From 62451557f98f17a237a4d0c23d8f017ef0c14789 Mon Sep 17 00:00:00 2001 From: Alex Volkov Date: Wed, 25 Mar 2026 19:00:57 -0400 Subject: [PATCH 1/9] Revert the fix for #2461 in search of a more general solution --- src/cc65/codeoptutil.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/cc65/codeoptutil.c b/src/cc65/codeoptutil.c index b098845b4..9c9699a6a 100644 --- a/src/cc65/codeoptutil.c +++ b/src/cc65/codeoptutil.c @@ -1197,10 +1197,8 @@ void AddOpHigh (StackOpData* D, opc_t OPC, LoadInfo* LI, int KeepResult) InsertEntry (D, X, D->IP++); } - /* If this is the right hand side, we can remove the load. */ - if (LI == &D->Rhs) { - LI->X.Flags |= LI_REMOVE; - } + /* In both cases, we can remove the load */ + LI->X.Flags |= LI_REMOVE; } else { From ac69017ca537ab91c2a13f8b4318831ca3c70cc0 Mon Sep 17 00:00:00 2001 From: Alex Volkov Date: Wed, 25 Mar 2026 19:01:37 -0400 Subject: [PATCH 2/9] Remove redundant LoadEntry clears; DelEntry() already handles clearing them, and the redundant clears are a red herring in the code. --- src/cc65/codeoptutil.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/cc65/codeoptutil.c b/src/cc65/codeoptutil.c index 9c9699a6a..6d64d89e2 100644 --- a/src/cc65/codeoptutil.c +++ b/src/cc65/codeoptutil.c @@ -1229,7 +1229,6 @@ void RemoveRegLoads (StackOpData* D, LoadInfo* LI) if (LI->A.LoadIndex >= 0 && (LI->A.LoadEntry->Flags & CEF_DONT_REMOVE) == 0) { DelEntry (D, LI->A.LoadIndex); - LI->A.LoadEntry = 0; } if (LI->A.LoadYIndex >= 0 && (LI->A.LoadYEntry->Flags & CEF_DONT_REMOVE) == 0) { @@ -1252,7 +1251,6 @@ void RemoveRegLoads (StackOpData* D, LoadInfo* LI) if (LI->X.LoadIndex >= 0 && (LI->X.LoadEntry->Flags & CEF_DONT_REMOVE) == 0) { DelEntry (D, LI->X.LoadIndex); - LI->X.LoadEntry = 0; } if (LI->X.LoadYIndex >= 0 && (LI->X.LoadYEntry->Flags & CEF_DONT_REMOVE) == 0) { From 40a20ab97351ac988a73cfea4ec8fca1ce100f7c Mon Sep 17 00:00:00 2001 From: Alex Volkov Date: Wed, 25 Mar 2026 19:03:00 -0400 Subject: [PATCH 3/9] Expand bug1374 tests to a few other conditions --- test/val/bug1374.c | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/test/val/bug1374.c b/test/val/bug1374.c index b55e75fee..86f0223a9 100644 --- a/test/val/bug1374.c +++ b/test/val/bug1374.c @@ -24,6 +24,15 @@ int test1b(void) return (z == 0x89ab) ? 0 : 1; } +int test1c(void) +{ + uint8_t x = 0x95; + uint8_t y = 0xab; + uint16_t z = (x * 0) | y; + printf("%x\n", z); + return (z == 0x00ab) ? 0 : 1; +} + int test2(void) { uint16_t x = 0x8900; @@ -33,6 +42,15 @@ int test2(void) return (z == 0x89ab) ? 0 : 1; } +int test2c(void) +{ + uint16_t x = 0x6795; + uint8_t y = 0xab; + uint16_t z = (x * 0) | y; + printf("%x\n", z); + return (z == 0x00ab) ? 0 : 1; +} + int test3(void) { uint16_t x = 0x89; @@ -69,6 +87,15 @@ int test4b(void) return (z == 0x89ab) ? 0 : 1; } +int test4c(void) +{ + uint8_t x = 0x95; + uint16_t y = 0xabcd; + uint16_t z = (x * 0) | y; + printf("%x\n", z); + return (z == 0xabcd) ? 0 : 1; +} + int main(void) { res |= test1(); @@ -78,6 +105,9 @@ int main(void) res |= test1b(); res |= test3b(); res |= test4b(); + res |= test1c(); + res |= test2c(); + res |= test4c(); printf("res: %d\n", res); return res; } From c9b885b144c09c39106686f71cf3aef984a95010 Mon Sep 17 00:00:00 2001 From: Alex Volkov Date: Wed, 25 Mar 2026 19:08:18 -0400 Subject: [PATCH 4/9] Prevent the removal of a ldaxysp runtime call as a "load instruction" for only the A or the X side, when the other is non-removable. Fixes issue #2942 / #2461. --- src/cc65/codeoptutil.c | 86 ++++++++++++++++++++++++++++++++++++------ 1 file changed, 74 insertions(+), 12 deletions(-) diff --git a/src/cc65/codeoptutil.c b/src/cc65/codeoptutil.c index 6d64d89e2..c2c815e4c 100644 --- a/src/cc65/codeoptutil.c +++ b/src/cc65/codeoptutil.c @@ -1123,6 +1123,8 @@ void AddOpLow (StackOpData* D, opc_t OPC, LoadInfo* LI) InsertEntry (D, X, D->IP++); if (LI->A.LoadEntry->OPC == OP65_JSR) { + /* This should only happen in one known ldaxysp case. */ + CHECK (CE_IsCallTo (LI->A.LoadEntry, "ldaxysp")); /* opc (c_sp),y */ X = NewCodeEntry (OPC, AM65_ZP_INDY, "c_sp", 0, D->OpEntry->LI); } else { @@ -1133,7 +1135,7 @@ void AddOpLow (StackOpData* D, opc_t OPC, LoadInfo* LI) } - /* In both cases, we can remove the load */ + /* In both cases, we may try removing the load */ LI->A.Flags |= LI_REMOVE; } else { @@ -1188,6 +1190,8 @@ void AddOpHigh (StackOpData* D, opc_t OPC, LoadInfo* LI, int KeepResult) InsertEntry (D, X, D->IP++); if (LI->X.LoadEntry->OPC == OP65_JSR) { + /* This should only happen in one known ldaxysp case. */ + CHECK (CE_IsCallTo (LI->X.LoadEntry, "ldaxysp")); /* opc (c_sp),y */ X = NewCodeEntry (OPC, AM65_ZP_INDY, "c_sp", 0, D->OpEntry->LI); } else { @@ -1197,7 +1201,7 @@ void AddOpHigh (StackOpData* D, opc_t OPC, LoadInfo* LI, int KeepResult) InsertEntry (D, X, D->IP++); } - /* In both cases, we can remove the load */ + /* In both cases, we may try removing the load */ LI->X.Flags |= LI_REMOVE; } else { @@ -1225,12 +1229,65 @@ void AddOpHigh (StackOpData* D, opc_t OPC, LoadInfo* LI, int KeepResult) void RemoveRegLoads (StackOpData* D, LoadInfo* LI) /* Remove register load insns */ { - if ((LI->A.Flags & LI_REMOVE) == LI_REMOVE) { - if (LI->A.LoadIndex >= 0 && - (LI->A.LoadEntry->Flags & CEF_DONT_REMOVE) == 0) { - DelEntry (D, LI->A.LoadIndex); - } + int CanRemoveA; + int CanRemoveX; + + /* When either A or X load insn is a call to ldaxysp runtime, the load + ** affects both and must be treated as a single A/X unit. A request to + ** remove the runtime call for just one (A or X) load is not valid in that + ** case. Either both must be removable+removed, or the other load is + ** independent, or ldaxysp runtime call must remain. + */ + CanRemoveA = (LI->A.Flags & LI_REMOVE) == LI_REMOVE && + LI->A.LoadEntry != 0 && + (LI->A.LoadEntry->Flags & CEF_DONT_REMOVE) == 0; + + CanRemoveX = (LI->X.Flags & LI_REMOVE) == LI_REMOVE && + LI->X.LoadEntry != 0 && + (LI->X.LoadEntry->Flags & CEF_DONT_REMOVE) == 0; + + /* The only runtime calls removable as load insns are calls to ldaxysp. */ + CHECK (!CanRemoveA || LI->A.LoadEntry->OPC != OP65_JSR || + CE_IsCallTo (LI->A.LoadEntry, "ldaxysp")); + CHECK (!CanRemoveX || LI->X.LoadEntry->OPC != OP65_JSR || + CE_IsCallTo (LI->X.LoadEntry, "ldaxysp")); + + /* When the A load is a runtime call and the X load cannot be removed, + ** we cannot remove the A load either. + */ + if (CanRemoveA && LI->A.LoadEntry->OPC == OP65_JSR && !CanRemoveX) { + /* A load is not removable */ + LI->A.LoadEntry->Flags |= CEF_DONT_REMOVE; + CanRemoveA = 0; + } + + /* When the X load is a runtime call and the A load cannot be removed, + ** we cannot remove the X load either. + */ + if (CanRemoveX && LI->X.LoadEntry->OPC == OP65_JSR && !CanRemoveA) { + /* X load is not removable */ + LI->X.LoadEntry->Flags |= CEF_DONT_REMOVE; + CanRemoveX = 0; + } + + /* A request to remove a "change" insn (ChgIndex) when there is no + ** corresponing load insn (LoadIndex) is never valid. + */ + CHECK ((LI->A.Flags & LI_REMOVE) == 0 || LI->A.ChgIndex < 0 || + LI->A.LoadIndex >= 0); + CHECK ((LI->X.Flags & LI_REMOVE) == 0 || LI->X.ChgIndex < 0 || + LI->X.LoadIndex >= 0); + + if (CanRemoveA) { + + CHECK (LI->A.LoadIndex >= 0); + DelEntry (D, LI->A.LoadIndex); + + /* Only remove the Y load if it is not shared with a non-removable + ** X load. A shared load will be removed here otherwise. + */ if (LI->A.LoadYIndex >= 0 && + (LI->A.LoadYIndex != LI->X.LoadYIndex || CanRemoveX) && (LI->A.LoadYEntry->Flags & CEF_DONT_REMOVE) == 0) { DelEntry (D, LI->A.LoadYIndex); } @@ -1247,11 +1304,16 @@ void RemoveRegLoads (StackOpData* D, LoadInfo* LI) LI->A.LoadYEntry->Flags |= CEF_DONT_REMOVE; } - if ((LI->X.Flags & LI_REMOVE) == LI_REMOVE) { - if (LI->X.LoadIndex >= 0 && - (LI->X.LoadEntry->Flags & CEF_DONT_REMOVE) == 0) { - DelEntry (D, LI->X.LoadIndex); - } + /* The X load may have already been removed above if it were a runtime + ** call (i.e. "jsr ldaxysp"), so need to check again. + */ + if (CanRemoveX && LI->X.LoadIndex >= 0) { + + DelEntry (D, LI->X.LoadIndex); + + /* Only remove the Y load if it is not shared with non-removable A load. + ** If it is shared and still needed, it was flaged non-removable above. + */ if (LI->X.LoadYIndex >= 0 && (LI->X.LoadYEntry->Flags & CEF_DONT_REMOVE) == 0) { DelEntry (D, LI->X.LoadYIndex); From 00a1f1d447838f8025036db0bf47aada597af5e1 Mon Sep 17 00:00:00 2001 From: Alex Volkov Date: Wed, 25 Mar 2026 19:14:14 -0400 Subject: [PATCH 5/9] Clearly delineate load instruction "must remove" conditions from "may remove" with a new LI_MUST_REMOVE flag. A demand to remove a non-removable load is fatal. --- src/cc65/codeoptutil.c | 6 ++++ src/cc65/codeoptutil.h | 1 + src/cc65/coptstop.c | 72 ++++++++++++++++++++++++------------------ 3 files changed, 48 insertions(+), 31 deletions(-) diff --git a/src/cc65/codeoptutil.c b/src/cc65/codeoptutil.c index c2c815e4c..7b81abb10 100644 --- a/src/cc65/codeoptutil.c +++ b/src/cc65/codeoptutil.c @@ -1270,6 +1270,12 @@ void RemoveRegLoads (StackOpData* D, LoadInfo* LI) CanRemoveX = 0; } + /* A load removal demand which cannot be satisifed is a fatal condition */ + if (((LI->A.Flags & LI_MUST_REMOVE) != 0 && !CanRemoveA) || + ((LI->X.Flags & LI_MUST_REMOVE) != 0 && !CanRemoveX)) { + Internal ("Cannot remove a load instruction which must be removed"); + } + /* A request to remove a "change" insn (ChgIndex) when there is no ** corresponing load insn (LoadIndex) is never valid. */ diff --git a/src/cc65/codeoptutil.h b/src/cc65/codeoptutil.h index 06a9d4ce3..c62ff28f3 100644 --- a/src/cc65/codeoptutil.h +++ b/src/cc65/codeoptutil.h @@ -70,6 +70,7 @@ typedef enum { LI_USED_BY_Y = 0x4000, /* Content used by RegY */ LI_SP = 0x8000, /* Content on stack */ LI_LOAD_INSN = 0x010000, /* Is a load insn */ + LI_MUST_REMOVE = 0x020000, /* Load must be removed */ } LI_FLAGS; /* Structure that tells us how to load the lhs values */ diff --git a/src/cc65/coptstop.c b/src/cc65/coptstop.c index 0bc533297..57985fda0 100644 --- a/src/cc65/coptstop.c +++ b/src/cc65/coptstop.c @@ -436,9 +436,9 @@ static unsigned Opt_toseqax_tosneax (StackOpData* D, const char* BoolTransformer X = NewCodeEntry (OP65_CMP, LoadA->AM, LoadA->Arg, 0, D->OpEntry->LI); InsertEntry (D, X, D->IP++); - /* Rhs load entries must be removed */ - D->Rhs.X.Flags |= LI_REMOVE; - D->Rhs.A.Flags |= LI_REMOVE; + /* Rhs load entries must be removed because Lhs must be in effect */ + D->Rhs.X.Flags |= (LI_REMOVE | LI_MUST_REMOVE); + D->Rhs.A.Flags |= (LI_REMOVE | LI_MUST_REMOVE); } else if (RhsIsRemovable (D)) { @@ -454,6 +454,10 @@ static unsigned Opt_toseqax_tosneax (StackOpData* D, const char* BoolTransformer /* Add operand for high byte */ AddOpHigh (D, OP65_CMP, &D->Rhs, 0); + /* Rhs load entries must be removed because Lhs must be in effect */ + D->Rhs.X.Flags |= LI_MUST_REMOVE; + D->Rhs.A.Flags |= LI_MUST_REMOVE; + } else { /* Implement the op via a temp ZP location */ @@ -1018,9 +1022,9 @@ static unsigned Opt_tosgeax (StackOpData* D) X = NewCodeEntry (OP65_ROL, AM65_ACC, "a", 0, D->OpEntry->LI); InsertEntry (D, X, D->IP++); - /* Rhs load entries must be removed */ - D->Rhs.X.Flags |= LI_REMOVE; - D->Rhs.A.Flags |= LI_REMOVE; + /* Rhs load entries must be removed because Lhs must be in effect */ + D->Rhs.X.Flags |= LI_MUST_REMOVE; + D->Rhs.A.Flags |= LI_MUST_REMOVE; /* Remove the push and the call to the tosgeax function */ RemoveRemainders (D); @@ -1075,9 +1079,9 @@ static unsigned Opt_tosltax (StackOpData* D) X = NewCodeEntry (OP65_ROL, AM65_ACC, "a", 0, D->OpEntry->LI); InsertEntry (D, X, D->IP++); - /* Rhs load entries must be removed */ - D->Rhs.X.Flags |= LI_REMOVE; - D->Rhs.A.Flags |= LI_REMOVE; + /* Rhs load entries must be removed because Lhs must be in effect */ + D->Rhs.X.Flags |= LI_MUST_REMOVE; + D->Rhs.A.Flags |= LI_MUST_REMOVE; /* Remove the push and the call to the tosltax function */ RemoveRemainders (D); @@ -1158,9 +1162,9 @@ static unsigned Opt_tossubax (StackOpData* D) /* Add code for high operand */ AddOpHigh (D, OP65_SBC, &D->Rhs, 1); - /* Rhs load entries must be removed */ - D->Rhs.X.Flags |= LI_REMOVE; - D->Rhs.A.Flags |= LI_REMOVE; + /* Rhs load entries must be removed because Lhs must be in effect */ + D->Rhs.X.Flags |= LI_MUST_REMOVE; + D->Rhs.A.Flags |= LI_MUST_REMOVE; /* Remove the push and the call to the tossubax function */ RemoveRemainders (D); @@ -1200,9 +1204,9 @@ static unsigned Opt_tosugeax (StackOpData* D) X = NewCodeEntry (OP65_ROL, AM65_ACC, "a", 0, D->OpEntry->LI); InsertEntry (D, X, D->IP++); - /* Rhs load entries must be removed */ - D->Rhs.X.Flags |= LI_REMOVE; - D->Rhs.A.Flags |= LI_REMOVE; + /* Rhs load entries must be removed because Lhs must be in effect */ + D->Rhs.X.Flags |= LI_MUST_REMOVE; + D->Rhs.A.Flags |= LI_MUST_REMOVE; /* Remove the push and the call to the tosugeax function */ RemoveRemainders (D); @@ -1246,9 +1250,9 @@ static unsigned Opt_tosugtax (StackOpData* D) X = NewCodeEntry (OP65_JSR, AM65_ABS, "boolugt", 0, D->OpEntry->LI); InsertEntry (D, X, D->IP++); - /* Rhs load entries must be removed */ - D->Rhs.X.Flags |= LI_REMOVE; - D->Rhs.A.Flags |= LI_REMOVE; + /* Rhs load entries must be removed because Lhs must be in effect */ + D->Rhs.X.Flags |= LI_MUST_REMOVE; + D->Rhs.A.Flags |= LI_MUST_REMOVE; /* Remove the push and the call to the operator function */ RemoveRemainders (D); @@ -1292,9 +1296,9 @@ static unsigned Opt_tosuleax (StackOpData* D) X = NewCodeEntry (OP65_JSR, AM65_ABS, "boolule", 0, D->OpEntry->LI); InsertEntry (D, X, D->IP++); - /* Rhs load entries must be removed */ - D->Rhs.X.Flags |= LI_REMOVE; - D->Rhs.A.Flags |= LI_REMOVE; + /* Rhs load entries must be removed because Lhs must be in effect */ + D->Rhs.X.Flags |= LI_MUST_REMOVE; + D->Rhs.A.Flags |= LI_MUST_REMOVE; /* Remove the push and the call to the operator function */ RemoveRemainders (D); @@ -1326,9 +1330,9 @@ static unsigned Opt_tosultax (StackOpData* D) X = NewCodeEntry (OP65_JSR, AM65_ABS, "boolult", 0, D->OpEntry->LI); InsertEntry (D, X, D->IP++); - /* Rhs load entries must be removed */ - D->Rhs.X.Flags |= LI_REMOVE; - D->Rhs.A.Flags |= LI_REMOVE; + /* Rhs load entries must be removed because Lhs must be in effect */ + D->Rhs.X.Flags |= LI_MUST_REMOVE; + D->Rhs.A.Flags |= LI_MUST_REMOVE; /* Remove the push and the call to the operator function */ RemoveRemainders (D); @@ -1396,8 +1400,8 @@ static unsigned Opt_a_tosbitwise (StackOpData* D, opc_t OPC) X = NewCodeEntry (OPC, D->Rhs.A.LoadEntry->AM, D->Rhs.A.LoadEntry->Arg, 0, D->OpEntry->LI); InsertEntry (D, X, D->IP++); - /* Rhs load entries must be removed */ - D->Rhs.A.Flags |= LI_REMOVE; + /* Rhs A load must be removed because Lhs must be in effect */ + D->Rhs.A.Flags |= (LI_REMOVE | LI_MUST_REMOVE); } else if (RegIsDirectNonStackLoaded (&D->Lhs.A)) { @@ -1433,10 +1437,12 @@ static unsigned Opt_a_tosbitwise (StackOpData* D, opc_t OPC) /* Since this is a "same X" EOR, the result is always 0. */ X = NewCodeEntry (OP65_LDX, AM65_IMM, MakeHexArg (0), 0, D->Rhs.X.ChgEntry->LI); InsertEntry (D, X, D->IP++); + + /* Rhs X load may be removed (but this is not a demand) */ D->Rhs.X.Flags |= LI_REMOVE; } } else { - /* Rhs load entries may be removed */ + /* Rhs X load may be removed (but this is not a demand) */ D->Rhs.X.Flags |= LI_REMOVE; } @@ -1464,7 +1470,7 @@ static unsigned Opt_a_toscmpbool (StackOpData* D, const char* BoolTransformer) /* Rhs low-byte load must be removed and hi-byte load may be removed */ D->Rhs.X.Flags |= LI_REMOVE; - D->Rhs.A.Flags |= LI_REMOVE; + D->Rhs.A.Flags |= LI_MUST_REMOVE; } else if (RegIsDirectLoaded (&D->Lhs.A)) { /* If the lhs is direct (but not stack relative), encode compares with lhs, @@ -1604,7 +1610,9 @@ static unsigned Opt_a_tosicmp (StackOpData* D) InsertEntry (D, X, D->IP++); } - /* RHS may be removed */ + /* RHS may be removed, but it is not a demand because Lhs was + ** reloaded at Op and is in effect. + */ D->Rhs.A.Flags |= LI_REMOVE; D->Rhs.X.Flags |= LI_REMOVE; } @@ -1703,9 +1711,11 @@ static unsigned Opt_a_tossub (StackOpData* D) InsertEntry (D, X, D->IP++); } - /* Rhs load entries must be removed */ + /* Rhs low-byte load must be removed (because Lhs must be in effect) + ** and hi-byte load may be removed. + */ D->Rhs.X.Flags |= LI_REMOVE; - D->Rhs.A.Flags |= LI_REMOVE; + D->Rhs.A.Flags |= LI_MUST_REMOVE; /* Remove the push and the call to the tossubax function */ RemoveRemainders (D); From 6027487c307437536bbed745eda77547190db0dd Mon Sep 17 00:00:00 2001 From: Alex Volkov Date: Thu, 26 Mar 2026 18:22:39 -0400 Subject: [PATCH 6/9] Expand bug2461 tests to other operators and Lhs/Rhs conditions --- test/val/bug2461.c | 190 +++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 176 insertions(+), 14 deletions(-) diff --git a/test/val/bug2461.c b/test/val/bug2461.c index e8340b224..77a8dff57 100644 --- a/test/val/bug2461.c +++ b/test/val/bug2461.c @@ -2,13 +2,22 @@ /* Note: The values for MASK1, MASK2, the return values of GarbleAX and the * arguments for CALC() are carefully chosen to elicit the bug. + * CALCLX() errors appear with cc65 -Osi optimizations. */ #include #define MASK1 0x000FU #define MASK2 0x00FFU -#define CALC(num, op) (((num) & (~MASK1)) op ((num) & MASK2)) +#define CALCLA(num, op) (((num) & (~MASK1)) op ((num) & MASK2)) +/* + 0x100 here invokes g_inc(), case CF_INT: if (val <= 0x300), when +** CodeSizeFactor >= 200; then g_inc() produces a single "inx" +*/ +#define CALCLX(num, op) (((num) + 0x100) op ((num) & MASK2)) +#define CALCRX(num, op) (((num) & MASK2) op ((num) << 8)) + +#define CALCX(num, op) (((num) + 0x100) op ((num) & (~MASK1))) + static unsigned Failures = 0; static unsigned TestCount = 0; @@ -16,33 +25,157 @@ static unsigned TestCount = 0; unsigned GarbleAX(void) { static const unsigned Garbage[] = { - 0x1234, 0x0000, 0x1234, 0x1234 + 0x1234, 0x1234, 0x0000, 0x1234, 0x1234, /* Lhs A: Add, Sub, And, Or, Xor */ + 0x0057, 0x0057, 0x0037, 0x0057, /* Lhs A: Eq, Neq, Gte, Lte */ + + 0x1234, 0x1234, 0x2003, 0x1002, 0x5678, /* Lhs X: Add, Sub, And, Or, Xor */ + 0x0101, 0xFF00, 0xFF00, 0xFF00, /* Lhs X: Eq, Neq, Gte, Lte */ + + 0x1234, 0x1234, 0xFFFF, 0xFFFF, 0xFFFF, /* Rhs X: Add, Sub, And, Or, Xor */ }; return Garbage[TestCount - 1]; } -unsigned WrongAdd(unsigned num) +unsigned LhsAAdd(unsigned num) { unsigned ret=GarbleAX(); - return CALC(num, +); + return CALCLA(num, +); } -unsigned WrongAnd(unsigned num) +unsigned LhsASub(unsigned num) { unsigned ret=GarbleAX(); - return CALC(num, &); + return CALCLA(num, -); } -unsigned WrongOr(unsigned num) +unsigned LhsAAnd(unsigned num) { unsigned ret=GarbleAX(); - return CALC(num, |); + return CALCLA(num, &); } -unsigned WrongXor(unsigned num) +unsigned LhsAOr(unsigned num) { unsigned ret=GarbleAX(); - return CALC(num, ^); + return CALCLA(num, |); +} + +unsigned LhsAXor(unsigned num) +{ + unsigned ret=GarbleAX(); + return CALCLA(num, ^); +} + + +unsigned LhsAEq(unsigned num) +{ + unsigned ret=GarbleAX(); + return CALCLA(num, ==); +} + +unsigned LhsANeq(unsigned num) +{ + unsigned ret=GarbleAX(); + return CALCLA(num, !=); +} + +unsigned LhsAGte(unsigned num) +{ + unsigned ret=GarbleAX(); + return CALCLA(num, >=); +} + +unsigned LhsALte(unsigned num) +{ + unsigned ret=GarbleAX(); + return CALCLA(num, <=); +} + + +unsigned LhsXAdd(unsigned num) +{ + unsigned ret=GarbleAX(); + return CALCX(num, +); +} + +unsigned LhsXSub(unsigned num) +{ + unsigned ret=GarbleAX(); + return CALCX(num, -); +} + +unsigned LhsXAnd(unsigned num) +{ + unsigned ret=GarbleAX(); + return CALCX(num, &); +} + +unsigned LhsXOr(unsigned num) +{ + unsigned ret=GarbleAX(); + return CALCLX(num, |); +} + +unsigned LhsXXor(unsigned num) +{ + unsigned ret=GarbleAX(); + return CALCLX(num, ^); +} + + +unsigned LhsXEq(unsigned num) +{ + unsigned ret=GarbleAX(); + return CALCLX(num, ==); +} + +unsigned LhsXNeq(unsigned num) +{ + unsigned ret=GarbleAX(); + return CALCX(num, !=); +} + +unsigned LhsXGte(unsigned num) +{ + unsigned ret=GarbleAX(); + return CALCLX(num, >=); +} + +unsigned LhsXLte(unsigned num) +{ + unsigned ret=GarbleAX(); + return CALCLX(num, <=); +} + + +unsigned RhsXAdd(unsigned num) +{ + unsigned ret=GarbleAX(); + return CALCRX(num, +); +} + +unsigned RhsXSub(unsigned num) +{ + unsigned ret=GarbleAX(); + return CALCRX(num, -); +} + +unsigned RhsXAnd(unsigned num) +{ + unsigned ret=GarbleAX(); + return CALCRX(num, &); +} + +unsigned RhsXOr(unsigned num) +{ + unsigned ret=GarbleAX(); + return CALCRX(num, |); +} + +unsigned RhsXXor(unsigned num) +{ + unsigned ret=GarbleAX(); + return CALCRX(num, ^); } void Test(unsigned (*F)(unsigned), unsigned Num, unsigned Ref) @@ -58,10 +191,39 @@ void Test(unsigned (*F)(unsigned), unsigned Num, unsigned Ref) int main(void) { - Test(WrongAdd, 0x4715, CALC(0x4715, +)); - Test(WrongAnd, 0x4715, CALC(0x4715, &)); - Test(WrongOr, 0x4715, CALC(0x4715, |)); - Test(WrongXor, 0x4715, CALC(0x4715, ^)); + /* Test 1+ */ + Test(LhsAAdd, 0x4715, CALCLA(0x4715, +)); + Test(LhsASub, 0x4715, CALCLA(0x4715, -)); + Test(LhsAAnd, 0x4715, CALCLA(0x4715, &)); + Test(LhsAOr, 0x4715, CALCLA(0x4715, |)); + Test(LhsAXor, 0x4715, CALCLA(0x4715, ^)); + + /* Test 6+ */ + Test(LhsAEq, 0x4750, CALCLA(0x4750, ==)); + Test(LhsANeq, 0x4750, CALCLA(0x4750, !=)); + Test(LhsAGte, 0x4750, CALCLA(0x4750, >=)); + Test(LhsALte, 0x4750, CALCLA(0x4750, <=)); + + /* Test 10+ */ + Test(LhsXAdd, 0x3F15, CALCX(0x3F15, +)); + Test(LhsXSub, 0x3F15, CALCX(0x3F15, -)); + Test(LhsXAnd, 0x3F15, CALCX(0x3F15, &)); + Test(LhsXOr, 0x3F15, CALCLX(0x3F15, |)); + Test(LhsXXor, 0x3F15, CALCLX(0x3F15, ^)); + + /* Test 15+ */ + Test(LhsXEq, 0xFF50, CALCLX(0xFF50, ==)); + Test(LhsXNeq, 0xFF50, CALCX(0xFF50, !=)); + Test(LhsXGte, 0xFF50, CALCLX(0xFF50, >=)); + Test(LhsXLte, 0xFF50, CALCLX(0xFF50, <=)); + + /* Test 19+ */ + Test(RhsXAdd, 0x3F15, CALCRX(0x3F15, +)); + Test(RhsXSub, 0x3F15, CALCRX(0x3F15, -)); + Test(RhsXAnd, 0x3F15, CALCRX(0x3F15, &)); + Test(RhsXOr, 0x3F15, CALCRX(0x3F15, |)); + Test(RhsXXor, 0x3F15, CALCRX(0x3F15, ^)); + printf("Failures: %u\n", Failures); return Failures; } From 321e47f0f34ebac0055a90c739a7d73be4b2e0fb Mon Sep 17 00:00:00 2001 From: Alex Volkov Date: Thu, 26 Mar 2026 18:32:04 -0400 Subject: [PATCH 7/9] Rhs X removals in coptstop.c should no longer be dangerous. --- src/cc65/coptstop.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/cc65/coptstop.c b/src/cc65/coptstop.c index 57985fda0..fdc1f96da 100644 --- a/src/cc65/coptstop.c +++ b/src/cc65/coptstop.c @@ -1423,12 +1423,10 @@ static unsigned Opt_a_tosbitwise (StackOpData* D, opc_t OPC) InsertEntry (D, X, D->IP++); } - /* ### Bug to come: there are dangerous Rhs X removal attempts here. - ** Runtime function calls can be "loads", and must be treated as a - ** single A/X unit, so removal of X load alone is not valid. - ** This can be solved with an additional "removable Rhs" test, or - ** by simply not forcing the LI_REMOVE flag and letting other optimizers - ** remove any unnecessary loads. + /* Note: Eager Rhs X removals here can sometimes produce slightly worse + ** code after all other optimizers have run. This may need a general + ** study of which is better: eager removal, or letting other optimizers + ** take care of unneeded loads. */ /* Do high-byte operation only when its result is used */ if ((GetRegInfo (D->Code, D->IP, REG_X) & REG_X) != 0) { From de780483193c345c4a20792ef9da763963d1c3cb Mon Sep 17 00:00:00 2001 From: Alex Volkov Date: Sun, 29 Mar 2026 17:05:20 -0400 Subject: [PATCH 8/9] Generalize the shared, non-removable reg load tests in RemoveRegLoads() to "instruction affects the other register" --- src/cc65/codeoptutil.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/cc65/codeoptutil.c b/src/cc65/codeoptutil.c index 7b81abb10..6f4288921 100644 --- a/src/cc65/codeoptutil.c +++ b/src/cc65/codeoptutil.c @@ -1252,19 +1252,19 @@ void RemoveRegLoads (StackOpData* D, LoadInfo* LI) CHECK (!CanRemoveX || LI->X.LoadEntry->OPC != OP65_JSR || CE_IsCallTo (LI->X.LoadEntry, "ldaxysp")); - /* When the A load is a runtime call and the X load cannot be removed, - ** we cannot remove the A load either. + /* When the A load insn affects X reg (e.g. ldaxysp runtime), and the X + ** load cannot be removed, we cannot remove the A load either. */ - if (CanRemoveA && LI->A.LoadEntry->OPC == OP65_JSR && !CanRemoveX) { + if (CanRemoveA && (LI->A.LoadEntry->Chg & REG_X) != 0 && !CanRemoveX) { /* A load is not removable */ LI->A.LoadEntry->Flags |= CEF_DONT_REMOVE; CanRemoveA = 0; } - /* When the X load is a runtime call and the A load cannot be removed, - ** we cannot remove the X load either. + /* When the X load insn affects A reg (e.g. ldaxysp runtime), and the A + ** load cannot be removed, we cannot remove the X load either. */ - if (CanRemoveX && LI->X.LoadEntry->OPC == OP65_JSR && !CanRemoveA) { + if (CanRemoveX && (LI->X.LoadEntry->Chg & REG_A) != 0 && !CanRemoveA) { /* X load is not removable */ LI->X.LoadEntry->Flags |= CEF_DONT_REMOVE; CanRemoveX = 0; From 69c17a502b66e4d8e217e83b61bc3cea67caaa63 Mon Sep 17 00:00:00 2001 From: Alex Volkov Date: Wed, 1 Apr 2026 18:27:28 -0400 Subject: [PATCH 9/9] Additional synthetic, extreme register load reuse tests future-proofing issues #2942 and #2947. --- test/val/pr2951.c | 244 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 244 insertions(+) create mode 100644 test/val/pr2951.c diff --git a/test/val/pr2951.c b/test/val/pr2951.c new file mode 100644 index 000000000..de56c1f05 --- /dev/null +++ b/test/val/pr2951.c @@ -0,0 +1,244 @@ + +/* bug #2942/#2947: OptStackOps() sometimes removes Lhs loads shared with Rhs. +** +** These are synthetic, extreme reuse test cases: AX unchanged by pushax. +** These do not occur with the current codegen, runtime and optimizers, but +** may occur in the future. If the runtime or codegen change sufficiently to +** break these tests, they can be deleted. +*/ + +#include +#include + +int fail = 0; + +#define TRASH_AX 0x55AA + +int stat_val; + +int stat_add_full(int trash) +{ + (void)trash; + + __AX__ = stat_val; + /* AX = AX + AX */ + asm("jsr pushax"); + asm("jsr tosaddax"); + + return __AX__; +} + +int stat_xor_full(int trash) +{ + (void)trash; + + __AX__ = stat_val; + /* AX = AX ^ AX */ + asm("jsr pushax"); + asm("jsr tosxorax"); + + return __AX__; +} + +int stat_xor_and(int trash) +{ + (void)trash; + + __AX__ = stat_val; + /* AX = AX ^ (AX & 0xFFF0) */ + asm("jsr pushax"); + asm("and #$F0"); + asm("jsr tosxorax"); + + return __AX__; +} + +int stat_or_inx(int trash) +{ + (void)trash; + + __AX__ = stat_val; + /* AX = AX | (AX + 0x100) */ + asm("jsr pushax"); + asm("inx"); + asm("jsr tosorax"); + + return __AX__; +} + +int stat_and_add(int trash) +{ + (void)trash; + + __AX__ = stat_val; + /* AX = (AX & 0xFFF0) + (AX & 0xFFF0) */ + asm("and #$F0"); + asm("jsr pushax"); + asm("jsr tosaddax"); + + return __AX__; +} + +int stat_inx_add(int trash) +{ + (void)trash; + + __AX__ = stat_val; + /* AX = (AX + 0x100) + (AX + 0x100) */ + asm("inx"); + asm("jsr pushax"); + asm("jsr tosaddax"); + + return __AX__; +} + +int stat_inx_or(int trash) +{ + (void)trash; + + __AX__ = stat_val; + /* AX = (AX + 0x100) | (AX + 0x100) */ + asm("inx"); + asm("jsr pushax"); + asm("jsr tosorax"); + + return __AX__; +} + + +int loc_add_full(int val, int trash) +{ + (void)trash; + + __AX__ = val; + /* AX = AX + AX */ + asm("jsr pushax"); + asm("jsr tosaddax"); + + return __AX__; +} + +int loc_xor_full(int val, int trash) +{ + (void)trash; + + __AX__ = val; + /* AX = AX ^ AX */ + asm("jsr pushax"); + asm("jsr tosxorax"); + + return __AX__; +} + +int loc_xor_and(int val, int trash) +{ + (void)trash; + + __AX__ = val; + /* AX = AX ^ (AX & 0xFFF0) */ + asm("jsr pushax"); + asm("and #$F0"); + asm("jsr tosxorax"); + + return __AX__; +} + +int loc_or_inx(int val, int trash) +{ + (void)trash; + + __AX__ = val; + /* AX = AX | (AX + 0x100) */ + asm("jsr pushax"); + asm("inx"); + asm("jsr tosorax"); + + return __AX__; +} + +int loc_and_add(int val, int trash) +{ + (void)trash; + + __AX__ = val; + /* AX = (AX & 0xFFF0) + (AX & 0xFFF0) */ + asm("and #$F0"); + asm("jsr pushax"); + asm("jsr tosaddax"); + + return __AX__; +} + +int loc_inx_add(int val, int trash) +{ + (void)trash; + + __AX__ = val; + /* AX = (AX + 0x100) + (AX + 0x100) */ + asm("inx"); + asm("jsr pushax"); + asm("jsr tosaddax"); + + return __AX__; +} + +int loc_inx_or(int val, int trash) +{ + (void)trash; + + __AX__ = val; + /* AX = (AX + 0x100) | (AX + 0x100) */ + asm("inx"); + asm("jsr pushax"); + asm("jsr tosorax"); + + return __AX__; +} + + +void test_stat(int (*func)(int /*trash*/), int num, int ref, const char* expr) +{ + int res; + + stat_val = num; + res = func(TRASH_AX); + if (res != ref) { + printf("Fail (stat): %s -> 0x%x\n", expr, res); + ++fail; + } +} + +void test_loc(int (*func)(int, int /*trash*/), int num, int ref, const char* expr) +{ + int res; + + res = func(num, TRASH_AX); + if (res != ref) { + printf("Fail (loc): %s -> 0x%x\n", expr, res); + ++fail; + } +} + +#define TEST_STATIC(func, num, ref) test_stat(func, num, (ref), #ref) +#define TEST_LOCAL(func, num, ref) test_loc(func, num, (ref), #ref) + +int main(void) +{ + TEST_STATIC(stat_add_full, 0x321, 0x321 + 0x321); + TEST_STATIC(stat_xor_full, 0x321, 0x321 ^ 0x321); + TEST_STATIC(stat_xor_and, 0x321, 0x321 ^ (0x321 & 0xFFF0)); + TEST_STATIC(stat_or_inx, 0x321, 0x321 | (0x321 + 0x100)); + TEST_STATIC(stat_and_add, 0x321, (0x321 & 0xFFF0) + (0x321 & 0xFFF0)); + TEST_STATIC(stat_inx_add, 0x321, (0x321 + 0x100) + (0x321 + 0x100)); + TEST_STATIC(stat_inx_or, 0x321, (0x321 + 0x100) | (0x321 + 0x100)); + + TEST_LOCAL(loc_add_full, 0x321, 0x321 + 0x321); + TEST_LOCAL(loc_xor_full, 0x321, 0x321 ^ 0x321); + TEST_LOCAL(loc_xor_and, 0x321, 0x321 ^ (0x321 & 0xFFF0)); + TEST_LOCAL(loc_or_inx, 0x321, 0x321 | (0x321 + 0x100)); + TEST_LOCAL(loc_and_add, 0x321, (0x321 & 0xFFF0) + (0x321 & 0xFFF0)); + TEST_LOCAL(loc_inx_add, 0x321, (0x321 + 0x100) + (0x321 + 0x100)); + TEST_LOCAL(loc_inx_or, 0x321, (0x321 + 0x100) | (0x321 + 0x100)); + + return fail == 0 ? EXIT_SUCCESS : EXIT_FAILURE; +}