Loads and stores to 16-bit values ignore half of the word

[shepmaster]

This IR:

target datalayout = "e-p:16:8:8-i8:8:8-i16:8:8-i32:8:8-i64:8:8-f32:8:8-f64:8:8-n8"
target triple = "avr-atmel-none"

@COUNTER = internal global i16 0, align 1

define internal void @main() unnamed_addr #0 {
entry-block:
  store i16 1, i16* @COUNTER, align 1
  ret void
}

attributes #0 = { uwtable }

Produces this output from llc -march=avr -mcpu=atmega328p < global.ll:

main:                                   ; @main
    ldi r24, 1
    ldi r25, 0
    sts COUNTER+1, r25
    sts COUNTER, r24
    ret

.Lfunc_end0:
    .size   main, .Lfunc_end0-main

    .type   COUNTER,@object         ; @COUNTER
    .local  COUNTER
    .comm   COUNTER,2,1

Which seems correct. However, the actual object (from llc -march=avr -mcpu=atmega328p global.ll -filetype=obj), disassembled with objdump shows:

00000000 <main>:
   0:   81 e0           ldi r24, 0x01   ; 1
   2:   90 e0           ldi r25, 0x00   ; 0
   4:   90 93 00 00     sts 0x0000, r25
   8:   80 93 00 00     sts 0x0000, r24
   c:   08 95           ret

Note that both sts instructions are writing to the same memory location, clobbering half of the value and not updating the other half.

I see the same issue when loading 16-bit values as well.

[dylanmckay]

@shepmaster: I'm gonna take a look into this.

[dylanmckay]

Both sts and lds use the F32DM instruction format from AVRInstrFormats.td. This is probably not a coincidence.

[dylanmckay]

If we use llvm-mc to assemble the assembly file:

(test.s):

main:
    ldi r24, 1
    ldi r25, 0
    sts COUNTER+1, r25
    sts COUNTER, r24
    ret

llvm-mc test.s -arch=avr -mcpu=atmega328p -filetype=obj -o test.o

We also get the same result. This is not a codegen problem but an MC problem.

[dylanmckay]

The exact same problem comes up when COUNTER is not even defined. I think this is a problem with relocations.

What should be happening is that when a symbol such as COUNTER is used, a relocation is created to fix up the sts instruction symbol address + 1. Until the object is linked, the unresolved bits in the instruction would still be 0, which looks like this case.

objdump -r shows that the object file has no relocations. This is probably the problem.

[dylanmckay]

When AVR-GCC compiles this snippet:

  sts 3,   r5
  sts 255, r7
  sts SYMBOL+1, r25

It generates this

00000000 <foo>:
00000000 <foo>:
   0:   50 92 03 00     sts     0x0003, r5
   4:   70 92 ff 00     sts     0x00FF, r7
   8:   90 93 00 00     sts     0x0000, r25
                        a: R_AVR_16     SYMBOL+0x1

We should also be generating an R_AVR_16 reloc in this case.

[dylanmckay]

Fixed as of c97cbe5.

What was happening is that we said that the instruction opcode is i16imm - a 16-bit immediate.

For some reason, LLVM doesn't give us an error when a relocatable expression is used in this context, even though LLVM doesn't know how to lower it into machine code.

This was fixed by adding a new addr16 operand type, and writing an MC encoder to handle the case where the operand is an expression, in which case, a fixup_16 is generated, which gets lowered into an R_AVR_16 relocation during ELF file creation.

[dylanmckay]

This is likely going to be a problem for a few different instructions which expect immediates but can have expressions.

I will look into this and fix them up.

[shepmaster]

This was an exciting, informative whirlwind conversation you had with yourself! Thank you for tracking it here so I could attempt to understand it. Maybe more importantly, thank you for fixing it ^_^.