From 629691dfb311e0ed8c71bdc1ed3636a36676a6f2 Mon Sep 17 00:00:00 2001 From: Karol Stasiak Date: Tue, 16 Apr 2019 12:09:14 +0200 Subject: [PATCH] Fix stack variables on 8080 and LR35902 --- .../z80/opt/AlwaysGoodI80Optimizations.scala | 3 +- .../opt/RuleBasedAssemblyOptimization.scala | 17 +++- .../compiler/z80/Z80ExpressionCompiler.scala | 31 +++++++- .../scala/millfork/test/StackVarSuite.scala | 77 ++++++++++++++----- .../scala/millfork/test/StructSuite.scala | 3 +- .../emu/EmuUnoptimizedCrossPlatformRun.scala | 2 +- 6 files changed, 108 insertions(+), 25 deletions(-) diff --git a/src/main/scala/millfork/assembly/z80/opt/AlwaysGoodI80Optimizations.scala b/src/main/scala/millfork/assembly/z80/opt/AlwaysGoodI80Optimizations.scala index 2e6d352f..b0c7d035 100644 --- a/src/main/scala/millfork/assembly/z80/opt/AlwaysGoodI80Optimizations.scala +++ b/src/main/scala/millfork/assembly/z80/opt/AlwaysGoodI80Optimizations.scala @@ -173,7 +173,8 @@ object AlwaysGoodI80Optimizations { for7Registers(register => (Elidable & Is8BitLoadTo(register) & NoOffset & MatchSourceRegisterAndOffset(1)) ~ (Linear & Not(Concerns(register)) & DoesntChangeMatchedRegisterAndOffset(1)).* ~ - (Elidable & IsRegular8BitLoadFrom(register) & DoesntMatterWhatItDoesWith(register)) ~~> { code => + (Elidable & IsRegular8BitLoadFrom(register) & DoesntMatterWhatItDoesWith(register) & MatchTargetRegisterAndOffset(2)) ~ + Where(ctx => ctx.areCompatibleForLoad(2, 1)) ~~> { code => val last = code.last val head = code.head code.tail.init :+ ZLine(LD, (last.registers, head.registers) match { diff --git a/src/main/scala/millfork/assembly/z80/opt/RuleBasedAssemblyOptimization.scala b/src/main/scala/millfork/assembly/z80/opt/RuleBasedAssemblyOptimization.scala index d880d340..5e84cabb 100644 --- a/src/main/scala/millfork/assembly/z80/opt/RuleBasedAssemblyOptimization.scala +++ b/src/main/scala/millfork/assembly/z80/opt/RuleBasedAssemblyOptimization.scala @@ -100,7 +100,7 @@ class AssemblyMatchingContext(val compilationOptions: CompilationOptions) { def log: Logger = compilationOptions.log - override def toString: String = map.mkString(", ") + override def toString: String = if (map.isEmpty) "(empty context)" else map.mkString(", ") def addObject(i: Int, o: Any): Boolean = { if (map.contains(i)) { @@ -180,6 +180,21 @@ class AssemblyMatchingContext(val compilationOptions: CompilationOptions) { jumps.isEmpty } + def areCompatibleForLoad(target: Int, source: Int): Boolean = { + val t = get[RegisterAndOffset](target).register + val s = get[RegisterAndOffset](source).register + import ZRegister._ + if (t == A || s == A) return true + if (t == MEM_DE || s == MEM_DE || t == MEM_BC || s == MEM_BC) return false + if (t == B || t == C || t == D || t == E) return true + if (s == B || s == C || s == D || s == E) return true + if ((t == IXH || t == IXL) && (s == IXH || s == IXL)) return true + if ((t == IYH || t == IYL) && (s == IYH || s == IYL)) return true + if ((t == H || t == L) && (s == MEM_HL || s == MEM_IX_D || s == MEM_IY_D)) return true + if ((s == H || s == L) && (t == MEM_HL || t == MEM_IX_D || t == MEM_IY_D)) return true + false + } + } object HelperCheckers { diff --git a/src/main/scala/millfork/compiler/z80/Z80ExpressionCompiler.scala b/src/main/scala/millfork/compiler/z80/Z80ExpressionCompiler.scala index e8917eb2..ce8e6385 100644 --- a/src/main/scala/millfork/compiler/z80/Z80ExpressionCompiler.scala +++ b/src/main/scala/millfork/compiler/z80/Z80ExpressionCompiler.scala @@ -1410,8 +1410,37 @@ object Z80ExpressionCompiler extends AbstractExpressionCompiler[ZLine] { case VariableExpression(vname) => env.get[Variable](vname) match { case v: Variable => + import ZRegister._ val size = v.typ.size - compileByteReads(ctx, source, size, ZExpressionTarget.HL).zip(compileByteStores(ctx, target, size, includeStep = false)).flatMap(t => t._1 ++ t._2) + val reads = compileByteReads(ctx, source, size, ZExpressionTarget.HL) + val stores = compileByteStores(ctx, target, size, includeStep = true) + if (stores.exists(_.exists(_.changesRegister(HL)))) { + if (reads.tail.exists(_.exists(_.changesRegister(HL)))) { + // most likely stack-to-stack copy + // use DE as the secondary pointer + val fixedReads = reads.head.init ++ List(ZLine.ld8(E, L), ZLine.ld8(D, H), ZLine.ld8(A, MEM_DE)) :: reads.tail.map(_.map { + case l@ZLine0(LD, TwoRegisters(A, MEM_HL), _) => l.copy(registers = TwoRegisters(A, MEM_DE)) + case l@ZLine0(INC_16, OneRegister(HL), _) => l.copy(registers = OneRegister(DE)) + case l@ZLine0(DEC_16, OneRegister(HL), _) => l.copy(registers = OneRegister(DE)) + case l => l + }) + fixedReads.zip(stores).flatMap(t => t._1 ++ t._2) + } else { + val fixedReads = reads.head ++ List(ZLine.ld8(B, H)) :: reads.tail.map(_.map { + case l@ZLine0(LD, TwoRegisters(reg, H), _) => l.copy(registers = TwoRegisters(reg, B)) + case l@ZLine0(LD, TwoRegisters(reg, L), _) => l.copy(registers = TwoRegisters(reg, C)) + case l => l + }) + val fixedStores = stores.map(_.map { + case l@ZLine0(LD, TwoRegisters(H, reg), _) => l.copy(registers = TwoRegisters(B, reg)) + case l@ZLine0(LD, TwoRegisters(L, reg), _) => l.copy(registers = TwoRegisters(C, reg)) + case l => l + }) + fixedReads.zip(fixedStores).flatMap(t => t._1 ++ t._2) + } + } else { + reads.zip(stores).flatMap(t => t._1 ++ t._2) + } } case _ => ??? } diff --git a/src/test/scala/millfork/test/StackVarSuite.scala b/src/test/scala/millfork/test/StackVarSuite.scala index 01970341..420d14ba 100644 --- a/src/test/scala/millfork/test/StackVarSuite.scala +++ b/src/test/scala/millfork/test/StackVarSuite.scala @@ -1,7 +1,7 @@ package millfork.test import millfork.Cpu -import millfork.test.emu.{EmuCrossPlatformBenchmarkRun, EmuOptimizedSoftwareStackRun, EmuSoftwareStackBenchmarkRun, EmuUnoptimizedZ80Run} +import millfork.test.emu.{EmuCrossPlatformBenchmarkRun, EmuSoftwareStackBenchmarkRun, EmuUnoptimizedCrossPlatformRun} import org.scalatest.{FunSuite, Matchers} /** @@ -69,24 +69,26 @@ class StackVarSuite extends FunSuite with Matchers { """.stripMargin)(_.readWord(0xc000) should equal(21)) } - // ERROR: (8:9) Right-hand-side expression is too complex -// test("Stack byte subtraction") { -// EmuUnoptimizedRun(""" -// | byte output @$c000 -// | void main () { -// | stack byte a -// | stack byte b -// | b = $77 -// | a = $11 -// | b -= zzz() -// | b -= a -// | output = b -// | } -// | byte zzz() { -// | return $22 -// | } -// """.stripMargin).readByte(0xc000) should equal(0x44) -// } + test("Stack byte subtraction") { + EmuUnoptimizedCrossPlatformRun(Cpu.Mos, Cpu.Z80, Cpu.Intel8080)( + """ + | byte output @$c000 + | void main () { + | stack byte a + | stack byte b + | b = $77 + | a = $11 + | b -= zzz() + | b -= a + | output = b + | } + | byte zzz() { + | return $22 + | } + """.stripMargin) { m => + m.readByte(0xc000) should equal(0x44) + } + } test("Stack word addition") { EmuCrossPlatformBenchmarkRun(Cpu.StrictMos, Cpu.Z80, Cpu.Intel8080, Cpu.Sharp)(""" @@ -251,4 +253,41 @@ class StackVarSuite extends FunSuite with Matchers { m.readByte(0xc003) should equal(2) } } + + test("Large stack storage") { + EmuUnoptimizedCrossPlatformRun(Cpu.Mos, Cpu.Z80, Cpu.Intel8080, Cpu.Sharp)( + """ + | int32 output @$c000 + | void main () { + | stack int32 a + | a = f() + | barrier() + | output = a + | } + | noinline int32 f() = 400 + | noinline void barrier(){} + """.stripMargin) { m => + m.readLong(0xc000) should equal(400) + } + } + + test("Large stack-to-stack transfer") { + EmuCrossPlatformBenchmarkRun(Cpu.Mos, Cpu.Z80, Cpu.Intel8080, Cpu.Sharp)( + """ + | int32 output @$c000 + | void main () { + | stack int32 a + | stack int32 b + | a = f() + | barrier() + | b = a + | barrier() + | output = b + | } + | noinline int32 f() = 400 + | noinline void barrier(){} + """.stripMargin) { m => + m.readLong(0xc000) should equal(400) + } + } } diff --git a/src/test/scala/millfork/test/StructSuite.scala b/src/test/scala/millfork/test/StructSuite.scala index ceddcc1e..89b0eabc 100644 --- a/src/test/scala/millfork/test/StructSuite.scala +++ b/src/test/scala/millfork/test/StructSuite.scala @@ -10,8 +10,7 @@ import org.scalatest.{FunSuite, Matchers} class StructSuite extends FunSuite with Matchers { test("Basic struct support") { - // TODO: 8080 has broken stack operations, fix and uncomment! - EmuUnoptimizedCrossPlatformRun(Cpu.Mos, Cpu.Z80/*, Cpu.Intel8080*/)(""" + EmuUnoptimizedCrossPlatformRun(Cpu.Mos, Cpu.Z80, Cpu.Intel8080)(""" | struct point { | byte x | byte y diff --git a/src/test/scala/millfork/test/emu/EmuUnoptimizedCrossPlatformRun.scala b/src/test/scala/millfork/test/emu/EmuUnoptimizedCrossPlatformRun.scala index 8f7b4a96..f989f8cd 100644 --- a/src/test/scala/millfork/test/emu/EmuUnoptimizedCrossPlatformRun.scala +++ b/src/test/scala/millfork/test/emu/EmuUnoptimizedCrossPlatformRun.scala @@ -8,7 +8,7 @@ import millfork.output.MemoryBank */ object EmuUnoptimizedCrossPlatformRun { def apply(platforms: Cpu.Value*)(source: String)(verifier: MemoryBank => Unit): Unit = { - val (_, mm) = if (platforms.contains(Cpu.Mos)) EmuUnoptimizedRun.apply2(source) else Timings(-1, -1) -> null + val (_, mm) = if (platforms.contains(Cpu.Mos) || platforms.contains(Cpu.StrictMos)) EmuUnoptimizedRun.apply2(source) else Timings(-1, -1) -> null val (_, mc) = if (platforms.contains(Cpu.Cmos)) EmuUnoptimizedCmosRun.apply2(source) else Timings(-1, -1) -> null val (_, mn) = if (platforms.contains(Cpu.Ricoh)) EmuUnoptimizedRicohRun.apply2(source) else Timings(-1, -1) -> null val (_, mz) = if (platforms.contains(Cpu.Z80)) EmuUnoptimizedZ80Run.apply2(source) else Timings(-1, -1) -> null