From fdd2bcf8470c17b8c187fc4509069dee9424902f Mon Sep 17 00:00:00 2001 From: Andy McFadden Date: Wed, 1 Jul 2020 17:59:12 -0700 Subject: [PATCH] Fix some 65816 code generation issues Two basic problems: (1) cc65, being a one-pass assembler, can't tell if a forward-referenced label is 16-bit or 24-bit. If the operand is potentially ambiguous, such as "LDA label", we need to add an operand width disambiguator. (The existing tests managed to only do backward references.) (2) 64tass wants the labels on JMP/JSR absolute operands to have 24-bit values that match the current program bank. This is the opposite of cc65, which requires 16-bit values. We need to distinguish PBR vs. DBR instructions (i.e. "LDA abs" vs. "JMP abs") and handle them differently when formatting for "Common". Merlin32 doesn't care, and ACME doesn't work at all, so neither of those needed updating. The 20052-branches-and-banks test was expanded to cover the problematic cases. --- Asm65/OpDef.cs | 26 ++++ SourceGen/AsmGen/GenCommon.cs | 11 ++ SourceGen/LineListGen.cs | 3 + SourceGen/MainController.cs | 2 +- SourceGen/PseudoOp.cs | 19 ++- SourceGen/SGTestData/20052-branches-and-banks | Bin 83 -> 202 bytes .../SGTestData/20052-branches-and-banks.dis65 | 132 +++++++++++++++++- .../20052-branches-and-banks_64tass.S | 65 ++++++++- .../20052-branches-and-banks_Merlin32.S | 64 ++++++++- .../Expected/20052-branches-and-banks_acme.S | 6 +- .../Expected/20052-branches-and-banks_cc65.S | 65 ++++++++- .../20052-branches-and-banks_cc65.cfg | 4 +- .../Source/20052-branches-and-banks.S | 79 +++++++++++ 13 files changed, 459 insertions(+), 17 deletions(-) diff --git a/Asm65/OpDef.cs b/Asm65/OpDef.cs index 2afc422..b2d7865 100644 --- a/Asm65/OpDef.cs +++ b/Asm65/OpDef.cs @@ -252,6 +252,32 @@ namespace Asm65 { } } + /// + /// True if this is an absolute-address instruction whose operand is combined with + /// the Program Bank Register. + /// + /// + /// As noted in Eyes & Lichty, the Absolute addressing mode uses the Data Bank Register + /// if locating data, or the Program Bank Register if transferring control. When + /// we "LDA symbol" we can only care about the 16-bit value because we can't know what B + /// will hold at run time. When we "JMP symbol" we can either (1) complain if it's in + /// a different bank, (2) complain if a bank is specified at all, or (3) just not care. + /// All three approaches are in use. + /// + /// This call is a way to know that the instruction is merged with the PBR rather than + /// the DBR. + /// + public bool IsAbsolutePBR { + get { + // Too lazy to add new field. Set for: + // JSR abs + // JMP abs + // JMP (abs,X) + // JSR (abs,X) + return Opcode == 0x20 || Opcode == 0x4c || Opcode == 0x7c || Opcode == 0xfc; + } + } + /// /// True if the operand's width is uniquely determined by the opcode mnemonic, even /// if the operation supports operands with varying widths. diff --git a/SourceGen/AsmGen/GenCommon.cs b/SourceGen/AsmGen/GenCommon.cs index de3b1d3..621e35e 100644 --- a/SourceGen/AsmGen/GenCommon.cs +++ b/SourceGen/AsmGen/GenCommon.cs @@ -201,6 +201,14 @@ namespace SourceGen.AsmGen { wdis = OpDef.WidthDisambiguation.ForceDirect; } } + if (wdis == OpDef.WidthDisambiguation.ForceLongMaybe && + gen.Quirks.SinglePassAssembler && + IsForwardLabelReference(gen, offset)) { + // Assemblers like cc65 can't tell if a symbol reference is Absolute or + // Long if they haven't seen the symbol yet. Irrelevant for ACME, which + // doesn't currently handle 65816 outside bank 0. + wdis = OpDef.WidthDisambiguation.ForceLong; + } string opcodeStr = formatter.FormatOpcode(op, wdis); @@ -228,6 +236,9 @@ namespace SourceGen.AsmGen { int branchDist = attr.Address - attr.OperandAddress; isPcRelBankWrap = branchDist > 32767 || branchDist < -32768; } + if (op.IsAbsolutePBR) { + opFlags |= PseudoOp.FormatNumericOpFlags.IsAbsolutePBR; + } // 16-bit operands outside bank 0 need to include the bank when computing // symbol adjustment. diff --git a/SourceGen/LineListGen.cs b/SourceGen/LineListGen.cs index ee295ec..6eb3d45 100644 --- a/SourceGen/LineListGen.cs +++ b/SourceGen/LineListGen.cs @@ -1240,6 +1240,9 @@ namespace SourceGen { op.AddrMode == OpDef.AddressMode.ImmLongXY) { opFlags = PseudoOp.FormatNumericOpFlags.HasHashPrefix; } + if (op.IsAbsolutePBR) { + opFlags |= PseudoOp.FormatNumericOpFlags.IsAbsolutePBR; + } // Use the OperandAddress when available. This is important for relative branch // instructions and PER, where we want to show the target address rather than the diff --git a/SourceGen/MainController.cs b/SourceGen/MainController.cs index 4cb8fad..f640ad5 100644 --- a/SourceGen/MainController.cs +++ b/SourceGen/MainController.cs @@ -1160,7 +1160,7 @@ namespace SourceGen { /// Full path to file. /// Project object. /// Returns true if we want to cancel the attempt. - /// + /// File data. private byte[] FindValidDataFile(ref string dataPathName, DisasmProject proj, out bool cancel) { FileInfo fi = new FileInfo(dataPathName); diff --git a/SourceGen/PseudoOp.cs b/SourceGen/PseudoOp.cs index 0519cff..291413e 100644 --- a/SourceGen/PseudoOp.cs +++ b/SourceGen/PseudoOp.cs @@ -558,8 +558,9 @@ namespace SourceGen { public enum FormatNumericOpFlags { None = 0, IsPcRel = 1, // opcode is PC relative, e.g. branch or PER - HasHashPrefix = 1 << 1, // operand has a leading '#', reducing ambiguity - OmitLabelPrefixSuffix = 1 << 2, // don't show annotation char or non-unique prefix + 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 } /// @@ -786,6 +787,20 @@ namespace SourceGen { symbolValue &= 0xffff; } + if ((flags & FormatNumericOpFlags.IsAbsolutePBR) != 0) { + if ((operandValue & 0x00ff0000) == (symbolValue & 0x00ff0000)) { + // JMP or JSR to something within the same bank. We don't need to + // mask the value. + 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; + } + } + bool needMask = false; if (symbolValue > mask) { // Post-shift value won't fit in an operand-size box. diff --git a/SourceGen/SGTestData/20052-branches-and-banks b/SourceGen/SGTestData/20052-branches-and-banks index d6683104e85c4c284124bea7823bd8fb9674232c..182ba4dab56e0c6fd560c8a33509d0a5dff713cb 100644 GIT binary patch delta 126 zcmWGO#W*3TB1XU{x|ac7`;*uGg1&YQjjoG zs5MfkGg7EGdbNn*kEGF`DkFtFBZWdEg$yHwY$JsdBZYFK28kL;qnawC91Z~>x&uZ) MgMrp4@F`>i0M0on!T> 16) - bne _L200E + bne skip jml [lodat] -_L200E nop +skip nop jsr j2 j2 jsr j2+3 jsr j2-3 jsl longsym - rts + jml bank54 + + .here + .logical $543210 +bank54 cmp bank54 + bra L54321C + +backchk nop + nop +L543218 rts + +backval .long backchk + +L54321C lda backchk + lda fwdchk + lda $543216 + lda $54327d + lda 0+(backchk & $ffff)+1 + lda 0+(backchk & $ffff)-1 + lda 0+(fwdchk & $ffff)+1 + lda 0+(fwdval & $ffff)+2 + nop + jsr backchk + jsr backchk+1 + jsr L543218 + jsr fwdchk + jsr fwdchk+1 + jsr L54327F + nop + ldx #$00 + jsr (backval,x) + jsr (fwdval,x) + jsr _L54326E + jsr _L543271 + jsr _L543268 + jsr _L54326B + jsr _L543274 + jsr _L543277 + bra L543280 + +_L543268 jmp (backval,x) + +_L54326B jmp (fwdval,x) + +_L54326E jmp ($1008) + +_L543271 jmp ($1008) + +_L543274 jml [$1008] + +_L543277 jml [$1008] + +fwdval .long fwdchk + +fwdchk nop + nop +L54327F rts + +L543280 jsr skip+$540000 + 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 2ada6dc..8f697c0 100644 --- a/SourceGen/SGTestData/Expected/20052-branches-and-banks_Merlin32.S +++ b/SourceGen/SGTestData/Expected/20052-branches-and-banks_Merlin32.S @@ -36,13 +36,71 @@ high44 beq :L44FFCB :L2000 bit :L2000 pea dat44 pea ^dat44 - bne :L200E + bne skip jml [lodat] -:L200E nop +skip nop jsr j2 j2 jsr j2+3 jsr j2-3 jsl longsym - rts + jml bank54 + + org $543210 +bank54 cmpl bank54 + bra L54321C + +backchk nop + nop +L543218 rts + +backval adr backchk + +L54321C ldal backchk + ldal fwdchk + ldal $543216 + ldal $54327d + lda backchk+1 + lda backchk-1 + lda fwdchk+1 + lda fwdval+2 + nop + jsr backchk + jsr backchk+1 + jsr L543218 + jsr fwdchk + jsr fwdchk+1 + jsr L54327F + nop + ldx #$00 + jsr (backval,x) + jsr (fwdval,x) + jsr :L54326E + jsr :L543271 + jsr :L543268 + jsr :L54326B + jsr :L543274 + jsr :L543277 + bra L543280 + +:L543268 jmp (backval,x) + +:L54326B jmp (fwdval,x) + +:L54326E jmp ($1008) + +:L543271 jmp ($1008) + +:L543274 jml [$1008] + +:L543277 jml [$1008] + +fwdval adr $54327d + +fwdchk nop + nop +L54327F rts + +L543280 jsr skip + rtl diff --git a/SourceGen/SGTestData/Expected/20052-branches-and-banks_acme.S b/SourceGen/SGTestData/Expected/20052-branches-and-banks_acme.S index bf7210c..7705eda 100644 --- a/SourceGen/SGTestData/Expected/20052-branches-and-banks_acme.S +++ b/SourceGen/SGTestData/Expected/20052-branches-and-banks_acme.S @@ -3,5 +3,9 @@ !pseudopc $1000 { !hex 18fbe2305c000044000102cf000044af000044ad0000a50030f562b2ffd0b082 !hex a9ff1700170044cfc0ff44f005303c8239005c0020002c0020f41700f44400d0 - !hex 03dc0810ea201220201520200f202256341260 + !hex 03dc0810ea201220201520200f20225634125c103254cf1032548006eaea6016 + !hex 3254af163254af7d3254af163254af7d3254ad1732ad1532ad7e32ad7c32ea20 + !hex 1632201732201832207d32207e32207f32eaa200fc1932fc7a32206e32207132 + !hex 206832206b3220743220773280187c19327c7a326c08106c0810dc0810dc0810 + !hex 7d3254eaea60200e206b } ;!pseudopc diff --git a/SourceGen/SGTestData/Expected/20052-branches-and-banks_cc65.S b/SourceGen/SGTestData/Expected/20052-branches-and-banks_cc65.S index 2e54e76..e4d6c35 100644 --- a/SourceGen/SGTestData/Expected/20052-branches-and-banks_cc65.S +++ b/SourceGen/SGTestData/Expected/20052-branches-and-banks_cc65.S @@ -43,13 +43,72 @@ high44: beq @L44FFCB @L2000: bit @L2000 pea dat44 & $ffff pea dat44 >> 16 - bne @L200E + bne skip jml [lodat] -@L200E: nop +skip: nop jsr j2 j2: jsr j2+3 jsr j2-3 jsl longsym - rts + jml bank54 + +; .segment "SEG004" + .org $543210 +bank54: cmp bank54 + bra L54321C + +backchk: nop + nop +L543218: rts + +backval: .faraddr backchk + +L54321C: lda backchk + lda f:fwdchk + lda $543216 + lda $54327d + lda backchk & $ffff +1 + lda backchk & $ffff -1 + lda fwdchk & $ffff +1 + lda fwdval & $ffff +2 + nop + jsr backchk & $ffff + jsr backchk & $ffff +1 + jsr L543218 & $ffff + jsr fwdchk & $ffff + jsr fwdchk & $ffff +1 + jsr L54327F & $ffff + nop + ldx #$00 + jsr (backval & $ffff,x) + jsr (fwdval & $ffff,x) + jsr @L54326E & $ffff + jsr @L543271 & $ffff + jsr @L543268 & $ffff + jsr @L54326B & $ffff + jsr @L543274 & $ffff + jsr @L543277 & $ffff + bra L543280 + +@L543268: jmp (backval & $ffff,x) + +@L54326B: jmp (fwdval & $ffff,x) + +@L54326E: jmp ($1008) + +@L543271: jmp ($1008) + +@L543274: jml [$1008] + +@L543277: jml [$1008] + +fwdval: .faraddr fwdchk + +fwdchk: nop + nop +L54327F: rts + +L543280: jsr skip + rtl diff --git a/SourceGen/SGTestData/Expected/20052-branches-and-banks_cc65.cfg b/SourceGen/SGTestData/Expected/20052-branches-and-banks_cc65.cfg index e3ec4e4..71e65e4 100644 --- a/SourceGen/SGTestData/Expected/20052-branches-and-banks_cc65.cfg +++ b/SourceGen/SGTestData/Expected/20052-branches-and-banks_cc65.cfg @@ -4,7 +4,8 @@ MEMORY { # MEM000: file=%O, start=$1000, size=11; # MEM001: file=%O, start=$440000, size=28; # MEM002: file=%O, start=$44ffc0, size=15; -# MEM003: file=%O, start=$2000, size=29; +# MEM003: file=%O, start=$2000, size=32; +# MEM004: file=%O, start=$543210, size=116; } SEGMENTS { CODE: load=MAIN, type=rw; @@ -12,6 +13,7 @@ SEGMENTS { # SEG001: load=MEM001, type=rw; # SEG002: load=MEM002, type=rw; # SEG003: load=MEM003, type=rw; +# SEG004: load=MEM004, type=rw; } FEATURES {} SYMBOLS {} diff --git a/SourceGen/SGTestData/Source/20052-branches-and-banks.S b/SourceGen/SGTestData/Source/20052-branches-and-banks.S index 3fdfae4..369aeae 100644 --- a/SourceGen/SGTestData/Source/20052-branches-and-banks.S +++ b/SourceGen/SGTestData/Source/20052-branches-and-banks.S @@ -51,4 +51,83 @@ j2: jsr j3 ;EDIT: set label j3: jsr j1 jsl symlong + jml bank54 + + + + .org $543210 +bank54: cmp f:bank54 + bra nxt54a + +backchk: + nop ;EDIT: set label + nop rts +backval: + .word backchk & $ffff + .byte bank54 >> 16 + +nxt54a: +; Test forward/backward refs. In cc65 this makes a difference because it's +; a one-pass assembler. + lda f:backchk ;EDIT: use symbol + lda f:fwdchk ;EDIT: use symbol + lda f:backchk ;EDIT: use hex + lda f:fwdchk ;EDIT: use hex + + lda a:backchk & $ffff + 1 ;EDIT: use symbol + lda a:backchk & $ffff - 1 ;EDIT: use symbol + lda a:fwdchk & $ffff + 1 ;EDIT: use symbol + lda a:fwdchk & $ffff - 1 ;EDIT: use symbol + +; Test non-bank-0 JSRs. The behavior varies significantly by assembler. The +; trick is that the high byte comes from the 'K' register. cc65 wants you +; to remove it for a 16-bit JSR (or fails because the operand is too big), +; 64tass wants you to keep it in place (or fails because you're trying to +; jump to the wrong bank), and Merlin32 doesn't care. + + nop + jsr backchk & $ffff + jsr backchk & $ffff + 1 ;EDIT: set to "backchk" + jsr backchk & $ffff + 2 ;leave in hex + jsr fwdchk & $ffff + jsr fwdchk & $ffff + 1 ;EDIT: set to "fwdchk" + jsr fwdchk & $ffff + 2 ;leave in hex + + nop + ldx #$00 + jsr (backval & $ffff,X) + jsr (fwdval & $ffff,X) + jsr jmp1b & $ffff + jsr jmp1f & $ffff + jsr jmp2b & $ffff + jsr jmp2f & $ffff + jsr jmp3b & $ffff + jsr jmp3f & $ffff + bra nxt54b + +; Fun fact: "JMP (addr)" and "JML [addr]" always load the indirect value +; from zero page. They do *not* use B or K. The (addr,X) mode uses the +; program bank (K). According to Eyes & Lichty, this is because the +; non-indexed form assumes you're jumping through a variable, while the +; indexed form assumes you've got an address table in your code. (Think +; how they would be used in ROM.) +jmp2b: jmp (backval & $ffff,X) +jmp2f: jmp (fwdval & $ffff,X) +jmp1b: jmp (lodat) +jmp1f: jmp (lodat) +jmp3b: jmp [lodat] +jmp3f: jmp [lodat] + +fwdval: + .word fwdchk & $ffff + .byte bank54 >> 16 + +fwdchk: + nop ;EDIT: set label + nop + rts + +nxt54b: + jsr skip ;EDIT: set to "skip" label + rtl