Have 'optnone' respect -fast-isel=false

[probinson]

Summary

The 'optnone' attribute was forcing FastISel on. For debugging 'optnone' issues, it would be useful to be able to disable this from the command line, so now the -fast-isel option will factor into the decision to enable FastISel for 'optnone' functions.

[probinson]

I'm not entirely sure it's kosher to describe backend command-line options in the IR LangRef, but Eric had asked for this feature to be documented and I'm not sure how else to do it. Advice welcome.

[echristo]

Some inline comments which should change quite a bit of the patch.

Thanks!

-eric

docs/LangRef.rst

1294 ↗

(On Diff #40573)

You can remove everything after the semicolon and replace it with a period.

include/llvm/Target/TargetMachine.h

105 ↗

(On Diff #40573)

No need for this.

[probinson]

include/llvm/Target/TargetMachine.h

105 ↗

(On Diff #40573)

This is where LLVMTargetMachine records the state of the -fast-isel command-line option, which SelectionDAGISel needs. SelectionDAGISel does not have direct access to the EnableFastISelOption flag.

Unless you're now saying 'optnone' must always use FastISel? That wasn't the conclusion in the llvm-dev thread.

[echristo]

Just make it an enum ala FloatABI with a Default, SelectionDAG, FastISel
and use that everywhere?

-eric

[probinson]

In D14792#293572, @echristo wrote:

Just make it an enum ala FloatABI with a Default, SelectionDAG, FastISel
and use that everywhere?

-eric

"Everywhere" is exactly two places, hardly seems worth the trouble. Maybe rename the flag to "O0UsesFastISel" instead? which then can be used directly in both LLVMTargetMachine and OptLevelChanger with obviously consistent semantics.

[dsanders]

In D14792#293634, @probinson wrote:

In D14792#293572, @echristo wrote:

Just make it an enum ala FloatABI with a Default, SelectionDAG, FastISel
and use that everywhere?

-eric

"Everywhere" is exactly two places, hardly seems worth the trouble. Maybe rename the flag to "O0UsesFastISel" instead? which then can be used directly in both LLVMTargetMachine and OptLevelChanger with obviously consistent semantics.

Given the current choice of SelectionDAG vs FastISel, I agree with Paul that this isn't worth the trouble. I think Eric is accounting for the addition of Global ISel in the near future which would presumably add a third choice to the enum he's requesting. If that is the reason, then I'd suggest leaving that for Global ISel since it's going to have to replace the -fast-isel option with an enum based option anyway.

I also agree with the renaming. FWIW, I was going to suggest 'CanUseFastISel' but I think 'O0UsesFastISel' or maybe 'O0WantsFastISel' is better.

[probinson]

include/llvm/Target/TargetMachine.h

105 ↗

(On Diff #40573)

Renamed to 'O0WantsFastISel' as suggested by dsanders.

[probinson]

Rename flag as requested.

[dsanders]

This LGTM.

llvm/trunk/docs/LangRef.rst

 ``nounwind``
     This function attribute indicates that the function never raises an
     exception. If the function does raise an exception, its runtime
     behavior is undefined. However, functions marked nounwind may still
     trap or generate asynchronous exceptions. Exception handling schemes
     that are recognized by LLVM to handle asynchronous exceptions, such
     as SEH, will still provide their implementation defined semantics.
 ``optnone``
-    This function attribute indicates that the function is not optimized
-    by any optimization or code generator passes with the
-    exception of interprocedural optimization passes.
+    This function attribute indicates that most optimization passes will skip
+    this function, with the exception of interprocedural optimization passes.
+    Code generation defaults to the "fast" instruction selector.
     This attribute cannot be used together with the ``alwaysinline``
     attribute; this attribute is also incompatible
     with the ``minsize`` attribute and the ``optsize`` attribute.
 
     This attribute requires the ``noinline`` attribute to be specified on
     the function as well, so the function is never inlined into any caller.
     Only functions with the ``alwaysinline`` attribute are valid
     candidates for inlining into the body of this function.

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

protected: // Can only create subclasses.
   /// Contains target specific asm information.
   const MCAsmInfo *AsmInfo;
 
   const MCRegisterInfo *MRI;
   const MCInstrInfo *MII;
   const MCSubtargetInfo *STI;
 
   unsigned RequireStructuredCFG : 1;
+  unsigned O0WantsFastISel : 1;
 
   /// This API is here to support the C API, deprecated in 3.7 release.
   /// This should never be used outside of legacy existing client.
   const DataLayout &getDataLayout() const { return DL; }
   friend struct C_API_PRIVATE_ACCESS;
 
 public:
   mutable TargetOptions Options;
public:
 
   /// Returns the optimization level: None, Less, Default, or Aggressive.
   CodeGenOpt::Level getOptLevel() const;
 
   /// \brief Overrides the optimization level.
   void setOptLevel(CodeGenOpt::Level Level) const;
 
   void setFastISel(bool Enable) { Options.EnableFastISel = Enable; }
+  bool getO0WantsFastISel() { return O0WantsFastISel; }
+  void setO0WantsFastISel(bool Enable) { O0WantsFastISel = Enable; }
 
   bool shouldPrintMachineCode() const { return Options.PrintMachineCode; }
 
   /// Returns the default value of asm verbosity.
   ///
   bool getAsmVerbosityDefault() const {
     return Options.MCOptions.AsmVerbose;
   }

llvm/trunk/lib/CodeGen/LLVMTargetMachine.cpp

addPassesToGenerateCode(LLVMTargetMachine *TM, PassManagerBase &PM,
   MachineModuleInfo *MMI = new MachineModuleInfo(
       *TM->getMCAsmInfo(), *TM->getMCRegisterInfo(), TM->getObjFileLowering());
   PM.add(MMI);
 
   // Set up a MachineFunction for the rest of CodeGen to work on.
   PM.add(new MachineFunctionAnalysis(*TM, MFInitializer));
 
   // Enable FastISel with -fast, but allow that to be overridden.
+  TM->setO0WantsFastISel(EnableFastISelOption != cl::BOU_FALSE);
   if (EnableFastISelOption == cl::BOU_TRUE ||
       (TM->getOptLevel() == CodeGenOpt::None &&
-       EnableFastISelOption != cl::BOU_FALSE))
+       TM->getO0WantsFastISel()))
     TM->setFastISel(true);
 
   // Ask the target for an isel.
   if (PassConfig->addInstSelector())
     return nullptr;
 
   PassConfig->addMachinePasses();
 

llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp

namespace llvm {
   public:
     OptLevelChanger(SelectionDAGISel &ISel,
                     CodeGenOpt::Level NewOptLevel) : IS(ISel) {
       SavedOptLevel = IS.OptLevel;
       if (NewOptLevel == SavedOptLevel)
         return;
       IS.OptLevel = NewOptLevel;
       IS.TM.setOptLevel(NewOptLevel);
-      SavedFastISel = IS.TM.Options.EnableFastISel;
-      if (NewOptLevel == CodeGenOpt::None)
-        IS.TM.setFastISel(true);
       DEBUG(dbgs() << "\nChanging optimization level for Function "
             << IS.MF->getFunction()->getName() << "\n");
       DEBUG(dbgs() << "\tBefore: -O" << SavedOptLevel
             << " ; After: -O" << NewOptLevel << "\n");
+      SavedFastISel = IS.TM.Options.EnableFastISel;
+      if (NewOptLevel == CodeGenOpt::None) {
+        IS.TM.setFastISel(IS.TM.getO0WantsFastISel());
+        DEBUG(dbgs() << "\tFastISel is "
+              << (IS.TM.Options.EnableFastISel ? "enabled" : "disabled")
+              << "\n");
+      }
     }
 
     ~OptLevelChanger() {
       if (IS.OptLevel == SavedOptLevel)
         return;
       DEBUG(dbgs() << "\nRestoring optimization level for Function "
             << IS.MF->getFunction()->getName() << "\n");
       DEBUG(dbgs() << "\tBefore: -O" << IS.OptLevel

llvm/trunk/test/CodeGen/Mips/emergency-spill-slot-near-fp.ll

 ; Check that register scavenging spill slot is close to $fp.
-; RUN: llc -march=mipsel -O0 -fast-isel=false < %s | FileCheck %s
+; RUN: llc -march=mipsel -O0 < %s | FileCheck %s
 
 ; CHECK: sw ${{.*}}, 8($sp)
 ; CHECK: lw ${{.*}}, 8($sp)
 
 define i32 @main(i32 signext %argc, i8** %argv) #0 {
 entry:
   %retval = alloca i32, align 4
   %argc.addr = alloca i32, align 4
entry:
   store <16 x i8> %1, <16 x i8>* %v1, align 16
   %2 = load <16 x i8>, <16 x i8>* %v0, align 16
   %3 = load <16 x i8>, <16 x i8>* %v1, align 16
   %mul = mul <16 x i8> %2, %3
   store <16 x i8> %mul, <16 x i8>* %result, align 16
   ret i32 0
 }
 
-attributes #0 = { noinline optnone "no-frame-pointer-elim"="true" }
+attributes #0 = { noinline "no-frame-pointer-elim"="true" }

llvm/trunk/test/Feature/optnone-llc.ll

 ; RUN: llc -O0 -debug %s -o /dev/null 2>&1 | FileCheck %s --check-prefix=LLC-O0
 ; RUN: llc -O1 -debug %s -o /dev/null 2>&1 | FileCheck %s --check-prefix=LLC-Ox
 ; RUN: llc -O2 -debug %s -o /dev/null 2>&1 | FileCheck %s --check-prefix=LLC-Ox
 ; RUN: llc -O3 -debug %s -o /dev/null 2>&1 | FileCheck %s --check-prefix=LLC-Ox
 ; RUN: llc -misched-postra -debug %s -o /dev/null 2>&1 | FileCheck %s --check-prefix=LLC-MORE
+; RUN: llc -O1 -debug-only=isel %s -o /dev/null 2>&1 | FileCheck %s --check-prefix=FAST
+; RUN: llc -O1 -debug-only=isel -fast-isel=false %s -o /dev/null 2>&1 | FileCheck %s --check-prefix=NOFAST
 
 ; REQUIRES: asserts, default_triple
 
 ; This test verifies that we don't run Machine Function optimizations
-; on optnone functions.
+; on optnone functions, and that we can turn off FastISel.
 
 ; Function Attrs: noinline optnone
 define i32 @_Z3fooi(i32 %x) #0 {
 entry:
   %x.addr = alloca i32, align 4
   store i32 %x, i32* %x.addr, align 4
   br label %while.cond
 

 ; LLC-Ox-DAG: Skipping pass 'Optimize machine instruction PHIs'
 ; LLC-Ox-DAG: Skipping pass 'Peephole Optimizations'
 ; LLC-Ox-DAG: Skipping pass 'Post{{.*}}RA{{.*}}{{[Ss]}}cheduler'
 ; LLC-Ox-DAG: Skipping pass 'Remove dead machine instructions'
 ; LLC-Ox-DAG: Skipping pass 'Tail Duplication'
 
 ; Alternate post-RA scheduler.
 ; LLC-MORE: Skipping pass 'PostRA Machine Instruction Scheduler'
+
+; Selectively disable FastISel for optnone functions.
+; FAST:   FastISel is enabled
+; NOFAST: FastISel is disabled