whole pipeline tests and some corrections/improvements to earlier cha…

…pter 19 tests

test_framework/tacky/common.py

@@ -23,7 +23,11 @@ class TackyOptimizationTest(basic.TestChapter):
 
     In the second kind of test, we still compile the program, run it, and validate its behavior,
     but we also inspect its assembly code to make sure it's been optimized. These test methods
-    should call run_and_parse, defined below.
+    should call run_and_parse or run_and_parse_all, defined below.
+
+    This class defines two test methods used in dead store elimination and whole pipeline tests:
+    * store_eliminated_test: Test that stores of particular constants were eliminated
+    * return_const_test: Test that the only thing this function does is return a specific consatnt
 
     Notes:
     * This class isn't designed to test intermediate stages
@@ -79,6 +83,8 @@ def store_eliminated_test(
         self, *, source_file: Path, redundant_consts: List[int]
     ) -> None:
         """Make sure any stores of the form mov $const, <something> were eliminated.
+
+        The test program should contain a single 'target' function.
         Args:
             source_file: absolute path to program under test
             redundant_consts: any constants that were sources of mov instructions in the
@@ -110,7 +116,10 @@ def is_dead_store(i: asm.AsmItem) -> bool:
         )
 
     def return_const_test(self, *, source_file: Path, returned_const: int) -> None:
-        """Validate that the function doesn't do anything except return a constant."""
+        """Validate that the function doesn't do anything except return a constant.
+
+        The test program should contain a single 'target' function.
+        """
 
         def ok(i: asm.AsmItem) -> bool:
             """We should optimize out everything except prologue, epilogue, and mov into EAX"""

test_framework/tacky/copy_prop.py

@@ -217,7 +217,7 @@ def retval_test(self, expected_retval: Union[int, str], program_path: Path) -> N
     def arg_test(
         self, expected_args: Mapping[str, Sequence[Optional[int]]], program: Path
     ) -> None:
-        """Validate that propagate expected values into function arguments.
+        """Validate that we propagate expected values into function arguments.
 
         The copy propagation pass should be able to determine the constant values of
         some arguments to some function calls. Make sure we move these constants into
@@ -227,7 +227,8 @@ def arg_test(
             * expected_args: mapping from function names to expected constant
               value of each argument.
               An argument's value is None if we don't expect to know it at compile time.
-            * program_path: absolute path to source file"""
+            * program_path: absolute path to source file
+        """
 
         # convert constants to assembly operands
         expected_ops: Mapping[str, List[Optional[asm.Operand]]] = {
@@ -366,18 +367,22 @@ def ok(i: asm.AsmItem) -> bool:
 
 # programs we'll validate with same_arg_test
 SAME_ARG_TESTS = [
-    "store_doesnt_kill.c",
-    "copy_struct.c",
+    # int only
     "different_source_values_same_copy.c",
     "propagate_static_var.c",
     "propagate_var.c",
     "propagate_params.c",
+    # other types
+    "store_doesnt_kill.c",
+    "copy_struct.c",
     "char_type_conversion.c",
 ]
 
 # programs we'll validate with redundant_copies_test
 REDUNDANT_COPIES_TESTS = [
+    # int only
     "redundant_copies.c",
+    # other types
     "redundant_double_copies.c",
     "redundant_struct_copies.c",
 ]

test_framework/tacky/dead_store_elim.py

@@ -18,8 +18,8 @@ class TestDeadStoreElimination(common.TackyOptimizationTest):
     Each dynamically generated test calls one of the following main test methods:
 
     * compile_and_run, defined in TestChapter: validate behavior but don't inspect assembly
-    * store_eliminated_test: make sure a particular mov instruction was eliminated
-    * return_const_test: make sure entire funcion is reduce to a return instruction
+    * store_eliminated_test, defined in TackyOptimizationTest: make sure a particular mov instruction was eliminated
+    * return_const_test, defined in TackyOptimizationTest: make sure entire funcion is reduce to a return instruction
     """
 
     test_dir = common.TEST_DIR / "dead_store_elimination"
@@ -42,7 +42,6 @@ class TestDeadStoreElimination(common.TackyOptimizationTest):
 
 # programs to validate with return_const_test, with expected return value
 RETURN_CONST = {
-    "self_copy.c": 5,
     "delete_arithmetic_ops.c": 5,
     "simple.c": 3,
     "delete_dead_pt_ii_instructions.c": 5,

test_framework/tacky/pipeline.py

@@ -3,6 +3,15 @@
 from pathlib import Path
 from typing import Callable
 
+from ..parser.asm import (
+    AsmItem,
+    Opcode,
+    Label,
+    Immediate,
+    Memory,
+    Register,
+    Instruction,
+)
 from . import common
 
 
@@ -13,20 +22,78 @@ class TestWholePipeline(common.TackyOptimizationTest):
 
     test_dir = common.TEST_DIR / "whole_pipeline"
 
+    def fold_const_test(self, *, source_file: Path) -> None:
+        """Constant folding should eliminate all computations from the target_* functions
+
+        Similar to TackyOptimizationTest::return_const_test
+        but we allow any immediate (or RIP-relative operand, in case its a double)
+        rather than requiring a specific immediate
+        """
+
+        parsed_asm = self.run_and_parse_all(source_file)
+
+        def ok(i: AsmItem) -> bool:
+            if common.is_prologue_or_epilogue(i):
+                return True
+
+            # zeroing out EAX with xor is okay
+            if i == Instruction(Opcode.XOR, [Register.AX, Register.AX]):
+                return True
+
+            if isinstance(i, Label) or i.opcode != Opcode.MOV:
+                return False  # aside from prologue/epilogue or zeroing EAX, only mov allowed
+
+            # can only mov into return register
+            src, dst = i.operands[0], i.operands[1]
+            if dst not in [Register.XMM0, Register.AX]:
+                return False
+
+            # source must be immediate or RIP-relative
+            if isinstance(src, Immediate):
+                return True
+
+            if isinstance(src, Memory) and src.base == Register.IP:
+                return True
+
+            # source isn't immediate or RIP-relative
+            return False
+
+        for fn_name, fn_body in parsed_asm.items():
+            if fn_name.startswith("target"):
+                bad_instructions = [i for i in fn_body.instructions if not ok(i)]
+                self.assertFalse(
+                    bad_instructions,
+                    msg=common.build_msg(
+                        "Found instructions that should have been constant folded",
+                        bad_instructions=bad_instructions,
+                        full_prog=fn_body,
+                        program_path=source_file,
+                    ),
+                )
+
 
 RETVAL_TESTS = {
+    # Part I
     "dead_condition.c": 10,
     "elim_and_copy_prop.c": 10,
     "remainder_test.c": 1,
-    "unsigned_compare.c": 1,
-    "unsigned_wraparound.c": 0,
-    "const_fold_sign_extend.c": -1000,
-    "const_fold_sign_extend_2.c": -1000,
-    "char_round_trip.c": 1,
-    "signed_unsigned_conversion.c": -11,
-    "not_char.c": 1,
+    "listing_19_5.c": 9,
+    "int_min.c": -2147483648,
+    # Part II
+    "listing_19_5_more_types.c": 9,
+    "integer_promotions.c": 0,
 }
 STORE_ELIMINATED = {"alias_analysis_change.c": [5, 10]}
+FOLD_CONST_TESTS = {
+    "fold_cast_to_double.c",
+    "fold_cast_from_double.c",
+    "fold_negative_zero.c",
+    "fold_infinity.c",
+    "fold_negative_values.c",
+    "signed_unsigned_conversion.c",
+    "fold_char_condition.c",
+    "fold_extension_and_truncation.c",
+}
 
 
 def make_whole_pipeline_test(program: Path) -> Callable[[TestWholePipeline], None]:
@@ -42,6 +109,11 @@ def test(self: TestWholePipeline) -> None:
         def test(self: TestWholePipeline) -> None:
             self.store_eliminated_test(source_file=program, redundant_consts=consts)
 
+    elif program.name in FOLD_CONST_TESTS:
+
+        def test(self: TestWholePipeline) -> None:
+            self.fold_const_test(source_file=program)
+
     else:
         raise RuntimeError(f"Don't know what to do with {program.name}")
 

test_properties.json

@@ -373,7 +373,6 @@
     },
     "requires_mathlib": [
         "chapter_13/valid/function_calls/standard_library_call.c",
-        "chapter_13/valid/special_values/copysign.c",
         "chapter_13/valid/libraries/double_params_and_result.c"
     ],
     "libs": {

tests/chapter_16/valid/chars/access_through_char_pointer.c

@@ -51,7 +51,4 @@ int main(void) {
     }
 
     return 0;
-
-
-
 }

tests/chapter_19/constant_folding/all_types/fold_cast_from_double.c

@@ -15,6 +15,7 @@ int target_to_int(void) {
 }
 
 unsigned target_to_uint(void) {
+    // constant in the range of uint but not int
     return (unsigned)2147483750.5;
 }
 
@@ -25,8 +26,8 @@ long target_to_long(void) {
 }
 
 unsigned long target_to_ulong(void) {
-    // same constant from chapter13/valid/explicit_casts/double_to_ulong.c
-    return (unsigned long)3458764513821589504.0;
+    // constant in the range of ulong but not long
+    return (unsigned long)13835058055282163712.5;
 }
 
 unsigned long target_implicit(void) {
@@ -50,7 +51,7 @@ int main(void) {
     if (target_to_long() != 9223372036854774784l) {
         return 5;
     }
-    if (target_to_ulong() != 3458764513821589504ul) {
+    if (target_to_ulong() != 13835058055282163712ul) {
         return 6;
     }
     if (target_implicit() != 3458764513821589504ul) {

tests/chapter_19/constant_folding/all_types/fold_conditional_jump.c

@@ -0,0 +1,54 @@
+/* Test constant folding of JumpIfZero and JumpIfNotZero instructions
+ * resulting from && and || operations, with operand types other than int.
+ * Identical to chapter_19/constant_folding/int_only/fold_conditional_jump.c
+ * but with non-int operands
+ * */
+#if defined SUPPRESS_WARNINGS && defined __clang__
+#pragma clang diagnostic ignored "-Wliteral-conversion"
+#endif
+
+// We'll emit two TACKY instructions of the form
+// JumpIfZero(0, false_label)
+// both should be rewritten as Jump instructions
+int target_jz_to_jmp(void) {
+    return 0l && 0; // 0
+}
+
+// We'll emit two TACKY instructions of the form
+// JumpIfZero(1, false_label)
+// both should be removed
+int target_remove_jz(void) {
+    return 1u && 1.; // 1
+}
+
+// We'll emit two JumpIfNotZero instructions:
+// JumpIfNotZero(3, true_label)
+// JumpIfNotZero(99, true_label)
+// both should be written as Jump instructions
+int target_jnz_to_jmp(void) {
+    return 3.5 || 99ul; // 1
+}
+
+// We'll emit two JumpIfNotZero instructions:
+// JumpIfNotZero(0, true_label)
+// JumpIfNotZero(1, true_label)
+// we should remove the first, rewrite the second as a Jump instruction
+int target_remove_jnz(void) {
+    return 0ul || 1; // 1
+}
+
+int main(void) {
+    if (target_jz_to_jmp() != 0) {
+        return 1;
+    }
+    if (target_remove_jz() != 1) {
+        return 2;
+    }
+    if (target_jnz_to_jmp() != 1) {
+        return 3;
+    }
+    if (target_remove_jnz() != 1) {
+        return 4;
+    }
+    return 0; // success
+}

tests/chapter_19/constant_folding/all_types/fold_double.c

@@ -10,8 +10,7 @@
 #pragma GCC diagnostic ignored "-Woverflow"
 #endif
 #endif
-// TODO in copy prop section, also include test cases for constant folding w/
-// special values? negative zero, infinity...
+
 double target_add(void) {
     // Because 1.2345e60 is so large, adding one to it doesn't change its value
     return 1.2345e60 + 1.;
@@ -130,10 +129,10 @@ int main(void) {
         return 11;
     }
     if (target_lt()) {
-        return 11;
+        return 12;
     }
     if (target_le()) {
-        return 12;
+        return 13;
     }
 
     // infinity

tests/chapter_19/constant_folding/all_types/negative_zero.c

@@ -0,0 +1,19 @@
+/* If we deduplicate floating-point StaticConstant constructs, make sure we
+ * distinguish between constants with the same value but different alignments.
+ * Specifically, if we've already added an ordinary constant -0.0, and then we
+ * need a 16-byte aligned -0.0 to use for negation, don't just reuse the
+ * previous 8-byte aligned one. (It's okay to either keep them as separate
+ * constants, or merge them and keep the higher alignment.) This is a regression
+ * test for a bug in the reference implementation. Note that we can only catch
+ * this bug once we implement constant folding; before then, we don't add
+ * positive StaticConstants.
+ * No 'target' function here because we're just looking for correctness,
+ * not inspecting assembly.
+ * */
+
+double x = 5.0;
+
+int main(void) {
+    double d = -0.0;  // add normal constant -0. to list of top-level constants
+    return (-x > d); // add 16-byte-aligned constant -0. to negate x
+}

tests/chapter_19/copy_propagation/README.md

@@ -4,9 +4,9 @@ In some test programs, the copy propagation and constant folding passes make it
 
 In some programs, copy propagation should replace the arguments to certain function calls with constants. In other programs, copy propagation should propagate the same value two different function arguments. The test script validates these programs by checking which values are copied into the parameter-passing registers before the `call` instruction.
 
-Register coalescing, which we implement in Chapter 20, can make it appear that the same value is passed in two different parameter-passing reigsters, even if copy propagation wasn't performed. The tests are designed to prevent register coalescing in those cases, so they'll still test the intended cases after you complete Chapter 20.
+Register coalescing, which we implement in Chapter 20, can make it look like the same value is passed in two different parameter-passing registers, even if that value wasn't propagated to both parameters. The tests are designed to prevent register coalescing in those cases, so they'll still test the intended cases after you complete Chapter 20.
 
-In one program (`redundant_copies.c`), removing a redundant copy makes a whole branch dead, allowing unreachable code elimination to remove that branch. The test script validates that this program contains no control-flow instructions.
+In a few programs, including `redundant_copies.c`, removing a redundant copy makes a whole branch dead, allowing unreachable code elimination to remove that branch. The test script validates that these programs contains no control-flow instructions.
 
 In `pointer_arithmetic.c`, the test script validates that we optimize away all computation instructions (e.g. arithmetic instructions like `imul` and type conversions like `movsx`).
 

tests/chapter_19/copy_propagation/all_types/copy_struct.c

@@ -5,7 +5,19 @@ struct s {
 };
 
 int callee(struct s a, struct s b) {
-    return a.x == 3 && a.y == 4 && b.x == 3 && b.y == 4;
+    if (a.x != 3) {
+        return 1; // fail
+    }
+    if (a.y != 4) {
+        return 2; // fail
+    }
+    if (b.x != 3) {
+        return 3; // fail
+    }
+    if (b.y != 4) {
+        return 4; // fail
+    }
+    return 0; // success
 }
 
 int target(void) {

tests/chapter_19/copy_propagation/all_types/propagate_into_type_conversions.c

@@ -12,5 +12,8 @@ int target(void) {
 }
 
 int main(void) {
-    return target() == 83826;
+    if (target() != 83826) {
+        return 1; // fail
+    }
+    return 0; // success
 }