From 478afa542e51d2220683e2b864a1f8e2f713ad57 Mon Sep 17 00:00:00 2001 From: Andy McFadden Date: Mon, 9 Aug 2021 13:47:04 -0700 Subject: [PATCH] Fix 64tass code gen corner case On the 65816, if you say "JSR foo" from bank $12, but "foo" is an address in bank 0, most assemblers will conclude that you're forming a 16-bit argument with a 16-bit address and assemble happily. 64tass halts with an error. Up until v1.55 or so, you could fake it out by supplying a large offset. This no longer works. The preferred way to say "no really I mean to do this" is to append ",k" to the operand. We now do that as needed. I didn't want to define a new ExpressionMode for 64tass just to support an operand modifier that should probably never actually get generated (you can't call across banks with JSR!), so this is implemented with a quirk and an op flag. 64tass v1.56.2625 is now the default. (issue #104) --- SourceGen/AsmGen/AsmTass64.cs | 4 +- SourceGen/AsmGen/GenCommon.cs | 8 ++- SourceGen/AsmGen/IGenerator.cs | 7 +++ SourceGen/PseudoOp.cs | 20 +++++-- SourceGen/RuntimeData/Help/codegen.html | 6 +- SourceGen/SGTestData/20052-branches-and-banks | Bin 238 -> 266 bytes .../SGTestData/20052-branches-and-banks.dis65 | 52 +++++++++++++++++- .../20052-branches-and-banks_64tass.S | 18 +++++- .../Expected/20052-branches-and-banks_acme.S | 3 +- .../Expected/20052-branches-and-banks_cc65.S | 16 +++++- .../20052-branches-and-banks_cc65.cfg | 2 +- .../20052-branches-and-banks_merlin32.S | 16 +++++- SourceGen/SGTestData/README.md | 2 +- .../Source/20052-branches-and-banks.S | 18 +++++- 14 files changed, 150 insertions(+), 22 deletions(-) diff --git a/SourceGen/AsmGen/AsmTass64.cs b/SourceGen/AsmGen/AsmTass64.cs index ef25f5c..38f51ef 100644 --- a/SourceGen/AsmGen/AsmTass64.cs +++ b/SourceGen/AsmGen/AsmTass64.cs @@ -127,6 +127,7 @@ namespace SourceGen.AsmGen { // Version we're coded against. private static CommonUtil.Version V1_53 = new CommonUtil.Version(1, 53, 1515); + private static CommonUtil.Version V1_56 = new CommonUtil.Version(1, 56, 2625); // Pseudo-op string constants. private static PseudoOp.PseudoOpNames sDataOpNames = @@ -178,13 +179,14 @@ namespace SourceGen.AsmGen { if (asmVersion != null) { mAsmVersion = asmVersion.Version; // Use the actual version. } else { - mAsmVersion = V1_53; // No assembler installed, use default. + mAsmVersion = V1_56; // No assembler installed, use default. } Quirks.StackIntOperandIsImmediate = true; Quirks.LeadingUnderscoreSpecial = true; Quirks.Need24BitsForAbsPBR = true; Quirks.BitNumberIsArg = true; + Quirks.BankZeroAbsPBRRestrict = true; mWorkDirectory = workDirectory; mFileNameBase = fileNameBase; diff --git a/SourceGen/AsmGen/GenCommon.cs b/SourceGen/AsmGen/GenCommon.cs index 73b5463..8a55161 100644 --- a/SourceGen/AsmGen/GenCommon.cs +++ b/SourceGen/AsmGen/GenCommon.cs @@ -215,7 +215,8 @@ namespace SourceGen.AsmGen { string formattedOperand = null; int operandLen = instrLen - 1; - PseudoOp.FormatNumericOpFlags opFlags = PseudoOp.FormatNumericOpFlags.OmitLabelPrefixSuffix; + PseudoOp.FormatNumericOpFlags opFlags = + PseudoOp.FormatNumericOpFlags.OmitLabelPrefixSuffix; bool isPcRelBankWrap = false; // Tweak branch instructions. We want to show the absolute address rather @@ -243,6 +244,11 @@ namespace SourceGen.AsmGen { if (op.IsAbsolutePBR) { opFlags |= PseudoOp.FormatNumericOpFlags.IsAbsolutePBR; } + if (gen.Quirks.BankZeroAbsPBRRestrict) { + // Hack to avoid having to define a new FormatConfig.ExpressionMode for 64tass. + // Get rid of this 64tass gets its own exp mode. + opFlags |= PseudoOp.FormatNumericOpFlags.Is64Tass; + } // 16-bit operands outside bank 0 need to include the bank when computing // symbol adjustment. diff --git a/SourceGen/AsmGen/IGenerator.cs b/SourceGen/AsmGen/IGenerator.cs index 7603a47..2269a75 100644 --- a/SourceGen/AsmGen/IGenerator.cs +++ b/SourceGen/AsmGen/IGenerator.cs @@ -207,6 +207,13 @@ namespace SourceGen.AsmGen { /// Enumeration of quirky or buggy behavior that GenCommon needs to handle. /// public class AssemblerQuirks { + /// + /// Does the assembler require a qualifier to be added to the operand when an instruction + /// formed with the Program Bank Register (JMP/JSR) attempts to access a bank zero + /// address from outside bank zero? + /// + public bool BankZeroAbsPBRRestrict { get; set; } + /// /// Does the assembler expect the bit index for BBR/BBS/RMB/SMB to be expressed as /// a separate argument? diff --git a/SourceGen/PseudoOp.cs b/SourceGen/PseudoOp.cs index c47ef6f..8ba882a 100644 --- a/SourceGen/PseudoOp.cs +++ b/SourceGen/PseudoOp.cs @@ -562,6 +562,7 @@ namespace SourceGen { IsAbsolutePBR = 1 << 1, // operand implicitly uses 'K' on 65816 (JMP/JSR) HasHashPrefix = 1 << 2, // operand has a leading '#', reducing ambiguity OmitLabelPrefixSuffix = 1 << 3, // don't show annotation char or non-unique prefix + Is64Tass = 1 << 4, // hack to allow 64tass to keep using "common" exp } /// @@ -798,6 +799,7 @@ namespace SourceGen { symbolValue &= 0xffff; } + bool needCommaK = false; if ((flags & FormatNumericOpFlags.IsAbsolutePBR) != 0) { if ((operandValue & 0x00ff0000) == (symbolValue & 0x00ff0000)) { // JMP or JSR to something within the same bank. We don't need to @@ -805,10 +807,16 @@ namespace SourceGen { symbolValue &= 0xffff; } else { // This is an absolute JMP/JSR to an out-of-bank location, which is - // bogus and should probably be prevented at a higher level. We handle - // it by altering the mask so an adjustment that covers the difference - // in bank values is generated. - mask = 0x00ffffff; + // bogus and should probably be prevented at a higher level. Most + // assemblers are perfectly happy with this because we're passing a + // 16-bit argument to a 16-bit operation, but 64tass doesn't like it + // when you do this. We used to just alter the mask to generate a + // (potentially very large) offset, but 64tass stopped accepting that. + // The correct approach is to add ",k" to the operand. + //mask = 0x00ffffff; + if ((flags & FormatNumericOpFlags.Is64Tass) != 0) { + needCommaK = true; + } } } @@ -863,6 +871,10 @@ namespace SourceGen { if (sb[0] == '(' && (flags & FormatNumericOpFlags.HasHashPrefix) == 0) { sb.Insert(0, "0+"); } + + if (needCommaK) { + sb.Append(",k"); + } } else { Debug.Assert(false, "bad numeric len"); sb.Append("?????"); diff --git a/SourceGen/RuntimeData/Help/codegen.html b/SourceGen/RuntimeData/Help/codegen.html index 5367d48..74cb95d 100644 --- a/SourceGen/RuntimeData/Help/codegen.html +++ b/SourceGen/RuntimeData/Help/codegen.html @@ -166,7 +166,7 @@ code, but also needs to know how to handle the corner cases.

64tass

-

Tested versions: v1.53.1515, v1.54.1900 +

Tested versions: v1.53.1515, v1.54.1900, v1.56.2625 [web site]

Bugs:

@@ -202,7 +202,9 @@ code, but also needs to know how to handle the corner cases.

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). + is invalid outside bank 0). Attempting to JSR to a label in bank + 0 from outside bank 0 causes an error, even though it is technically + a 16-bit operand.
  • 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 43e83e03da664c2bf4f7e18ed1e33ee83cbc16d3..a212b0e30d821fde32b9d19231118429f6dfc958 100644 GIT binary patch delta 40 ucmaFI*u^yArQ%vXg|$iw3VaF*N(z7Y6fQ8=0GX%Xu08!0$UOb_RW<-8VGz&& delta 12 TcmeBTddE27C9A^ex397RBfbVF diff --git a/SourceGen/SGTestData/20052-branches-and-banks.dis65 b/SourceGen/SGTestData/20052-branches-and-banks.dis65 index ad45105..9d1c12d 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":4, -"FileDataLength":238, -"FileDataCrc32":2077431201, +"FileDataLength":266, +"FileDataCrc32":-1640516088, "ProjectProps":{ "CpuName":"65816", "IncludeUndocumentedInstr":false, @@ -264,6 +264,54 @@ "SubFormat":"Symbol", "SymbolRef":{ "Label":"skip", +"Part":"Low"}}, + +"233":{ +"Length":3, +"Format":"NumericLE", +"SubFormat":"Symbol", +"SymbolRef":{ +"Label":"skip", +"Part":"Low"}}, + +"236":{ +"Length":3, +"Format":"NumericLE", +"SubFormat":"Symbol", +"SymbolRef":{ +"Label":"skip", +"Part":"Low"}}, + +"239":{ +"Length":3, +"Format":"NumericLE", +"SubFormat":"Symbol", +"SymbolRef":{ +"Label":"skip", +"Part":"Low"}}, + +"242":{ +"Length":3, +"Format":"NumericLE", +"SubFormat":"Symbol", +"SymbolRef":{ +"Label":"skip", +"Part":"Low"}}, + +"245":{ +"Length":3, +"Format":"NumericLE", +"SubFormat":"Symbol", +"SymbolRef":{ +"Label":"skip", +"Part":"Low"}}, + +"250":{ +"Length":3, +"Format":"NumericLE", +"SubFormat":"Symbol", +"SymbolRef":{ +"Label":"skip", "Part":"Low"}}}, "LvTables":{ diff --git a/SourceGen/SGTestData/Expected/20052-branches-and-banks_64tass.S b/SourceGen/SGTestData/Expected/20052-branches-and-banks_64tass.S index 48a9d62..7750ff5 100644 --- a/SourceGen/SGTestData/Expected/20052-branches-and-banks_64tass.S +++ b/SourceGen/SGTestData/Expected/20052-branches-and-banks_64tass.S @@ -107,7 +107,7 @@ fwdchk nop nop rts -L543280 jsr skip+$540000 +L543280 jsr skip,k nop rep #$30 .al @@ -132,8 +132,20 @@ L543280 jsr skip+$540000 lda #$eaea rep #$30 nop - jsr $54edcb - nop + lda skip + lda skip+20 + jsr skip,k + jsr skip+20,k + jsr (skip,k,x) + bne _L5432B7 + jmp (skip,k,x) + +_L5432B7 jsr $54edcb + lda $edcb + bne _L5432C2 + jmp ($54edcb,x) + +_L5432C2 nop rtl .here diff --git a/SourceGen/SGTestData/Expected/20052-branches-and-banks_acme.S b/SourceGen/SGTestData/Expected/20052-branches-and-banks_acme.S index 5065ef6..a2c23e1 100644 --- a/SourceGen/SGTestData/Expected/20052-branches-and-banks_acme.S +++ b/SourceGen/SGTestData/Expected/20052-branches-and-banks_acme.S @@ -8,5 +8,6 @@ !hex 1632201732201832207d32207e32207f32eaa200fc1932fc7a32206e32207132 !hex 206832206b3220743220773280187c19327c7a326c08106c0810dc0810dc0810 !hex 7d3254eaea60200e20eac23008a90000e230a90028a9eaeae23008a900c230a9 - !hex 000028a9eaeac230ea20cbedea6b + !hex 000028a9eaeac230eaad0e20ad2220200e20202220fc0e20d0037c0e2020cbed + !hex adcbedd0037ccbedea6b } ;!pseudopc diff --git a/SourceGen/SGTestData/Expected/20052-branches-and-banks_cc65.S b/SourceGen/SGTestData/Expected/20052-branches-and-banks_cc65.S index 988010b..7fba5fd 100644 --- a/SourceGen/SGTestData/Expected/20052-branches-and-banks_cc65.S +++ b/SourceGen/SGTestData/Expected/20052-branches-and-banks_cc65.S @@ -134,7 +134,19 @@ L543280: jsr skip lda #$eaea rep #$30 nop - jsr $edcb - nop + lda skip + lda skip+20 + jsr skip + jsr skip+20 + jsr (skip,x) + bne @L5432B7 + jmp (skip,x) + +@L5432B7: jsr $edcb + lda $edcb + bne @L5432C2 + jmp ($edcb,x) + +@L5432C2: 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 eed875b..cff2daf 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=152; +# MEM004: file=%O, start=$543210, size=180; } SEGMENTS { CODE: load=MAIN, type=rw; diff --git a/SourceGen/SGTestData/Expected/20052-branches-and-banks_merlin32.S b/SourceGen/SGTestData/Expected/20052-branches-and-banks_merlin32.S index eaa5b92..ba9b42a 100644 --- a/SourceGen/SGTestData/Expected/20052-branches-and-banks_merlin32.S +++ b/SourceGen/SGTestData/Expected/20052-branches-and-banks_merlin32.S @@ -123,7 +123,19 @@ L543280 jsr skip lda #$eaea rep #$30 nop - jsr $edcb - nop + lda skip + lda skip+20 + jsr skip + jsr skip+20 + jsr (skip,x) + bne :L5432B7 + jmp (skip,x) + +:L5432B7 jsr $edcb + lda $edcb + bne :L5432C2 + jmp ($edcb,x) + +:L5432C2 nop rtl diff --git a/SourceGen/SGTestData/README.md b/SourceGen/SGTestData/README.md index 776df66..807993a 100644 --- a/SourceGen/SGTestData/README.md +++ b/SourceGen/SGTestData/README.md @@ -6,7 +6,7 @@ NOTE: some tests may fail if you use a version of the assembler that is different from the one used to generate the expected output. The current set was generated for: - * 64tass v1.53.1515 + * 64tass v1.56.2625 * ACME v0.97 * cc65 v2.18 * Merlin 32 v1.0 diff --git a/SourceGen/SGTestData/Source/20052-branches-and-banks.S b/SourceGen/SGTestData/Source/20052-branches-and-banks.S index 48b0184..1d2600e 100644 --- a/SourceGen/SGTestData/Source/20052-branches-and-banks.S +++ b/SourceGen/SGTestData/Source/20052-branches-and-banks.S @@ -2,7 +2,7 @@ ; See the LICENSE.txt file for distribution terms (Apache 2.0). ; ; Assembler: cc65 -; (cl65 --target none -C .cfg .S) +; cl65 --target none -C 20052-branches-and-banks.cfg 20052-branches-and-banks.S ; ; For the 65816 we want to exercise some additional things. @@ -169,8 +169,22 @@ nxt54b: .i16 nop -; try a 16-bit JSR with no symbol +; try some 16-bit stuff with and without symbols + lda skip ;EDIT: set these to "skip" label + lda skip+20 + jsr skip + jsr skip+20 + jsr (skip,X) + bne past1 + jmp (skip,X) +past1: + +; try a few without symbols jsr $edcb + lda $edcb + bne past2 + jmp ($edcb,X) +past2: nop rtl