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

[AVR] Microchip changed the semantics of 16 bit register writes on newer cores #58395

Closed
G33KatWork opened this issue Oct 16, 2022 · 4 comments
Closed
Assignees

Comments

@G33KatWork
Copy link

When accessing 16 bit registers like counters on an 8 bit AVR core, these accesses have to be split into two 8 bit register writes. To do this, the peripherals have a TEMP register that the CPU automatically writes to on the first write and a write to the other half of the target register yields a write of this temporary register and the currently written value into the 16 bit target register in the same cycle.

On old cores like an ATmega8, the datasheet states that you need to write the high byte first and then the low byte. See ATmega8 datasheet, page 78.. LLVM takes care of this here for writes and here for reads.

However, newer cores like the tinyAVR 1-series like an ATtiny817 require these writes to happen with the LOW byte first, followed by the high byte. See ATtiny817 datasheet, page 55. I didn't check any other datasheets yet about which other cores might be affected by this change.

I just ran into this when trying to use Rust on an ATtiny817. Writing to the counter register with a u16 seemingly caused the low byte to be ignored or otherwise wrong all the time. Once I converted my writes manually to two u8 writes in the order specified by the datasheet, things started to work. Flipping them once more showed the same broken results as the single u16 write.

I also confirmed the wrong order by disassembling the resulting Rust program and comparing it to an equivalent C program compiled by avr-gcc. The avr-gcc compiled program was equivalent except for the write order of the two register halves.

@G33KatWork
Copy link
Author

I did some poking a the avr-gcc code to figure out which devices are affected by this change. Apparently all XMEGA cores require the low byte to be written first. Here are a few functions in gcc responsible for this:

And here is a list of all the XMEGA cores.

Maybe that helps a bit.

@benshi001
Copy link
Member

fixed by https://reviews.llvm.org/D141752

@sprintersb
Copy link

fixed by https://reviews.llvm.org/D141752

Not completely, take for example

volatile int* stv (int volatile *p, int volatile *q)
{
    *q = 0;
    *p++ = 0;
    return p;
}

compiled with -S -mmcu=avr5 -Os. Output is:

stv:                                    ; @stv
; %bb.0:
	movw	r26, r24
	ldi	r24, 0
	ldi	r25, 0
	movw	r30, r22
	st	Z, r24
	std	Z+1, r25
	st	X+, r24
	st	X+, r25
	movw	r24, r26
	ret

I didn't try all addressing modes, but at least the above two (indirect and post-increment) are not correct, because we have to obey the following access orders for multi-byte volatile data:

non-Xmega Xmega
Load low first low first
Store high first low first

For an explanation, see for example Does avr-gcc properly work with 16-bit AVR I/O registers? on stackoverflow.

A device is Xmega iff __AVR_ARCH__ >= 100.

--version
Ubuntu clang version 16.0.0 (++20230118052744+29d25f9e9a4c-1~exp1~20230118172840.512)

@benshi001
Copy link
Member

I have uploaded a new version of my patch https://reviews.llvm.org/D141752, your new test is correctly
compiled to

        .text
.set __tmp_reg__, 0
.set __zero_reg__, 1
.set __SREG__, 63
.set __SP_H__, 62
.set __SP_L__, 61
        .file   "a.c"
        .globl  stv                             ; -- Begin function stv
        .p2align        1
        .type   stv,@function
stv:                                    ; @stv
; %bb.0:
        ldi     r18, 0
        ldi     r19, 0
        movw    r30, r22
        std     Z+1, r19
        st      Z, r18
        movw    r30, r24
        std     Z+1, r19
        st      Z, r18
        adiw    r24, 2
        ret
.Lfunc_end0:
        .size   stv, .Lfunc_end0-stv
                                        ; -- End function
        .addrsig

flemairen6 pushed a commit to Xilinx/llvm-project that referenced this issue May 10, 2023
For 16-bit ports, the normal devices reqiure writing high byte first
and then low byte. But the XMEGA devices require the reverse order.

Fixes llvm#58395

Reviewed By: aykevl, jacquesguan

Differential Revision: https://reviews.llvm.org/D141752
veselypeta pushed a commit to veselypeta/cherillvm that referenced this issue Aug 21, 2024
For 16-bit ports, the normal devices reqiure writing high byte first
and then low byte. But the XMEGA devices require the reverse order.

Fixes llvm/llvm-project#58395

Reviewed By: aykevl, jacquesguan

Differential Revision: https://reviews.llvm.org/D141752
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants