added inlining certain trivial non-asm subroutine calls

.idea/misc.xml

@@ -22,9 +22,4 @@
   <component name="ProjectRootManager" version="2" languageLevel="JDK_11" project-jdk-name="11" project-jdk-type="JavaSDK">
     <output url="file://$PROJECT_DIR$/out" />
   </component>
-  <component name="SwUserDefinedSpecifications">
-    <option name="specTypeByUrl">
-      <map />
-    </option>
-  </component>
 </project>

codeOptimizers/src/prog8/optimizer/Extensions.kt

@@ -55,11 +55,9 @@ fun Program.optimizeStatements(errors: IErrorReporter,
 }
 
 fun Program.inlineSubroutines(): Int {
-    // TODO implement the inliner
-//    val inliner = Inliner(this)
-//    inliner.visit(this)
-//    return inliner.applyModifications()
-    return 0
+    val inliner = Inliner(this)
+    inliner.visit(this)
+    return inliner.applyModifications()
 }
 
 fun Program.simplifyExpressions(errors: IErrorReporter) : Int {

codeOptimizers/src/prog8/optimizer/Inliner.kt

@@ -3,53 +3,106 @@ package prog8.optimizer
 import prog8.ast.IFunctionCall
 import prog8.ast.Node
 import prog8.ast.Program
-import prog8.ast.expressions.FunctionCallExpression
+import prog8.ast.expressions.*
 import prog8.ast.statements.*
 import prog8.ast.walk.AstWalker
 import prog8.ast.walk.IAstModification
 import prog8.ast.walk.IAstVisitor
+import prog8.code.core.InternalCompilerException
+
+
+private  fun isEmptyReturn(stmt: Statement): Boolean = stmt is Return && stmt.value==null
+
 
 class Inliner(val program: Program): AstWalker() {
 
-    class DetermineInlineSubs(program: Program): IAstVisitor {
+    class DetermineInlineSubs(val program: Program): IAstVisitor {
+        private val modifications = mutableListOf<IAstModification>()
+
         init {
             visit(program)
+            modifications.forEach { it.perform() }
+            modifications.clear()
         }
 
         override fun visit(subroutine: Subroutine) {
             if(!subroutine.isAsmSubroutine && !subroutine.inline && subroutine.parameters.isEmpty()) {
                 val containsSubsOrVariables = subroutine.statements.any { it is VarDecl || it is Subroutine}
                 if(!containsSubsOrVariables) {
-                    if(subroutine.statements.size==1 || (subroutine.statements.size==2 && subroutine.statements[1] is Return)) {
+                    if(subroutine.statements.size==1 || (subroutine.statements.size==2 && isEmptyReturn(subroutine.statements[1]))) {
                         // subroutine is possible candidate to be inlined
                         subroutine.inline =
                             when(val stmt=subroutine.statements[0]) {
                                 is Return -> {
-                                    if(stmt.value!!.isSimple) {
-                                        makeFullyScoped(stmt)
+                                    if(stmt.value is NumericLiteral)
                                         true
+                                    else if (stmt.value is IdentifierReference) {
+                                        makeFullyScoped(stmt.value as IdentifierReference)
+                                        true
+                                    } else if(stmt.value!! is IFunctionCall && (stmt.value as IFunctionCall).args.size<=1 && (stmt.value as IFunctionCall).args.all { it is NumericLiteral || it is IdentifierReference }) {
+                                        when (stmt.value) {
+                                            is BuiltinFunctionCall -> {
+                                                makeFullyScoped(stmt.value as BuiltinFunctionCall)
+                                                true
+                                            }
+                                            is FunctionCallExpression -> {
+                                                makeFullyScoped(stmt.value as FunctionCallExpression)
+                                                true
+                                            }
+                                            else -> false
+                                        }
                                     } else
                                         false
                                 }
                                 is Assignment -> {
-                                    val inline = stmt.value.isSimple && (stmt.target.identifier!=null || stmt.target.memoryAddress?.addressExpression?.isSimple==true)
+                                    if(stmt.value.isSimple) {
+                                        val targetInline =
+                                            if(stmt.target.identifier!=null) {
+                                                makeFullyScoped(stmt.target.identifier!!)
+                                                true
+                                            } else if(stmt.target.memoryAddress?.addressExpression is NumericLiteral || stmt.target.memoryAddress?.addressExpression is IdentifierReference) {
+                                                if(stmt.target.memoryAddress?.addressExpression is IdentifierReference)
+                                                    makeFullyScoped(stmt.target.memoryAddress?.addressExpression as IdentifierReference)
+                                                true
+                                            } else
+                                                false
+                                        val valueInline =
+                                            if(stmt.value is IdentifierReference) {
+                                                makeFullyScoped(stmt.value as IdentifierReference)
+                                                true
+                                            } else if((stmt.value as? DirectMemoryRead)?.addressExpression is NumericLiteral || (stmt.value as? DirectMemoryRead)?.addressExpression is IdentifierReference) {
+                                                if((stmt.value as? DirectMemoryRead)?.addressExpression is IdentifierReference)
+                                                    makeFullyScoped((stmt.value as? DirectMemoryRead)?.addressExpression as IdentifierReference)
+                                                true
+                                            } else
+                                                false
+                                        targetInline || valueInline
+                                    } else
+                                        false
+                                }
+                                is BuiltinFunctionCallStatement -> {
+                                    val inline = stmt.args.size<=1 && stmt.args.all { it is NumericLiteral || it is IdentifierReference }
                                     if(inline)
                                         makeFullyScoped(stmt)
                                     inline
                                 }
-                                is BuiltinFunctionCallStatement,
                                 is FunctionCallStatement -> {
-                                    stmt as IFunctionCall
-                                    val inline = stmt.args.size<=1 && stmt.args.all { it.isSimple }
+                                    val inline = stmt.args.size<=1 && stmt.args.all { it is NumericLiteral || it is IdentifierReference }
                                     if(inline)
                                         makeFullyScoped(stmt)
                                     inline
                                 }
                                 is PostIncrDecr -> {
-                                    val inline = (stmt.target.identifier!=null || stmt.target.memoryAddress?.addressExpression?.isSimple==true)
-                                    if(inline)
-                                        makeFullyScoped(stmt)
-                                    inline
+                                    if(stmt.target.identifier!=null) {
+                                        makeFullyScoped(stmt.target.identifier!!)
+                                        true
+                                    }
+                                    else if(stmt.target.memoryAddress?.addressExpression is NumericLiteral || stmt.target.memoryAddress?.addressExpression is IdentifierReference) {
+                                        if(stmt.target.memoryAddress?.addressExpression is IdentifierReference)
+                                            makeFullyScoped(stmt.target.memoryAddress?.addressExpression as IdentifierReference)
+                                        true
+                                    } else
+                                        false
                                 }
                                 is Jump, is GoSub -> true
                                 else -> false
@@ -60,20 +113,51 @@ class Inliner(val program: Program): AstWalker() {
             super.visit(subroutine)
         }
 
-        private fun makeFullyScoped(incrdecr: PostIncrDecr) {
-            TODO("Not yet implemented")
+        private fun makeFullyScoped(identifier: IdentifierReference) {
+            val scoped = (identifier.targetStatement(program)!! as INamedStatement).scopedName
+            val scopedIdent = IdentifierReference(scoped, identifier.position)
+            modifications += IAstModification.ReplaceNode(identifier, scopedIdent, identifier.parent)
         }
 
-        private fun makeFullyScoped(call: IFunctionCall) {
-            TODO("Not yet implemented")
+        private fun makeFullyScoped(call: BuiltinFunctionCallStatement) {
+            val scopedArgs = makeScopedArgs(call.args)
+            val scopedCall = BuiltinFunctionCallStatement(call.target.copy(), scopedArgs.toMutableList(), call.position)
+            modifications += IAstModification.ReplaceNode(call, scopedCall, call.parent)
         }
 
-        private fun makeFullyScoped(assign: Assignment) {
-            TODO("Not yet implemented")
+        private fun makeFullyScoped(call: FunctionCallStatement) {
+            val sub = call.target.targetSubroutine(program)!!
+            val scopedName = IdentifierReference(sub.scopedName, call.target.position)
+            val scopedArgs = makeScopedArgs(call.args)
+            val scopedCall = FunctionCallStatement(scopedName, scopedArgs.toMutableList(), call.void, call.position)
+            modifications += IAstModification.ReplaceNode(call, scopedCall, call.parent)
         }
 
-        private fun makeFullyScoped(ret: Return) {
-            TODO("Not yet implemented")
+        private fun makeFullyScoped(call: BuiltinFunctionCall) {
+            val sub = call.target.targetSubroutine(program)!!
+            val scopedName = IdentifierReference(sub.scopedName, call.target.position)
+            val scopedArgs = makeScopedArgs(call.args)
+            val scopedCall = BuiltinFunctionCall(scopedName, scopedArgs.toMutableList(), call.position)
+            modifications += IAstModification.ReplaceNode(call, scopedCall, call.parent)
+        }
+
+        private fun makeFullyScoped(call: FunctionCallExpression) {
+            val scopedArgs = makeScopedArgs(call.args)
+            val scopedCall = FunctionCallExpression(call.target.copy(), scopedArgs.toMutableList(), call.position)
+            modifications += IAstModification.ReplaceNode(call, scopedCall, call.parent)
+        }
+
+        private fun makeScopedArgs(args: List<Expression>): List<Expression> {
+            return args.map {
+                when (it) {
+                    is NumericLiteral -> it.copy()
+                    is IdentifierReference -> {
+                        val scoped = (it.targetStatement(program)!! as INamedStatement).scopedName
+                        IdentifierReference(scoped, it.position)
+                    }
+                    else -> throw InternalCompilerException("expected only number or identifier arg, otherwise too complex")
+                }
+            }
         }
     }
 
@@ -84,24 +168,41 @@ class Inliner(val program: Program): AstWalker() {
 
     override fun after(gosub: GoSub, parent: Node): Iterable<IAstModification> {
         val sub = gosub.identifier.targetStatement(program) as? Subroutine
-        if(sub!=null && sub.inline) {
-            val inlined = sub.statements
-            TODO("INLINE GOSUB:  $gosub --->  $inlined")
+        if(sub!=null && sub.inline && sub.parameters.isEmpty()) {
+            require(sub.statements.size == 1 || (sub.statements.size == 2 && isEmptyReturn(sub.statements[1])))
+            return if(sub.isAsmSubroutine) {
+                // simply insert the asm for the argument-less routine
+                listOf(IAstModification.ReplaceNode(gosub, sub.statements.single().copy(), parent))
+            } else {
+                // note that we don't have to process any args, because we online inline parameterless subroutines.
+                when (val toInline = sub.statements.first()) {
+                    is Return -> noModifications
+                    else -> listOf(IAstModification.ReplaceNode(gosub, toInline.copy(), parent))
+                }
+            }
+
         }
         return noModifications
     }
 
-    override fun after(functionCallExpr: FunctionCallExpression, parent: Node): Iterable<IAstModification> = inlineCall(functionCallExpr as IFunctionCall, parent)
-
-    override fun after(functionCallStatement: FunctionCallStatement, parent: Node): Iterable<IAstModification> = inlineCall(functionCallStatement as IFunctionCall, parent)
-
-    private fun inlineCall(call: IFunctionCall, parent: Node): Iterable<IAstModification> {
-        val sub = call.target.targetStatement(program) as? Subroutine
-        if(sub!=null && sub.inline) {
-            val inlined = sub.statements
-            TODO("INLINE FCALL:  $call --->  $inlined")
+    override fun after(functionCallStatement: FunctionCallStatement, parent: Node): Iterable<IAstModification>  {
+        val sub = functionCallStatement.target.targetStatement(program) as? Subroutine
+        if(sub!=null && sub.inline && sub.parameters.isEmpty()) {
+            require(sub.statements.size==1 || (sub.statements.size==2 && isEmptyReturn(sub.statements[1])))
+            return if(sub.isAsmSubroutine) {
+                // simply insert the asm for the argument-less routine
+                listOf(IAstModification.ReplaceNode(functionCallStatement, sub.statements.single().copy(), parent))
+            } else {
+                // note that we don't have to process any args, because we online inline parameterless subroutines.
+                when (val toInline = sub.statements.first()) {
+                    is Return -> noModifications
+                    else -> listOf(IAstModification.ReplaceNode(functionCallStatement, toInline.copy(), parent))
+                }
+            }
         }
         return noModifications
     }
+
+    // TODO also inline function call expressions, and remove it from the StatementOptimizer
 }
 

compiler/src/prog8/compiler/IntermediateAstMaker.kt

@@ -4,10 +4,9 @@ import com.github.michaelbull.result.Ok
 import com.github.michaelbull.result.Result
 import com.github.michaelbull.result.getOrElse
 import com.github.michaelbull.result.mapError
-import prog8.ast.IFunctionCall
-import prog8.ast.IStatementContainer
 import prog8.ast.Program
 import prog8.ast.base.FatalAstException
+import prog8.ast.determineGosubArguments
 import prog8.ast.expressions.*
 import prog8.ast.statements.*
 import prog8.code.ast.*
@@ -242,32 +241,10 @@ class IntermediateAstMaker(val program: Program) {
         // Gather the Goto and any preceding parameter assignments back into a single Function call node.
         // (the reason it was split up in the first place, is because the Compiler Ast optimizers
         // can then work on any complex expressions that are used as arguments.)
-        val parent = gosub.parent as IStatementContainer
-        val gosubIdx = parent.statements.indexOf(gosub)
-        val previousNodes = parent.statements.subList(0, gosubIdx).reversed()
-        val paramValues = mutableMapOf<String, Expression>()
-        for (node in previousNodes) {
-            if(node !is Assignment || node.origin!=AssignmentOrigin.PARAMETERASSIGN)
-                break
-            paramValues[node.target.identifier!!.nameInSource.last()] = node.value
-        }
-        // instead of just assigning to the parameters, another way is to use push()/pop()
-        if(previousNodes.isNotEmpty()) {
-            val first = previousNodes[0] as? IFunctionCall
-            if(first!=null && (first.target.nameInSource.singleOrNull() in arrayOf("pop", "popw"))) {
-                val numPops = previousNodes.indexOfFirst { (it as? IFunctionCall)?.target?.nameInSource?.singleOrNull() !in arrayOf("pop", "popw") }
-                val pops = previousNodes.subList(0, numPops)
-                val pushes = previousNodes.subList(numPops, numPops+numPops).reversed()
-                for ((push, pop) in pushes.zip(pops)) {
-                    val name = ((pop as IFunctionCall).args.single() as IdentifierReference).nameInSource.last()
-                    val arg = (push as IFunctionCall).args.single()
-                    paramValues[name] = arg
-                }
-            }
-        }
+        val arguments = determineGosubArguments(gosub)
 
         val parameters = gosub.identifier.targetSubroutine(program)!!.parameters
-        if(paramValues.size != parameters.size)
+        if(arguments.size != parameters.size)
             throw FatalAstException("mismatched number of parameter assignments for function call")
 
         val target = transform(gosub.identifier)
@@ -275,7 +252,7 @@ class IntermediateAstMaker(val program: Program) {
 
         // put arguments in correct order for the parameters
         parameters.forEach {
-            val argument = paramValues.getValue(it.name)
+            val argument = arguments.getValue(it.name)
             call.add(transformExpression(argument))
         }
 

compilerAst/src/prog8/ast/Extensions.kt

@@ -1,10 +1,10 @@
 package prog8.ast
 
 import prog8.ast.base.FatalAstException
+import prog8.ast.expressions.Expression
+import prog8.ast.expressions.IdentifierReference
 import prog8.ast.expressions.InferredTypes
-import prog8.ast.statements.VarDecl
-import prog8.ast.statements.VarDeclOrigin
-import prog8.ast.statements.VarDeclType
+import prog8.ast.statements.*
 import prog8.code.core.DataType
 import prog8.code.core.Position
 import prog8.code.core.ZeropageWish
@@ -56,3 +56,32 @@ fun getTempRegisterName(dt: InferredTypes.InferredType): List<String> {
         else -> throw FatalAstException("invalid dt $dt")
     }
 }
+
+fun determineGosubArguments(gosub: GoSub): Map<String, Expression> {
+    val parent = gosub.parent as IStatementContainer
+    val gosubIdx = parent.statements.indexOf(gosub)
+    val previousNodes = parent.statements.subList(0, gosubIdx).reversed()
+
+    val arguments = mutableMapOf<String, Expression>()
+    for (node in previousNodes) {
+        if(node !is Assignment || node.origin!=AssignmentOrigin.PARAMETERASSIGN)
+            break
+        arguments[node.target.identifier!!.nameInSource.last()] = node.value
+    }
+
+    // instead of just assigning to the parameters, another way is to use push()/pop()
+    if(previousNodes.isNotEmpty()) {
+        val first = previousNodes[0] as? IFunctionCall
+        if(first!=null && (first.target.nameInSource.singleOrNull() in arrayOf("pop", "popw"))) {
+            val numPops = previousNodes.indexOfFirst { (it as? IFunctionCall)?.target?.nameInSource?.singleOrNull() !in arrayOf("pop", "popw") }
+            val pops = previousNodes.subList(0, numPops)
+            val pushes = previousNodes.subList(numPops, numPops+numPops).reversed()
+            for ((push, pop) in pushes.zip(pops)) {
+                val name = ((pop as IFunctionCall).args.single() as IdentifierReference).nameInSource.last()
+                val arg = (push as IFunctionCall).args.single()
+                arguments[name] = arg
+            }
+        }
+    }
+    return arguments
+}

compilerAst/src/prog8/ast/expressions/AstExpressions.kt

@@ -932,7 +932,10 @@ class FunctionCallExpression(override var target: IdentifierReference,
     }
 
     override fun copy() = FunctionCallExpression(target.copy(), args.map { it.copy() }.toMutableList(), position)
-    override val isSimple = target.nameInSource.size==1 && (target.nameInSource[0] in arrayOf("msb", "lsb", "peek", "peekw"))
+    override val isSimple =
+        target.nameInSource.size==1
+                && target.nameInSource[0] in arrayOf("msb", "lsb", "peek", "peekw", "mkword")
+                && args.all { it.isSimple }
 
     override fun replaceChildNode(node: Node, replacement: Node) {
         if(node===target)

compilerAst/src/prog8/ast/statements/AstStatements.kt

@@ -613,7 +613,10 @@ class FunctionCallStatement(override var target: IdentifierReference,
         args.forEach { it.linkParents(this) }
     }
 
-    override fun copy() = throw NotImplementedError("no support for duplicating a FunctionCallStatement")
+    override fun copy(): FunctionCallStatement {
+        val argsCopies = args.map { it.copy() }
+        return FunctionCallStatement(target.copy(), argsCopies.toMutableList(), void, position)
+    }
 
     override fun replaceChildNode(node: Node, replacement: Node) {
         if(node===target)
@@ -637,7 +640,7 @@ class InlineAssembly(val assembly: String, override val position: Position) : St
         this.parent = parent
     }
 
-    override fun copy() = throw NotImplementedError("no support for duplicating a InlineAssembly")
+    override fun copy() = InlineAssembly(assembly, position)
 
     override fun replaceChildNode(node: Node, replacement: Node) = throw FatalAstException("can't replace here")
     override fun accept(visitor: IAstVisitor) = visitor.visit(this)

docs/source/todo.rst

@@ -3,7 +3,6 @@ TODO
 
 For next release
 ^^^^^^^^^^^^^^^^
-- complete the Inliner
 - add McCarthy evaluation to shortcircuit and/or expressions. First do ifs by splitting them up? Then do expressions that compute a value?
 
 ...
@@ -27,6 +26,7 @@ Compiler:
 - vm: how to remove all unused subroutines? (in the assembly codegen, we let 64tass solve this for us)
 - vm: rather than being able to jump to any 'address' (IPTR), use 'blocks' that have entry and exit points -> even better dead code elimination possible too
 - vm: add more assignments to translateInplaceAssign()
+- Inliner: also inline function call expressions, and remove it from the StatementOptimizer
 - when the vm is stable and *if* its language can get promoted to prog8 IL, the variable allocation should be changed.
   It's now done before the vm code generation, but the IL should probably not depend on the allocations already performed.
   So the CodeGen doesn't do VariableAlloc *before* the codegen, but as a last step.

examples/test.p8

@@ -7,46 +7,29 @@
 
 ; NOTE: meant to test to virtual machine output target (use -target vitual)
 
-main {
+other {
     ubyte value = 42
 
-    sub inline_candidate() -> ubyte {
-        return math.sin8u(value)
+    sub getter() -> ubyte {
+        return value
     }
+}
 
-    sub inline_candidate2() {
-        value++
-        return
-    }
+main {
 
-    sub add(ubyte first, ubyte second) -> ubyte {
-        return first + second
-    }
-
-    sub mul(ubyte first, ubyte second) -> ubyte {
-        return first * second
-    }
-
-    ubyte ix
 
     sub start() {
-
-        ubyte @shared value1 = inline_candidate()
-        txt.print_ub(value) ; 42
-        txt.spc()
-        inline_candidate2()
-        inline_candidate2()
-        inline_candidate2()
-        txt.print_ub(value) ; 45
-        txt.nl()
-        txt.print_ub(inline_candidate())
-        txt.nl()
-
-        ubyte @shared value=99      ; TODO compiler warning about shadowing
-        txt.print_ub(value)
-        txt.nl()
-
-        ubyte @shared add=99      ; TODO compiler warning about shadowing
+        ubyte @shared ix = other.getter()
+        ix = other.getter()
+        ix++
+        ix = other.getter()
+        ix++
+        ix = other.getter()
+        ix++
+        ix = other.getter()
+        ix++
+        ix = other.getter()
+        ix++
 
 ;        ; a "pixelshader":
 ;        sys.gfx_enable(0)       ; enable lo res screen