[IndirectThunks] Make generated MF structure as expected by all instruction selectors.

[kristof.beyls]

Summary

This also enables running the AArch64 SLSHardening pass with GlobalISel,
so add a test for that.

[arsenm]

llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp

2496

Why was this violated? Seems like it should be straightforward to fix? Can you add a pure IR test that just runs the IRTranslator?

[kristof.beyls]

llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp

2496

The violation comes from thunks produced by the ThunkInserter. They are naked functions, not sure if that could trigger this issue.
I'll look into if it's possible to have a pure IR test trigger this.

[kristof.beyls]

This new revision fixes the root cause of GlobalISel asserting on an IndirectThunk: the IndirectThunk creation code was producing a MachineFunction with a slightly different structure than what gets produced from a naked function implemented in C code and processed by clang.
This fixes this by letting the IndirectThunk generator produce a MachineFunction structure that is the same as what gets produced by clang for an empty naked function written in C.

[zbrid]

LGTM

Could you explain how not adding the MBB entry block is different from removing extra MBB later? Curious on the details about the differences here since the change makes sense functionally and I don't see any issues, but I can't understand why adding the extra MBB entry block at the point we create the machine function and then deleting extra blocks results in something different than the automatically created entry block. Is it some attributes or configuration in the basic block or something?

Also slightly unrelated question: Thanks for moving the IndirectThunk code to be of use to more Targets earlier in the commit stack! Along those lines, I was wondering if you had additional thoughts recently on adding retpolines for AArch64 since you are working near the x86 retpoline code and there was an llvm-dev thread from a team member of mine a week or two ago?

llvm/include/llvm/CodeGen/IndirectThunks.h

67 ↗

(On Diff #270195)

nit: update this comment to only mention MachineFunctions since MBBs don't need to be created.

[zbrid]

This comment was removed by zbrid.

[kristof.beyls]

In D81403#2097148, @zbrid wrote:

LGTM

Could you explain how not adding the MBB entry block is different from removing extra MBB later? Curious on the details about the differences here since the change makes sense functionally and I don't see any issues, but I can't understand why adding the extra MBB entry block at the point we create the machine function and then deleting extra blocks results in something different than the automatically created entry block. Is it some attributes or configuration in the basic block or something?

This was triggering the following assert in GlobalISel:
https://github.com/llvm/llvm-project/blob/master/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp#L2495

The X86 target does not use globalisel by default, but the AArch64 target does use it by default at -O0. So now that IndirectThunks is used by AArch64 also, that has become a blocker.
Globalisel runs between the place where the MBB entry block was added and the block was removed later.
Apart from this assert blocking the compilation flow, I wouldn't expect code generation to be different.

[zbrid]

Thanks for the explanation!

[kristof.beyls]

llvm/include/llvm/CodeGen/IndirectThunks.h

67 ↗

(On Diff #270195)

Thanks, I fixed that in a follow-up commit.

llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp

for (const MachineBasicBlock::RegisterMaskPair &LiveIn : EntryBB->liveins())
     NewEntryBB.addLiveIn(LiveIn);
   NewEntryBB.sortUniqueLiveIns();
 
   // Get rid of the now empty basic block.
   EntryBB->removeSuccessor(&NewEntryBB);
   MF->remove(EntryBB);
   MF->DeleteMachineBasicBlock(EntryBB);
 
-  assert(&MF->front() == &NewEntryBB &&
-         "New entry wasn't next in the list of basic block!");
+  if (&MF->front() != &NewEntryBB) {
+    OptimizationRemarkMissed R("gisel-irtranslator", "GISelFailure",
+                               F.getSubprogram(), &F.getEntryBlock());
+    R << "unable to lower CFG structure.";
+    reportTranslationError(*MF, *TPC, *ORE, R);
+    return false;
+  }
 
   // Initialize stack protector information.
   StackProtector &SP = getAnalysis<StackProtector>();
   SP.copyToMachineFrameInfo(MF->getFrameInfo());
 
   return false;
 }

llvm/test/CodeGen/AArch64/speculation-hardening-sls.ll

 ; RUN: llc -mattr=harden-sls-retbr,harden-sls-blr -verify-machineinstrs -mtriple=aarch64-none-linux-gnu < %s | FileCheck %s --check-prefixes=CHECK,ISBDSB --dump-input-on-failure
 ; RUN: llc -mattr=harden-sls-retbr,harden-sls-blr -mattr=+sb -verify-machineinstrs -mtriple=aarch64-none-linux-gnu < %s | FileCheck %s --check-prefixes=CHECK,SB --dump-input-on-failure
+; RUN: llc -global-isel -global-isel-abort=0 -mattr=harden-sls-retbr,harden-sls-blr -verify-machineinstrs -mtriple=aarch64-none-linux-gnu < %s | FileCheck %s --check-prefixes=CHECK,ISBDSB --dump-input-on-failure
+; RUN: llc -global-isel -global-isel-abort=0 -mattr=harden-sls-retbr,harden-sls-blr -mattr=+sb -verify-machineinstrs -mtriple=aarch64-none-linux-gnu < %s | FileCheck %s --check-prefixes=CHECK,SB --dump-input-on-failure
 
 ; Function Attrs: norecurse nounwind readnone
 define dso_local i32 @double_return(i32 %a, i32 %b) {
 entry:
   %cmp = icmp sgt i32 %a, 0
   br i1 %cmp, label %if.then, label %if.else
 
 if.then:                                          ; preds = %entry