Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inline assembly, missing txa in some (not all) cases #428

Closed
cwedgwood opened this issue Dec 18, 2023 · 1 comment
Closed

Inline assembly, missing txa in some (not all) cases #428

cwedgwood opened this issue Dec 18, 2023 · 1 comment

Comments

@cwedgwood
Copy link

Using this code:

Testing with the mos-sim target (or c64 or atari8, but not mos-nes) -Os (others too):

#include <stdint.h>
#include <stdio.h>

// http://www.6502.org/source/general/SWN.html
// this requires noinline or else it gets things wrong
/* __attribute__((noinline)) */ uint8_t nybbleswap(uint8_t v) {

  asm(
      // put these is so we can locate the instructions when inline
      "sei \n cli \n"
      //
      "asl      \n"
      "adc #$80 \n"
      "rol      \n"
      "asl      \n"
      "adc #$80 \n"
      "rol      \n"

      : "+a"(v)
      :
      : "a", "p");
  return v;
}

int main() {
  for (auto i = 0; i <= 0xfe; i++) {
    auto v2 = nybbleswap(i);
    *(volatile char *)0x2000 = i;
    *(volatile char *)0x2001 = v2;
    // printf("%x %x\n", i, v2);
  }
  return 0;
}

We get the following:

00000221 <main>:
     221: a2 00         ldx     #$0
     223: 8a            txa
     224: 78            sei
     225: 58            cli
     226: 0a            asl
     227: 69 80         adc     #$80
     229: 2a            rol
     22a: 0a            asl
     22b: 69 80         adc     #$80
     22d: 2a            rol
     22e: 8e 00 20      stx     $2000                   ; 0x2000 <__heap_start+0x1dbe>
     231: 8d 01 20      sta     $2001                   ; 0x2001 <__heap_start+0x1dbf>
     234: e8            inx
     235: e0 ff         cpx     #$ff
     237: d0 ea         bne     $223 <main+0x2>
     239: a2 00         ldx     #$0
     23b: 8a            txa
     23c: 60            rts

If we uncomment the printf:

00000224 <main>:
     224: 18            clc
     225: a5 00         lda     $0                      ; 0x0 <__zp_data_size>
     227: 69 fc         adc     #$fc
     229: 85 00         sta     $0                      ; 0x0 <__zp_data_size>
     22b: a5 01         lda     $1                      ; 0x1 <__rc1>
     22d: 69 ff         adc     #$ff
     22f: 85 01         sta     $1                      ; 0x1 <__rc1>
     231: a2 00         ldx     #$0
     233: 86 55         stx     $55                     ; 0x55 <__zp_bss_start+0x35>
     235: 78            sei
     236: 58            cli
     237: 0a            asl
     238: 69 80         adc     #$80
     23a: 2a            rol
     23b: 0a            asl
     23c: 69 80         adc     #$80
     23e: 2a            rol
     23f: 8e 00 20      stx     $2000                   ; 0x2000 <__heap_start+0x6e2>
     242: 8d 01 20      sta     $2001                   ; 0x2001 <__heap_start+0x6e3>
     245: a0 02         ldy     #$2
     247: 91 00         sta     ($0),y                  ; 0x0 <__zp_data_size>
     249: a4 55         ldy     $55                     ; 0x55 <__zp_bss_start+0x35>
     24b: 8a            txa
     24c: 91 00         sta     ($0),y                  ; 0x0 <__zp_data_size>
     24e: 86 56         stx     $56                     ; 0x56 <__zp_bss_start+0x36>
     250: a9 00         lda     #$0
     252: a0 01         ldy     #$1
     254: 91 00         sta     ($0),y                  ; 0x0 <__zp_data_size>
     256: a0 03         ldy     #$3
     258: 91 00         sta     ($0),y                  ; 0x0 <__zp_data_size>
     25a: 20 7a 02      jsr     $27a <printf>
     25d: a6 56         ldx     $56                     ; 0x56 <__zp_bss_start+0x36>
     25f: e8            inx
     260: e0 ff         cpx     #$ff
     262: d0 d1         bne     $235 <main+0x11>

There is a missing txa at 0x235

@mysterymath mysterymath transferred this issue from llvm-mos/llvm-mos-sdk Dec 19, 2023
@mysterymath
Copy link
Member

"Clobber descriptions may not in any way overlap with an input or output operand."
https://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html#Clobbers-and-Scratch-Registers

Accordingly, this is undefined behavior. I think I'd like the compiler to emit a warning for this if that's possible; it appears that there are similar warnings in Clang for other platforms, so it may be a matter of hooking it up. See #385 for this.

I'm bumping #385 up to a P0 due to the number of times this has come up; a bad inline assemble experience may be a serious risk for the project. I'll take a look at this in the near future and see if there's anything we can do about the user experience; but the rules themselves are out of our hands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants