fix problematic optimizations to logical expressions

compiler/src/prog8/compiler/astprocessing/AstPreprocessor.kt

@@ -115,17 +115,6 @@ class AstPreprocessor(val program: Program, val errors: IErrorReporter, val comp
         return noModifications
     }
 
-    override fun after(expr: PrefixExpression, parent: Node): Iterable<IAstModification> {
-        if(expr.operator == "not") {
-            // not(x)  -->  x==0
-            // this means that "not" will never occur anywhere again in the ast after this
-            val dt = expr.expression.inferType(program).getOr(DataType.UBYTE)
-            val replacement = BinaryExpression(expr.expression, "==", NumericLiteral(dt,0.0, expr.position), expr.position)
-            return listOf(IAstModification.ReplaceNodeSafe(expr, replacement, parent))
-        }
-        return noModifications
-    }
-
     override fun after(decl: VarDecl, parent: Node): Iterable<IAstModification> {
         val nextAssignment = decl.nextSibling() as? Assignment
         if(nextAssignment!=null && nextAssignment.origin!=AssignmentOrigin.VARINIT) {

compiler/src/prog8/compiler/astprocessing/NotExpressionChanger.kt

@@ -8,6 +8,7 @@ import prog8.ast.expressions.NumericLiteral
 import prog8.ast.expressions.PrefixExpression
 import prog8.ast.walk.AstWalker
 import prog8.ast.walk.IAstModification
+import prog8.code.core.DataType
 import prog8.code.core.IErrorReporter
 import prog8.code.core.IntegerDatatypes
 
@@ -39,54 +40,6 @@ internal class NotExpressionChanger(val program: Program, val errors: IErrorRepo
                 }
             }
         }
-
-        val left = expr.left as? BinaryExpression
-        val right = expr.right as? BinaryExpression
-        val leftValue = left?.right?.constValue(program)?.number
-        val rightValue = right?.right?.constValue(program)?.number
-
-        if(expr.operator == "or") {
-            if(left?.operator=="==" && right?.operator=="==" && leftValue==0.0 && rightValue==0.0) {
-                // (a==0) or (b==0) -> (a and b)==0
-                val orExpr = BinaryExpression(left.left, "and", right.left, expr.position)
-                val equalsZero = BinaryExpression(orExpr, "==", NumericLiteral.fromBoolean(false, expr.position), expr.position)
-                return listOf(IAstModification.ReplaceNode(expr, equalsZero, parent))
-            }
-        }
-        else if(expr.operator == "and") {
-            if(left?.operator=="==" && right?.operator=="==" && leftValue==0.0 && rightValue==0.0) {
-                // (a==0) and (b==0) -> (a or b)==0
-                val orExpr = BinaryExpression(left.left, "or", right.left, expr.position)
-                val equalsZero = BinaryExpression(orExpr, "==", NumericLiteral.fromBoolean(false, expr.position), expr.position)
-                return listOf(IAstModification.ReplaceNode(expr, equalsZero, parent))
-            }
-        }
-        return noModifications
-    }
-
-    override fun after(expr: BinaryExpression, parent: Node): Iterable<IAstModification> {
-        // not a or not b  -> not(a and b)
-        if(expr.operator=="or") {
-            val left = expr.left as? PrefixExpression
-            val right = expr.right as? PrefixExpression
-            if(left?.operator=="not" && right?.operator=="not") {
-                val andExpr = BinaryExpression(left.expression, "and", right.expression, expr.position)
-                val notExpr = PrefixExpression("not", andExpr, expr.position)
-                return listOf(IAstModification.ReplaceNode(expr, notExpr, parent))
-            }
-        }
-
-        // not a and not b -> not(a or b)
-        if(expr.operator=="and") {
-            val left = expr.left as? PrefixExpression
-            val right = expr.right as? PrefixExpression
-            if(left?.operator=="not" && right?.operator=="not") {
-                val andExpr = BinaryExpression(left.expression, "or", right.expression, expr.position)
-                val notExpr = PrefixExpression("not", andExpr, expr.position)
-                return listOf(IAstModification.ReplaceNode(expr, notExpr, parent))
-            }
-        }
-
         return noModifications
     }
 
@@ -116,6 +69,12 @@ internal class NotExpressionChanger(val program: Program, val errors: IErrorRepo
                     return listOf(IAstModification.ReplaceNode(expr, subBinExpr, parent))
                 }
             }
+
+            // all other not(x)  -->  x==0
+            // this means that "not" will never occur anywhere again in the ast after this
+            val dt = expr.expression.inferType(program).getOr(DataType.UBYTE)
+            val replacement = BinaryExpression(expr.expression, "==", NumericLiteral(dt,0.0, expr.position), expr.position)
+            return listOf(IAstModification.ReplaceNodeSafe(expr, replacement, parent))
         }
         return noModifications
     }

compiler/test/TestOptimization.kt

@@ -251,15 +251,15 @@ class TestOptimization: FunSpec({
         (initY2.value as NumericLiteral).number shouldBe 11.0
     }
 
-    xtest("various 'not' operator rewrites even without optimizations on") {
+    test("various 'not' operator rewrites even without optimizations on") {
         val src = """
             main {
                 sub start() {
                     ubyte a1
                     ubyte a2
                     a1 = not not a1             ; a1 = a1==0
-                    a1 = not a1 or not a2       ; a1 = (a1 and a2)==0
+                    a1 = not a1 or not a2       ; a1 = a1==0 | a2==0
-                    a1 = not a1 and not a2      ; a1 = (a1 or a2)==0
+                    a1 = not a1 and not a2      ; a1 = a1==0 & a2==0
                 }
             }
         """
@@ -273,20 +273,8 @@ class TestOptimization: FunSpec({
         val value3 = (stmts[6] as Assignment).value as BinaryExpression
         value1.operator shouldBe "=="
         value1.right shouldBe NumericLiteral(DataType.UBYTE, 0.0, Position.DUMMY)
-        value2.operator shouldBe "=="
+        value2.operator shouldBe "|"
-        value2.right shouldBe NumericLiteral(DataType.UBYTE, 0.0, Position.DUMMY)
+        value3.operator shouldBe "&"
-        value3.operator shouldBe "=="
-        value3.right shouldBe NumericLiteral(DataType.UBYTE, 0.0, Position.DUMMY)
-        val left1 = value1.left as IdentifierReference
-        val left2 = value2.left as BinaryExpression
-        val left3 = value3.left as BinaryExpression
-        left1.nameInSource shouldBe listOf("a1")
-        left2.operator shouldBe "and"
-        (left2.left as IdentifierReference).nameInSource shouldBe listOf("a1")
-        (left2.right as IdentifierReference).nameInSource shouldBe listOf("a2")
-        left3.operator shouldBe "or"
-        (left3.left as IdentifierReference).nameInSource shouldBe listOf("a1")
-        (left3.right as IdentifierReference).nameInSource shouldBe listOf("a2")
     }
 
     test("intermediate assignment steps generated for typecasted expression") {

docs/source/todo.rst

@@ -3,8 +3,6 @@ TODO
 
 For next release
 ^^^^^^^^^^^^^^^^
-- re-enable unittest "various 'not' operator rewrites even without optimizations on" when not-problem is fixed
-
 - petaxian roller.p8 line 49  (also see test.p8) generates large code compared to 8.2
 
 - code gen for if statements has become inefficient? vm/6502?
@@ -22,8 +20,6 @@ For next release
 - compiling logical.p8 to virtual with optimization generates a lot larger code as without optimizations.
   this is not the case for the 6502 codegen.
 
-- bin expr splitter: split logical expressions on ands/ors/xors ?
-
 - add some more optimizations in vmPeepholeOptimizer
 - vm Instruction needs to know what the read-registers/memory are, and what the write-register/memory is.
   this info is needed for more advanced optimizations and later code generation steps.

examples/test.p8

@@ -8,41 +8,39 @@ main  {
     sub start() {
 
         ubyte a1 = 0
-        ubyte a2 = 128
         uword w1 = 0
+        ubyte vv
 
-;        if not a1 and not w1
+        if not a1
-;            txt.print("1")
-;        if (0==a1) and (0==w1)
-;            txt.print("a")
-;        txt.nl()
-        a1 = 0
-        w1 = 4096
-        if not a1 and not w1
-            txt.print("fail ")
-        else
             txt.print("ok ")
-        if (0==a1) and (0==w1)
-            txt.print("fail ")
         else
-            txt.print("ok ")
+            txt.print("fail ")
+        vv = not a1
+        txt.print_ub(vv)
         txt.nl()
-
         a1=128
-        w1=2
+        if not a1
-        if not a1 and not w1
-            txt.print("fail")
-        if (0==a1) and (0==w1)
             txt.print("fail ")
+        else
+            txt.print("ok ")
+        vv = not a1
+        txt.print_ub(vv)
         txt.nl()
-        w1=2
+        if not w1
-        if not a1 and not w1
+            txt.print("ok ")
-            txt.print("fail")
+        else
-        if (0==a1) and (0==w1)
             txt.print("fail ")
+        vv = not w1
+        txt.print_ub(vv)
+        txt.nl()
+        w1 = 4096
+        if not w1
+            txt.print("fail ")
+        else
+            txt.print("ok ")
+        vv = not w1
+        txt.print_ub(vv)
         txt.nl()
-
-
 
 
         ; petaxian roller.p8 line 49