[GlobalISel] Fix insertion of stack-protector epilogue

[petpav01]

Summary

SelectionDAG implements an optimization for stack-protector which causes the StackProtector pass to only insert prologue instrumentation but generating epilogue instrumentation is deferred to SelectionDAG.

The StackProtector pass does the same for GlobalISel but this selection framework currently does not implement the same optimization as SelectionDAG which means no epilogue instrumentation gets inserted by the compiler. This primarily affects AArch64 at -O0 because this configuration enables GlobalISel by default.

The proposed patch fixes the immediate problem by having the StackProtector pass generate the epilogue instrumentation when GlobalISel is enabled, similarly to what is done when FastISel is active.

Notes:

• The patch changes meaning of TargetOptions::EnableGlobalISel. The flag was previously set only when a target switched on GlobalISel but it is now always set when the GlobalISel pipeline is enabled. This makes the flag consistent with TargetOptions::EnableFastISel and allows its use in StackProtector to determine when GlobalISel is enabled.
  
  The EnableGlobalISel flag had previouly only one use in TargetPassConfig::isGlobalISelAbortEnabled(). The method used its value to determine whether GlobalISel was enabled by a target and returned false in such a case. To preserve the current behaviour, a new flag TargetOptions::GlobalISelAbort is introduced to separately record the abort behaviour.

• StackProtector::InsertStackProtectors() is updated to find a stack guard slot by searching for the llvm.stackprotector intrinsic when the prologue was not created by StackProtector itself but the pass still needs to generate the epilogue instrumentation. This fixes a problem when the pass would abort because the stack guard AllocInst pointer was null when generating the epilogue. This was hit by test/CodeGen/AArch64/GlobalISel/arm64-irtranslator-stackprotect.ll.

[petpav01]

Ping.

[aemerson]

Hi Petr,

This looks ok to me, can you separate this into two separate commits though? One for the option changes and the other for the SP fix.

Thanks,
Amara

test/CodeGen/AArch64/GlobalISel/irtranslator-stackprotect-check.ll

1 ↗

(On Diff #174002)

We don't yet officially support GISel on AArch64 except for -O0, so can you add this to the run line?

[petpav01]

Thanks Amara. Committed with the suggested change and splitting up the patch as rL347861 and rL347862.

[davezarzycki]

Hello,

r347862 of this change broke the ability to run the test suite with the ARM64 backend enabled BUT without the ARM backend enabled. Was this intentional? See below:

Dave

---

FAIL: LLVM :: CodeGen/AArch64/GlobalISel/irtranslator-stackprotect-check.ll (18353 of 44268)

• TEST 'LLVM :: CodeGen/AArch64/GlobalISel/irtranslator-stackprotect-check.ll' FAILED ****

Script:

: 'RUN: at line 1'; /home/dave/s/lc/t/bin/llc -O0 -stop-before=irtranslator -global-isel /home/dave/s/lc/test/CodeGen/AArch64/GlobalISel/irtranslator-stackprotect-check.ll -o - | /home/dave/s/lc/t/bin/FileCheck /home/dave/s/lc/test/CodeGen/AArch64/GlobalISel/irtranslator-stackprotect-check.ll

: 'RUN: at line 2'; /home/dave/s/lc/t/bin/llc -O0 -stop-after=irtranslator -verify-machineinstrs -global-isel /home/dave/s/lc/test/CodeGen/AArch64/GlobalISel/irtranslator-stackprotect-check.ll -o - | /home/dave/s/lc/t/bin/FileCheck --check-prefixes CHECK,CHECK-MIR /home/dave/s/lc/test/CodeGen/AArch64/GlobalISel/irtranslator-stackprotect-check.ll

Exit Code: 2

Command Output (stderr):

/home/dave/s/lc/t/bin/llc: error: : error: unable to get target for 'armv8-arm-none-eabi', see --version and --triple.
FileCheck error: '-' is empty.
FileCheck command line: /home/dave/s/lc/t/bin/FileCheck /home/dave/s/lc/test/CodeGen/AArch64/GlobalISel/irtranslator-stackprotect-check.ll

[petpav01]

Apologies for the breakage. This was supposed to be an AArch64 test but the triple is incorrect. Should be fixed in rL348111.

llvm/trunk/include/llvm/Target/TargetMachine.h

public:
 
   /// Overrides the optimization level.
   void setOptLevel(CodeGenOpt::Level Level);
 
   void setFastISel(bool Enable) { Options.EnableFastISel = Enable; }
   bool getO0WantsFastISel() { return O0WantsFastISel; }
   void setO0WantsFastISel(bool Enable) { O0WantsFastISel = Enable; }
   void setGlobalISel(bool Enable) { Options.EnableGlobalISel = Enable; }
+  void setGlobalISelAbort(GlobalISelAbortMode Mode) {
+    Options.GlobalISelAbort = Mode;
+  }
   void setMachineOutliner(bool Enable) {
     Options.EnableMachineOutliner = Enable;
   }
   void setSupportsDefaultOutlining(bool Enable) {
     Options.SupportsDefaultOutlining = Enable;
   }
 
   bool shouldPrintMachineCode() const { return Options.PrintMachineCode; }

llvm/trunk/include/llvm/Target/TargetOptions.h

namespace llvm {
   /// that option should take precedence over the tuning.
   enum class DebuggerKind {
     Default,  // No specific tuning requested.
     GDB,      // Tune debug info for gdb.
     LLDB,     // Tune debug info for lldb.
     SCE       // Tune debug info for SCE targets (e.g. PS4).
   };
 
+  /// Enable abort calls when global instruction selection fails to lower/select
+  /// an instruction.
+  enum class GlobalISelAbortMode {
+    Disable,        // Disable the abort.
+    Enable,         // Enable the abort.
+    DisableWithDiag // Disable the abort but emit a diagnostic on failure.
+  };
+
   class TargetOptions {
   public:
     TargetOptions()
         : PrintMachineCode(false), UnsafeFPMath(false), NoInfsFPMath(false),
           NoNaNsFPMath(false), NoTrappingFPMath(false),
           NoSignedZerosFPMath(false),
           HonorSignDependentRoundingFPMathOption(false), NoZerosInBSS(false),
           GuaranteedTailCallOpt(false), StackSymbolOrdering(true),
public:
     /// EnableFastISel - This flag enables fast-path instruction selection
     /// which trades away generated code quality in favor of reducing
     /// compile time.
     unsigned EnableFastISel : 1;
 
     /// EnableGlobalISel - This flag enables global instruction selection.
     unsigned EnableGlobalISel : 1;
 
+    /// EnableGlobalISelAbort - Control abort behaviour when global instruction
+    /// selection fails to lower/select an instruction.
+    GlobalISelAbortMode GlobalISelAbort = GlobalISelAbortMode::Enable;
+
     /// UseInitArray - Use .init_array instead of .ctors for static
     /// constructors.
     unsigned UseInitArray : 1;
 
     /// Disable the integrated assembler.
     unsigned DisableIntegratedAS : 1;
 
     /// Compress DWARF debug sections.

llvm/trunk/lib/CodeGen/TargetPassConfig.cpp

 static cl::opt<cl::boolOrDefault> EnableGlobalISelOption(
     "global-isel", cl::Hidden,
     cl::desc("Enable the \"global\" instruction selector"));
 
 static cl::opt<std::string> PrintMachineInstrs(
     "print-machineinstrs", cl::ValueOptional, cl::desc("Print machine instrs"),
     cl::value_desc("pass-name"), cl::init("option-unspecified"), cl::Hidden);
 
-static cl::opt<int> EnableGlobalISelAbort(
+static cl::opt<GlobalISelAbortMode> EnableGlobalISelAbort(
     "global-isel-abort", cl::Hidden,
     cl::desc("Enable abort calls when \"global\" instruction selection "
-             "fails to lower/select an instruction: 0 disable the abort, "
-             "1 enable the abort, and "
-             "2 disable the abort but emit a diagnostic on failure"),
-    cl::init(1));
+             "fails to lower/select an instruction"),
+    cl::values(
+        clEnumValN(GlobalISelAbortMode::Disable, "0", "Disable the abort"),
+        clEnumValN(GlobalISelAbortMode::Enable, "1", "Enable the abort"),
+        clEnumValN(GlobalISelAbortMode::DisableWithDiag, "2",
+                   "Disable the abort but emit a diagnostic on failure")));
 
 // Temporary option to allow experimenting with MachineScheduler as a post-RA
 // scheduler. Targets can "properly" enable this with
 // substitutePass(&PostRASchedulerID, &PostMachineSchedulerID).
 // Targets can return true in targetSchedulesPostRAScheduling() and
 // insert a PostRA scheduling pass wherever it wants.
 cl::opt<bool> MISchedPostRA("misched-postra", cl::Hidden,
   cl::desc("Run MachineScheduler post regalloc (independent of preRA sched)"));
TargetPassConfig::TargetPassConfig(LLVMTargetMachine &TM, PassManagerBase &pm)
   else {
     // If not explicitly specified, use target default.
     TM.Options.EnableIPRA = TM.useIPRA();
   }
 
   if (TM.Options.EnableIPRA)
     setRequiresCodeGenSCCOrder();
 
+  if (EnableGlobalISelAbort.getNumOccurrences())
+    TM.Options.GlobalISelAbort = EnableGlobalISelAbort;
+
   setStartStopPasses();
 }
 
 CodeGenOpt::Level TargetPassConfig::getOptLevel() const {
   return TM->getOptLevel();
 }
 
 /// Insert InsertedPassID pass after TargetPassID.
void TargetPassConfig::addISelPrepare() {
   if (!DisableVerify)
     addPass(createVerifierPass());
 }
 
 bool TargetPassConfig::addCoreISelPasses() {
   // Enable FastISel with -fast-isel, but allow that to be overridden.
   TM->setO0WantsFastISel(EnableFastISelOption != cl::BOU_FALSE);
   if (EnableFastISelOption == cl::BOU_TRUE ||
-      (TM->getOptLevel() == CodeGenOpt::None && TM->getO0WantsFastISel()))
+      (TM->getOptLevel() == CodeGenOpt::None && TM->getO0WantsFastISel() &&
+       !TM->Options.EnableGlobalISel)) {
     TM->setFastISel(true);
+    TM->setGlobalISel(false);
+  }
 
   // Ask the target for an instruction selector.
   // Explicitly enabling fast-isel should override implicitly enabled
   // global-isel.
   if (EnableGlobalISelOption == cl::BOU_TRUE ||
       (EnableGlobalISelOption == cl::BOU_UNSET &&
        TM->Options.EnableGlobalISel && EnableFastISelOption != cl::BOU_TRUE)) {
+    TM->setGlobalISel(true);
     TM->setFastISel(false);
 
     SaveAndRestore<bool> SavedAddingMachinePasses(AddingMachinePasses, true);
     if (addIRTranslator())
       return true;
 
     addPreLegalizeMachineIR();
 
if (EnableBlockPlacementStats)
       addPass(&MachineBlockPlacementStatsID);
   }
 }
 
 //===---------------------------------------------------------------------===//
 /// GlobalISel Configuration
 //===---------------------------------------------------------------------===//
 bool TargetPassConfig::isGlobalISelAbortEnabled() const {
-  if (EnableGlobalISelAbort.getNumOccurrences() > 0)
-    return EnableGlobalISelAbort == 1;
-
-  // When no abort behaviour is specified, we don't abort if the target says
-  // that GISel is enabled.
-  return !TM->Options.EnableGlobalISel;
+  return TM->Options.GlobalISelAbort == GlobalISelAbortMode::Enable;
 }
 
 bool TargetPassConfig::reportDiagnosticWhenGlobalISelFallback() const {
-  return EnableGlobalISelAbort == 2;
+  return TM->Options.GlobalISelAbort == GlobalISelAbortMode::DisableWithDiag;
 }

llvm/trunk/lib/Target/AArch64/AArch64TargetMachine.cpp

if (getMCAsmInfo()->usesWindowsCFI()) {
     // is a call.
     //
     // FIXME: We could elide the trap if the next instruction would be in
     // the same region anyway.
     this->Options.TrapUnreachable = true;
   }
 
   // Enable GlobalISel at or below EnableGlobalISelAt0.
-  if (getOptLevel() <= EnableGlobalISelAtO)
+  if (getOptLevel() <= EnableGlobalISelAtO) {
     setGlobalISel(true);
+    setGlobalISelAbort(GlobalISelAbortMode::Disable);
+  }
 
   // AArch64 supports the MachineOutliner.
   setMachineOutliner(true);
 
   // AArch64 supports default outlining behaviour.
   setSupportsDefaultOutlining(true);
 }