[AVR] Don't adjust addresses by 2 for absolute values

[aykevl]

Summary

Adjusting by 2 breaks DWARF output. With this fix, programs start to compile and produce valid DWARF output.

---

I'm not quite sure whether this is the correct fix. I feel like this patches over a bug somewhere else. Feel free to suggest a different way to do it.

[aykevl]

After some investigation, it appears that "temporary labels" can be constructed like this:

1:
    jmp 1b

I tried to make a test for this but it seems like adjustFixupValue is only called when actually generating an object file. Therefore, to test this we'd need to disassemble the code and check whether the raw bytes have the correct relocation addend. So I think a good test for this would need to wait until there is better disassembly support.

[dylanmckay]

This comment was removed by dylanmckay.

[dylanmckay]

Great work, thanks @aykevl.

I suspect it is possible to write a test for this using llvc -show-encoding

llc ~/projects/llvm/llvm-project/llvm/test/CodeGen/AVR/and.ll -o - --show-mc-encoding -march=avr -mcpu=atmega328p

Here's part of the output

andi    r18, 253                ; encoding: [0x2d,0x7f]
andi    r20, 155                ; encoding: [0x4b,0x79]
andi    r21, 88                 ; encoding: [0x58,0x75]
andi    r22, 76                 ; encoding: [0x6c,0x74]
andi    r23, 73                 ; encoding: [0x79,0x74]
andi    r24, 31                 ; encoding: [0x8f,0x71]
andi    r25, 242                ; encoding: [0x92,0x7f]
ret                             ; encoding: [0x08,0x95]

I believe that relocations will also be included in the output, as fixup values.

If that's not possible, then I agree that the tests will have to come in a pater patch.

[aykevl]

I found a way to test at least the case where the value should not be shifted. If it is incorrectly shifted, the instruction becomes something other than a nop and thus will fail the test.
For the other case (when the value should be shifted), the AVR backend needs to be able to disassemble branch instructions. llc does not trigger the relevant code path.

[dylanmckay]

Nice!

llvm/lib/Target/AVR/MCTargetDesc/AVRAsmBackend.cpp

void AVRAsmBackend::adjustFixupValue(const MCFixup &Fixup,
   unsigned Kind = Fixup.getKind();
 
   // Parsed LLVM-generated temporary labels are already
   // adjusted for instruction size, but normal labels aren't.
   //
   // To handle both cases, we simply un-adjust the temporary label
   // case so it acts like all other labels.
   if (const MCSymbolRefExpr *A = Target.getSymA()) {
-    if (A->getSymbol().isTemporary())
+    if (A->getSymbol().isTemporary()) {
+      switch (Kind) {
+      case FK_Data_1:
+      case FK_Data_2:
+      case FK_Data_4:
+      case FK_Data_8:
+        // Don't shift value for absolute addresses.
+        break;
+      default:
         Value += 2;
       }
+    }
+  }
 
   switch (Kind) {
   default:
     llvm_unreachable("unhandled fixup");
   case AVR::fixup_7_pcrel:
     adjust::fixup_7_pcrel(Size, Fixup, Value, Ctx);
     break;
   case AVR::fixup_13_pcrel:

llvm/test/MC/AVR/relocations-abs.s

+; RUN: llvm-mc -filetype=obj -triple=avr %s | llvm-objdump -dr - | FileCheck %s
+
+; CHECK: bar:
+; CHECK-NEXT: 00 00 nop
+; CHECK-NEXT: R_AVR_16 .text+0x2
+bar:
+    .short 1f
+1: