[llvm] [MC] Fuse relaxation and layout into a single forward pass (PR #184544)

[Hans Wennborg via llvm-commits]

zmodem wrote:

I think I'm starting to see what the problem is.

Here's an asm based reproducer: [a.s.gz](https://github.com/user-attachments/files/26348152/a.s.gz)

```
$ build/bin/llvm-mc -filetype=obj -o /tmp/a.bad.o -triple=thumbv7-linux-androideabi29 /tmp/a.s
/tmp/a.s:43:10: error: out of range pc-relative fixup value
        cbz     r0, .LBB0_11
                    ^
```

If we comment out the error message we can see what code it's trying to emit:

```
$ git diff
diff --git a/llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp b/llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp
index cc21844b2635..be985bbd3468 100644
--- a/llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp
+++ b/llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp
@@ -747,7 +747,7 @@ unsigned ARMAsmBackend::adjustFixupValue(const MCAssembler &Asm,
     // so ensure that the raw value LSB is zero and it lies in [2, 130].
     // An offset of 2 will be relaxed to a NOP.
     if ((int64_t)Value < 2 || Value > 0x82 || Value & 1) {
-      Ctx.reportError(Fixup.getLoc(), "out of range pc-relative fixup value");
+      llvm::errs() << "FIXUP VALUE: " << Value << "\n";
       return 0;
     }
     // Offset by 4 and don't encode the lower bit, which is always 0.
```

```
$ build/bin/llvm-mc -filetype=obj -o /tmp/a.bad.o -triple=thumbv7-linux-androideabi29 /tmp/a.s
$ build/bin/llvm-objdump -d /tmp/a.bad.o --triple=thumbv7m-arm-none-eabi | tee /tmp/bad.txt
```

and compare it with the output before this change:

```
$ build/bin/llvm-mc.good -filetype=obj -o /tmp/a.good.o -triple=thumbv7-linux-androideabi29 /tmp/a.s
$ build/bin/llvm-objdump -d /tmp/a.good.o --triple=thumbv7m-arm-none-eabi | tee /tmp/good.txt
```

Here are the output files for convenience:  [good.txt](https://github.com/user-attachments/files/26348297/good.txt), [bad.txt](https://github.com/user-attachments/files/26348302/bad.txt)

>From good.txt:

```
      2c: b3e8         	cbz	r0, 0xaa <_ZN4fuzz5mojom14FuzzUnion_Data8ValidateEv+0xaa> @ imm = #0x7a
...
      56: 4a17         	ldr	r2, [pc, #0x5c]         @ 0xb4 <_ZN4fuzz5mojom14FuzzUnion_Data8ValidateEv+0xb4>
...
      8a: 4a0d         	ldr	r2, [pc, #0x34]         @ 0xc0 <_ZN4fuzz5mojom14FuzzUnion_Data8ValidateEv+0xc0>
...
      9c: 4a09         	ldr	r2, [pc, #0x24]         @ 0xc4 <_ZN4fuzz5mojom14FuzzUnion_Data8ValidateEv+0xc4>
...
      aa: 4620         	mov	r0, r4
      ac: bdb0         	pop	{r4, r5, r7, pc}
```

>From bad.txt:

```
      2c: b100         	cbz	r0, ...     <--- Trying to branch to B0 but the immediate doesn't fit.
...
      56: f8df 2060    	ldr.w	r2, [pc, #0x60]         @ 0xb8 <_ZN4fuzz5mojom14FuzzUnion_Data8ValidateEv+0xb8>
...
      8c: f8df 2034    	ldr.w	r2, [pc, #0x34]         @ 0xc4 <_ZN4fuzz5mojom14FuzzUnion_Data8ValidateEv+0xc4>
...
      a0: f8df 2024    	ldr.w	r2, [pc, #0x24]         @ 0xc8 <_ZN4fuzz5mojom14FuzzUnion_Data8ValidateEv+0xc8>
...
      b0: 4620         	mov	r0, r4
      b2: bdb0         	pop	{r4, r5, r7, pc}
```

There seems to be two issues:

1. After this PR, the assembler is relaxing a bunch of `LDR` instructions to the wide encoding, `LDR.W`. If I'm reading https://developer.arm.com/documentation/ddi0406/c/Application-Level-Architecture/Instruction-Details/Alphabetical-list-of-instructions/LDR--literal-?lang=en right, the narrow `LDR` encoding supports offsets from 0--1020, so these relaxations are all unnecessary.

   Regardless of the out-of-range fixup error, this seems like an unfortunate binary size regression.

2. Using the wider `LDR.W` instructions has pushed the `CBZ`'s branch target out of range (the fixup has gone from 126 to 132 bytes), causing the error we hit.

   The `CBZ` instruction is formed by `ARMConstantIslands::optimizeThumb2Branches()`. It relies on the `BBInfoVector`'s offset information, which is ultimately based on `ARMBaseInstrInfo::getInstSizeInBytes()`. That doesn't take into account that these `LDR` instructions (2-byte `tLDRpci`) can get relaxed to 4-byte `t2LDRpci` by the assembler (`ARMAsmBackend::relaxInstruction()`).

   Should `ARMBaseInstrInfo::getInstSizeInBytes()` have special cases for instructions that can get relaxed to wider encodings by the assembler? (Maybe it could call `ARMAsmBackend::getRelaxedOpcode()`?)

+ at smithp35 for Arm expertise.

https://github.com/llvm/llvm-project/pull/184544