From f790182f0b0f29b0a2a1631f225c8b990452f20d Mon Sep 17 00:00:00 2001 From: Irmen de Jong Date: Mon, 30 Oct 2023 00:09:18 +0100 Subject: [PATCH] adding short-circuit boolean expression evaluation (in IR codegen) also -noshortcircuit cli option --- .../src/prog8/code/core/CompilationOptions.kt | 1 + .../codegen/intermediate/ExpressionGen.kt | 60 ++++++++++----- compiler/src/prog8/CompilerMain.kt | 3 + compiler/src/prog8/compiler/Compiler.kt | 2 + compiler/test/TestCompilerOnExamples.kt | 1 + compiler/test/TestCompilerOptionLibdirs.kt | 1 + compiler/test/helpers/compileXyz.kt | 1 + docs/source/compiling.rst | 5 ++ docs/source/todo.rst | 21 +++-- examples/test.p8 | 76 +++++++++++++++---- .../src/prog8/http/TestHttp.kt | 1 + .../src/prog8/intermediate/IRInstructions.kt | 8 +- virtualmachine/src/prog8/vm/VirtualMachine.kt | 12 ++- 13 files changed, 146 insertions(+), 46 deletions(-) diff --git a/codeCore/src/prog8/code/core/CompilationOptions.kt b/codeCore/src/prog8/code/core/CompilationOptions.kt index a47deac2c..71aaf804f 100644 --- a/codeCore/src/prog8/code/core/CompilationOptions.kt +++ b/codeCore/src/prog8/code/core/CompilationOptions.kt @@ -18,6 +18,7 @@ class CompilationOptions(val output: OutputType, var optimize: Boolean = false, var asmQuiet: Boolean = false, var asmListfile: Boolean = false, + var shortCircuit: Boolean = true, var includeSourcelines: Boolean = false, var experimentalCodegen: Boolean = false, var varsHighBank: Int? = null, diff --git a/codeGenIntermediate/src/prog8/codegen/intermediate/ExpressionGen.kt b/codeGenIntermediate/src/prog8/codegen/intermediate/ExpressionGen.kt index aef5c8b88..c172cf9c2 100644 --- a/codeGenIntermediate/src/prog8/codegen/intermediate/ExpressionGen.kt +++ b/codeGenIntermediate/src/prog8/codegen/intermediate/ExpressionGen.kt @@ -697,35 +697,59 @@ internal class ExpressionGen(private val codeGen: IRCodeGen) { private fun operatorAnd(binExpr: PtBinaryExpression, vmDt: IRDataType): ExpressionCodeResult { val result = mutableListOf() - return if(binExpr.right is PtNumber) { - val tr = translateExpression(binExpr.left) - addToResult(result, tr, tr.resultReg, -1) - addInstr(result, IRInstruction(Opcode.AND, vmDt, reg1 = tr.resultReg, immediate = (binExpr.right as PtNumber).number.toInt()), null) - ExpressionCodeResult(result, vmDt, tr.resultReg, -1) - } else { + if(codeGen.options.shortCircuit && (!binExpr.left.isSimple() && !binExpr.right.isSimple())) { + // short-circuit LEFT and RIGHT --> if LEFT then RIGHT else LEFT (== if !LEFT then LEFT else RIGHT) val leftTr = translateExpression(binExpr.left) addToResult(result, leftTr, leftTr.resultReg, -1) + val shortcutLabel = codeGen.createLabelName() + addInstr(result, IRInstruction(Opcode.BSTEQ, labelSymbol = shortcutLabel), null) val rightTr = translateExpression(binExpr.right) - addToResult(result, rightTr, rightTr.resultReg, -1) - addInstr(result, IRInstruction(Opcode.ANDR, vmDt, reg1 = leftTr.resultReg, reg2 = rightTr.resultReg), null) - ExpressionCodeResult(result, vmDt, leftTr.resultReg, -1) + addToResult(result, rightTr, leftTr.resultReg, -1) + result += IRCodeChunk(shortcutLabel, null) + return ExpressionCodeResult(result, vmDt, leftTr.resultReg, -1) + } else { + return if(binExpr.right is PtNumber) { + val tr = translateExpression(binExpr.left) + addToResult(result, tr, tr.resultReg, -1) + addInstr(result, IRInstruction(Opcode.AND, vmDt, reg1 = tr.resultReg, immediate = (binExpr.right as PtNumber).number.toInt()), null) + ExpressionCodeResult(result, vmDt, tr.resultReg, -1) + } else { + val leftTr = translateExpression(binExpr.left) + addToResult(result, leftTr, leftTr.resultReg, -1) + val rightTr = translateExpression(binExpr.right) + addToResult(result, rightTr, rightTr.resultReg, -1) + addInstr(result, IRInstruction(Opcode.ANDR, vmDt, reg1 = leftTr.resultReg, reg2 = rightTr.resultReg), null) + ExpressionCodeResult(result, vmDt, leftTr.resultReg, -1) + } } } private fun operatorOr(binExpr: PtBinaryExpression, vmDt: IRDataType): ExpressionCodeResult { val result = mutableListOf() - return if(binExpr.right is PtNumber) { - val tr = translateExpression(binExpr.left) - addToResult(result, tr, tr.resultReg, -1) - addInstr(result, IRInstruction(Opcode.OR, vmDt, reg1 = tr.resultReg, immediate = (binExpr.right as PtNumber).number.toInt()), null) - ExpressionCodeResult(result, vmDt, tr.resultReg, -1) - } else { + if(codeGen.options.shortCircuit && (!binExpr.left.isSimple() && !binExpr.right.isSimple())) { + // short-circuit LEFT or RIGHT --> if LEFT then LEFT else RIGHT val leftTr = translateExpression(binExpr.left) addToResult(result, leftTr, leftTr.resultReg, -1) + val shortcutLabel = codeGen.createLabelName() + addInstr(result, IRInstruction(Opcode.BSTNE, labelSymbol = shortcutLabel), null) val rightTr = translateExpression(binExpr.right) - addToResult(result, rightTr, rightTr.resultReg, -1) - addInstr(result, IRInstruction(Opcode.ORR, vmDt, reg1 = leftTr.resultReg, reg2 = rightTr.resultReg), null) - ExpressionCodeResult(result, vmDt, leftTr.resultReg, -1) + addToResult(result, rightTr, leftTr.resultReg, -1) + result += IRCodeChunk(shortcutLabel, null) + return ExpressionCodeResult(result, vmDt, leftTr.resultReg, -1) + } else { + return if(binExpr.right is PtNumber) { + val tr = translateExpression(binExpr.left) + addToResult(result, tr, tr.resultReg, -1) + addInstr(result, IRInstruction(Opcode.OR, vmDt, reg1 = tr.resultReg, immediate = (binExpr.right as PtNumber).number.toInt()), null) + ExpressionCodeResult(result, vmDt, tr.resultReg, -1) + } else { + val leftTr = translateExpression(binExpr.left) + addToResult(result, leftTr, leftTr.resultReg, -1) + val rightTr = translateExpression(binExpr.right) + addToResult(result, rightTr, rightTr.resultReg, -1) + addInstr(result, IRInstruction(Opcode.ORR, vmDt, reg1 = leftTr.resultReg, reg2 = rightTr.resultReg), null) + ExpressionCodeResult(result, vmDt, leftTr.resultReg, -1) + } } } diff --git a/compiler/src/prog8/CompilerMain.kt b/compiler/src/prog8/CompilerMain.kt index 04d9c7777..e4501f63a 100644 --- a/compiler/src/prog8/CompilerMain.kt +++ b/compiler/src/prog8/CompilerMain.kt @@ -50,6 +50,7 @@ private fun compileMain(args: Array): Boolean { val sourceDirs by cli.option(ArgType.String, fullName="srcdirs", description = "list of extra paths, separated with ${File.pathSeparator}, to search in for imported modules").multiple().delimiter(File.pathSeparator) val includeSourcelines by cli.option(ArgType.Boolean, fullName = "sourcelines", description = "include original Prog8 source lines in generated asm code") val splitWordArrays by cli.option(ArgType.Boolean, fullName = "splitarrays", description = "treat all word arrays as tagged with @split to make them lsb/msb split in memory") + val noShortCircuit by cli.option(ArgType.Boolean, fullName = "noshortcircuit", description = "do not apply McCarthy/short-circuit evaluation to boolean expressions") val breakpointCpuInstruction by cli.option(ArgType.Boolean, fullName = "breakinstr", description = "also use a CPU instruction for %breakpoint") val compilationTarget by cli.option(ArgType.String, fullName = "target", description = "target output of the compiler (one of '${C64Target.NAME}', '${C128Target.NAME}', '${Cx16Target.NAME}', '${AtariTarget.NAME}', '${PETTarget.NAME}', '${VMTarget.NAME}') (required)") val startVm by cli.option(ArgType.Boolean, fullName = "vm", description = "load and run a .p8ir IR source file in the VM") @@ -148,6 +149,7 @@ private fun compileMain(args: Array): Boolean { warnSymbolShadowing == true, quietAssembler == true, asmListfile == true, + noShortCircuit != true, includeSourcelines == true, experimentalCodegen == true, varsHighBank, @@ -224,6 +226,7 @@ private fun compileMain(args: Array): Boolean { warnSymbolShadowing == true, quietAssembler == true, asmListfile == true, + noShortCircuit != true, includeSourcelines == true, experimentalCodegen == true, varsHighBank, diff --git a/compiler/src/prog8/compiler/Compiler.kt b/compiler/src/prog8/compiler/Compiler.kt index 451ef2123..02d00025a 100644 --- a/compiler/src/prog8/compiler/Compiler.kt +++ b/compiler/src/prog8/compiler/Compiler.kt @@ -33,6 +33,7 @@ class CompilerArguments(val filepath: Path, val warnSymbolShadowing: Boolean, val quietAssembler: Boolean, val asmListfile: Boolean, + val shortCircuit: Boolean, val includeSourcelines: Boolean, val experimentalCodegen: Boolean, val varsHighBank: Int?, @@ -76,6 +77,7 @@ fun compileProgram(args: CompilerArguments): CompilationResult? { optimize = args.optimize asmQuiet = args.quietAssembler asmListfile = args.asmListfile + shortCircuit = args.shortCircuit includeSourcelines = args.includeSourcelines experimentalCodegen = args.experimentalCodegen breakpointCpuInstruction = args.breakpointCpuInstruction diff --git a/compiler/test/TestCompilerOnExamples.kt b/compiler/test/TestCompilerOnExamples.kt index 778212b25..6743654e8 100644 --- a/compiler/test/TestCompilerOnExamples.kt +++ b/compiler/test/TestCompilerOnExamples.kt @@ -30,6 +30,7 @@ private fun compileTheThing(filepath: Path, optimize: Boolean, target: ICompilat warnSymbolShadowing = false, quietAssembler = true, asmListfile = false, + shortCircuit = true, includeSourcelines = false, experimentalCodegen = false, varsHighBank = null, diff --git a/compiler/test/TestCompilerOptionLibdirs.kt b/compiler/test/TestCompilerOptionLibdirs.kt index 60c9d69dd..4e26a1e56 100644 --- a/compiler/test/TestCompilerOptionLibdirs.kt +++ b/compiler/test/TestCompilerOptionLibdirs.kt @@ -28,6 +28,7 @@ class TestCompilerOptionSourcedirs: FunSpec({ warnSymbolShadowing = false, quietAssembler = true, asmListfile = false, + shortCircuit = true, includeSourcelines = false, experimentalCodegen = false, varsHighBank = null, diff --git a/compiler/test/helpers/compileXyz.kt b/compiler/test/helpers/compileXyz.kt index 27fb66d4c..6f1785251 100644 --- a/compiler/test/helpers/compileXyz.kt +++ b/compiler/test/helpers/compileXyz.kt @@ -27,6 +27,7 @@ internal fun compileFile( warnSymbolShadowing = false, quietAssembler = true, asmListfile = false, + shortCircuit = true, includeSourcelines = false, experimentalCodegen = false, varsHighBank = null, diff --git a/docs/source/compiling.rst b/docs/source/compiling.rst index 35ab3685d..1d56c16b8 100644 --- a/docs/source/compiling.rst +++ b/docs/source/compiling.rst @@ -195,6 +195,11 @@ One or more .p8 module files This removes the need to add @split yourself but some programs may fail to compile with this option as not all array operations are implemented yet on split arrays. +``-noshortcircuit`` + Do *not* apply `McCarthy/short-circuit evaluation `_ to boolean expressions. + This is a new feature and changes the behavior of existing programs so it can be turned off again for now. + This toggle will disappear eventually. + ``-vm`` load and run a p8-virt or p8-ir listing in the internal VirtualMachine instead of compiling a prog8 program file.. diff --git a/docs/source/todo.rst b/docs/source/todo.rst index d013d03eb..f7a270e02 100644 --- a/docs/source/todo.rst +++ b/docs/source/todo.rst @@ -2,7 +2,11 @@ TODO ==== -- [on branch: shortcircuit] investigate McCarthy evaluation again? this may also reduce code size perhaps for things like if a>4 or a<2 .... +- [on branch: shortcircuit] complete McCarthy evaluation. This may also reduce code size perhaps for things like if a>4 or a<2 .... + - vm ircodegen (DONE) + - in 6502 codegen (see vm's ExpressionGen operatorAnd / operatorOr) + +- IR: reduce amount of CMP/CMPI after instructions that set the status bits correctly (LOADs? INC? Bitwise operations, etc), but only after setting the status bits is verified! ... @@ -11,7 +15,6 @@ Future Things and Ideas ^^^^^^^^^^^^^^^^^^^^^^^ Compiler: -- (after shortcircuit is in:) What happens when we make all subs return a boolean not as ubyte in A, but in the cpu's Carry flag? - Multidimensional arrays and chained indexing, purely as syntactic sugar over regular arrays. - make a form of "manual generics" possible like: varsub routine(T arg)->T where T is expanded to a specific type (this is already done hardcoded for several of the builtin functions) @@ -27,7 +30,6 @@ Compiler: - (need separate step in codegen and IR to write the "golden" variables) - do we need (array)variable alignment tag instead of block alignment tag? You want to align the data, not the code in the block? -- ir: proper code gen for the CALLI instruction and that it (optionally) returns a word value that needs to be assigned to a reg - ir: getting it in shape for code generation - ir: related to the one above: block alignment doesn't translate well to variables in the block (the actual stuff that needs to be aligned in memory) but: need variable alignment tag instead of block alignment tag, really - ir: idea: (but LLVM IR simply keeps the variables, so not a good idea then?...): replace all scalar variables by an allocated register. Keep a table of the variable to register mapping (including the datatype) @@ -48,7 +50,7 @@ Compiler: Libraries: -- once a VAL_1 implementation is merged into the X16 kernal properly, remove all the workarounds in cx16 floats.parse_f() . Prototype parse routine in examples/cx16/floatparse.p8 +- once a VAL_1 implementation is merged into the X16 kernal properly, remove all the workarounds in cx16 floats.parse_f() - fix the problems in atari target, and flesh out its libraries. - c128 target: make syslib more complete (missing kernal routines)? - pet32 target: make syslib more complete (missing kernal routines)? @@ -56,6 +58,8 @@ Libraries: Optimizations: +- give a warning for variables that could be a const - or even make them a const (if not @shared)? +- treat every scalar variable decl with initialization value, as const by default, unless the variable gets assigned to somewhere (or has its address taken, or is @shared) - VariableAllocator: can we think of a smarter strategy for allocating variables into zeropage, rather than first-come-first-served? for instance, vars used inside loops first, then loopvars, then uwords used as pointers, then the rest - various optimizers skip stuff if compTarget.name==VMTarget.NAME. Once 6502-codegen is done from IR code, @@ -78,6 +82,9 @@ What if we were to re-introduce Structs in prog8? Some thoughts: Other language/syntax features to think about --------------------------------------------- -- add (rom/ram)bank support to romsub. A call will then automatically switch banks, use callfar and something else when in banked ram. - challenges: how to not make this too X16 specific? How does the compiler know what bank to switch (ram/rom)? - How to make it performant when we want to (i.e. NOT have it use callfar/auto bank switching) ? +- module directive to set the text encoding for that whole file (iso, petscii, etc.) +- chained assignments `x=y=z=99` +- declare multiple variables `ubyte x,y,z` (if init value present, all get that init value) +- chained comparisons `10calc_x2() + txt.print("* 1b and fail\n") + + txt.print("\n1c:\n") + if calc_a1()>calc_x1() and calc_a2()<=calc_x2() + txt.print("* 1c and fail\n") + + txt.print("\n2a:\n") + if calc_a1()calc_x2() + txt.print("* 2b or ok\n") + + txt.print("\n3a:\n") + if calc_a1()>calc_x1() or calc_a2()<=calc_x2() + txt.print("* 3a or ok\n") + + txt.print("\n3b:\n") + if calc_a1()>calc_x1() or calc_a2()>calc_x2() + txt.print("* 3b or fail\n") + + txt.print("\n4a:\n") + bool result = calc_a1()calc_x2() + txt.print_ub(result) + txt.nl() + txt.print("\n4b:\n") + result = calc_a1()>=calc_x1() and calc_a2()>calc_x2() + txt.print_ub(result) + txt.nl() + @($4000) &= 22 + + sub calc_a1() -> ubyte { + txt.print("calc_a1\n") + return a1+zero + } + sub calc_a2() -> ubyte { + txt.print("calc_a2\n") + return a2+zero + } + sub calc_x1() -> ubyte { + txt.print("calc_x1\n") + return x1+zero + } + sub calc_x2() -> ubyte { + txt.print("calc_x2\n") + return x2+zero + } } } diff --git a/httpCompilerService/src/prog8/http/TestHttp.kt b/httpCompilerService/src/prog8/http/TestHttp.kt index 85b589d8e..ac98a539a 100644 --- a/httpCompilerService/src/prog8/http/TestHttp.kt +++ b/httpCompilerService/src/prog8/http/TestHttp.kt @@ -40,6 +40,7 @@ class RequestParser : Take { quietAssembler = false, includeSourcelines = false, asmListfile = false, + shortCircuit = true, experimentalCodegen = false, splitWordArrays = false, breakpointCpuInstruction = false, diff --git a/intermediate/src/prog8/intermediate/IRInstructions.kt b/intermediate/src/prog8/intermediate/IRInstructions.kt index 15c9401c3..0beeb48cc 100644 --- a/intermediate/src/prog8/intermediate/IRInstructions.kt +++ b/intermediate/src/prog8/intermediate/IRInstructions.kt @@ -14,10 +14,10 @@ Program to execute is not stored in the system memory, it's just a separate list 65536 virtual floating point registers (64 bits double precision) fr0-fr65535 65536 bytes of memory. Thus memory pointers (addresses) are limited to 16 bits. Value stack, max 128 entries of 1 byte each. -Status flags: Carry, Zero, Negative. NOTE: status flags are only affected by the CMP instruction or explicit CLC/SEC!!! - LOAD instructions DO affect the Z and N flags - INC/DEC instructions DO affect the Z and N flags - other instructions such as logical or arithmetic operations DO NOT AFFECT THE STATUS FLAGS UNLESS EXPLICITLY NOTED! +Status flags: Carry, Zero, Negative. NOTE: status flags are only affected by the CMP instruction or explicit CLC/SEC, + LOAD instructions DO affect the Z and N flags. + INC/DEC instructions DO affect the Z and N flags, + other instructions only affect Z an N flags if the value in a result register is written. Instruction set is mostly a load/store architecture, there are few instructions operating on memory directly. diff --git a/virtualmachine/src/prog8/vm/VirtualMachine.kt b/virtualmachine/src/prog8/vm/VirtualMachine.kt index ad4130338..acd198d57 100644 --- a/virtualmachine/src/prog8/vm/VirtualMachine.kt +++ b/virtualmachine/src/prog8/vm/VirtualMachine.kt @@ -311,8 +311,16 @@ class VirtualMachine(irProgram: IRProgram) { private inline fun setResultReg(reg: Int, value: Int, type: IRDataType) { when(type) { - IRDataType.BYTE -> registers.setUB(reg, value.toUByte()) - IRDataType.WORD -> registers.setUW(reg, value.toUShort()) + IRDataType.BYTE -> { + registers.setUB(reg, value.toUByte()) + statusZero = value==0 + statusNegative = value>=0x80 + } + IRDataType.WORD -> { + registers.setUW(reg, value.toUShort()) + statusZero = value==0 + statusNegative = value>=0x8000 + } IRDataType.FLOAT -> throw IllegalArgumentException("attempt to set integer result register but float type") } }