fixed some incrdecr optimization issues

il65/emit/assignment.py

@@ -21,7 +21,8 @@ def generate_assignment(ctx: Context) -> None:
 
 
 def generate_aug_assignment(ctx: Context) -> None:
-    # for instance: value += 3  (value = 0-255 for now)
+    # for instance: value += 33
+    # (note that with += and -=, values 0..255 usually occur as the more efficient incrdecr statements instead)
     # left: Register, SymbolName, or Dereference. right: Expression/LiteralValue
     out = ctx.out
     stmt = ctx.stmt
@@ -36,7 +37,7 @@ def generate_aug_assignment(ctx: Context) -> None:
                     if stmt.operator not in ("<<=", ">>=") or rvalue.value != 0:
                         _generate_aug_reg_int(out, lvalue, stmt.operator, rvalue.value, "", ctx.scope)
                 else:
-                    raise CodeError("aug. assignment value must be 0..255", rvalue)
+                    raise CodeError("aug. assignment value must be 0..255", rvalue)    # @todo value > 255
             else:
                 raise CodeError("constant integer literal or variable required for now", rvalue)   # XXX
         elif isinstance(rvalue, SymbolName):
@@ -46,7 +47,7 @@ def generate_aug_assignment(ctx: Context) -> None:
                     if symdef.datatype.isinteger() and 0 <= symdef.value.const_value() <= 255:  # type: ignore
                         _generate_aug_reg_int(out, lvalue, stmt.operator, symdef.value.const_value(), "", ctx.scope)   # type: ignore
                     else:
-                        raise CodeError("aug. assignment value must be integer 0..255", rvalue)
+                        raise CodeError("aug. assignment value must be integer 0..255", rvalue)  # @todo value > 255
                 elif symdef.datatype == DataType.BYTE:
                     _generate_aug_reg_int(out, lvalue, stmt.operator, 0, symdef.name, ctx.scope)
                 else:
@@ -60,7 +61,7 @@ def generate_aug_assignment(ctx: Context) -> None:
         elif isinstance(rvalue, Dereference):
             print("warning: {}: using indirect/dereferece is very costly".format(rvalue.sourceref))
             if rvalue.datatype != DataType.BYTE:
-                raise CodeError("aug. assignment value must be a byte, 0..255", rvalue)
+                raise CodeError("aug. assignment value must be a byte, 0..255", rvalue)  # @todo value > 255
             if isinstance(rvalue.operand, (LiteralValue, SymbolName)):
                 if isinstance(rvalue.operand, LiteralValue):
                     what = to_hex(rvalue.operand.value)

il65/emit/incrdecr.py

@@ -1,14 +1,14 @@
 """
 Programming Language for 6502/6510 microprocessors, codename 'Sick'
 This is the code generator for the in-place incr and decr instructions.
-Incrementing or decrementing variables by a small value 0..255 (for integers)
+Incrementing or decrementing variables by a small byte value 0..255
 is quite frequent and this generates assembly code tweaked for this case.
 
 Written by Irmen de Jong (irmen@razorvine.net) - license: GNU GPL 3.0
 """
 
-from ..plyparse import VarType, VarDef, Register, IncrDecr, SymbolName, Dereference, LiteralValue, datatype_of
-from ..datatypes import DataType, REGISTER_BYTES
+from ..plyparse import VarDef, Register, IncrDecr, SymbolName, Dereference, LiteralValue
+from ..datatypes import VarType, DataType, REGISTER_BYTES
 from . import CodeError, preserving_registers, to_hex, Context, scoped_name
 
 
@@ -17,10 +17,10 @@ def generate_incrdecr(ctx: Context) -> None:
     stmt = ctx.stmt
     scope = ctx.scope
     assert isinstance(stmt, IncrDecr)
-    assert isinstance(stmt.howmuch, (int, float)) and stmt.howmuch >= 0
-    assert stmt.operator in ("++", "--")
     if stmt.howmuch == 0:
         return
+    if not 0 <= stmt.howmuch <= 255:
+        raise CodeError("incr/decr value must be 0..255 - other values should have been converted into an AugAssignment")
     target = stmt.target        # one of Register/SymbolName/Dereference, or a VarDef
     if isinstance(target, SymbolName):
         symdef = scope.lookup(target.name)
@@ -28,10 +28,6 @@ def generate_incrdecr(ctx: Context) -> None:
             target = symdef     # type: ignore
         else:
             raise CodeError("cannot incr/decr this", symdef)
-
-    if stmt.howmuch > 255:
-        if datatype_of(target, scope) != DataType.FLOAT:
-            raise CodeError("only supports integer incr/decr by up to 255 for now")
     howmuch_str = str(stmt.howmuch)
 
     if isinstance(target, Register):

il65/optimize.py

@@ -39,7 +39,6 @@ class Optimizer:
         self.create_aug_assignments()
         self.optimize_assignments()
         self.remove_superfluous_assignments()
-        self.combine_assignments_into_multi()
         # @todo optimize addition with self into shift 1  (A+=A -> A<<=1)
         self.optimize_goto_compare_with_zero()
         self.join_incrdecrs()
@@ -48,9 +47,54 @@ class Optimizer:
         # @todo analyse for unreachable code and remove that (f.i. code after goto or return that has no label so can never be jumped to)
 
     def join_incrdecrs(self) -> None:
+        def combine(incrdecrs: List[IncrDecr], scope: Scope) -> None:
+            # combine the separate incrdecrs
+            replaced = False
+            total = 0
+            for i in incrdecrs:
+                if i.operator == "++":
+                    total += i.howmuch
+                else:
+                    total -= i.howmuch
+            if total == 0:
+                replaced = True
+                for x in incrdecrs:
+                    scope.remove_node(x)
+            else:
+                is_float = False
+                if isinstance(target, SymbolName):
+                    symdef = target.my_scope().lookup(target.name)
+                    is_float = isinstance(symdef, VarDef) and symdef.datatype == DataType.FLOAT
+                elif isinstance(target, Dereference):
+                    is_float = target.datatype == DataType.FLOAT
+                if is_float or -255 <= total <= 255:
+                    replaced = True
+                    for x in incrdecrs[1:]:
+                        scope.remove_node(x)
+                    incrdecr = self._make_incrdecr(incrdecrs[0], target, abs(total), "++" if total >= 0 else "--")
+                    scope.replace_node(incrdecrs[0], incrdecr)
+                else:
+                    # total is > or < than 255, make an augmented assignment out of it instead of an incrdecr
+                    aug_assign = AugAssignment(operator="-=" if total < 0 else "+=", sourceref=incrdecrs[0].sourceref)  # type: ignore
+                    left = incrdecrs[0].target
+                    right = LiteralValue(value=abs(total), sourceref=incrdecrs[0].sourceref)  # type: ignore
+                    left.parent = aug_assign
+                    right.parent = aug_assign
+                    aug_assign.nodes.append(left)
+                    aug_assign.nodes.append(right)
+                    aug_assign.mark_lhs()
+                    replaced = True
+                    for x in incrdecrs[1:]:
+                        scope.remove_node(x)
+                    scope.replace_node(incrdecrs[0], aug_assign)
+            if replaced:
+                self.optimizations_performed = True
+                self.num_warnings += 1
+                print_warning("{}: merged a sequence of incr/decrs or augmented assignments".format(incrdecrs[0].sourceref))
+
         for scope in self.module.all_nodes(Scope):
-            incrdecrs = []      # type: List[IncrDecr]
             target = None
+            incrdecrs = []  # type: List[IncrDecr]
             for node in list(scope.nodes):
                 if isinstance(node, IncrDecr):
                     if target is None:
@@ -61,54 +105,17 @@ class Optimizer:
                         incrdecrs.append(node)
                         continue
                 if len(incrdecrs) > 1:
-                    # optimize...
-                    replaced = False
-                    total = 0
-                    for i in incrdecrs:
-                        if i.operator == "++":
-                            total += i.howmuch
-                        else:
-                            total -= i.howmuch
-                    if total == 0:
-                        replaced = True
-                        for x in incrdecrs:
-                            scope.remove_node(x)
-                    else:
-                        is_float = False
-                        if isinstance(target, SymbolName):
-                            symdef = target.my_scope().lookup(target.name)
-                            if isinstance(symdef, VarDef) and symdef.datatype == DataType.FLOAT:
-                                is_float = True
-                        elif isinstance(target, Dereference):
-                            is_float = target.datatype == DataType.FLOAT
-                        if is_float:
-                            replaced = True
-                            for x in incrdecrs[1:]:
-                                scope.remove_node(x)
-                            incrdecr = self._make_incrdecr(incrdecrs[0], target, abs(total), "++" if total >= 0 else "--")
-                            scope.replace_node(incrdecrs[0], incrdecr)
-                        elif 0 < total <= 255:
-                            replaced = True
-                            for x in incrdecrs[1:]:
-                                scope.remove_node(x)
-                            incrdecr = self._make_incrdecr(incrdecrs[0], target, total, "++")
-                            scope.replace_node(incrdecrs[0], incrdecr)
-                        elif -255 <= total < 0:
-                            replaced = True
-                            total = -total
-                            for x in incrdecrs[1:]:
-                                scope.remove_node(x)
-                            incrdecr = self._make_incrdecr(incrdecrs[0], target, total, "--")
-                            scope.replace_node(incrdecrs[0], incrdecr)
-                    if replaced:
-                        self.optimizations_performed = True
-                        self.num_warnings += 1
-                        print_warning("{}: merged a sequence of incr/decrs or augmented assignments".format(incrdecrs[0].sourceref))
+                    combine(incrdecrs, scope)   # type: ignore
                 incrdecrs.clear()
                 target = None
                 if isinstance(node, IncrDecr):
-                    incrdecrs.append(node)
-                    target = node.target
+                    # it was an incrdecr with a different target than what we had gathered so far.
+                    if target is None:
+                        target = node.target
+                        incrdecrs.append(node)
+            if len(incrdecrs) > 1:
+                # combine remaining incrdecrs at the bottom of the block
+                combine(incrdecrs, scope)   # type: ignore
 
     def _same_target(self, node1: Union[Register, SymbolName, Dereference],
                      node2: Union[Register, SymbolName, Dereference]) -> bool:
@@ -181,7 +188,7 @@ class Optimizer:
     def optimize_assignments(self) -> None:
         # remove assignment statements that do nothing (A=A)
         # remove augmented assignments that have no effect (x+=0, x-=0, x/=1, x//=1, x*=1)
-        # convert augmented assignments to simple incr/decr if possible (A+=10 =>  A++ by 10)
+        # convert augmented assignments to simple incr/decr if value allows it (A+=10 =>  incr A by 10)
         # simplify some calculations (x*=0, x**=0) to simple constant value assignment
         # @todo remove or simplify logical aug assigns like A |= 0, A |= true, A |= false  (or perhaps turn them into byte values first?)
         for assignment in self.module.all_nodes():
@@ -273,33 +280,6 @@ class Optimizer:
         a.parent = old_stmt.parent
         return a
 
-    def combine_assignments_into_multi(self) -> None:
-        # fold multiple consecutive assignments with the same rvalue into one multi-assignment
-        for scope in self.module.all_nodes(Scope):
-            rvalue = None
-            assignments = []    # type: List[Assignment]
-            for stmt in list(scope.nodes):
-                if isinstance(stmt, Assignment):
-                    if assignments:
-                        if stmt.right == rvalue:
-                            assignments.append(stmt)
-                            continue
-                        elif len(assignments) > 1:
-                            # replace the first assignment by a multi-assign with all the others
-                            for assignment in assignments[1:]:
-                                print("{}: joined with previous assignment".format(assignment.sourceref))
-                                assignments[0].left.nodes.extend(assignment.left.nodes)
-                                scope.remove_node(assignment)
-                                self.optimizations_performed = True
-                            rvalue = None
-                            assignments.clear()
-                    else:
-                        rvalue = stmt.right
-                        assignments.append(stmt)
-                else:
-                    rvalue = None
-                    assignments.clear()
-
     @no_type_check
     def remove_unused_subroutines(self) -> None:
         # some symbols are used by the emitted assembly code from the code generator,

il65/plyparse.py

@@ -22,7 +22,7 @@ __all__ = ["ProgramFormat", "ZpOptions", "math_functions", "builtin_functions",
            "UndefinedSymbolError", "AstNode", "Directive", "Scope", "Block", "Module", "Label", "Expression",
            "Register", "Subroutine", "LiteralValue", "AddressOf", "SymbolName", "Dereference", "IncrDecr",
            "ExpressionWithOperator", "Goto", "SubCall", "VarDef", "Return", "Assignment", "AugAssignment",
-           "InlineAssembly", "BuiltinFunction", "TokenFilter", "parser", "connect_parents",
+           "InlineAssembly", "BuiltinFunction", "TokenFilter", "parser", "connect_parents", "DatatypeNode",
            "parse_file", "coerce_constant_value", "datatype_of", "check_symbol_definition", "NotCompiletimeConstantError"]
 
 
@@ -546,7 +546,8 @@ class Dereference(Expression):
 
 @attr.s(cmp=False)
 class IncrDecr(AstNode):
-    # increment or decrement something by a CONSTANT value (1 or more)
+    # increment or decrement something by a small CONSTANT value (1..255)
+    # larger values will be treated/converted as an augmented assignment += or -=.
     # one subnode: target (Register, SymbolName, or Dereference).
     operator = attr.ib(type=str, validator=attr.validators.in_(["++", "--"]))
     howmuch = attr.ib(default=1)
@@ -567,7 +568,7 @@ class IncrDecr(AstNode):
         self.nodes.append(target)
 
     def __attrs_post_init__(self):
-        # make sure the amount is always >= 0
+        # make sure the amount is always >= 0, flip the operator if needed
         if self.howmuch < 0:
             self.howmuch = -self.howmuch
             self.operator = "++" if self.operator == "--" else "--"

tests/test_core.py

@@ -1,6 +1,6 @@
 import pytest
-from il65.datatypes import DataType, VarType, STRING_DATATYPES, FLOAT_MAX_POSITIVE, FLOAT_MAX_NEGATIVE, char_to_bytevalue
-from il65.plyparse import coerce_constant_value, LiteralValue, Scope, AddressOf, SymbolName, VarDef
+from il65.datatypes import DataType, STRING_DATATYPES, FLOAT_MAX_POSITIVE, FLOAT_MAX_NEGATIVE, char_to_bytevalue
+from il65.plyparse import coerce_constant_value, LiteralValue, Scope, SymbolName, VarDef
 from il65.compile import ParseError
 from il65.plylex import SourceRef
 from il65.emit import to_hex, to_mflpt5

tests/test_optimizer.py

@@ -0,0 +1,92 @@
+import pytest
+from il65.plyparse import IncrDecr, AugAssignment, VarDef, SymbolName
+from il65.optimize import optimize
+from .test_parser import parse_source
+
+
+def test_incrdecr_joins_nonfloat():
+    src = """~ test {
+        X ++
+        X ++
+        X += 10
+        Y--
+        Y--
+        Y-=20
+    }"""
+    result = parse_source(src)
+    testscope = result.scope.nodes[0].nodes[0]
+    assert len(testscope.nodes) == 6
+    assert isinstance(testscope.nodes[0], IncrDecr)
+    assert testscope.nodes[0].howmuch == 1
+    assert isinstance(testscope.nodes[1], IncrDecr)
+    assert testscope.nodes[1].howmuch == 1
+    assert isinstance(testscope.nodes[2], AugAssignment)
+    assert testscope.nodes[2].right.value == 10
+    assert isinstance(testscope.nodes[3], IncrDecr)
+    assert testscope.nodes[3].howmuch == 1
+    assert isinstance(testscope.nodes[4], IncrDecr)
+    assert testscope.nodes[4].howmuch == 1
+    assert isinstance(testscope.nodes[5], AugAssignment)
+    assert testscope.nodes[5].right.value == 20
+    # now optimize the incrdecrs (joins them)
+    optimize(result)
+    testscope = result.scope.nodes[0].nodes[0]
+    assert len(testscope.nodes) == 2                    # @todo broken optimization right now
+    assert isinstance(testscope.nodes[0], IncrDecr)
+    assert testscope.nodes[0].operator == "++"
+    assert testscope.nodes[0].howmuch == 12
+    assert isinstance(testscope.nodes[1], IncrDecr)
+    assert testscope.nodes[1].operator == "--"
+    assert testscope.nodes[1].howmuch == 22
+
+
+def test_incrdecr_joins_float():
+    src = """~ test {
+        var .float flt = 0
+        flt ++
+        flt ++
+        flt += 10
+        flt --
+        flt --
+        flt --
+        flt -= 5 
+    }"""
+    result = parse_source(src)
+    testscope = result.scope.nodes[0].nodes[0]
+    assert len(testscope.nodes) == 8
+    # now optimize the incrdecrs (joins them)
+    optimize(result)
+    testscope = result.scope.nodes[0].nodes[0]
+    assert len(testscope.nodes) == 2
+    assert isinstance(testscope.nodes[0], VarDef)
+    assert isinstance(testscope.nodes[1], IncrDecr)
+    assert testscope.nodes[1].operator == "++"
+    assert testscope.nodes[1].howmuch == 4
+    assert isinstance(testscope.nodes[1].target, SymbolName)
+    assert testscope.nodes[1].target.name == "flt"
+
+
+def test_large_incrdecr_to_augassign():
+    src = """~ test {
+        X ++
+        X ++
+        X += 255
+        Y --
+        Y --
+        Y -= 255
+    }"""
+    result = parse_source(src)
+    testscope = result.scope.nodes[0].nodes[0]
+    assert len(testscope.nodes) == 6
+    # now optimize; joins the incrdecrs then converts to augassign because values are too large.
+    optimize(result)
+    testscope = result.scope.nodes[0].nodes[0]
+    assert len(testscope.nodes) == 2
+    assert isinstance(testscope.nodes[0], AugAssignment)
+    assert testscope.nodes[0].left.name == "X"
+    assert testscope.nodes[0].operator == "+="
+    assert testscope.nodes[0].right.value == 257
+    assert isinstance(testscope.nodes[1], AugAssignment)
+    assert testscope.nodes[1].left.name == "Y"
+    assert testscope.nodes[1].operator == "-="
+    assert testscope.nodes[1].right.value == 257

tests/test_parser.py

@@ -165,8 +165,7 @@ def test_block_nodes():
 
 
 def test_parser_2():
-    src = """
-    ~ {
+    src = """~ test {
         999(1,2)
         [zz]()
     }
@@ -186,8 +185,7 @@ def test_parser_2():
 
 
 def test_typespec():
-    src = """
-    ~ {
+    src = """~ test {
         [$c000.word] = 5
         [$c000 .byte] = 5
         [AX .word] = 5
@@ -232,8 +230,7 @@ def test_typespec():
 
 
 def test_char_string():
-    src = """
-    ~ {
+    src = """~ test {
         var x1 = '@'
         var x2 = 'π'
         var x3 = 'abc'
@@ -255,8 +252,7 @@ def test_char_string():
 
 
 def test_boolean_int():
-    src = """
-    ~ {
+    src = """~ test {
         var x1 = true
         var x2 = false
         A = true
@@ -272,7 +268,7 @@ def test_boolean_int():
     assert type(assgn2.right.value) is int and assgn2.right.value == 0
 
 
-def test_incrdecr():
+def test_incrdecr_operators():
     sref = SourceRef("test", 1, 1)
     with pytest.raises(ValueError):
         IncrDecr(operator="??", sourceref=sref)
@@ -350,8 +346,7 @@ def test_symbol_lookup():
 
 
 def test_const_numeric_expressions():
-    src = """
-~ {
+    src = """~ test {
         A = 1+2+3+4+5
         X = 1+2*5+2
         Y = (1+2)*(5+2)
@@ -391,8 +386,7 @@ def test_const_numeric_expressions():
 
 
 def test_const_logic_expressions():
-    src = """
-~ {
+    src = """~ test {
         A = true or false
         X = true and false
         Y = true xor false
@@ -420,8 +414,7 @@ def test_const_logic_expressions():
 
 
 def test_const_other_expressions():
-    src = """
-~ {
+    src = """~ test {
         memory memvar = $c123
         A = &memvar     ; constant
         X = &sin        ; non-constant
@@ -444,8 +437,7 @@ def test_const_other_expressions():
 
 
 def test_vdef_const_folds():
-    src = """
-~ {
+    src = """~ test {
     const  cb1 = 123
     const  cb2 = cb1
     const  cb3 = cb1*3
@@ -490,8 +482,7 @@ def test_vdef_const_folds():
 
 
 def test_vdef_const_expressions():
-    src = """
-~ {
+    src = """~ test {
     var bvar = 99
     var .float fvar = sin(1.2-0.3)
     var .float flt2 = -9.87e-6

tests/test_vardef.py

@@ -1,6 +1,7 @@
 import pytest
-from il65.datatypes import DataType
-from il65.plyparse import LiteralValue, VarDef, VarType, DatatypeNode, ExpressionWithOperator, Scope, AddressOf, SymbolName, UndefinedSymbolError
+from il65.datatypes import DataType, VarType
+from il65.plyparse import (LiteralValue, VarDef, DatatypeNode, ExpressionWithOperator,
+                           Scope, AddressOf, SymbolName, UndefinedSymbolError)
 from il65.plylex import SourceRef