[CodeGen][MachineLastInstrsCleanup] fix INLINEASM_BR hazard

[nickdesaulniers]

Summary

If the removable definition resides in an INLINEASM_BR target, the
reuseable candidate might not dominate the INLINEASM_BR.

bb0:
   INLINEASM_BR &"" %bb.1
   renamable $x8 = MOVi64imm 29273397577910035
   B %bb.2
   ...
 bb1:
   renamable $x8 = MOVi64imm 29273397577910035
   renamable $x8 = ADDXri killed renamable $x8, 2048, 0
 bb2:

Removing the second mov is a hazard when the inline asm branches to bb1.

Skip such replacements when the to be removed instruction is in the
target of such an INLINEASM_BR instruction.

We could get more aggressive about this in the future, but for now
simply abort.

This is causing a boot failure on linux-4.19.y branches of the LTS Linux
kernel for ARCH=arm64 with CONFIG_RANDOMIZE_BASE=y (KASLR) and
CONFIG_UNMAP_KERNEL_AT_EL0=y (KPTI).

Link: https://reviews.llvm.org/D123394
Link: https://github.com/ClangBuiltLinux/linux/issues/1837

Thanks to @nathanchance for the report, and @ardb for debugging.

[nickdesaulniers]

• new change

[nickdesaulniers]

(This will need to be backported to release/16.x since release/16.x contains 5ecd363)

[efriedma]

If I'm following correctly, the issue is specifically with definitions after the INLINEASM_BR (which only execute in the fallthrough case)? In that case, you could theoretically still do some sort of analysis... but it gets a lot more complicated, so this fix seems sufficient. LGTM

[nickdesaulniers]

In D149191#4300468, @efriedma wrote:

If I'm following correctly, the issue is specifically with definitions after the INLINEASM_BR (which only execute in the fallthrough case)? In that case, you could theoretically still do some sort of analysis... but it gets a lot more complicated, so this fix seems sufficient. LGTM

Yeah, this should be something quick that is low risk for a backport. I started hacking up the more precise fix, but it turns out you need to find the corresponding INLINEASM_BR in the pred MBB (could be multiple) and check which dominates. That doesn't seem as low risk as this for backporting.

Thanks for the review!

llvm/lib/CodeGen/MachineLateInstrsCleanup.cpp

static bool isCandidate(const MachineInstr *MI, Register &DefedReg,
   return DefedReg.isValid();
 }
 
 bool MachineLateInstrsCleanup::processBlock(MachineBasicBlock *MBB) {
   bool Changed = false;
   Reg2DefMap &MBBDefs = RegDefs[MBB->getNumber()];
 
   // Find reusable definitions in the predecessor(s).
-  if (!MBB->pred_empty() && !MBB->isEHPad()) {
+  if (!MBB->pred_empty() && !MBB->isEHPad() &&
+      !MBB->isInlineAsmBrIndirectTarget()) {
     MachineBasicBlock *FirstPred = *MBB->pred_begin();
     for (auto [Reg, DefMI] : RegDefs[FirstPred->getNumber()])
       if (llvm::all_of(
               drop_begin(MBB->predecessors()),
               [&, &Reg = Reg, &DefMI = DefMI](const MachineBasicBlock *Pred) {
                 auto PredDefI = RegDefs[Pred->getNumber()].find(Reg);
                 return PredDefI != RegDefs[Pred->getNumber()].end() &&
                        DefMI->isIdenticalTo(*PredDefI->second);

llvm/test/CodeGen/AArch64/machine-latecleanup-inlineasm.mir

body:             |
   ; CHECK-NEXT:   renamable $x23 = COPY $x1
   ; CHECK-NEXT:   renamable $x24 = COPY $x0
   ; CHECK-NEXT:   INLINEASM_BR &"b $0", 1 /* sideeffect attdialect */, 13 /* imm */, %bb.1
   ; CHECK-NEXT:   renamable $x8 = MOVi64imm 29273397577910035
   ; CHECK-NEXT:   B %bb.2
   ; CHECK-NEXT: {{  $}}
   ; CHECK-NEXT: bb.1.bb8 (machine-block-address-taken, inlineasm-br-indirect-target):
   ; CHECK-NEXT:   successors: %bb.2(0x80000000)
-  ; CHECK-NEXT:   liveins: $w20, $x19, $x21, $x22, $x23, $x24, $x8
+  ; CHECK-NEXT:   liveins: $w20, $x19, $x21, $x22, $x23, $x24
   ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   renamable $x8 = MOVi64imm 29273397577910035
   ; CHECK-NEXT:   renamable $x8 = ADDXri killed renamable $x8, 2048, 0
   ; CHECK-NEXT: {{  $}}
   ; CHECK-NEXT: bb.2.bb9:
   ; CHECK-NEXT:   successors: %bb.3(0x80000000)
   ; CHECK-NEXT:   liveins: $w20, $x8, $x19, $x21, $x22, $x23, $x24
   ; CHECK-NEXT: {{  $}}
   ; CHECK-NEXT:   renamable $x25 = COPY $xzr
   ; CHECK-NEXT:   renamable $w27 = MOVi32imm 1, implicit-def $x27