[PATCH] D81405: [AArch64] Avoid incompatibility between SLSBLR mitigation and BTI codegen.

[Oliver Stannard (Linaro) via Phabricator via llvm-commits]

ostannard added a comment.

I have the same concern as in D81402 <https://reviews.llvm.org/D81402>: will this still work if the call to the thunk goes through a long-branch veneer or PLT, which could clobber x16 and x17?



================
Comment at: llvm/lib/Target/AArch64/AArch64FastISel.cpp:3274
+    const MCInstrDesc &II =
+        TII.get(Addr.getReg() ? AArch64::BLRCall : AArch64::BL);
     MIB = BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc, II);
----------------
Do we need to conditionally emit `BLRCallX16X17` here too?


================
Comment at: llvm/lib/Target/AArch64/AArch64FastISel.cpp:3307
 
-    const MCInstrDesc &II = TII.get(AArch64::BLR);
+    const MCInstrDesc &II = TII.get(AArch64::BLRCall);
     CallReg = constrainOperandRegClass(II, CallReg, 0);
----------------
Again, might we need to emit `BLRCallX16X17` here?


================
Comment at: llvm/lib/Target/AArch64/AArch64FrameLowering.cpp:1129
 
-      BuildMI(MBB, MBBI, DL, TII->get(AArch64::BLR))
+      BuildMI(MBB, MBBI, DL, TII->get(AArch64::BLRCallX16X17))
           .addReg(AArch64::X16, RegState::Kill)
----------------
Why is this hard-coded to BLRCallX16X17, never BLRCall?


================
Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.cpp:6562
     } else {
-      assert(Call->getOpcode() == AArch64::BLR);
+      assert(Call->getOpcode() == AArch64::BLRCall);
       TailOpcode = AArch64::TCRETURNriALL;
----------------
Would it be possible for a `BLRCallX16X17` to reach here?


================
Comment at: llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp:779
   if (!IsTailCall)
-    return IsIndirect ? AArch64::BLR : AArch64::BL;
+    return IsIndirect ? AArch64::BLRCall : AArch64::BL;
 
----------------
Same as fast-sel, do we need to emit `BLRCallX16X17` here?


================
Comment at: llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp:2874
   // silly).
-  MIB.buildInstr(AArch64::BLR, {}, {Load})
+  MIB.buildInstr(AArch64::BLRCall, {}, {Load})
       .addDef(AArch64::X0, RegState::Implicit)
----------------
Same as fast-sel, do we need to emit `BLRCallX16X17` here?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81405/new/

https://reviews.llvm.org/D81405