preparing optimizing pointer indexing

codeOptimizers/src/prog8/optimizer/UnusedCodeRemover.kt

@@ -28,23 +28,23 @@ class UnusedCodeRemover(private val program: Program,
 
     override fun before(breakStmt: Break, parent: Node): Iterable<IAstModification> {
         reportUnreachable(breakStmt)
-        return emptyList()
+        return noModifications
     }
 
     override fun before(jump: Jump, parent: Node): Iterable<IAstModification> {
         reportUnreachable(jump)
-        return emptyList()
+        return noModifications
     }
 
     override fun before(returnStmt: Return, parent: Node): Iterable<IAstModification> {
         reportUnreachable(returnStmt)
-        return emptyList()
+        return noModifications
     }
 
     override fun before(functionCallStatement: FunctionCallStatement, parent: Node): Iterable<IAstModification> {
         if(functionCallStatement.target.nameInSource.last() == "exit")
             reportUnreachable(functionCallStatement)
-        return emptyList()
+        return noModifications
     }
 
     private fun reportUnreachable(stmt: Statement) {

compiler/src/prog8/compiler/astprocessing/AstChecker.kt

@@ -9,6 +9,7 @@ import prog8.ast.walk.IAstVisitor
 import prog8.code.core.*
 import prog8.code.target.VMTarget
 import prog8.compiler.BuiltinFunctions
+import prog8.compiler.InplaceModifyingBuiltinFunctions
 import prog8.compiler.builtinFunctionReturnType
 import java.io.CharConversionException
 import java.io.File
@@ -1041,7 +1042,7 @@ internal class AstChecker(private val program: Program,
                 }
             }
 
-            if(funcName[0] in arrayOf("rol", "ror", "rol2", "ror2", "swap", "sort", "reverse")) {
+            if(funcName[0] in InplaceModifyingBuiltinFunctions) {
                 // in-place modification, can't be done on literals
                 if(functionCallStatement.args.any { it !is IdentifierReference && it !is ArrayIndexedExpression && it !is DirectMemoryRead }) {
                     errors.err("invalid argument to a in-place modifying function", functionCallStatement.args.first().position)

compiler/src/prog8/compiler/astprocessing/AstExtensions.kt

@@ -94,7 +94,7 @@ internal fun Program.checkIdentifiers(errors: IErrorReporter, options: Compilati
     checker2.visit(this)
 
     if(errors.noErrors()) {
-        val transforms = AstVariousTransforms(this)
+        val transforms = AstOnetimeTransforms(this)
         transforms.visit(this)
         transforms.applyModifications()
         val lit2decl = LiteralsToAutoVars(this, options.compTarget, errors)

compiler/src/prog8/compiler/astprocessing/AstOnetimeTransforms.kt

@@ -0,0 +1,67 @@
+package prog8.compiler.astprocessing
+
+import prog8.ast.IFunctionCall
+import prog8.ast.Node
+import prog8.ast.Program
+import prog8.ast.expressions.ArrayIndexedExpression
+import prog8.ast.expressions.BinaryExpression
+import prog8.ast.expressions.DirectMemoryRead
+import prog8.ast.statements.AssignTarget
+import prog8.ast.statements.DirectMemoryWrite
+import prog8.ast.statements.VarDecl
+import prog8.ast.walk.AstWalker
+import prog8.ast.walk.IAstModification
+import prog8.code.core.DataType
+import prog8.compiler.InplaceModifyingBuiltinFunctions
+
+
+internal class AstOnetimeTransforms(private val program: Program) : AstWalker() {
+
+    override fun after(arrayIndexedExpression: ArrayIndexedExpression, parent: Node): Iterable<IAstModification> {
+        if(parent !is VarDecl) {
+            // TODO move this / remove this, and make the codegen better instead.
+            // If the expression is pointervar[idx] where pointervar is uword and not a real array,
+            // replace it by a @(pointervar+idx) expression.
+            // Don't replace the initializer value in a vardecl - this will be moved to a separate
+            // assignment statement soon in after(VarDecl)
+            return replacePointerVarIndexWithMemreadOrMemwrite(arrayIndexedExpression, parent)
+        }
+        return noModifications
+    }
+
+    private fun replacePointerVarIndexWithMemreadOrMemwrite(arrayIndexedExpression: ArrayIndexedExpression, parent: Node): Iterable<IAstModification> {
+        val arrayVar = arrayIndexedExpression.arrayvar.targetVarDecl(program)
+        if(arrayVar!=null && arrayVar.datatype == DataType.UWORD) {
+            // rewrite   pointervar[index]  into  @(pointervar+index)
+            val indexer = arrayIndexedExpression.indexer
+            val add = BinaryExpression(arrayIndexedExpression.arrayvar.copy(), "+", indexer.indexExpr, arrayIndexedExpression.position)
+            if(parent is AssignTarget) {
+                // we're part of the target of an assignment, we have to actually change the assign target itself
+                val memwrite = DirectMemoryWrite(add, arrayIndexedExpression.position)
+                val newtarget = AssignTarget(null, null, memwrite, arrayIndexedExpression.position)
+                return listOf(IAstModification.ReplaceNode(parent, newtarget, parent.parent))
+            } else {
+                val fcall = parent as? IFunctionCall
+                return if(fcall!=null) {
+                    val fname = fcall.target.nameInSource
+                    if(fname.size==1 && fname[0] in InplaceModifyingBuiltinFunctions) {
+                        // TODO for now, swap() etc don't work on pointer var indexed args
+                        val memread = DirectMemoryRead(add, arrayIndexedExpression.position)
+                        listOf(IAstModification.ReplaceNode(arrayIndexedExpression, memread, parent))
+                    } else {
+                        // TODO first candidate for optimization is to remove this:
+                        val memread = DirectMemoryRead(add, arrayIndexedExpression.position)
+                        listOf(IAstModification.ReplaceNode(arrayIndexedExpression, memread, parent))
+                    }
+                } else {
+                    val memread = DirectMemoryRead(add, arrayIndexedExpression.position)
+                    listOf(IAstModification.ReplaceNode(arrayIndexedExpression, memread, parent))
+                }
+            }
+        }
+
+        return noModifications
+    }
+}
+
+

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

@@ -25,6 +25,32 @@ class AstPreprocessor(val program: Program, val errors: IErrorReporter, val comp
         return super.before(string, parent)
     }
 
+    override fun before(pipe: Pipe, parent: Node): Iterable<IAstModification> {
+        if(pipe.source is PipeExpression) {
+            // correct Antlr parse tree quirk: turn nested pipe into single flat pipe
+            val psrc = pipe.source as PipeExpression
+            val newSource = psrc.source
+            val newSegments = psrc.segments
+            newSegments += pipe.segments.single()
+            return listOf(IAstModification.ReplaceNode(pipe as Node, Pipe(newSource, newSegments, pipe.position), parent))
+        } else if(pipe.source is IPipe)
+            throw InternalCompilerException("pipe source should have been adjusted to be a normal expression")
+        return noModifications
+    }
+
+    override fun before(pipeExpr: PipeExpression, parent: Node): Iterable<IAstModification> {
+        if(pipeExpr.source is PipeExpression) {
+            // correct Antlr parse tree quirk; turn nested pipe into single flat pipe
+            val psrc = pipeExpr.source as PipeExpression
+            val newSource = psrc.source
+            val newSegments = psrc.segments
+            newSegments += pipeExpr.segments.single()
+            return listOf(IAstModification.ReplaceNode(pipeExpr as Node, PipeExpression(newSource, newSegments, pipeExpr.position), parent))
+        } else if(pipeExpr.source is IPipe)
+            throw InternalCompilerException("pipe source should have been adjusted to be a normal expression")
+        return noModifications
+    }
+
     override fun after(range: RangeExpression, parent: Node): Iterable<IAstModification> {
         // has to be done before the constant folding, otherwise certain checks there will fail on invalid range sizes
         val modifications = mutableListOf<IAstModification>()
@@ -111,29 +137,25 @@ class AstPreprocessor(val program: Program, val errors: IErrorReporter, val comp
         return noModifications
     }
 
-    override fun before(pipe: Pipe, parent: Node): Iterable<IAstModification> {
+    override fun after(subroutine: Subroutine, parent: Node): Iterable<IAstModification> {
-        if(pipe.source is PipeExpression) {
+        // For non-kernal subroutines and non-asm parameters:
-            // correct Antlr parse tree quirk: turn nested pipe into single flat pipe
+        // inject subroutine params as local variables (if they're not there yet).
-            val psrc = pipe.source as PipeExpression
+        val symbolsInSub = subroutine.allDefinedSymbols
-            val newSource = psrc.source
+        val namesInSub = symbolsInSub.map{ it.first }.toSet()
-            val newSegments = psrc.segments
+        if(subroutine.asmAddress==null) {
-            newSegments += pipe.segments.single()
+            if(!subroutine.isAsmSubroutine && subroutine.parameters.isNotEmpty()) {
-            return listOf(IAstModification.ReplaceNode(pipe as Node, Pipe(newSource, newSegments, pipe.position), parent))
+                val vars = subroutine.statements.asSequence().filterIsInstance<VarDecl>().map { it.name }.toSet()
-        } else if(pipe.source is IPipe)
+                if(!vars.containsAll(subroutine.parameters.map{it.name})) {
-            throw InternalCompilerException("pipe source should have been adjusted to be a normal expression")
+                    return subroutine.parameters
-        return noModifications
+                        .filter { it.name !in namesInSub }
+                        .map {
+                            val vardecl = VarDecl.fromParameter(it)
+                            IAstModification.InsertFirst(vardecl, subroutine)
+                        }
+                }
+            }
         }
 
-    override fun before(pipeExpr: PipeExpression, parent: Node): Iterable<IAstModification> {
-        if(pipeExpr.source is PipeExpression) {
-            // correct Antlr parse tree quirk; turn nested pipe into single flat pipe
-            val psrc = pipeExpr.source as PipeExpression
-            val newSource = psrc.source
-            val newSegments = psrc.segments
-            newSegments += pipeExpr.segments.single()
-            return listOf(IAstModification.ReplaceNode(pipeExpr as Node, PipeExpression(newSource, newSegments, pipeExpr.position), parent))
-        } else if(pipeExpr.source is IPipe)
-            throw InternalCompilerException("pipe source should have been adjusted to be a normal expression")
         return noModifications
     }
 }

compiler/src/prog8/compiler/astprocessing/AstVariousTransforms.kt

@@ -1,42 +0,0 @@
-package prog8.compiler.astprocessing
-
-import prog8.ast.Node
-import prog8.ast.Program
-import prog8.ast.expressions.ArrayIndexedExpression
-import prog8.ast.statements.Subroutine
-import prog8.ast.statements.VarDecl
-import prog8.ast.walk.AstWalker
-import prog8.ast.walk.IAstModification
-
-
-internal class AstVariousTransforms(private val program: Program) : AstWalker() {
-
-    // TODO can this be integrated in another walker
-
-    override fun after(subroutine: Subroutine, parent: Node): Iterable<IAstModification> {
-        // For non-kernal subroutines and non-asm parameters:
-        // inject subroutine params as local variables (if they're not there yet).
-        val symbolsInSub = subroutine.allDefinedSymbols
-        val namesInSub = symbolsInSub.map{ it.first }.toSet()
-        if(subroutine.asmAddress==null) {
-            if(!subroutine.isAsmSubroutine && subroutine.parameters.isNotEmpty()) {
-                val vars = subroutine.statements.asSequence().filterIsInstance<VarDecl>().map { it.name }.toSet()
-                if(!vars.containsAll(subroutine.parameters.map{it.name})) {
-                    return subroutine.parameters
-                            .filter { it.name !in namesInSub }
-                            .map {
-                                val vardecl = VarDecl.fromParameter(it)
-                                IAstModification.InsertFirst(vardecl, subroutine)
-                            }
-                }
-            }
-        }
-
-        return noModifications
-    }
-
-    // TODO move this / remove this.  Needed here atm otherwise a replacement error occurs later in StatementReorderer
-    override fun after(arrayIndexedExpression: ArrayIndexedExpression, parent: Node): Iterable<IAstModification> {
-        return replacePointerVarIndexWithMemreadOrMemwrite(program, arrayIndexedExpression, parent)
-    }
-}

compiler/src/prog8/compiler/astprocessing/BeforeAsmAstChanger.kt

@@ -229,7 +229,7 @@ internal class BeforeAsmAstChanger(val program: Program,
         //       in that case: do *not* split it off but just keep it as it is (otherwise code size increases)
         // NOTE: do NOT move this to an earler ast transform phase (such as StatementReorderer or StatementOptimizer) - it WILL result in larger code.
 
-        if(options.compTarget.name==VMTarget.NAME)  // don't apply this optimizer for Vm target
+        if(options.compTarget.name==VMTarget.NAME)  // don't apply this optimization for Vm target
             return CondExprSimplificationResult(null, null, null, null)
 
         var leftAssignment: Assignment? = null
@@ -279,7 +279,7 @@ internal class BeforeAsmAstChanger(val program: Program,
             return noModifications
         }
 
-        if(options.compTarget.name!=VMTarget.NAME) {
+        if(options.compTarget.name!=VMTarget.NAME) {    // don't apply this optimization for Vm target
             val index = arrayIndexedExpression.indexer.indexExpr
             if (index !is NumericLiteral && index !is IdentifierReference) {
                 // replace complex indexing expression with a temp variable to hold the computed index first
@@ -343,5 +343,4 @@ internal class BeforeAsmAstChanger(val program: Program,
         )
         return modifications
     }
-
 }

compiler/src/prog8/compiler/astprocessing/StatementReorderer.kt

@@ -189,15 +189,6 @@ internal class StatementReorderer(val program: Program,
         return modifications + parameterChanges + varsChanges
     }
 
-    override fun after(arrayIndexedExpression: ArrayIndexedExpression, parent: Node): Iterable<IAstModification> {
-        if(parent !is VarDecl) {
-            // don't replace the initializer value in a vardecl - this will be moved to a separate
-            // assignment statement soon in after(VarDecl)
-            return replacePointerVarIndexWithMemreadOrMemwrite(program, arrayIndexedExpression, parent)
-        }
-        return noModifications
-    }
-
     override fun after(expr: BinaryExpression, parent: Node): Iterable<IAstModification> {
 
         // ConstValue <associativeoperator> X -->  X <associativeoperator> ConstValue
@@ -576,25 +567,3 @@ private fun makeGosubWithArgsViaCpuStack(
     }
     return listOf(IAstModification.ReplaceNode(call as Node, scope, parent))
 }
-
-
-
-internal fun replacePointerVarIndexWithMemreadOrMemwrite(program: Program, arrayIndexedExpression: ArrayIndexedExpression, parent: Node): Iterable<IAstModification> {
-    val arrayVar = arrayIndexedExpression.arrayvar.targetVarDecl(program)
-    if(arrayVar!=null && arrayVar.datatype == DataType.UWORD) {
-        // rewrite   pointervar[index]  into  @(pointervar+index)
-        val indexer = arrayIndexedExpression.indexer
-        val add = BinaryExpression(arrayIndexedExpression.arrayvar.copy(), "+", indexer.indexExpr, arrayIndexedExpression.position)
-        return if(parent is AssignTarget) {
-            // we're part of the target of an assignment, we have to actually change the assign target itself
-            val memwrite = DirectMemoryWrite(add, arrayIndexedExpression.position)
-            val newtarget = AssignTarget(null, null, memwrite, arrayIndexedExpression.position)
-            listOf(IAstModification.ReplaceNode(parent, newtarget, parent.parent))
-        } else {
-            val memread = DirectMemoryRead(add, arrayIndexedExpression.position)
-            listOf(IAstModification.ReplaceNode(arrayIndexedExpression, memread, parent))
-        }
-    }
-
-    return emptyList()
-}

compilerAst/src/prog8/compiler/BuiltinFunctions.kt

@@ -129,7 +129,7 @@ private val functionSignatures: List<FSignature> = listOf(
 )
 
 val BuiltinFunctions = functionSignatures.associateBy { it.name }
-
+val InplaceModifyingBuiltinFunctions = setOf("rol", "ror", "rol2", "ror2", "swap", "sort", "reverse")
 
 private fun builtinAny(array: List<Double>): Double = if(array.any { it!=0.0 }) 1.0 else 0.0
 

docs/source/todo.rst

@@ -3,7 +3,8 @@ TODO
 
 For next release
 ^^^^^^^^^^^^^^^^
-- try to integrated AstVariousTransforms into another walker
+- optimize pointervar indexing codegen: reading (expressions) via optimized codegen instead of @(pointer+idx)
+- optimize pointervar indexing codegen: writing (all sorts of things)
 - why is this code so much larger:
                 uword xx
                 for xx in 0 to size-1 {

examples/test.p8

@@ -28,13 +28,6 @@ main {
 ;        txt.spc()
 ;    }
 
-    sub crash () {
-        uword eRef
-        if eRef[3] and 10  {
-          return
-        }
-    }
-
     sub start() {
         ; mcCarthy()
 
@@ -42,6 +35,9 @@ main {
         ubyte size = 9
         ubyte[10] data = [11,22,33,4,5,6,7,8,9,10]
         uword bitmapbuf = &data
+        value = bitmapbuf[2]
+        txt.print_ub(value)    ;; 33
+        txt.nl()
 
 ;        ; 11 22 33
 ;        txt.print_ub(bitmapbuf[0])