From 2ff86889db1a121977fd0d7a1fbd147b6ab2c2f9 Mon Sep 17 00:00:00 2001 From: Karol Stasiak Date: Tue, 24 Sep 2019 17:37:06 +0200 Subject: [PATCH] Fix sign extension in arithmetic promotions again. --- .../compiler/AbstractExpressionCompiler.scala | 10 +- .../m6809/M6809ExpressionCompiler.scala | 107 +++++++++++------- .../compiler/m6809/M6809MacroExpander.scala | 65 +++++++++++ .../compiler/mos/MosExpressionCompiler.scala | 22 +++- .../compiler/z80/Z80ExpressionCompiler.scala | 12 +- .../millfork/output/M6809Assembler.scala | 2 + src/test/scala/millfork/test/MacroSuite.scala | 4 +- .../millfork/test/SignExtensionSuite.scala | 37 +++++- 8 files changed, 202 insertions(+), 57 deletions(-) create mode 100644 src/main/scala/millfork/compiler/m6809/M6809MacroExpander.scala diff --git a/src/main/scala/millfork/compiler/AbstractExpressionCompiler.scala b/src/main/scala/millfork/compiler/AbstractExpressionCompiler.scala index b938d348..321c1f6a 100644 --- a/src/main/scala/millfork/compiler/AbstractExpressionCompiler.scala +++ b/src/main/scala/millfork/compiler/AbstractExpressionCompiler.scala @@ -4,6 +4,7 @@ import millfork.env._ import millfork.node._ import millfork.error.{ConsoleLogger, Logger} import millfork.assembly.AbstractCode +import millfork.output.NoAlignment /** * @author Karol Stasiak @@ -572,7 +573,12 @@ object AbstractExpressionCompiler { def lookupFunction(env: Environment, log: Logger, f: FunctionCallExpression): MangledFunction = { val paramsWithTypes = f.expressions.map(x => getExpressionType(env, log, x) -> x) - env.lookupFunction(f.functionName, paramsWithTypes).getOrElse( - log.fatal(s"Cannot find function `${f.functionName}` with given params `${paramsWithTypes.map(_._1).mkString("(", ",", ")")}`", f.position)) + env.lookupFunction(f.functionName, paramsWithTypes).getOrElse { + log.error(s"Cannot find function `${f.functionName}` with given params `${paramsWithTypes.map(_._1).mkString("(", ",", ")")}`", f.position) + val signature = NormalParamSignature(paramsWithTypes.map { case (t, _) => + UninitializedMemoryVariable("?", t, VariableAllocationMethod.Auto, None, NoAlignment, isVolatile = false) + }) + ExternFunction(f.functionName, NullType, signature, Constant.Zero, env, None) + } } } diff --git a/src/main/scala/millfork/compiler/m6809/M6809ExpressionCompiler.scala b/src/main/scala/millfork/compiler/m6809/M6809ExpressionCompiler.scala index 21de60cb..76bb3828 100644 --- a/src/main/scala/millfork/compiler/m6809/M6809ExpressionCompiler.scala +++ b/src/main/scala/millfork/compiler/m6809/M6809ExpressionCompiler.scala @@ -4,7 +4,7 @@ import millfork.assembly.m6809.{DAccumulatorIndexed, Indexed, MLine, MOpcode, Tw import millfork.compiler.{AbstractExpressionCompiler, BranchIfFalse, BranchIfTrue, BranchSpec, ComparisonType, CompilationContext, NoBranching} import millfork.node.{DerefExpression, Expression, FunctionCallExpression, GeneratedConstantExpression, IndexedExpression, LhsExpression, LiteralExpression, M6809Register, SumExpression, VariableExpression} import millfork.assembly.m6809.MOpcode._ -import millfork.env.{AssemblyParamSignature, Constant, ConstantBooleanType, ConstantPointy, ExternFunction, FatBooleanType, M6809RegisterVariable, MathOperator, MemoryVariable, NormalFunction, NormalParamSignature, NumericConstant, StackVariablePointy, Type, Variable, VariablePointy} +import millfork.env.{AssemblyParamSignature, Constant, ConstantBooleanType, ConstantPointy, ExternFunction, FatBooleanType, FunctionInMemory, M6809RegisterVariable, MacroFunction, MathOperator, MemoryVariable, NormalFunction, NormalParamSignature, NumericConstant, StackVariablePointy, Type, Variable, VariablePointy} import scala.collection.GenTraversableOnce @@ -285,8 +285,9 @@ object M6809ExpressionCompiler extends AbstractExpressionCompiler[MLine] { env.maybeGet[Type](fce.functionName) match { case Some(typ) => val sourceType = validateTypeCastAndGetSourceExpressionType(ctx, typ, params) - return sourceType.size match { - case 1 => compileToB(ctx, params.head) ++ targetifyB(ctx, target, isSigned = sourceType.isSigned) + return typ.size match { + // TODO: alternating signedness? + case 1 => compileToB(ctx, params.head) ++ targetifyB(ctx, target, isSigned = typ.isSigned) case 2 => compileToD(ctx, params.head) ++ targetifyD(ctx, target) case _ => ??? } @@ -294,47 +295,71 @@ object M6809ExpressionCompiler extends AbstractExpressionCompiler[MLine] { // fallthrough to the lookup below } val f = lookupFunction(ctx, fce) - val prepareParams: List[MLine] = f.params match { - case NormalParamSignature(List(param)) if param.typ.size == 1 => - compileToB(ctx, params.head) - case NormalParamSignature(List(param)) if param.typ.size == 2 => - compileToD(ctx, params.head) - case NormalParamSignature(signature) => - params.zip(signature).flatMap { case (paramExpr, paramVar) => - val callCtx = callingContext(ctx, f.name, paramVar) - paramVar.typ.size match { - case 1 => - compileToB(ctx, paramExpr) ++ storeB(callCtx, VariableExpression(paramVar.name + "`aa")) - case 2 => - compileToD(ctx, paramExpr) ++ storeD(callCtx, VariableExpression(paramVar.name + "`aa")) - case _ => + f match { + case function: MacroFunction => + val (paramPreparation, statements) = M6809MacroExpander.inlineFunction(ctx, function, params, expr.position) + paramPreparation ++ statements.flatMap { s => + M6809StatementCompiler.compile(ctx, s) match { + case (code, Nil) => code + case (code, _) => ctx.log.error("Invalid statement in macro expansion", fce.position); code + } + } + case _:FunctionInMemory => + val prepareParams: List[MLine] = f.params match { + case NormalParamSignature(List(param)) if param.typ.size == 1 => + compileToB(ctx, params.head) + case NormalParamSignature(List(param)) if param.typ.size == 2 => + compileToD(ctx, params.head) + case NormalParamSignature(signature) => + params.zip(signature).flatMap { case (paramExpr, paramVar) => + val callCtx = callingContext(ctx, f.name, paramVar) + paramVar.typ.size match { + case 1 => + compileToB(ctx, paramExpr) ++ storeB(callCtx, VariableExpression(paramVar.name + "`aa")) + case 2 => + compileToD(ctx, paramExpr) ++ storeD(callCtx, VariableExpression(paramVar.name + "`aa")) + case _ => + ??? + } + } + case AssemblyParamSignature(signature) => + params.zip(signature).flatMap { case (e, a) => + val compiled = a.variable match { + case M6809RegisterVariable(M6809Register.A, _) => compileToA(ctx, e) + case M6809RegisterVariable(M6809Register.B, _) => compileToB(ctx, e) + case M6809RegisterVariable(M6809Register.D, _) => compileToD(ctx, e) + case M6809RegisterVariable(M6809Register.X, _) => compileToX(ctx, e) + case M6809RegisterVariable(M6809Register.Y, _) => compileToY(ctx, e) + case M6809RegisterVariable(M6809Register.U, _) => compileToU(ctx, e) + case _ => ??? + } + if (compiled.length > 1) ??? + compiled + } + case _ => ??? + } + val actualCall = f match { + case nf:NormalFunction => + List(MLine.absolute(JSR, nf.toAddress)) + case nf:ExternFunction => + List(MLine.absolute(JSR, nf.toAddress)) + case _ => + println("Unsupported function: " + f.name) + ??? + } + val storeResult = f.returnType.size match { + case 1 => targetifyB(ctx, target, f.returnType.isSigned) + case 2 => targetifyD(ctx, target) + case _ => + if (target == MExpressionTarget.NOTHING) { + Nil + } else { + println("Unsupported function: " + f.name) ??? - } + } } - case AssemblyParamSignature(signature) => - params.zip(signature).flatMap { case (e, a) => - val compiled = a.variable match { - case M6809RegisterVariable(M6809Register.A, _) => compileToA(ctx, e) - case M6809RegisterVariable(M6809Register.B, _) => compileToB(ctx, e) - case M6809RegisterVariable(M6809Register.D, _) => compileToD(ctx, e) - case M6809RegisterVariable(M6809Register.X, _) => compileToX(ctx, e) - case M6809RegisterVariable(M6809Register.Y, _) => compileToY(ctx, e) - case M6809RegisterVariable(M6809Register.U, _) => compileToU(ctx, e) - case _ => ??? - } - if (compiled.length > 1) ??? - compiled - } - case _ => ??? + prepareParams ++ actualCall ++ storeResult } - val actualCall = f match { - case nf:NormalFunction => - List(MLine.absolute(JSR, nf.toAddress)) - case nf:ExternFunction => - List(MLine.absolute(JSR, nf.toAddress)) - case _ => ??? - } - prepareParams ++ actualCall } case _ => ??? } diff --git a/src/main/scala/millfork/compiler/m6809/M6809MacroExpander.scala b/src/main/scala/millfork/compiler/m6809/M6809MacroExpander.scala new file mode 100644 index 00000000..e1f3ce0a --- /dev/null +++ b/src/main/scala/millfork/compiler/m6809/M6809MacroExpander.scala @@ -0,0 +1,65 @@ +package millfork.compiler.m6809 + +import java.util.Locale + +import millfork.assembly.m6809.MLine +import millfork.assembly.z80.ZLine +import millfork.compiler.{CompilationContext, MacroExpander} +import millfork.env._ +import millfork.node._ + +/** + * @author Karol Stasiak + */ +object M6809MacroExpander extends MacroExpander[MLine] { + + override def prepareAssemblyParams(ctx: CompilationContext, assParams: List[AssemblyParam], params: List[Expression], code: List[ExecutableStatement]): (List[MLine], List[ExecutableStatement]) = { + var paramPreparation = List[MLine]() + var actualCode = code + var hadRegisterParam = false + assParams.zip(params).foreach { + case (AssemblyParam(typ, Placeholder(ph, phType), AssemblyParameterPassingBehaviour.ByReference), actualParam) => + actualParam match { + case VariableExpression(vname) => + ctx.env.get[ThingInMemory](vname) + case l: LhsExpression => + // TODO: ?? + M6809ExpressionCompiler.compileToB(ctx, l) + case _ => + ctx.log.error("A non-assignable expression was passed to an inlineable function as a `ref` parameter", actualParam.position) + } + actualCode = actualCode.map { + case a@MosAssemblyStatement(_, _, expr, _) => + a.copy(expression = expr.replaceVariable(ph, actualParam)) + case x => x + } + case (AssemblyParam(typ, Placeholder(ph, phType), AssemblyParameterPassingBehaviour.ByConstant), actualParam) => + ctx.env.eval(actualParam).getOrElse(ctx.env.errorConstant("Non-constant expression was passed to an inlineable function as a `const` parameter", actualParam.position)) + actualCode = actualCode.map { + case a@Z80AssemblyStatement(_, _, _, expr, _) => + a.copy(expression = expr.replaceVariable(ph, actualParam)) + case x => x + } + case (AssemblyParam(typ, v@M6809RegisterVariable(register, _), AssemblyParameterPassingBehaviour.Copy), actualParam) => + if (hadRegisterParam) { + ctx.log.error("Only one macro assembly function parameter can be passed via a register", actualParam.position) + } + hadRegisterParam = true + paramPreparation = (register, typ.size) match { + case (M6809Register.A, 1) => M6809ExpressionCompiler.compileToA(ctx, actualParam) + case (M6809Register.B, 1) => M6809ExpressionCompiler.compileToB(ctx, actualParam) + case (M6809Register.D, 2) => M6809ExpressionCompiler.compileToD(ctx, actualParam) + case (M6809Register.X, 2) => M6809ExpressionCompiler.compileToX(ctx, actualParam) + case (M6809Register.Y, 2) => M6809ExpressionCompiler.compileToY(ctx, actualParam) + case (M6809Register.U, 2) => M6809ExpressionCompiler.compileToU(ctx, actualParam) + case _ => + ctx.log.error(s"Invalid parameter for macro: ${typ.name} ${register.toString.toLowerCase(Locale.ROOT)}", actualParam.position) + Nil + } + case (AssemblyParam(_, _, AssemblyParameterPassingBehaviour.Copy), actualParam) => + ??? + case (_, actualParam) => + } + paramPreparation -> actualCode + } + } diff --git a/src/main/scala/millfork/compiler/mos/MosExpressionCompiler.scala b/src/main/scala/millfork/compiler/mos/MosExpressionCompiler.scala index 374f3cd5..b0495136 100644 --- a/src/main/scala/millfork/compiler/mos/MosExpressionCompiler.scala +++ b/src/main/scala/millfork/compiler/mos/MosExpressionCompiler.scala @@ -1638,10 +1638,26 @@ object MosExpressionCompiler extends AbstractExpressionCompiler[AssemblyLine] { env.maybeGet[Type](f.functionName) match { case Some(typ) => val sourceType = validateTypeCastAndGetSourceExpressionType(ctx, typ, params) - val newExprTypeAndVariable = exprTypeAndVariable.map { i => - (if (i._1.size == sourceType.size) i._1 else sourceType) -> i._2 + exprTypeAndVariable match { + case None => + return compile(ctx, params.head, None, branches) + case Some((_, targetVariable)) => + val targetType = targetVariable.typ + val officialType: Type = if (targetType.size == typ.size) { + typ + } else if (targetType.size == sourceType.size) { + targetType + } else if (typ.isSigned == sourceType.isSigned) { + targetType + } else if (typ.size == 1) { + return compileToA(ctx, f) ++ expressionStorageFromA(ctx, exprTypeAndVariable, position = expr.position, signedSource = typ.isSigned) + } else if (typ.size == 2) { + return compileToAX(ctx, f) ++ expressionStorageFromAX(ctx, exprTypeAndVariable, position = expr.position) + } else { + ctx.log.fatal(s"Cannot compile $typ($sourceType(...)) to $targetType") + } + return compile(ctx, params.head, Some(officialType -> targetVariable), branches) } - return compile(ctx, params.head, newExprTypeAndVariable, branches) case None => // fallthrough to the lookup below } diff --git a/src/main/scala/millfork/compiler/z80/Z80ExpressionCompiler.scala b/src/main/scala/millfork/compiler/z80/Z80ExpressionCompiler.scala index 494ad7eb..a52033d1 100644 --- a/src/main/scala/millfork/compiler/z80/Z80ExpressionCompiler.scala +++ b/src/main/scala/millfork/compiler/z80/Z80ExpressionCompiler.scala @@ -1202,8 +1202,9 @@ object Z80ExpressionCompiler extends AbstractExpressionCompiler[ZLine] { env.maybeGet[Type](f.functionName) match { case Some(typ) => val sourceType = validateTypeCastAndGetSourceExpressionType(ctx, typ, params) - return sourceType.size match { - case 1 => targetifyA(ctx, target, compileToA(ctx, params.head), isSigned = sourceType.isSigned) + return typ.size match { + // TODO: alternating signedness? + case 1 => targetifyA(ctx, target, compileToA(ctx, params.head), isSigned = typ.isSigned) case 2 => targetifyHL(ctx, target, compileToHL(ctx, params.head)) case _ => ??? } @@ -1213,8 +1214,11 @@ object Z80ExpressionCompiler extends AbstractExpressionCompiler[ZLine] { lookupFunction(ctx, f) match { case function: MacroFunction => val (paramPreparation, statements) = Z80MacroExpander.inlineFunction(ctx, function, params, expression.position) - paramPreparation ++ statements.map { - case _ => ??? + paramPreparation ++ statements.flatMap { s => + Z80StatementCompiler.compile(ctx, s) match { + case (code, Nil) => code + case (code, _) => ctx.log.error("Invalid statement in macro expansion", expression.position); code + } } case function: EmptyFunction => ??? // TODO: type conversion? diff --git a/src/main/scala/millfork/output/M6809Assembler.scala b/src/main/scala/millfork/output/M6809Assembler.scala index a83a8a58..a28545b5 100644 --- a/src/main/scala/millfork/output/M6809Assembler.scala +++ b/src/main/scala/millfork/output/M6809Assembler.scala @@ -87,6 +87,8 @@ class M6809Assembler(program: Program, index case MLine0(op, NonExistent, _) if MOpcode.NoopDiscard(op) => index + case MLine0(CHANGED_MEM, _, _) => + index case MLine0(TFR, TwoRegisters(source, target), param) if M6809Register.registerSize(source) == M6809Register.registerSize(target) && param.isProvablyZero => writeByte(bank, index, 0x1f) diff --git a/src/test/scala/millfork/test/MacroSuite.scala b/src/test/scala/millfork/test/MacroSuite.scala index e4f2fbae..8a4ea1e0 100644 --- a/src/test/scala/millfork/test/MacroSuite.scala +++ b/src/test/scala/millfork/test/MacroSuite.scala @@ -10,7 +10,7 @@ import org.scalatest.{FunSuite, Matchers} class MacroSuite extends FunSuite with Matchers { test("Most basic test") { - EmuCrossPlatformBenchmarkRun(Cpu.Mos, Cpu.Z80, Cpu.Intel8086)( + EmuCrossPlatformBenchmarkRun(Cpu.Mos, Cpu.Z80, Cpu.Intel8086, Cpu.Motorola6809)( """ | macro void run(byte x) { | output = x @@ -29,7 +29,7 @@ class MacroSuite extends FunSuite with Matchers { } test("Macros in assembly") { - EmuCrossPlatformBenchmarkRun(Cpu.Mos, Cpu.Z80, Cpu.Intel8086)( + EmuCrossPlatformBenchmarkRun(Cpu.Mos, Cpu.Z80, Cpu.Intel8086, Cpu.Motorola6809)( """ | macro void run(byte x) { | output = x diff --git a/src/test/scala/millfork/test/SignExtensionSuite.scala b/src/test/scala/millfork/test/SignExtensionSuite.scala index 545790da..4aecfc2b 100644 --- a/src/test/scala/millfork/test/SignExtensionSuite.scala +++ b/src/test/scala/millfork/test/SignExtensionSuite.scala @@ -1,7 +1,7 @@ package millfork.test import millfork.Cpu -import millfork.test.emu.{EmuCrossPlatformBenchmarkRun, EmuUnoptimizedCrossPlatformRun} +import millfork.test.emu.{EmuCrossPlatformBenchmarkRun, EmuUnoptimizedCrossPlatformRun, ShouldNotCompile} import org.scalatest.{FunSuite, Matchers} /** @@ -10,7 +10,7 @@ import org.scalatest.{FunSuite, Matchers} class SignExtensionSuite extends FunSuite with Matchers { test("Sbyte to Word") { - EmuCrossPlatformBenchmarkRun(Cpu.Mos, Cpu.Z80, Cpu.Intel8080, Cpu.Sharp, Cpu.Intel8086)(""" + EmuCrossPlatformBenchmarkRun(Cpu.Mos, Cpu.Z80, Cpu.Intel8080, Cpu.Sharp, Cpu.Intel8086, Cpu.Motorola6809)(""" | word output @$c000 | void main () { | sbyte b @@ -22,7 +22,7 @@ class SignExtensionSuite extends FunSuite with Matchers { } } test("Sbyte to Word 2") { - EmuCrossPlatformBenchmarkRun(Cpu.Mos, Cpu.Z80, Cpu.Intel8080, Cpu.Sharp, Cpu.Intel8086)(""" + EmuCrossPlatformBenchmarkRun(Cpu.Mos, Cpu.Z80, Cpu.Intel8080, Cpu.Sharp, Cpu.Intel8086, Cpu.Motorola6809)(""" | word output @$c000 | void main () { | output = b() @@ -71,7 +71,7 @@ class SignExtensionSuite extends FunSuite with Matchers { } test("Byte to Word") { - EmuUnoptimizedCrossPlatformRun(Cpu.Mos, Cpu.Z80)(""" + EmuUnoptimizedCrossPlatformRun(Cpu.Mos, Cpu.Z80, Cpu.Motorola6809)(""" | word output @$c000 | void main () { | sbyte b @@ -85,7 +85,7 @@ class SignExtensionSuite extends FunSuite with Matchers { } test("Byte to Word 2") { - EmuUnoptimizedCrossPlatformRun(Cpu.Mos, Cpu.Z80)(""" + EmuUnoptimizedCrossPlatformRun(Cpu.Mos, Cpu.Z80, Cpu.Motorola6809)(""" | word output @$c000 | void main () { | sbyte b @@ -97,4 +97,31 @@ class SignExtensionSuite extends FunSuite with Matchers { m.readWord(0xc000) should equal(0x00ff) } } + + test("Returning sbyte as word") { + EmuUnoptimizedCrossPlatformRun(Cpu.Mos, Cpu.Z80, Cpu.Motorola6809)(""" + | word output @$c000 + | void main () { + | output = f($ff) + | } + | noinline word f(byte x) = sbyte(x) + """.stripMargin){m => + m.readWord(0xc000) should equal(0xffff) + } + } + + test("Check multilayered conversions of signed and unsigned types") { + EmuUnoptimizedCrossPlatformRun(Cpu.Mos, Cpu.Z80)( + """ + |long output @$c000 + |noinline sbyte f() = $FE + |void main() { + | // should yield $0000fffe + | output = word(f()) + |} + |""".stripMargin) { m => + m.readLong(0xc000) should equal(0xfffe) + } + } + }