Merge pull request #2949 from avolkov76/anv-bug-2947

[cc65] Prevent Lhs loads shared with the Rhs from being removed in `OptStackOps()`

src/cc65/codeoptutil.c

@@ -509,6 +509,68 @@ void SetIfOperandLoadUnremovable (LoadInfo* LI, unsigned Used)
 
 
 
+static int IsRegInsnSameAsOtherAX (int InsnIndex, const LoadInfo* OtherLI)
+/* Helper: Return true if the other side (Rhs or Lhs) refers to the same insn */
+{
+    if (InsnIndex < 0) {
+        /* No, do not have an insn */
+        return 0;
+    }
+
+    /* Check if this insn matches any load/change on the other side */
+    return InsnIndex == OtherLI->A.LoadIndex ||
+           InsnIndex == OtherLI->A.ChgIndex ||
+           InsnIndex == OtherLI->X.LoadIndex ||
+           InsnIndex == OtherLI->X.ChgIndex;
+}
+
+
+
+static int IsRegInsnSameAsOtherY (int InsnIndex, const LoadInfo* OtherLI)
+/* Helper: Return true if the other side (Rhs or Lhs) refers to the same insn */
+{
+    if (InsnIndex < 0) {
+        /* No, do not have an insn */
+        return 0;
+    }
+
+    /* Check if this insn matches any load/change on the other side */
+    return InsnIndex == OtherLI->Y.LoadIndex ||
+           InsnIndex == OtherLI->Y.ChgIndex;
+}
+
+
+
+void SetUnremovableIfUsedByOther (LoadInfo* LI, const LoadInfo* OtherLI)
+/* Check and flag operand loads unremovable if the other side (Rhs or Lhs)
+** uses the same load insn (checked by index).
+*/
+{
+    /* Disallow removing the loads if they are shared between Lhs and Rhs.
+    ** Note: For safety, we check the ChgIndex as well, which accounts for
+    ** conditions where a label is encountered in the block.
+    */
+    if (IsRegInsnSameAsOtherAX (LI->A.LoadIndex, OtherLI) ||
+        IsRegInsnSameAsOtherAX (LI->A.ChgIndex, OtherLI)) {
+        /* Parts of A load are used by other and cannot be removed */
+        LI->A.Flags |= LI_DONT_REMOVE;
+    }
+
+    if (IsRegInsnSameAsOtherAX (LI->X.LoadIndex, OtherLI) ||
+        IsRegInsnSameAsOtherAX (LI->X.ChgIndex, OtherLI)) {
+        /* Parts of X load are used by other and cannot be removed */
+        LI->X.Flags |= LI_DONT_REMOVE;
+    }
+
+    if (IsRegInsnSameAsOtherY (LI->Y.LoadIndex, OtherLI) ||
+        IsRegInsnSameAsOtherY (LI->Y.ChgIndex, OtherLI)) {
+        /* Parts of Y load are used by other and cannot be removed */
+        LI->Y.Flags |= LI_DONT_REMOVE;
+    }
+}
+
+
+
 unsigned int TrackLoads (LoadInfo* LI, CodeSeg* S, int I)
 /* Track loads for a code entry.
 ** Return used registers.

src/cc65/codeoptutil.h

@@ -180,6 +180,11 @@ void SetIfOperandSrcAffected (LoadInfo* LLI, CodeEntry* E);
 void SetIfOperandLoadUnremovable (LoadInfo* LI, unsigned Used);
 /* Check and flag operand load that may be unremovable */
 
+void SetUnremovableIfUsedByOther (LoadInfo* LI, const LoadInfo* OtherLI);
+/* Check and flag operand loads unremovable if the other side (Rhs or Lhs)
+** uses the same load insn (checked by index).
+*/
+
 unsigned int TrackLoads (LoadInfo* LI, CodeSeg* S, int I);
 /* Track loads for a code entry.
 ** Return used registers.

src/cc65/coptstop.c

@@ -1434,8 +1434,6 @@ static unsigned Opt_a_tosbitwise (StackOpData* D, opc_t OPC)
             X = NewCodeEntry (OP65_LDX, AM65_IMM, MakeHexArg (0), 0, D->Rhs.X.ChgEntry->LI);
             InsertEntry (D, X, D->IP++);
             D->Rhs.X.Flags |= LI_REMOVE;
-        } else {
-            D->Rhs.X.Flags |= LI_DONT_REMOVE;
         }
     } else {
         /* Rhs load entries may be removed */
@@ -2092,6 +2090,12 @@ unsigned OptStackOps (CodeSeg* S)
                     }
                 }
 
+                /* Lhs and Rhs may shared some of the load insns; or due to how
+                ** the labels are processed, "change" insns may refer to the
+                ** others' loads. In either case, it is unsafe to remove them.
+                */
+                SetUnremovableIfUsedByOther (&Data.Lhs, &Data.Rhs);
+
                 /* Flag entries that can't be removed */
                 SetDontRemoveEntryFlags (&Data);
 

test/val/bug2395.c

@@ -43,7 +43,7 @@ int main(void) {
     a = c + (d != 0);
     b = c + 1;
 
-    printf("%u ^ (%u != 0) => %u\n", c, d, a);
+    printf("%u + (%u != 0) => %u\n", c, d, a);
     if (a != b) {
       printf("ADD error: a %d instead of %d\n", a, b);
       fails++;
@@ -52,7 +52,7 @@ int main(void) {
     a = c - (d != 0);
     b = c - 1;
 
-    printf("%u ^ (%u != 0) => %u\n", c, d, a);
+    printf("%u - (%u != 0) => %u\n", c, d, a);
     if (a != b) {
       printf("SUB error: a %d instead of %d\n", a, b);
       fails++;
@@ -88,7 +88,7 @@ int main(void) {
     a = c + (d >= 0);
     b = c + 1;
 
-    printf("%u ^ (%u >= 0) => %u\n", c, d, a);
+    printf("%u + (%u >= 0) => %u\n", c, d, a);
     if (a != b) {
       printf("ADD error: a %d instead of %d\n", a, b);
       fails++;
@@ -97,14 +97,12 @@ int main(void) {
     a = c - (d >= 0);
     b = c - 1;
 
-    printf("%u ^ (%u >= 0) => %u\n", c, d, a);
+    printf("%u - (%u >= 0) => %u\n", c, d, a);
     if (a != b) {
       printf("SUB error: a %d instead of %d\n", a, b);
       fails++;
     }
 
-    printf("%d errors\n", fails);
-
     a = c ^ (i >= 0);
     b = c ^ 1;
 
@@ -135,7 +133,7 @@ int main(void) {
     a = c + (i >= 0);
     b = c + 1;
 
-    printf("%u ^ (%d >= 0) => %u\n", c, i, a);
+    printf("%u + (%d >= 0) => %u\n", c, i, a);
     if (a != b) {
       printf("ADD int cmp error: a %d instead of %d\n", a, b);
       fails++;
@@ -144,7 +142,7 @@ int main(void) {
     a = c - (i >= 0);
     b = c - 1;
 
-    printf("%u ^ (%d >= 0) => %u\n", c, i, a);
+    printf("%u - (%d >= 0) => %u\n", c, i, a);
     if (a != b) {
       printf("SUB int cmp error: a %d instead of %d\n", a, b);
       fails++;

test/val/bug2947.c

@@ -1,22 +1,349 @@
 
-/* bug #2947: Opt_a_tosbitwise() attempts to remove non-removable Rhs X */
+/* bug #2947: OptStackOps() attempts to remove non-removable Rhs X */
 
 #include <stdio.h>
 #include <stdlib.h>
 
-unsigned char a, b = 0;
+unsigned char t8 = 0;
+int t16 = -1;
+unsigned char r8a, r8b;
+int r16;
 unsigned char c = 10;
 unsigned char d = 1;
 
-int main(void) {
+static int fail = 0;
+
+void r8a_xor_test(void)
+{
     /* 'if' needed to produce a label below, moved by OptJumpTarget3  */
-    if (b != 0) {
+    if (t8 != 0) {
-        /* Operation not important; it only affects the removal of one LDX #$00 */
+        /* Keep X reg at 0 */
-        b = 0;
+        t8 = 0;
     }
 
     /* d >= 0 is const, A/X=1 (Warning: Result of comparison is always true) */
-    a = c ^ (d >= 0);
+    r8a = c ^ (d >= 0);
 
-    return !(a == 11);
+    if (r8a != 11) {
+        ++fail;
+        printf("r8a = c ^ (d >= 0) : fail\n");
+    }
+}
+
+void r8b_xor_test(void)
+{
+    /* 'if' needed to produce a label below, moved by OptJumpTarget3  */
+    if (t8 != 0) {
+        /* Clobber X reg */
+        t16 = -1;
+    }
+
+    /* d >= 0 is const, A/X=1 (Warning: Result of comparison is always true) */
+    r8b = c ^ (d >= 0);
+
+    if (r8b != 11) {
+        ++fail;
+        printf("r8b = c ^ (d >= 0) : fail\n");
+    }
+}
+
+void r8a_or_test(void)
+{
+    /* 'if' needed to produce a label below, moved by OptJumpTarget3  */
+    if (t8 != 0) {
+        /* Keep X reg at 0 */
+        t8 = 0;
+    }
+
+    /* d >= 0 is const, A/X=1 (Warning: Result of comparison is always true) */
+    r8a = c | (d >= 0);
+
+    if (r8a != 11) {
+        ++fail;
+        printf("r8a = c | (d >= 0) : fail\n");
+    }
+}
+
+void r8b_or_test(void)
+{
+    /* 'if' needed to produce a label below, moved by OptJumpTarget3  */
+    if (t8 != 0) {
+        /* Clobber X reg */
+        t16 = -1;
+    }
+
+    /* d >= 0 is const, A/X=1 (Warning: Result of comparison is always true) */
+    r8b = c | (d >= 0);
+
+    if (r8b != 11) {
+        ++fail;
+        printf("r8b = c | (d >= 0) : fail\n");
+    }
+}
+
+void r8a_add_test(void)
+{
+    /* 'if' needed to produce a label below, moved by OptJumpTarget3  */
+    if (t8 != 0) {
+        /* Keep X reg at 0 */
+        t8 = 0;
+    }
+
+    /* d >= 0 is const, A/X=1 (Warning: Result of comparison is always true) */
+    r8a = c + (d >= 0);
+
+    if (r8a != 11) {
+        ++fail;
+        printf("r8a = c + (d >= 0) : fail\n");
+    }
+}
+
+void r8b_add_test(void)
+{
+    /* 'if' needed to produce a label below, moved by OptJumpTarget3  */
+    if (t8 != 0) {
+        /* Clobber X reg */
+        t16 = -1;
+    }
+
+    /* d >= 0 is const, A/X=1 (Warning: Result of comparison is always true) */
+    r8b = c + (d >= 0);
+
+    if (r8b != 11) {
+        ++fail;
+        printf("r8b = c + (d >= 0) : fail\n");
+    }
+}
+
+void r8a_sub_test(void)
+{
+    /* 'if' needed to produce a label below, moved by OptJumpTarget3  */
+    if (t8 != 0) {
+        /* Keep X reg at 0 */
+        t8 = 0;
+    }
+
+    /* d >= 0 is const, A/X=1 (Warning: Result of comparison is always true) */
+    r8a = c - (d >= 0);
+
+    if (r8a != 9) {
+        ++fail;
+        printf("r8a = c - (d >= 0) : fail\n");
+    }
+}
+
+void r8b_sub_test(void)
+{
+    /* 'if' needed to produce a label below, moved by OptJumpTarget3  */
+    if (t8 != 0) {
+        /* Clobber X reg */
+        t16 = -1;
+    }
+
+    /* d >= 0 is const, A/X=1 (Warning: Result of comparison is always true) */
+    r8b = c - (d >= 0);
+
+    if (r8b != 9) {
+        ++fail;
+        printf("r8b = c - (d >= 0) : fail\n");
+    }
+}
+
+void r8a_ge_test(void)
+{
+    /* 'if' needed to produce a label below, moved by OptJumpTarget3  */
+    if (t8 != 0) {
+        /* Keep X reg at 0 */
+        t8 = 0;
+    }
+
+    /* d >= 0 is const, A/X=1 (Warning: Result of comparison is always true) */
+    r8a = c >= (d >= 0);
+
+    if (r8a != 1) {
+        ++fail;
+        printf("r8a = c >= (d >= 0) : fail\n");
+    }
+}
+
+void r8b_ge_test(void)
+{
+    /* 'if' needed to produce a label below, moved by OptJumpTarget3  */
+    if (t8 != 0) {
+        /* Clobber X reg */
+        t16 = -1;
+    }
+
+    /* d >= 0 is const, A/X=1 (Warning: Result of comparison is always true) */
+    r8b = c >= (d >= 0);
+
+    if (r8b != 1) {
+        ++fail;
+        printf("r8b = c >= (d >= 0) : fail\n");
+    }
+}
+
+void r8a_le_test(void)
+{
+    /* 'if' needed to produce a label below, moved by OptJumpTarget3  */
+    if (t8 != 0) {
+        /* Keep X reg at 0 */
+        t8 = 0;
+    }
+
+    /* d >= 0 is const, A/X=1 (Warning: Result of comparison is always true) */
+    r8a = d <= (d >= 0);
+
+    if (r8a != 1) {
+        ++fail;
+        printf("r8a = d <= (d >= 0) : fail\n");
+    }
+}
+
+void r8b_le_test(void)
+{
+    /* 'if' needed to produce a label below, moved by OptJumpTarget3  */
+    if (t8 != 0) {
+        /* Clobber X reg */
+        t16 = -1;
+    }
+
+    /* d >= 0 is const, A/X=1 (Warning: Result of comparison is always true) */
+    r8b = d <= (d >= 0);
+
+    if (r8b != 1) {
+        ++fail;
+        printf("r8b = d <= (d >= 0) : fail\n");
+    }
+}
+
+void r16_xor_test(void)
+{
+    /* 'if' needed to produce a label below */
+    if (t16 != 0) {
+        /* Clobber X reg */
+        t16 = -1;
+    }
+
+    /* d >= 0 is const, A/X=1 (Warning: Result of comparison is always true) */
+    r16 = c ^ (d >= 0);
+
+    if (r16 != 11) {
+        ++fail;
+        printf("r16 = c ^ (d >= 0) : fail\n");
+    }
+}
+
+void r16_or_test(void)
+{
+    /* 'if' needed to produce a label below */
+    if (t16 != 0) {
+        /* Clobber X reg */
+        t16 = -1;
+    }
+
+    /* d >= 0 is const, A/X=1 (Warning: Result of comparison is always true) */
+    r16 = c | (d >= 0);
+
+    if (r16 != 11) {
+        ++fail;
+        printf("r16 = c | (d >= 0) : fail\n");
+    }
+}
+
+void r16_add_test(void)
+{
+    /* 'if' needed to produce a label below */
+    if (t16 != 0) {
+        /* Clobber X reg */
+        t16 = -1;
+    }
+
+    /* d >= 0 is const, A/X=1 (Warning: Result of comparison is always true) */
+    r16 = c + (d >= 0);
+
+    if (r16 != 11) {
+        ++fail;
+        printf("r16 = c + (d >= 0) : fail\n");
+    }
+}
+
+void r16_sub_test(void)
+{
+    /* 'if' needed to produce a label below */
+    if (t16 != 0) {
+        /* Clobber X reg */
+        t16 = -1;
+    }
+
+    /* d >= 0 is const, A/X=1 (Warning: Result of comparison is always true) */
+    r16 = c - (d >= 0);
+
+    if (r16 != 9) {
+        ++fail;
+        printf("r16 = c - (d >= 0) : fail\n");
+    }
+}
+
+void r16_ge_test(void)
+{
+    /* 'if' needed to produce a label below */
+    if (t16 != 0) {
+        /* Clobber X reg */
+        t16 = -1;
+    }
+
+    /* d >= 0 is const, A/X=1 (Warning: Result of comparison is always true) */
+    r16 = c >= (d >= 0);
+
+    if (r16 != 1) {
+        ++fail;
+        printf("r16 = c >= (d >= 0) : fail\n");
+    }
+}
+
+void r16_le_test(void)
+{
+    /* 'if' needed to produce a label below */
+    if (t16 != 0) {
+        /* Keep X reg at 0 */
+        t16 = -1;
+    }
+
+    /* d >= 0 is const, A/X=1 (Warning: Result of comparison is always true) */
+    r16 = d <= (unsigned int)(d >= 0);
+
+    if (r16 != 1) {
+        ++fail;
+        printf("r16 = d <= (d >= 0) : fail\n");
+    }
+}
+
+int main(void)
+{
+    r8a_xor_test();
+    r8b_xor_test();
+    r8a_or_test();
+    r8b_or_test();
+
+    r8a_add_test();
+    r8b_add_test();
+    r8a_sub_test();
+    r8b_sub_test();
+
+    r8a_ge_test();
+    r8b_ge_test();
+    r8a_le_test();
+    r8b_le_test();
+
+    r16_xor_test();
+    r16_or_test();
+
+    r16_add_test();
+    r16_sub_test();
+
+    r16_ge_test();
+    r16_le_test();
+
+    return fail == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
 }