From 9732bc2cb798fa92612f2284dab84b0e73aac3a3 Mon Sep 17 00:00:00 2001 From: Alex Volkov Date: Sun, 22 Mar 2026 18:42:01 -0400 Subject: [PATCH 1/4] Correct some printed messages and remove one print in bug2395 test (c&p errors) --- test/val/bug2395.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/test/val/bug2395.c b/test/val/bug2395.c index 7cdf8b30b..f02518474 100644 --- a/test/val/bug2395.c +++ b/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++; From 7988d9aefbbb26ddb7ba169f122261f3256444de Mon Sep 17 00:00:00 2001 From: Alex Volkov Date: Sun, 22 Mar 2026 18:44:16 -0400 Subject: [PATCH 2/4] Regression tests for #2947 expanded to other operators and conditions --- test/val/bug2947.c | 343 +++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 335 insertions(+), 8 deletions(-) diff --git a/test/val/bug2947.c b/test/val/bug2947.c index dbb479f2c..e48a298bb 100644 --- a/test/val/bug2947.c +++ b/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 #include -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) { - /* Operation not important; it only affects the removal of one LDX #$00 */ - b = 0; + if (t8 != 0) { + /* Keep X reg at 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; } From af285ba9e5c1ca98162e249587a925f6ce4aa0db Mon Sep 17 00:00:00 2001 From: Alex Volkov Date: Sun, 22 Mar 2026 18:49:12 -0400 Subject: [PATCH 3/4] Remove red herring flag set. It is far too late to set LI_DONT_REMOVE in a subopt. --- src/cc65/coptstop.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/cc65/coptstop.c b/src/cc65/coptstop.c index 07ad6cb71..945dc1c54 100644 --- a/src/cc65/coptstop.c +++ b/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 */ From d711b6d6fa3526095cf6013c31fa4fba2c3141dc Mon Sep 17 00:00:00 2001 From: Alex Volkov Date: Sun, 22 Mar 2026 19:42:57 -0400 Subject: [PATCH 4/4] Prevent Lhs load insns shared with the Rhs from being inadvertently removed by Rhs optimizers. Fixes issue #2947. --- src/cc65/codeoptutil.c | 62 ++++++++++++++++++++++++++++++++++++++++++ src/cc65/codeoptutil.h | 5 ++++ src/cc65/coptstop.c | 6 ++++ 3 files changed, 73 insertions(+) diff --git a/src/cc65/codeoptutil.c b/src/cc65/codeoptutil.c index 44174c5fc..b098845b4 100644 --- a/src/cc65/codeoptutil.c +++ b/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. diff --git a/src/cc65/codeoptutil.h b/src/cc65/codeoptutil.h index cf7743605..06a9d4ce3 100644 --- a/src/cc65/codeoptutil.h +++ b/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. diff --git a/src/cc65/coptstop.c b/src/cc65/coptstop.c index 945dc1c54..0bc533297 100644 --- a/src/cc65/coptstop.c +++ b/src/cc65/coptstop.c @@ -2090,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);