Merge pull request #246 from greg-king5/shift-signed-char

Fix a signed char right-shift compiler optimization.

src/cc65/coptshift.c

@@ -307,59 +307,87 @@ NextEntry:
 
 
 
-unsigned OptShift2(CodeSeg* S)
-/* A call to the asrax1 routines may get replaced by something simpler, if
-** X is not used later:
+unsigned OptShift2 (CodeSeg* S)
+/* The sequence
+**
+**      bpl     L
+**      dex
+** L:   jsr     asraxN
+**
+** might be replaced by N copies of
 **
 **      cmp     #$80
 **      ror     a
+**
+** if X is not used later (X is assumed to be zero on entry).
+** If the sequence is followed immediately by another
+**
+**      jsr     asraxN
+**
+** then their shifts are combined.
 */
 {
     unsigned Changes = 0;
-    unsigned I;
+    unsigned I = 0;
 
     /* Walk over the entries */
-    I = 0;
     while (I < CS_GetEntryCount (S)) {
-
         unsigned Shift;
-        unsigned Count;
+        unsigned Count, Count2;
+        unsigned K;
+        CodeEntry* L[4];
 
         /* Get next entry */
-        CodeEntry* E = CS_GetEntry (S, I);
+        L[0] = CS_GetEntry (S, I);
 
         /* Check for the sequence */
-        if (E->OPC == OP65_JSR                          &&
-            (Shift = GetShift (E->Arg)) != SHIFT_NONE   &&
-            SHIFT_TYPE (Shift) == SHIFT_TYPE_ASR        &&
-            (Count = SHIFT_COUNT (Shift)) > 0           &&
-            Count * 100 <= S->CodeSizeFactor    &&
-            !RegXUsed (S, I+1)) {
+        if ((L[0]->OPC == OP65_BPL || L[0]->OPC == OP65_BCC)            &&
+            L[0]->JumpTo != 0                                           &&
+            CS_GetEntries (S, L+1, I+1, 3)                              &&
+            L[1]->OPC == OP65_DEX                                       &&
+            L[0]->JumpTo->Owner == L[2]                                 &&
+            !CS_RangeHasLabel (S, I, 2)                                 &&
+            L[2]->OPC == OP65_JSR                                       &&
+            SHIFT_TYPE (Shift = GetShift (L[2]->Arg)) == SHIFT_TYPE_ASR &&
+            (Count = SHIFT_COUNT (Shift)) > 0) {
 
-            CodeEntry* X;
-            unsigned J = I+1;
+            if (L[3]->OPC == OP65_JSR                                           &&
+                SHIFT_TYPE (Shift = GetShift (L[3]->Arg)) == SHIFT_TYPE_ASR     &&
+                (Count2 = SHIFT_COUNT (Shift)) > 0) {
 
-            /* Generate the replacement sequence */
-            while (Count--) {
-                /* cmp #$80 */
-                X = NewCodeEntry (OP65_CMP, AM65_IMM, "$80", 0, E->LI);
-                CS_InsertEntry (S, X, J++);
-
-                /* ror a */
-                X = NewCodeEntry (OP65_ROR, AM65_ACC, "a", 0, E->LI);
-                CS_InsertEntry (S, X, J++);
+                /* Found a second jsr asraxN */
+                Count += Count2;
+                K = 4;
+            } else {
+                K = 3;
             }
+            if (Count * 100 <= S->CodeSizeFactor        &&
+                !RegXUsed (S, I+K)) {
 
-            /* Delete the call to asrax */
-            CS_DelEntry (S, I);
+                CodeEntry* X;
+                unsigned J = I+K;
 
-            /* Remember, we had changes */
-            ++Changes;
+                /* Generate the replacement sequence */
+                do {
+                    /* cmp #$80 */
+                    X = NewCodeEntry (OP65_CMP, AM65_IMM, "$80", 0, L[2]->LI);
+                    CS_InsertEntry (S, X, J++);
+
+                    /* ror a */
+                    X = NewCodeEntry (OP65_ROR, AM65_ACC, "a", 0, L[2]->LI);
+                    CS_InsertEntry (S, X, J++);
+                } while (--Count);
+
+                /* Remove the bpl/dex/jsr */
+                CS_DelEntries (S, I, K);
+
+                /* Remember, we had changes */
+                ++Changes;
+            }
         }
 
         /* Next entry */
         ++I;
-
     }
 
     /* Return the number of changes made */
@@ -412,7 +440,7 @@ unsigned OptShift3 (CodeSeg* S)
             (Shift = GetShift (L[2]->Arg)) != SHIFT_NONE        &&
             SHIFT_DIR (Shift) == SHIFT_DIR_RIGHT                &&
             (Count = SHIFT_COUNT (Shift)) > 0) {
-                                                
+
             /* Add the replacement insn instead */
             CodeEntry* X = NewCodeEntry (OP65_ROR, AM65_ACC, "a", 0, L[2]->LI);
             CS_InsertEntry (S, X, I+3);
@@ -421,7 +449,7 @@ unsigned OptShift3 (CodeSeg* S)
                 CS_InsertEntry (S, X, I+4);
             }
 
-            /* Remove the bcs/dex/jsr */
+            /* Remove the bcc/inx/jsr */
             CS_DelEntries (S, I, 3);
 
             /* Remember, we had changes */

src/cc65/coptshift.h

@@ -60,12 +60,19 @@ unsigned OptShift1 (CodeSeg* S);
 **  L1:
 */
 
-unsigned OptShift2(CodeSeg* S);
-/* A call to the asrax1 routines may get replaced by something simpler, if
-** X is not used later:
+unsigned OptShift2 (CodeSeg* S);
+/* The sequence
+**
+**      bpl     L
+**      dex
+** L:   jsr     asraxN
+**
+** might be replaced by N copies of
 **
 **      cmp     #$80
 **      ror     a
+**
+** if X is not used later (X is assumed to be zero on entry).
 */
 
 unsigned OptShift3 (CodeSeg* S);

test/val/rotate8.c

@@ -0,0 +1,156 @@
+/*
+  !!DESCRIPTION!! Optimized-shift signed ints right by a constant; and, assign to chars.
+  !!ORIGIN!!      cc65 regression tests
+  !!LICENCE!!     Public Domain
+  !!AUTHOR!!      Greg King
+*/
+
+#include <stdio.h>
+
+static unsigned char failures = 0;
+static unsigned char n = 0;
+
+/* This number must be read from a variable because
+** we want this program, not cc65, to do the shift.
+*/
+static const signed int aint0 = 0xAAC0;
+
+static signed char achar0, achar1;
+
+static void check(void)
+{
+    if ((unsigned char)achar0 != (unsigned char)achar1)
+        ++failures;
+}
+
+static void shift_right_0(void)
+{
+    achar0 = aint0 >> 0;
+    check();
+}
+
+static void shift_right_1(void)
+{
+    achar0 = aint0 >> 1;
+    check();
+}
+
+static void shift_right_2(void)
+{
+    achar0 = aint0 >> 2;
+    check();
+}
+
+static void shift_right_3(void)
+{
+    achar0 = aint0 >> 3;
+    check();
+}
+
+static void shift_right_4(void)
+{
+    achar0 = aint0 >> 4;
+    check();
+}
+
+static void shift_right_5(void)
+{
+    achar0 = aint0 >> 5;
+    check();
+}
+
+static void shift_right_6(void)
+{
+    achar0 = aint0 >> 6;
+    check();
+}
+
+static void shift_right_7(void)
+{
+    achar0 = aint0 >> 7;
+    check();
+}
+
+static void shift_right_8(void)
+{
+    achar0 = aint0 >> 8;
+    check();
+}
+
+static void shift_right_9(void)
+{
+    achar0 = aint0 >> 9;
+    check();
+}
+
+static void shift_right_10(void)
+{
+    achar0 = aint0 >> 10;
+    check();
+}
+
+static void shift_right_11(void)
+{
+    achar0 = aint0 >> 11;
+    check();
+}
+
+static void shift_right_12(void)
+{
+    achar0 = aint0 >> 12;
+    check();
+}
+
+static void shift_right_13(void)
+{
+    achar0 = aint0 >> 13;
+    check();
+}
+
+static void shift_right_14(void)
+{
+    achar0 = aint0 >> 14;
+    check();
+}
+
+static void shift_right_15(void)
+{
+    achar0 = aint0 >> 15;
+    check();
+}
+
+const struct {
+    signed char achar;
+    void (*func)(void);
+} tests[] = {
+    {0xC0, shift_right_0},
+    {0x60, shift_right_1},
+    {0xB0, shift_right_2},
+    {0x58, shift_right_3},
+    {0xAC, shift_right_4},
+    {0x56, shift_right_5},
+    {0xAB, shift_right_6},
+    {0x55, shift_right_7},
+    {0xAA, shift_right_8},
+    {0xD5, shift_right_9},
+    {0xEA, shift_right_10},
+    {0xF5, shift_right_11},
+    {0xFA, shift_right_12},
+    {0xFD, shift_right_13},
+    {0xFE, shift_right_14},
+    {0xFF, shift_right_15}
+};
+
+int main(void)
+{
+    do {
+        achar1 = tests[n].achar;
+        tests[n].func();
+    } while (++n < sizeof tests / sizeof tests[0]);
+
+    if (failures) {
+        printf("rotate8: failures: %u (of %u).\n",
+               failures, sizeof tests / sizeof tests[0]);
+    }
+    return failures;
+}