From 6d7fdff6b5180e94139887508de8999b25b3a898 Mon Sep 17 00:00:00 2001 From: Andy McFadden Date: Fri, 3 Jul 2020 13:58:18 -0700 Subject: [PATCH] Fix 65816 code generation issues Code generated for 64tass was incorrect for JSR/JMP to a location outside the file bounds. A test added to 20052-branches-and-banks revealed an issue with cc65 generation as well. --- SourceGen/AsmGen/AsmTass64.cs | 1 + SourceGen/AsmGen/GenCommon.cs | 7 +++--- SourceGen/AsmGen/IGenerator.cs | 6 +++++ SourceGen/PseudoOp.cs | 8 +++++++ SourceGen/RuntimeData/Help/codegen.html | 5 ++++ SourceGen/SGTestData/20052-branches-and-banks | Bin 202 -> 224 bytes .../SGTestData/20052-branches-and-banks.dis65 | 22 ++++++++++++++---- .../20052-branches-and-banks_64tass.S | 18 ++++++++++---- .../20052-branches-and-banks_Merlin32.S | 18 ++++++++++---- .../Expected/20052-branches-and-banks_acme.S | 2 +- .../Expected/20052-branches-and-banks_cc65.S | 18 ++++++++++---- .../20052-branches-and-banks_cc65.cfg | 2 +- 12 files changed, 86 insertions(+), 21 deletions(-) diff --git a/SourceGen/AsmGen/AsmTass64.cs b/SourceGen/AsmGen/AsmTass64.cs index 8e2b2a9..53bb811 100644 --- a/SourceGen/AsmGen/AsmTass64.cs +++ b/SourceGen/AsmGen/AsmTass64.cs @@ -164,6 +164,7 @@ namespace SourceGen.AsmGen { Quirks = new AssemblerQuirks(); Quirks.StackIntOperandIsImmediate = true; Quirks.LeadingUnderscoreSpecial = true; + Quirks.Need24BitsForAbsPBR = true; mWorkDirectory = workDirectory; mFileNameBase = fileNameBase; diff --git a/SourceGen/AsmGen/GenCommon.cs b/SourceGen/AsmGen/GenCommon.cs index 621e35e..b31f79c 100644 --- a/SourceGen/AsmGen/GenCommon.cs +++ b/SourceGen/AsmGen/GenCommon.cs @@ -292,10 +292,10 @@ namespace SourceGen.AsmGen { formattedOperand = hash + formatter.FormatHexValue(arg1, 2) + "," + hash + formatter.FormatHexValue(arg2, 2); } else { - if (operandLen == 2) { + if (operandLen == 2 && !(op.IsAbsolutePBR && gen.Quirks.Need24BitsForAbsPBR)) { // This is necessary for 16-bit operands, like "LDA abs" and "PEA val", // when outside bank zero. The bank is included in the operand address, - // but we don't want to show it here. + // but we don't want to show it here. We may need it for JSR/JMP though. operandForSymbol &= 0xffff; } formattedOperand = formatter.FormatHexValue(operandForSymbol, operandLen * 2); @@ -303,7 +303,8 @@ namespace SourceGen.AsmGen { } string operandStr = formatter.FormatOperand(op, formattedOperand, wdis); - if (gen.Quirks.StackIntOperandIsImmediate && op.AddrMode == OpDef.AddressMode.StackInt) { + if (gen.Quirks.StackIntOperandIsImmediate && + op.AddrMode == OpDef.AddressMode.StackInt) { // COP $02 is standard, but some require COP #$02 operandStr = '#' + operandStr; } diff --git a/SourceGen/AsmGen/IGenerator.cs b/SourceGen/AsmGen/IGenerator.cs index 03a26fd..1d97786 100644 --- a/SourceGen/AsmGen/IGenerator.cs +++ b/SourceGen/AsmGen/IGenerator.cs @@ -216,6 +216,12 @@ namespace SourceGen.AsmGen { /// public bool BlockMoveArgsReversed { get; set; } + /// + /// Do we need to specify a 24-bit value for 16-bit absolute arguments that are + /// formed with the Program Bank Register (JMP/JSR)? + /// + public bool Need24BitsForAbsPBR { get; set; } + /// /// Does the assembler support a type of label whose value can be redefined to /// act as a local variable? diff --git a/SourceGen/PseudoOp.cs b/SourceGen/PseudoOp.cs index 2907c13..fafcd64 100644 --- a/SourceGen/PseudoOp.cs +++ b/SourceGen/PseudoOp.cs @@ -633,6 +633,14 @@ namespace SourceGen { case FormatDescriptor.SubType.None: case FormatDescriptor.SubType.Hex: case FormatDescriptor.SubType.Address: + if ((formatter.ExpressionMode == Formatter.FormatConfig.ExpressionMode.Cc65 || + formatter.ExpressionMode == Formatter.FormatConfig.ExpressionMode.Merlin) && + (flags & FormatNumericOpFlags.IsAbsolutePBR) != 0) { + // cc65 really doesn't like 24-bit values for JMP/JSR. If it sees a + // 24-bit hex constant it emits JML/JSL. Merlin works either way, and + // I think it looks better as a 16-bit value. + operandValue &= 0xffff; + } return formatter.FormatHexValue(operandValue, hexMinLen); case FormatDescriptor.SubType.Decimal: return formatter.FormatDecimalValue(operandValue); diff --git a/SourceGen/RuntimeData/Help/codegen.html b/SourceGen/RuntimeData/Help/codegen.html index 915478e..b7b8c31 100644 --- a/SourceGen/RuntimeData/Help/codegen.html +++ b/SourceGen/RuntimeData/Help/codegen.html @@ -172,6 +172,11 @@ code, but also needs to know how to handle the corner cases.

  • For 65816, selecting the bank byte is done with the grave accent character ('`') rather than the caret ('^'). (There's a note in the docs to the effect that they plan to move to carets.)
  • +
  • Instructions whose argument is formed by combining with the + 65816 Program Bank Register (16-bit JMP/JSR) must be specified + as 24-bit values for code that lives outside bank 0. This is + true for both symbols and raw hex (e.g. JSR $1234 + is invalid outside bank 0).
  • The arguments to COP and BRK require immediate-mode syntax (COP #$03 rather than COP $03).
  • For historical reasons, the default behavior of the assembler is to diff --git a/SourceGen/SGTestData/20052-branches-and-banks b/SourceGen/SGTestData/20052-branches-and-banks index 182ba4dab56e0c6fd560c8a33509d0a5dff713cb..db4c8cf724ddc0b6ee3dcd6a18c7501e0917e7b8 100644 GIT binary patch delta 30 lcmX@b_<(W3N%2?StJgO;g(v~>tF;YI3Jp$w8k}Bb0|46u4oCn1 delta 8 PcmaFBc#3htNycmd5ZD89 diff --git a/SourceGen/SGTestData/20052-branches-and-banks.dis65 b/SourceGen/SGTestData/20052-branches-and-banks.dis65 index 5219ded..c29fd16 100644 --- a/SourceGen/SGTestData/20052-branches-and-banks.dis65 +++ b/SourceGen/SGTestData/20052-branches-and-banks.dis65 @@ -1,8 +1,8 @@ ### 6502bench SourceGen dis65 v1.0 ### { -"_ContentVersion":3, -"FileDataLength":202, -"FileDataCrc32":530517490, +"_ContentVersion":4, +"FileDataLength":224, +"FileDataCrc32":2055368095, "ProjectProps":{ "CpuName":"65816", "IncludeUndocumentedInstr":false, @@ -222,6 +222,12 @@ "Label":"backchk", "Part":"Low"}}, +"133":{ +"Length":3, +"Format":"NumericLE", +"SubFormat":"Hex", +"SymbolRef":null}, + "139":{ "Length":3, "Format":"NumericLE", @@ -230,6 +236,12 @@ "Label":"fwdchk", "Part":"Low"}}, +"142":{ +"Length":3, +"Format":"NumericLE", +"SubFormat":"Hex", +"SymbolRef":null}, + "148":{ "Length":3, "Format":"NumericLE", @@ -258,4 +270,6 @@ "Visualizations":[], "VisualizationAnimations":[], "VisualizationSets":{ -}} +}, + +"RelocList":null} diff --git a/SourceGen/SGTestData/Expected/20052-branches-and-banks_64tass.S b/SourceGen/SGTestData/Expected/20052-branches-and-banks_64tass.S index e32a69f..d2f7ea8 100644 --- a/SourceGen/SGTestData/Expected/20052-branches-and-banks_64tass.S +++ b/SourceGen/SGTestData/Expected/20052-branches-and-banks_64tass.S @@ -58,7 +58,7 @@ bank54 cmp bank54 backchk nop nop -L543218 rts + rts backval .long backchk @@ -73,10 +73,10 @@ L54321C lda backchk nop jsr backchk jsr backchk+1 - jsr L543218 + jsr $543218 jsr fwdchk jsr fwdchk+1 - jsr L54327F + jsr $54327f nop ldx #$00 jsr (backval,x) @@ -105,9 +105,19 @@ fwdval .long fwdchk fwdchk nop nop -L54327F rts + rts L543280 jsr skip+$540000 + nop + phk + plb + lda $544280 + jsl $544280 + nop + lda $4280 + jsr $544280 + jsr ($544280,x) + nop rtl .here diff --git a/SourceGen/SGTestData/Expected/20052-branches-and-banks_Merlin32.S b/SourceGen/SGTestData/Expected/20052-branches-and-banks_Merlin32.S index 8f697c0..d5523ea 100644 --- a/SourceGen/SGTestData/Expected/20052-branches-and-banks_Merlin32.S +++ b/SourceGen/SGTestData/Expected/20052-branches-and-banks_Merlin32.S @@ -52,7 +52,7 @@ bank54 cmpl bank54 backchk nop nop -L543218 rts + rts backval adr backchk @@ -67,10 +67,10 @@ L54321C ldal backchk nop jsr backchk jsr backchk+1 - jsr L543218 + jsr $3218 jsr fwdchk jsr fwdchk+1 - jsr L54327F + jsr $327f nop ldx #$00 jsr (backval,x) @@ -99,8 +99,18 @@ fwdval adr $54327d fwdchk nop nop -L54327F rts + rts L543280 jsr skip + nop + phk + plb + ldal $544280 + jsl $544280 + nop + lda $4280 + jsr $4280 + jsr ($4280,x) + nop rtl diff --git a/SourceGen/SGTestData/Expected/20052-branches-and-banks_acme.S b/SourceGen/SGTestData/Expected/20052-branches-and-banks_acme.S index 7705eda..9c3c485 100644 --- a/SourceGen/SGTestData/Expected/20052-branches-and-banks_acme.S +++ b/SourceGen/SGTestData/Expected/20052-branches-and-banks_acme.S @@ -7,5 +7,5 @@ !hex 3254af163254af7d3254af163254af7d3254ad1732ad1532ad7e32ad7c32ea20 !hex 1632201732201832207d32207e32207f32eaa200fc1932fc7a32206e32207132 !hex 206832206b3220743220773280187c19327c7a326c08106c0810dc0810dc0810 - !hex 7d3254eaea60200e206b + !hex 7d3254eaea60200e20ea4babaf80425422804254eaad8042208042fc8042ea6b } ;!pseudopc diff --git a/SourceGen/SGTestData/Expected/20052-branches-and-banks_cc65.S b/SourceGen/SGTestData/Expected/20052-branches-and-banks_cc65.S index e4d6c35..ae4ade4 100644 --- a/SourceGen/SGTestData/Expected/20052-branches-and-banks_cc65.S +++ b/SourceGen/SGTestData/Expected/20052-branches-and-banks_cc65.S @@ -60,7 +60,7 @@ bank54: cmp bank54 backchk: nop nop -L543218: rts + rts backval: .faraddr backchk @@ -75,10 +75,10 @@ L54321C: lda backchk nop jsr backchk & $ffff jsr backchk & $ffff +1 - jsr L543218 & $ffff + jsr $3218 jsr fwdchk & $ffff jsr fwdchk & $ffff +1 - jsr L54327F & $ffff + jsr $327f nop ldx #$00 jsr (backval & $ffff,x) @@ -107,8 +107,18 @@ fwdval: .faraddr fwdchk fwdchk: nop nop -L54327F: rts + rts L543280: jsr skip + nop + phk + plb + lda $544280 + jsl $544280 + nop + lda $4280 + jsr $4280 + jsr ($4280,x) + nop rtl diff --git a/SourceGen/SGTestData/Expected/20052-branches-and-banks_cc65.cfg b/SourceGen/SGTestData/Expected/20052-branches-and-banks_cc65.cfg index 71e65e4..49008c6 100644 --- a/SourceGen/SGTestData/Expected/20052-branches-and-banks_cc65.cfg +++ b/SourceGen/SGTestData/Expected/20052-branches-and-banks_cc65.cfg @@ -5,7 +5,7 @@ MEMORY { # MEM001: file=%O, start=$440000, size=28; # MEM002: file=%O, start=$44ffc0, size=15; # MEM003: file=%O, start=$2000, size=32; -# MEM004: file=%O, start=$543210, size=116; +# MEM004: file=%O, start=$543210, size=138; } SEGMENTS { CODE: load=MAIN, type=rw;