From d785d43ae7d2a25325e8bc6bef3118fe681215c5 Mon Sep 17 00:00:00 2001 From: Karol Stasiak Date: Wed, 28 Mar 2018 12:32:26 +0200 Subject: [PATCH] Many optimization improvements and bugfixes - fixed VariableToRegisterOptimization removing variables during superoptimization - fixed PointlessMathFromFlow giving results that do not fit a byte - fixed PointlessLoadBeforeReturn moving reads from before to after memory modification - achieved and exceeded CC65 performance when doing 16-bit Eratosthenes sieve --- CHANGELOG.md | 2 + .../scala/millfork/CompilationOptions.scala | 4 +- .../scala/millfork/OptimizationPresets.scala | 6 + .../millfork/assembly/AssemblyLine.scala | 3 + .../opt/AlwaysGoodOptimizations.scala | 144 ++++++++++++++++-- .../assembly/opt/ReverseFlowAnalyzer.scala | 72 ++++++++- .../opt/ReverseFlowAnalyzerPerOpcode.scala | 24 +-- .../opt/RuleBasedAssemblyOptimization.scala | 33 +++- .../assembly/opt/SuperOptimizer.scala | 9 +- .../opt/VariableToRegisterOptimization.scala | 9 +- .../opt/ZeropageRegisterOptimizations.scala | 11 +- .../millfork/compiler/StatementCompiler.scala | 8 +- .../scala/millfork/test/WordMathSuite.scala | 38 +++++ 13 files changed, 316 insertions(+), 47 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 314a0c80..15b5f591 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,8 @@ * Fixed several bugs, most importantly invalid offsets for branching instructions. +* Other improvements. + ## 0.2.2 * Allowed adding constant words to variable bytes without the zeropage pseudoregister. diff --git a/src/main/scala/millfork/CompilationOptions.scala b/src/main/scala/millfork/CompilationOptions.scala index ea401a8c..f1cea382 100644 --- a/src/main/scala/millfork/CompilationOptions.scala +++ b/src/main/scala/millfork/CompilationOptions.scala @@ -143,7 +143,9 @@ object CompilationFlag extends Enumeration { // warning options ExtraComparisonWarnings, RorWarning, - FatalWarnings = Value + FatalWarnings, + // special options for internal compiler use + InternalCurrentlyOptimizingForMeasurement = Value val allWarnings: Set[CompilationFlag.Value] = Set(ExtraComparisonWarnings) diff --git a/src/main/scala/millfork/OptimizationPresets.scala b/src/main/scala/millfork/OptimizationPresets.scala index b408d9d8..5d5cc034 100644 --- a/src/main/scala/millfork/OptimizationPresets.scala +++ b/src/main/scala/millfork/OptimizationPresets.scala @@ -76,6 +76,8 @@ object OptimizationPresets { AlwaysGoodOptimizations.PointlessStashingToIndexOverShortSafeBranch, AlwaysGoodOptimizations.LoopInvariantRegister, AlwaysGoodOptimizations.PointlessStackStashing, + AlwaysGoodOptimizations.PointlessStashingForLaterStore, + AlwaysGoodOptimizations.PointlessStashingForLaterLoad, AlwaysGoodOptimizations.RearrangeMath, EmptyMemoryStoreRemoval, AlwaysGoodOptimizations.PointlessLoadBeforeReturn, @@ -91,6 +93,7 @@ object OptimizationPresets { LaterOptimizations.PointlessLoadAfterStore, AlwaysGoodOptimizations.PoinlessLoadBeforeAnotherLoad, AlwaysGoodOptimizations.RearrangableLoadFromTheSameLocation, + AlwaysGoodOptimizations.ShiftingJustWrittenValue, LaterOptimizations.LoadingAfterShifting, EmptyMemoryStoreRemoval, @@ -173,6 +176,8 @@ object OptimizationPresets { AlwaysGoodOptimizations.PointlessRegisterTransfersBeforeReturn, AlwaysGoodOptimizations.PointlessSignCheck, AlwaysGoodOptimizations.PointlessStackStashing, + AlwaysGoodOptimizations.PointlessStashingForLaterLoad, + AlwaysGoodOptimizations.PointlessStashingForLaterStore, AlwaysGoodOptimizations.PointlessStashingToIndexOverShortSafeBranch, AlwaysGoodOptimizations.PoinlessStoreBeforeStore, AlwaysGoodOptimizations.PointlessStoreToTheSameVariable, @@ -180,6 +185,7 @@ object OptimizationPresets { AlwaysGoodOptimizations.RearrangeMath, AlwaysGoodOptimizations.RemoveNops, AlwaysGoodOptimizations.ReverseFlowAnalysis, + AlwaysGoodOptimizations.ShiftingJustWrittenValue, AlwaysGoodOptimizations.SimplifiableBitOpsSequence, AlwaysGoodOptimizations.SimplifiableCondition, AlwaysGoodOptimizations.SimplifiableIndexChanging, diff --git a/src/main/scala/millfork/assembly/AssemblyLine.scala b/src/main/scala/millfork/assembly/AssemblyLine.scala index 3e35a77c..936747cb 100644 --- a/src/main/scala/millfork/assembly/AssemblyLine.scala +++ b/src/main/scala/millfork/assembly/AssemblyLine.scala @@ -170,6 +170,9 @@ object OpcodeClasses { val AccessesWordInMemoryAlwaysIfNotImplied = Set( ) + val StoresByte = Set(STA, STX, STY, STZ, SAX) + val StoresWord = Set(STA_W, STX_W, STY_W, STZ_W) + val OverwritesA = Set( LDA, PLA, LDA_W, PLA_W, diff --git a/src/main/scala/millfork/assembly/opt/AlwaysGoodOptimizations.scala b/src/main/scala/millfork/assembly/opt/AlwaysGoodOptimizations.scala index 48436f81..1f12cc26 100644 --- a/src/main/scala/millfork/assembly/opt/AlwaysGoodOptimizations.scala +++ b/src/main/scala/millfork/assembly/opt/AlwaysGoodOptimizations.scala @@ -57,7 +57,7 @@ object AlwaysGoodOptimizations { }, (Elidable & MatchA(0) & HasSet(State.C) & HasOpcode(ROL) & HasAddrMode(Implied) & DoesntMatterWhatItDoesWith(State.C)) ~~> { (code, ctx) => - AssemblyLine.immediate(LDA, ctx.get[Int](0) * 2 + 1) :: Nil + AssemblyLine.immediate(LDA, (ctx.get[Int](0) * 2 + 1) & 0xff) :: Nil }, (Elidable & MatchA(0) & HasSet(State.C) & HasOpcode(ROR) & HasAddrMode(Implied) & DoesntMatterWhatItDoesWith(State.C)) ~~> { (code, ctx) => @@ -67,7 +67,7 @@ object AlwaysGoodOptimizations { MatchA(0) & MatchParameter(1) & HasOpcode(ADC) & HasAddrMode(Immediate) & HasClear(State.D) & HasClear(State.C) & DoesntMatterWhatItDoesWith(State.C, State.V)) ~~> { (code, ctx) => - AssemblyLine.immediate(LDA, ctx.get[Constant](1) + ctx.get[Int](0)) :: Nil + AssemblyLine.immediate(LDA, (ctx.get[Constant](1) + ctx.get[Int](0)).quickSimplify.loByte) :: Nil }, (Elidable & MatchA(0) & MatchParameter(1) & @@ -77,13 +77,13 @@ object AlwaysGoodOptimizations { case NumericConstant(x, _) => x == (x & 0xff) case _ => false }) ~~> { (code, ctx) => - AssemblyLine.immediate(LDA, ctx.get[Constant](1) + ctx.get[Int](0)) :: Nil + AssemblyLine.immediate(LDA, (ctx.get[Constant](1) + ctx.get[Int](0)).quickSimplify.loByte) :: Nil }, (Elidable & MatchA(0) & MatchParameter(1) & HasOpcode(ADC) & HasAddrMode(Immediate) & HasClear(State.D) & HasSet(State.C) & DoesntMatterWhatItDoesWith(State.C, State.V)) ~~> { (code, ctx) => - AssemblyLine.immediate(LDA, ctx.get[Constant](1) + ((ctx.get[Int](0) + 1) & 0xff)) :: Nil + AssemblyLine.immediate(LDA, (ctx.get[Constant](1) + (ctx.get[Int](0) + 1)).quickSimplify.loByte) :: Nil }, (Elidable & MatchA(0) & MatchParameter(1) & @@ -207,20 +207,117 @@ object AlwaysGoodOptimizations { ) + val PointlessStashingForLaterStore = new RuleBasedAssemblyOptimization("Pointless stashing for later store", + needsFlowInfo = FlowInfoRequirement.NoRequirement, + // LDA/TAX/TAY/TAZ will be cleaned by something else + (HasOpcode(STA) & MatchAddrMode(0) & MatchParameter(1)) ~ + (Elidable & Linear & DoesNotConcernMemoryAt(0, 1) & DoesntChangeIndexingInAddrMode(0)).*.capture(5) ~ + (Elidable & HasOpcode(LDA) & MatchAddrMode(0) & MatchParameter(1)) ~ + (Elidable & HasOpcode(STA) & HasAddrModeIn(Set(ZeroPage, Absolute, LongAbsolute)) & DoesNotConcernMemoryAt(0, 1)).capture(4) ~ + Where(ctx => { + val sta = ctx.get[List[AssemblyLine]](4).head + val middleCode = ctx.get[List[AssemblyLine]](5) + middleCode.forall(line => HelperCheckers.memoryAccessDoesntOverlap(line, sta)) + }) ~~> { code => + code.last :: code.init + }, + (HasOpcode(TAX)) ~ + (Elidable & Linear & Not(ChangesX) & Not(HasOpcode(STX))).*.capture(5) ~ + (Elidable & HasOpcode(STX) & HasAddrModeIn(Set(ZeroPage, Absolute, LongAbsolute))).capture(4) ~ + Where(ctx => { + val sta = ctx.get[List[AssemblyLine]](4).head + val middleCode = ctx.get[List[AssemblyLine]](5) + middleCode.forall(line => HelperCheckers.memoryAccessDoesntOverlap(line, sta)) + }) ~~> { code => + code.last.copy(opcode = STA) :: code.init + }, + (HasOpcode(TAY)) ~ + (Elidable & Linear & Not(ChangesY) & Not(HasOpcode(STY))).*.capture(5) ~ + (Elidable & HasOpcode(STY) & HasAddrModeIn(Set(ZeroPage, Absolute, LongAbsolute))).capture(4) ~ + Where(ctx => { + val sta = ctx.get[List[AssemblyLine]](4).head + val middleCode = ctx.get[List[AssemblyLine]](5) + middleCode.forall(line => HelperCheckers.memoryAccessDoesntOverlap(line, sta)) + }) ~~> { code => + code.last.copy(opcode = STA) :: code.init + }, + (HasOpcode(TAZ)) ~ + (Elidable & Linear & Not(ChangesIZ) & Not(HasOpcode(STZ))).*.capture(5) ~ + (Elidable & HasOpcode(STZ) & HasAddrModeIn(Set(ZeroPage, Absolute, LongAbsolute))).capture(4) ~ + Where(ctx => { + val sta = ctx.get[List[AssemblyLine]](4).head + val middleCode = ctx.get[List[AssemblyLine]](5) + middleCode.forall(line => HelperCheckers.memoryAccessDoesntOverlap(line, sta)) + }) ~~> { code => + code.last.copy(opcode = STA) :: code.init + }, + ) + + val PointlessStashingForLaterLoad = new RuleBasedAssemblyOptimization("Pointless stashing for later load", + needsFlowInfo = FlowInfoRequirement.BackwardFlow, + (Elidable & HasOpcode(LDA) & MatchAddrMode(0) & MatchParameter(1)) ~ + (Elidable & HasOpcode(STA) & MatchAddrMode(10) & MatchParameter(11) & DoesntMatterWhatItDoesWith(State.A, State.Z, State.N)) ~ + (Elidable & Linear & DoesNotConcernMemoryAt(0, 1) & DoesNotConcernMemoryAt(10, 11) & DoesntChangeIndexingInAddrMode(0) & DoesntChangeIndexingInAddrMode(10)).* ~ + (Elidable & HasOpcode(LDA) & MatchAddrMode(10) & MatchParameter(11)) ~~> { code => + code.drop(2).init ++ code.take(2) + }, + (Elidable & HasOpcode(STA) & MatchAddrMode(0) & MatchParameter(1)) ~ + (Elidable & HasOpcode(STA) & MatchAddrMode(10) & MatchParameter(11) & DoesntMatterWhatItDoesWith(State.A, State.Z, State.N)) ~ + (Elidable & Linear & DoesNotConcernMemoryAt(0, 1) & DoesNotConcernMemoryAt(10, 11) & DoesntChangeIndexingInAddrMode(0) & DoesntChangeIndexingInAddrMode(10)).* ~ + (Elidable & HasOpcode(LDA) & MatchAddrMode(10) & MatchParameter(11)) ~~> { code => + code.head :: (code.drop(2).init ++ List(code.head.copy(opcode = LDA), code(1))) + }, + + (Elidable & HasOpcode(LDX) & MatchAddrMode(0) & MatchParameter(1) & DoesntMatterWhatItDoesWith(State.Z, State.N)) ~ + (Elidable & Linear & DoesNotConcernMemoryAt(0, 1) & DoesntChangeIndexingInAddrMode(0) & Not(ConcernsX)).* ~ + (Elidable & HasOpcode(STX)) ~ + (Elidable & HasOpcode(TXA)) ~~> { code => + code.tail.init.init ++ List(code.head.copy(opcode = LDA), code.init.last.copy(opcode = STA), AssemblyLine.implied(TAX)) + }, + (Elidable & HasOpcode(LDY) & MatchAddrMode(0) & MatchParameter(1) & DoesntMatterWhatItDoesWith(State.Z, State.N)) ~ + (Elidable & Linear & DoesNotConcernMemoryAt(0, 1) & DoesntChangeIndexingInAddrMode(0) & Not(ConcernsY)).* ~ + (Elidable & HasOpcode(STY)) ~ + (Elidable & HasOpcode(TYA)) ~~> { code => + code.tail.init.init ++ List(code.head.copy(opcode = LDA), code.init.last.copy(opcode = STA), AssemblyLine.implied(TAY)) + }, + (Elidable & HasOpcode(LDZ) & MatchAddrMode(0) & MatchParameter(1) & DoesntMatterWhatItDoesWith(State.Z, State.N)) ~ + (Elidable & Linear & DoesNotConcernMemoryAt(0, 1) & DoesntChangeIndexingInAddrMode(0) & Not(ConcernsIZ)).* ~ + (Elidable & HasOpcode(STZ)) ~ + (Elidable & HasOpcode(TZA)) ~~> { code => + code.tail.init.init ++ List(code.head.copy(opcode = LDA), code.init.last.copy(opcode = STA), AssemblyLine.implied(TAZ)) + }, + + (Elidable & HasOpcode(LDX) & MatchAddrMode(0) & MatchParameter(1) & DoesntMatterWhatItDoesWith(State.Z, State.N)) ~ + (Elidable & Linear & DoesNotConcernMemoryAt(0, 1) & DoesntChangeIndexingInAddrMode(0) & Not(ConcernsX)).* ~ + (Elidable & HasOpcode(TXA)) ~~> { code => + code.tail.init ++ List(code.head.copy(opcode = LDA), AssemblyLine.implied(TAX)) + }, + (Elidable & HasOpcode(LDY) & MatchAddrMode(0) & MatchParameter(1) & DoesntMatterWhatItDoesWith(State.Z, State.N)) ~ + (Elidable & Linear & DoesNotConcernMemoryAt(0, 1) & DoesntChangeIndexingInAddrMode(0) & Not(ConcernsY)).* ~ + (Elidable & HasOpcode(TYA)) ~~> { code => + code.tail.init ++ List(code.head.copy(opcode = LDA), AssemblyLine.implied(TAY)) + }, + (Elidable & HasOpcode(LDZ) & MatchAddrMode(0) & MatchParameter(1) & DoesntMatterWhatItDoesWith(State.Z, State.N)) ~ + (Elidable & Linear & DoesNotConcernMemoryAt(0, 1) & DoesntChangeIndexingInAddrMode(0) & Not(ConcernsIZ)).* ~ + (Elidable & HasOpcode(TZA)) ~~> { code => + code.tail.init ++ List(code.head.copy(opcode = LDA), AssemblyLine.implied(TAZ)) + }, + ) + val PointlessLoadBeforeReturn = new RuleBasedAssemblyOptimization("Pointless load before return", needsFlowInfo = FlowInfoRequirement.NoRequirement, (Set(LDA, TXA, TYA, EOR, AND, ORA, ANC) & Elidable) ~ (LinearOrLabel & Not(ConcernsA) & Not(ReadsNOrZ) & Not(HasOpcode(DISCARD_AF))).* ~ HasOpcode(DISCARD_AF) ~~> (_.tail), (Set(LDX, TAX, TSX, INX, DEX) & Elidable) ~ (LinearOrLabel & Not(ConcernsX) & Not(ReadsNOrZ) & Not(HasOpcode(DISCARD_XF))).* ~ HasOpcode(DISCARD_XF) ~~> (_.tail), (Set(LDY, TAY, INY, DEY) & Elidable) ~ (LinearOrLabel & Not(ConcernsY) & Not(ReadsNOrZ) & Not(HasOpcode(DISCARD_YF))).* ~ HasOpcode(DISCARD_YF) ~~> (_.tail), - (HasOpcode(LDX) & Elidable & MatchAddrMode(3)) ~ - (LinearOrLabel & Not(ConcernsX) & Not(ReadsNOrZ) & DoesntChangeIndexingInAddrMode(3)).*.capture(1) ~ + (HasOpcode(LDX) & Elidable & MatchAddrMode(3) & MatchParameter(4)) ~ + (LinearOrLabel & Not(ConcernsX) & Not(ReadsNOrZ) & DoesntChangeMemoryAt(3, 4) & DoesntChangeIndexingInAddrMode(3)).*.capture(1) ~ (HasOpcode(TXA) & Elidable) ~ ((LinearOrLabel & Not(ConcernsX) & Not(HasOpcode(DISCARD_XF))).* ~ HasOpcode(DISCARD_XF)).capture(2) ~~> { (c, ctx) => ctx.get[List[AssemblyLine]](1) ++ (c.head.copy(opcode = LDA) :: ctx.get[List[AssemblyLine]](2)) }, - (HasOpcode(LDY) & Elidable & MatchAddrMode(3)) ~ - (LinearOrLabel & Not(ConcernsY) & Not(ReadsNOrZ) & DoesntChangeIndexingInAddrMode(3)).*.capture(1) ~ + (HasOpcode(LDY) & Elidable & MatchAddrMode(3) & MatchParameter(4)) ~ + (LinearOrLabel & Not(ConcernsY) & Not(ReadsNOrZ) & DoesntChangeMemoryAt(3, 4) & DoesntChangeIndexingInAddrMode(3)).*.capture(1) ~ (HasOpcode(TYA) & Elidable) ~ ((LinearOrLabel & Not(ConcernsY) & Not(HasOpcode(DISCARD_YF))).* ~ HasOpcode(DISCARD_YF)).capture(2) ~~> { (c, ctx) => @@ -410,7 +507,10 @@ object AlwaysGoodOptimizations { needsFlowInfo = FlowInfoRequirement.NoRequirement, (AllDirectJumps & MatchParameter(0) & Elidable) ~ HasOpcodeIn(NoopDiscardsFlags).* ~ - (HasOpcode(LABEL) & MatchParameter(0)) ~~> (c => c.last :: Nil) + (HasOpcode(LABEL) & MatchParameter(0)) ~~> (c => c.last :: Nil), + (AllDirectJumps & MatchParameter(0) & Elidable) ~ + (HasOpcode(LABEL) & Not(MatchParameter(0))).* ~ + (HasOpcode(LABEL) & MatchParameter(0)) ~~> (_.tail), ) val ImpossibleBranchRemoval = new RuleBasedAssemblyOptimization("Impossible branch", @@ -1021,6 +1121,25 @@ object AlwaysGoodOptimizations { wordShifting(5, hiFirst = true, hiFromX = false), ) + val ShiftingJustWrittenValue = new RuleBasedAssemblyOptimization("Shifting just written value", + needsFlowInfo = FlowInfoRequirement.BackwardFlow, + (Elidable & HasOpcode(STA) & MatchAddrMode(0) & MatchParameter(1) & DoesntMatterWhatItDoesWith(State.C, State.Z, State.N, State.A)) ~ + (Linear & DoesNotConcernMemoryAt(0, 1) & DoesntChangeIndexingInAddrMode(0)).* ~ + (Elidable & HasOpcodeIn(Set(ASL, LSR)) & MatchAddrMode(0) & MatchParameter(1) & DoesntMatterWhatItDoesWith(State.C, State.Z, State.N)) ~~> { code => + code.last.copy(addrMode = AddrMode.Implied, parameter = Constant.Zero) :: code.init + }, + (Elidable & HasOpcode(STA) & MatchAddrMode(0) & MatchParameter(1) & DoesntMatterWhatItDoesWith(State.C, State.Z, State.N, State.A)) ~ + (Linear & DoesNotConcernMemoryAt(0, 1) & DoesntChangeIndexingInAddrMode(0) & Not(ChangesC)).* ~ + (Elidable & HasOpcodeIn(Set(ASL, LSR)) & MatchAddrMode(0) & MatchParameter(1) & DoesntMatterWhatItDoesWith(State.Z, State.N)) ~~> { code => + code.last.copy(addrMode = AddrMode.Implied, parameter = Constant.Zero) :: code.init + }, + (Elidable & HasOpcode(STA) & MatchAddrMode(0) & MatchParameter(1) & DoesntMatterWhatItDoesWith(State.Z, State.N, State.A)) ~ + (Linear & DoesNotConcernMemoryAt(0, 1) & DoesntChangeIndexingInAddrMode(0) & Not(ChangesC)).* ~ + (Elidable & HasOpcodeIn(Set(ROL, ROR)) & MatchAddrMode(0) & MatchParameter(1) & DoesntMatterWhatItDoesWith(State.Z, State.N)) ~~> { code => + code.last.copy(addrMode = AddrMode.Implied, parameter = Constant.Zero) :: code.init + }, + ) + val SmarterShiftingBytes = new RuleBasedAssemblyOptimization("Smarter shifting of bytes", needsFlowInfo = FlowInfoRequirement.NoRequirement, (Elidable & HasOpcode(ASL) & HasAddrMode(Implied)) ~ @@ -1422,6 +1541,13 @@ object AlwaysGoodOptimizations { (Elidable & HasOpcode(JMP) & MatchParameter(1))).capture(11) ~~> { (code, ctx) => ctx.get[List[AssemblyLine]](10) ++ ctx.get[List[AssemblyLine]](13).map(_.copy(parameter = ctx.get[Constant](1))) ++ ctx.get[List[AssemblyLine]](11) }, + (Elidable & HasOpcode(LABEL) & MatchParameter(0)) ~ + (Elidable & HasOpcode(JMP) & MatchParameter(1)) ~ + Not(MatchParameter(1)).* ~ + (HasOpcode(LABEL) & MatchParameter(1)) ~ + (HasOpcode(JMP) & MatchParameter(1)) ~~> { (code, ctx) => + code.head :: code.drop(2) + }, ) val IndexComparisonOptimization = new RuleBasedAssemblyOptimization("Index comparison optimization", diff --git a/src/main/scala/millfork/assembly/opt/ReverseFlowAnalyzer.scala b/src/main/scala/millfork/assembly/opt/ReverseFlowAnalyzer.scala index 80d97fe5..01e552de 100644 --- a/src/main/scala/millfork/assembly/opt/ReverseFlowAnalyzer.scala +++ b/src/main/scala/millfork/assembly/opt/ReverseFlowAnalyzer.scala @@ -1,7 +1,7 @@ package millfork.assembly.opt import millfork.CompilationOptions -import millfork.assembly.{AssemblyLine, Opcode, OpcodeClasses, State} +import millfork.assembly._ import millfork.env._ import millfork.error.ErrorReporting import millfork.node.Register @@ -45,8 +45,10 @@ case class CpuImportance(a: Importance = UnknownImportance, d: Importance = UnknownImportance, m: Importance = UnknownImportance, w: Importance = UnknownImportance, + r0: Importance = UnknownImportance, + r1: Importance = UnknownImportance, ) { - override def toString: String = s"A=$a,B=$ah,X=$x,Y=$y,Z=$iz; Z=$z,N=$n,C=$c,V=$v,D=$d,M=$m,X=$w" + override def toString: String = s"A=$a,B=$ah,X=$x,Y=$y,Z=$iz; Z=$z,N=$n,C=$c,V=$v,D=$d,M=$m,X=$w; R0=$r0,R1:$r1" def ~(that: CpuImportance) = new CpuImportance( a = this.a ~ that.a, @@ -60,6 +62,8 @@ case class CpuImportance(a: Importance = UnknownImportance, d = this.d ~ that.d, m = this.m ~ that.m, w = this.w ~ that.w, + r0 = this.r0 ~ that.r0, + r1 = this.r1 ~ that.r1, ) def isUnimportant(state: State.Value): Boolean = state match { @@ -77,11 +81,19 @@ case class CpuImportance(a: Importance = UnknownImportance, case State.M => m != Important case State.W => w != Important } + + def isPseudoregisterUnimportant(index: Int): Boolean = index match { + case 0 => r0 != Important + case 1 => r1 != Important + case _ => false + } } object ReverseFlowAnalyzer { val aluAdders = Set(Opcode.ADC, Opcode.SBC, Opcode.ISC, Opcode.DCP, Opcode.ADC_W, Opcode.SBC_W) + val actuallyRead = Set(AddrMode.IndexedZ, AddrMode.IndexedSY, AddrMode.IndexedY, AddrMode.LongIndexedY, AddrMode.LongIndexedZ, AddrMode.IndexedX, AddrMode.Indirect, AddrMode.AbsoluteIndexedX) + val absoluteLike = Set(AddrMode.ZeroPage, AddrMode.Absolute, AddrMode.LongAbsolute) //noinspection RedundantNewCaseClass def analyze(f: NormalFunction, code: List[AssemblyLine]): List[CpuImportance] = { @@ -93,7 +105,8 @@ object ReverseFlowAnalyzer { a = Important, ah = Important, x = Important, y = Important, iz = Important, c = Important, v = Important, d = Important, z = Important, n = Important, - m = Important, w = Important) + m = Important, w = Important, + r0 = Important, r1 = Important) changed = true while (changed) { changed = false @@ -105,7 +118,8 @@ object ReverseFlowAnalyzer { changed = true importanceArray(i) = currentImportance } - codeArray(i) match { + val currentLine = codeArray(i) + currentLine match { case AssemblyLine(opcode, Relative | LongRelative, MemoryAddressConstant(Label(l)), _) if OpcodeClasses.ShortConditionalBranching(opcode) => val L = l val labelIndex = codeArray.indexWhere { @@ -115,7 +129,7 @@ object ReverseFlowAnalyzer { currentImportance = if (labelIndex < 0) finalImportance else importanceArray(labelIndex) ~ currentImportance case _ => } - codeArray(i) match { + currentLine match { case AssemblyLine(JSR | JMP, Absolute | LongAbsolute, MemoryAddressConstant(fun: FunctionInMemory), _) => // this case has to be handled first, because the generic JSR importance handler is too conservative @@ -131,7 +145,9 @@ object ReverseFlowAnalyzer { v = Unimportant, d = Important, m = Important, - w = Important) + w = Important, + r0 = Unimportant, + r1 = Unimportant) fun.params match { case AssemblyParamSignature(params) => params.foreach(_.variable match { @@ -153,6 +169,9 @@ object ReverseFlowAnalyzer { }) case _ => } + if (ZeropageRegisterOptimizations.functionsThatUsePseudoregisterAsInput(fun.name)) { + result = result.copy(r0 = Important, r1 = Important) + } currentImportance = result case AssemblyLine(ANC, _, NumericConstant(0, _), _) => @@ -231,6 +250,47 @@ object ReverseFlowAnalyzer { else if (addrMode == IndexedZ /*|| addrMode == LongIndexedZ*/ ) currentImportance = currentImportance.copy(iz = Important) } + if (absoluteLike(currentLine.addrMode)) { + if (OpcodeClasses.StoresByte(currentLine.opcode)) { + currentLine.parameter match { + case MemoryAddressConstant(th: Thing) + if th.name == "__reg" => currentImportance = currentImportance.copy(r0 = Unimportant) + case CompoundConstant(MathOperator.Plus, MemoryAddressConstant(th: Thing), NumericConstant(1, _)) + if th.name == "__reg" => currentImportance = currentImportance.copy(r1 = Unimportant) + case _ => () + } + } + if (OpcodeClasses.StoresWord(currentLine.opcode)) { + currentLine.parameter match { + case MemoryAddressConstant(th: Thing) + if th.name == "__reg" => currentImportance = currentImportance.copy(r0 = Unimportant, r1 = Unimportant) + case _ => () + } + } + } + if (actuallyRead(currentLine.addrMode)) { + currentLine.parameter match { + case MemoryAddressConstant(th: Thing) + if th.name == "__reg" => currentImportance = currentImportance.copy(r0 = Important, r1 = Important) + case _ => () + } + } else if (OpcodeClasses.ReadsM(currentLine.opcode) || OpcodeClasses.ReadsMemoryIfNotImpliedOrImmediate(currentLine.opcode)) { + if (OpcodeClasses.AccessesWordInMemory(currentLine.opcode)) { + currentLine.parameter match { + case MemoryAddressConstant(th: Thing) + if th.name == "__reg" => currentImportance = currentImportance.copy(r0 = Important, r1 = Important) + case _ => () + } + } else { + currentLine.parameter match { + case MemoryAddressConstant(th: Thing) + if th.name == "__reg" => currentImportance = currentImportance.copy(r0 = Important) + case CompoundConstant(MathOperator.Plus, MemoryAddressConstant(th: Thing), NumericConstant(1, _)) + if th.name == "__reg" => currentImportance = currentImportance.copy(r1 = Important) + case _ => () + } + } + } } } // importanceArray.zip(codeArray).foreach{ diff --git a/src/main/scala/millfork/assembly/opt/ReverseFlowAnalyzerPerOpcode.scala b/src/main/scala/millfork/assembly/opt/ReverseFlowAnalyzerPerOpcode.scala index eaa6cda6..8fb5435b 100644 --- a/src/main/scala/millfork/assembly/opt/ReverseFlowAnalyzerPerOpcode.scala +++ b/src/main/scala/millfork/assembly/opt/ReverseFlowAnalyzerPerOpcode.scala @@ -118,9 +118,9 @@ object ReverseFlowAnalyzerPerOpcode { currentImportance.copy(y = currentImportance.x, x = currentImportance.y, m = Important, w = Important) }), - DISCARD_XF -> (_.copy(x = Unimportant, n = Unimportant, z = Unimportant, c = Unimportant, v = Unimportant)), - DISCARD_YF -> (_.copy(y = Unimportant, iz = Unimportant, n = Unimportant, z = Unimportant, c = Unimportant, v = Unimportant)), - DISCARD_AF -> (_.copy(a = Unimportant, n = Unimportant, z = Unimportant, c = Unimportant, v = Unimportant)), + DISCARD_XF -> (_.copy(x = Unimportant, n = Unimportant, z = Unimportant, c = Unimportant, v = Unimportant, r0 = Unimportant, r1 = Unimportant)), + DISCARD_YF -> (_.copy(y = Unimportant, iz = Unimportant, n = Unimportant, z = Unimportant, c = Unimportant, v = Unimportant, r0 = Unimportant, r1 = Unimportant)), + DISCARD_AF -> (_.copy(a = Unimportant, n = Unimportant, z = Unimportant, c = Unimportant, v = Unimportant, r0 = Unimportant, r1 = Unimportant)), LDA -> (_.copy(a = Unimportant, n = Unimportant, z = Unimportant, m = Important)), LDX -> (_.copy(x = Unimportant, n = Unimportant, z = Unimportant, w = Important)), @@ -181,16 +181,16 @@ object ReverseFlowAnalyzerPerOpcode { LDX_W -> (_.copy(x = Unimportant, n = Unimportant, z = Unimportant, w = Important)), LDY_W -> (_.copy(y = Unimportant, n = Unimportant, z = Unimportant, w = Important)), - STA -> (_.copy(a = Important, m = Important)), - STX -> (_.copy(x = Important, w = Important)), - STY -> (_.copy(y = Important, w = Important)), - STZ -> (_.copy(iz = Important, m = Important)), - SAX -> (_.copy(a = Important, x = Important)), + STA -> (importance => importance.copy(a = Important, m = Important)), + STX -> (importance => importance.copy(x = Important, w = Important)), + STY -> (importance => importance.copy(y = Important, w = Important)), + STZ -> (importance => importance.copy(iz = Important, m = Important)), + SAX -> (importance => importance.copy(a = Important, x = Important)), - STA_W -> (_.copy(a = Important, ah = Important, m = Important)), - STX_W -> (_.copy(x = Important, w = Important)), - STY_W -> (_.copy(y = Important, w = Important)), - STZ_W -> (_.copy(iz = Important, m = Important)), + STA_W -> (importance => importance.copy(a = Important, ah = Important, m = Important)), + STX_W -> (importance => importance.copy(x = Important, w = Important)), + STY_W -> (importance => importance.copy(y = Important, w = Important)), + STZ_W -> (importance => importance.copy(iz = Important, m = Important)), DEX -> (_.copy(x = Important, n = Unimportant, z = Unimportant, w = Important)), DEY -> (_.copy(y = Important, n = Unimportant, z = Unimportant, w = Important)), diff --git a/src/main/scala/millfork/assembly/opt/RuleBasedAssemblyOptimization.scala b/src/main/scala/millfork/assembly/opt/RuleBasedAssemblyOptimization.scala index b6aeabcf..9443f5fe 100644 --- a/src/main/scala/millfork/assembly/opt/RuleBasedAssemblyOptimization.scala +++ b/src/main/scala/millfork/assembly/opt/RuleBasedAssemblyOptimization.scala @@ -205,6 +205,13 @@ object HelperCheckers { val p2 = l2.parameter val w1 = OpcodeClasses.AccessesWordInMemory(l1.opcode) val w2 = OpcodeClasses.AccessesWordInMemory(l2.opcode) + + def distinctThings(a: String, b: String): Boolean = { + if (a == "__reg") return b != "__reg" + if (b == "__reg") return a != "__reg" + a.takeWhile(_ != '.') != b.takeWhile(_ != '.') + } + def handleKnownDistance(distance: Short): Boolean = { // `distance` is the distance between the first byte that can be addressed by l1 (b1) and the first byte that can be addressed by l2 (b2): (b2-b1) val indexingAddrModes = Set(AbsoluteIndexedX, AbsoluteX, ZeroPageX, AbsoluteY, ZeroPageY, LongAbsoluteX) @@ -238,7 +245,7 @@ object HelperCheckers { case (NumericConstant(_, _), CompoundConstant(MathOperator.Plus | MathOperator.Minus, MemoryAddressConstant(a: ThingInMemory), NumericConstant(_, _))) => true // TODO: ??? case (MemoryAddressConstant(a: ThingInMemory), MemoryAddressConstant(b: ThingInMemory)) => - a.name.takeWhile(_ != '.') != b.name.takeWhile(_ != '.') // TODO: ??? + distinctThings(a.name, b.name) // TODO: ??? case (CompoundConstant(op@(MathOperator.Plus | MathOperator.Minus), MemoryAddressConstant(a: ThingInMemory), NumericConstant(offset, _)), MemoryAddressConstant(b: ThingInMemory)) => if (a.name == b.name) { @@ -248,7 +255,7 @@ object HelperCheckers { handleKnownDistance(offset.toShort) } } else { - a.name.takeWhile(_ != '.') != b.name.takeWhile(_ != '.') // TODO: ??? + distinctThings(a.name, b.name) // TODO: ??? } case (MemoryAddressConstant(a: ThingInMemory), CompoundConstant(op@(MathOperator.Plus | MathOperator.Minus), MemoryAddressConstant(b: ThingInMemory), NumericConstant(offset, _))) => @@ -259,7 +266,7 @@ object HelperCheckers { handleKnownDistance(offset.toShort) } } else { - a.name.takeWhile(_ != '.') != b.name.takeWhile(_ != '.') // TODO: ??? + distinctThings(a.name, b.name) // TODO: ??? } case (CompoundConstant(op1@(MathOperator.Plus | MathOperator.Minus), MemoryAddressConstant(a: ThingInMemory), NumericConstant(o1, _)), CompoundConstant(op2@(MathOperator.Plus | MathOperator.Minus), MemoryAddressConstant(b: ThingInMemory), NumericConstant(o2, _))) => @@ -268,7 +275,7 @@ object HelperCheckers { val offset2 = if (op2==MathOperator.Plus) o2 else -o2 handleKnownDistance((offset2 - offset1).toShort) } else { - a.name.takeWhile(_ != '.') != b.name.takeWhile(_ != '.') // TODO: ??? + distinctThings(a.name, b.name) // TODO: ??? } case _ => false @@ -531,6 +538,16 @@ case class DoesntMatterWhatItDoesWith(states: State.Value*) extends AssemblyLine override def toString: String = states.mkString("[¯\\_(ツ)_/¯:", ",", "]") } +case class DoesntMatterWhatItDoesWithReg(index: Int) extends AssemblyLinePattern { + override def validate(needsFlowInfo: FlowInfoRequirement.Value): Unit = + FlowInfoRequirement.assertBackward(needsFlowInfo) + + override def matchLineTo(ctx: AssemblyMatchingContext, flowInfo: FlowInfo, line: AssemblyLine): Boolean = + flowInfo.importanceAfter.isPseudoregisterUnimportant(index) + + override def toString: String = s"[¯\\_(ツ)_/¯: __reg+$index]" +} + case class HasSet(state: State.Value) extends AssemblyLinePattern { override def validate(needsFlowInfo: FlowInfoRequirement.Value): Unit = FlowInfoRequirement.assertForward(needsFlowInfo) @@ -796,15 +813,15 @@ case class HasOpcode(op: Opcode.Value) extends TrivialAssemblyLinePattern { override def toString: String = op.toString } -case class RefersTo(identifier: String, offset: Int) extends TrivialAssemblyLinePattern { +case class RefersTo(identifier: String, offset: Int = 999) extends TrivialAssemblyLinePattern { override def apply(line: AssemblyLine): Boolean = { (line.addrMode == AddrMode.ZeroPage || line.addrMode == AddrMode.Absolute || line.addrMode == AddrMode.LongAbsolute) && (line.parameter match { case MemoryAddressConstant(th) => - offset == 0 && th.name == identifier + (offset == 999 || offset == 0) && th.name == identifier case CompoundConstant(MathOperator.Plus, MemoryAddressConstant(th), NumericConstant(nn, _)) => - offset == nn && th.name == identifier + (offset == 999 || offset == nn) && th.name == identifier case CompoundConstant(MathOperator.Plus, NumericConstant(nn, _), MemoryAddressConstant(th)) => - offset == nn && th.name == identifier + (offset == 999 || offset == nn) && th.name == identifier case _ => false }) } diff --git a/src/main/scala/millfork/assembly/opt/SuperOptimizer.scala b/src/main/scala/millfork/assembly/opt/SuperOptimizer.scala index 3aaad326..098711c0 100644 --- a/src/main/scala/millfork/assembly/opt/SuperOptimizer.scala +++ b/src/main/scala/millfork/assembly/opt/SuperOptimizer.scala @@ -49,18 +49,21 @@ object SuperOptimizer extends AssemblyOptimization { val quickScrub = List( UnusedLabelRemoval, + AlwaysGoodOptimizations.BranchInPlaceRemoval, + AlwaysGoodOptimizations.PointlessLoadBeforeReturn, AlwaysGoodOptimizations.UnusedCodeRemoval ) - val quicklyCleanedCode = quickScrub.foldLeft(code)((c, o) => o.optimize(m, c, options)) + val optionsForMeasurements = options.copy(commandLineFlags = options.commandLineFlags + (CompilationFlag.InternalCurrentlyOptimizingForMeasurement -> true)) + val quicklyCleanedCode = quickScrub.foldLeft(code)((c, o) => o.optimize(m, c, optionsForMeasurements)) seenSoFar += viewCode(quicklyCleanedCode) queue.enqueue(quickScrub.reverse -> quicklyCleanedCode) while(queue.nonEmpty) { val (optsSoFar, codeSoFar) = queue.dequeue() var isLeaf = true - (if (options.flag(CompilationFlag.SingleThreaded)) allOptimizers + (if (optionsForMeasurements.flag(CompilationFlag.SingleThreaded)) allOptimizers else allOptimizers.par).foreach { o => - val optimized = o.optimize(m, codeSoFar, options) + val optimized = o.optimize(m, codeSoFar, optionsForMeasurements) val view = viewCode(optimized) seenSoFar.synchronized{ if (!seenSoFar(view)) { diff --git a/src/main/scala/millfork/assembly/opt/VariableToRegisterOptimization.scala b/src/main/scala/millfork/assembly/opt/VariableToRegisterOptimization.scala index 78b1746c..daa6ab4b 100644 --- a/src/main/scala/millfork/assembly/opt/VariableToRegisterOptimization.scala +++ b/src/main/scala/millfork/assembly/opt/VariableToRegisterOptimization.scala @@ -134,6 +134,7 @@ object VariableToRegisterOptimization extends AssemblyOptimization { v.name -> VariableLifetime.apply(v.name, code) ).toMap + val removeVariablesForReal = !options.flag(CompilationFlag.InternalCurrentlyOptimizingForMeasurement) val costFunction: CyclesAndBytes => Int = if (options.flag(CompilationFlag.OptimizeForSpeed)) _.cycles else _.bytes val importances = ReverseFlowAnalyzer.analyze(f, code) val blastProcessing = options.flag(CompilationFlag.OptimizeForSonicSpeed) @@ -252,7 +253,7 @@ object VariableToRegisterOptimization extends AssemblyOptimization { reportOptimizedBlock(oldCode, newCode) output ++= newCode i = range.end - if (contains(range, variablesWithLifetimes(v))) { + if (removeVariablesForReal && contains(range, variablesWithLifetimes(v))) { f.environment.removeVariable(v) } done = true @@ -266,7 +267,7 @@ object VariableToRegisterOptimization extends AssemblyOptimization { reportOptimizedBlock(oldCode, newCode) output ++= newCode i = range.end - if (contains(range, variablesWithLifetimes(v))) { + if (removeVariablesForReal && contains(range, variablesWithLifetimes(v))) { f.environment.removeVariable(v) } done = true @@ -281,7 +282,7 @@ object VariableToRegisterOptimization extends AssemblyOptimization { reportOptimizedBlock(oldCode, newCode) output ++= newCode i = range.end - if (contains(range, variablesWithLifetimes(v))) { + if (removeVariablesForReal && contains(range, variablesWithLifetimes(v))) { f.environment.removeVariable(v) } done = true @@ -296,7 +297,7 @@ object VariableToRegisterOptimization extends AssemblyOptimization { reportOptimizedBlock(oldCode, newCode) output ++= newCode i = range.end - if (contains(range, variablesWithLifetimes(v))) { + if (removeVariablesForReal && contains(range, variablesWithLifetimes(v))) { f.environment.removeVariable(v) } done = true diff --git a/src/main/scala/millfork/assembly/opt/ZeropageRegisterOptimizations.scala b/src/main/scala/millfork/assembly/opt/ZeropageRegisterOptimizations.scala index 0ca3752e..a1f2d031 100644 --- a/src/main/scala/millfork/assembly/opt/ZeropageRegisterOptimizations.scala +++ b/src/main/scala/millfork/assembly/opt/ZeropageRegisterOptimizations.scala @@ -10,7 +10,7 @@ import millfork.env.{CompoundConstant, Constant, MathOperator} */ object ZeropageRegisterOptimizations { - private val functionsThatUsePseudoregisterAsInput = Set("__mul_u8u8u8") + val functionsThatUsePseudoregisterAsInput = Set("__mul_u8u8u8") val ConstantMultiplication = new RuleBasedAssemblyOptimization("Constant multiplication", needsFlowInfo = FlowInfoRequirement.ForwardFlow, @@ -22,6 +22,8 @@ object ZeropageRegisterOptimizations { code.init :+ AssemblyLine.immediate(LDA, product & 0xff) }, + // TODO: constants other than power of 2: + (Elidable & HasOpcode(STA) & RefersTo("__reg", 0) & MatchAddrMode(0) & MatchParameter(1)) ~ (Linear & Not(RefersTo("__reg", 1)) & DoesntChangeMemoryAt(0, 1)).* ~ (HasOpcode(STA) & RefersTo("__reg", 1) & MatchA(4)) ~ @@ -66,9 +68,16 @@ object ZeropageRegisterOptimizations { (HasOpcodeIn(Set(RTS, RTL)) | CallsAnyExcept(functionsThatUsePseudoregisterAsInput)) ~~> (_.tail), ) + val DeadRegStoreFromFlow = new RuleBasedAssemblyOptimization("Dead zeropage register store from flow", + needsFlowInfo = FlowInfoRequirement.BackwardFlow, + (Elidable & HasOpcode(STA) & RefersTo("__reg", 0) & DoesntMatterWhatItDoesWithReg(0)) ~~> (_.tail), + (Elidable & HasOpcode(STA) & RefersTo("__reg", 1) & DoesntMatterWhatItDoesWithReg(1)) ~~> (_.tail), + ) + val All: List[AssemblyOptimization] = List( ConstantMultiplication, DeadRegStore, + DeadRegStoreFromFlow, ) } diff --git a/src/main/scala/millfork/compiler/StatementCompiler.scala b/src/main/scala/millfork/compiler/StatementCompiler.scala index 4e1beff0..ffdcf071 100644 --- a/src/main/scala/millfork/compiler/StatementCompiler.scala +++ b/src/main/scala/millfork/compiler/StatementCompiler.scala @@ -326,15 +326,17 @@ object StatementCompiler { List(labelChunk(start), conditionBlock, branchChunk(jumpIfTrue, middle), jmpChunk(end), labelChunk(middle), bodyBlock, labelChunk(inc), incrementBlock, jmpChunk(start), labelChunk(end)).flatten } else { val conditionBlock = ExpressionCompiler.compile(ctx, condition, someRegisterA, NoBranching) - List(labelChunk(start), conditionBlock, branchChunk(jumpIfFalse, end), bodyBlock, labelChunk(inc), incrementBlock, jmpChunk(start), labelChunk(end)).flatten + List(jmpChunk(middle), labelChunk(start), bodyBlock, labelChunk(inc), incrementBlock, labelChunk(middle), conditionBlock, branchChunk(jumpIfTrue, start), labelChunk(end)).flatten +// List(labelChunk(start), conditionBlock, branchChunk(jumpIfFalse, end), bodyBlock, labelChunk(inc), incrementBlock, jmpChunk(start), labelChunk(end)).flatten } case BuiltInBooleanType => if (largeBodyBlock) { val conditionBlock = ExpressionCompiler.compile(ctx, condition, someRegisterA, BranchIfTrue(middle)) List(labelChunk(start), conditionBlock, jmpChunk(end), labelChunk(middle), bodyBlock, labelChunk(inc), incrementBlock, jmpChunk(start), labelChunk(end)).flatten } else { - val conditionBlock = ExpressionCompiler.compile(ctx, condition, someRegisterA, BranchIfFalse(end)) - List(labelChunk(start), conditionBlock, bodyBlock, labelChunk(inc), incrementBlock, jmpChunk(start), labelChunk(end)).flatten + val conditionBlock = ExpressionCompiler.compile(ctx, condition, someRegisterA, BranchIfTrue(start)) + List(jmpChunk(middle), labelChunk(start), bodyBlock, labelChunk(inc), incrementBlock, labelChunk(middle), conditionBlock, labelChunk(end)).flatten +// List(labelChunk(start), conditionBlock, bodyBlock, labelChunk(inc), incrementBlock, jmpChunk(start), labelChunk(end)).flatten } case _ => ErrorReporting.error(s"Illegal type for a condition: `$condType`", condition.position) diff --git a/src/test/scala/millfork/test/WordMathSuite.scala b/src/test/scala/millfork/test/WordMathSuite.scala index 18a9a802..5a210b40 100644 --- a/src/test/scala/millfork/test/WordMathSuite.scala +++ b/src/test/scala/millfork/test/WordMathSuite.scala @@ -165,6 +165,26 @@ class WordMathSuite extends FunSuite with Matchers { } } + test("Word addition 4") { + EmuBenchmarkRun(""" + | word output @$c000 + | void main () { + | word v + | word u + | v = w($308) + | u = w($601) + | barrier() + | output = u + v + | } + | noinline void barrier() { } + | noinline word w(word w) { + | return w + | } + """.stripMargin){ m => + m.readWord(0xc000) should equal(0x909) + } + } + test("Word bit ops 2") { EmuBenchmarkRun(""" | word output @$c000 @@ -180,4 +200,22 @@ class WordMathSuite extends FunSuite with Matchers { m.readWord(0xc000) should equal(0x483) } } + + test("Word shift") { + EmuBenchmarkRun(""" + | word output @$c000 + | void main () { + | word v + | v = w($123) + | barrier() + | output = v << 1 + | } + | noinline void barrier() { } + | noinline word w(word w) { + | return w + | } + """.stripMargin){ m => + m.readWord(0xc000) should equal(0x246) + } + } }