Correct the upper bound for a CBZ/CBNZ branch target.

[prakhar]

Summary

Fix for the upper bound check that was causing a build failure.

[olista01]

lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp

581–584

The 4-byte offset is being applied a few lines down, so should this actually be checking for the range [4, 130]?

[prakhar]

lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp

581–584

The valid range is actually [2, 130] as offsets of 2 are relaxed to NOP. But yes I forgot to take the offset correction below into account.

[prakhar]

Use the offset bounds before 4-byte correction

[olista01]

test/MC/ARM/thumb-cb-negative-offsets.s

23

The label is in-range of this instruction.

26

This should probably be a nop, since we are branching to it. I'd suggest making this two nops, with a label for each, so that you can test the exact bound on the offset for both instructions.

[prakhar]

test/MC/ARM/thumb-cb-negative-offsets.s

23

Yes, that is deliberate to test that this will not cause an error unlike the preceding instruction.

26

Okay.

[prakhar]

Clarify the upper bound tests

[olista01]

test/MC/ARM/thumb-cb-negative-offsets.s

27

This test won't fail if an error is emitted for this instruction. You'll need a CHECK-NOT line to ensure that no further diagnostics are emitted.

[prakhar]

CHECK-NOT added

[olista01]

LGTM

lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp

if (Ctx && !STI->getFeatureBits()[ARM::FeatureThumb2] && IsResolved) {
       if (FixupDiagnostic) {
         Ctx->reportError(Fixup.getLoc(), FixupDiagnostic);
         return 0;
       }
     }
     // Offset by 4, and don't encode the low two bits.
     return ((Value - 4) >> 2) & 0xff;
   case ARM::fixup_arm_thumb_cb: {
-    // CB instructions can only branch to offsets in [0, 126] in multiples of 2
-    if (Ctx && ((int64_t)Value < 0 || Value > 0x3e || Value & 1)) {
+    // CB instructions can only branch to offsets in [4, 126] in multiples of 2
+    // 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 (Ctx && ((int64_t)Value < 2 || Value > 0x82 || Value & 1)) {
       Ctx->reportError(Fixup.getLoc(), "out of range pc-relative fixup value");
       return 0;
     }
     // Offset by 4 and don't encode the lower bit, which is always 0.
     // FIXME: diagnose if no Thumb2
     uint32_t Binary = (Value - 4) >> 1;
     return ((Binary & 0x20) << 4) | ((Binary & 0x1f) << 3);
   }

test/MC/ARM/thumb-cb-negative-offsets.s

 
 @ CHECK: out of range pc-relative fixup value
   cbz r0, label1
 @ CHECK: out of range pc-relative fixup value
   cbnz r0, label1
 
   .space 1000
 label1:
-  .word 4
+  nop
+
+@ CHECK: out of range pc-relative fixup value
+  cbz r0, label2
+  .space 130
+label2:
+  nop
+
+@ Should be within range
+  cbnz r0, label3
+  .space 128
+label3:
+  nop