Stop resetting NoFramePointerElim in TargetMachine::resetTargetOptions.

This is part of the work to remove TargetMachine::resetTargetOptions.

In this patch, instead of updating global variable NoFramePointerElim in
resetTargetOptions, its use in DisableFramePointerElim is replaced with a call
to TargetFrameLowering::noFramePointerElim. This function determines on a
per-function basis if frame pointer elimination should be disabled.

There is no change in functionality except that cl:opt option "disable-fp-elim"
can now override function attribute "no-frame-pointer-elim". 


git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@238080 91177308-0d34-0410-b5e6-96231b3b80d8

15 files changed, 112 insertions, 38 deletions

diff --git a/include/llvm/CodeGen/CommandFlags.h b/include/llvm/CodeGen/CommandFlags.h
index d38129be1f4..a58c2de0841 100644
--- a/include/llvm/CodeGen/CommandFlags.h
+++ b/include/llvm/CodeGen/CommandFlags.h
@@ -230,6 +230,7 @@ static inline TargetOptions InitTargetOptionsFromCodeGenFlags() {
   TargetOptions Options;
   Options.LessPreciseFPMADOption = EnableFPMAD;
   Options.NoFramePointerElim = DisableFPElim;
+  Options.NoFramePointerElimOverride = DisableFPElim.getNumOccurrences() > 0;
   Options.AllowFPOpFusion = FuseFPOps;
   Options.UnsafeFPMath = EnableUnsafeFPMath;
   Options.NoInfsFPMath = EnableNoInfsFPMath;
diff --git a/include/llvm/Target/TargetFrameLowering.h b/include/llvm/Target/TargetFrameLowering.h
index 1250020db23..25d43c82cd7 100644
--- a/include/llvm/Target/TargetFrameLowering.h
+++ b/include/llvm/Target/TargetFrameLowering.h
@@ -173,6 +173,9 @@ public:
     return false;
   }
 
+  /// Return true if the target needs to disable frame pointer elimination.
+  virtual bool noFramePointerElim(const MachineFunction &MF) const;
+
   /// hasFP - Return true if the specified function should have a dedicated
   /// frame pointer register. For most targets this is true only if the function
   /// has variable sized allocas or if frame pointer elimination is disabled.
diff --git a/include/llvm/Target/TargetOptions.h b/include/llvm/Target/TargetOptions.h
index 8180f5a0c3b..435a0fd6286 100644
--- a/include/llvm/Target/TargetOptions.h
+++ b/include/llvm/Target/TargetOptions.h
@@ -20,6 +20,7 @@
 
 namespace llvm {
   class MachineFunction;
+  class Module;
   class StringRef;
 
   namespace FloatABI {
@@ -60,6 +61,7 @@ namespace llvm {
   public:
     TargetOptions()
         : PrintMachineCode(false), NoFramePointerElim(false),
+          NoFramePointerElimOverride(false),
           LessPreciseFPMADOption(false), UnsafeFPMath(false),
           NoInfsFPMath(false), NoNaNsFPMath(false),
           HonorSignDependentRoundingFPMathOption(false),
@@ -84,6 +86,9 @@ namespace llvm {
     /// elimination optimization, this option should disable it.
     unsigned NoFramePointerElim : 1;
 
+    /// This flag is true when "disable-fp-elim" appeared on the command line.
+    unsigned NoFramePointerElimOverride : 1;
+
     /// DisableFramePointerElim - This returns true if frame pointer elimination
     /// optimization should be disabled for the given machine function.
     bool DisableFramePointerElim(const MachineFunction &MF) const;
@@ -222,9 +227,14 @@ namespace llvm {
     MCTargetOptions MCOptions;
   };
 
-/// \brief Set function attributes of functions in Module M based on CPU and
-/// Features.
-void setFunctionAttributes(StringRef CPU, StringRef Features, Module &M);
+/// \brief Set function attributes of functions in Module M based on CPU,
+/// Features, and Options.
+/// If AlwaysRecordAttrs is true, it will always record the function attributes
+/// in Options regardless of whether those attributes were specified on the
+/// tool's command line.
+void setFunctionAttributes(StringRef CPU, StringRef Features,
+                           const TargetOptions &Options, Module &M,
+                           bool AlwaysRecordAttrs);
 
 // Comparison operators:
 
diff --git a/lib/CodeGen/TargetFrameLoweringImpl.cpp b/lib/CodeGen/TargetFrameLoweringImpl.cpp
index e3f01912b87..56383247ead 100644
--- a/lib/CodeGen/TargetFrameLoweringImpl.cpp
+++ b/lib/CodeGen/TargetFrameLoweringImpl.cpp
@@ -14,6 +14,7 @@
 #include "llvm/Target/TargetFrameLowering.h"
 #include "llvm/CodeGen/MachineFrameInfo.h"
 #include "llvm/CodeGen/MachineFunction.h"
+#include "llvm/IR/Function.h"
 #include "llvm/Target/TargetRegisterInfo.h"
 #include "llvm/Target/TargetSubtargetInfo.h"
 #include <cstdlib>
@@ -22,6 +23,12 @@ using namespace llvm;
 TargetFrameLowering::~TargetFrameLowering() {
 }
 
+/// The default implementation just looks at attribute "no-frame-pointer-elim".
+bool TargetFrameLowering::noFramePointerElim(const MachineFunction &MF) const {
+  auto Attr = MF.getFunction()->getFnAttribute("no-frame-pointer-elim");
+  return Attr.getValueAsString() == "true";
+}
+
 /// getFrameIndexOffset - Returns the displacement from the frame register to
 /// the stack frame of the specified index. This is the default implementation
 /// which is overridden for some targets.
diff --git a/lib/CodeGen/TargetOptionsImpl.cpp b/lib/CodeGen/TargetOptionsImpl.cpp
index c855ae51ce4..9d1c27ef51e 100644
--- a/lib/CodeGen/TargetOptionsImpl.cpp
+++ b/lib/CodeGen/TargetOptionsImpl.cpp
@@ -12,23 +12,26 @@
 //===----------------------------------------------------------------------===//
 
 #include "llvm/IR/Function.h"
+#include "llvm/IR/Module.h"
 #include "llvm/CodeGen/MachineFrameInfo.h"
 #include "llvm/CodeGen/MachineFunction.h"
+#include "llvm/Target/TargetFrameLowering.h"
 #include "llvm/Target/TargetOptions.h"
+#include "llvm/Target/TargetSubtargetInfo.h"
 using namespace llvm;
 
 /// DisableFramePointerElim - This returns true if frame pointer elimination
 /// optimization should be disabled for the given machine function.
 bool TargetOptions::DisableFramePointerElim(const MachineFunction &MF) const {
-  // Check to see if we should eliminate non-leaf frame pointers and then
-  // check to see if we should eliminate all frame pointers.
-  if (MF.getFunction()->hasFnAttribute("no-frame-pointer-elim-non-leaf") &&
-      !NoFramePointerElim) {
-    const MachineFrameInfo *MFI = MF.getFrameInfo();
-    return MFI->hasCalls();
-  }
+  // Check to see if we should eliminate all frame pointers.
+  if (MF.getSubtarget().getFrameLowering()->noFramePointerElim(MF))
+    return true;
+
+  // Check to see if we should eliminate non-leaf frame pointers.
+  if (MF.getFunction()->hasFnAttribute("no-frame-pointer-elim-non-leaf"))
+    return MF.getFrameInfo()->hasCalls();
 
-  return NoFramePointerElim;
+  return false;
 }
 
 /// LessPreciseFPMAD - This flag return true when -enable-fp-mad option
@@ -53,7 +56,9 @@ StringRef TargetOptions::getTrapFunctionName() const {
 }
 
 
-void llvm::setFunctionAttributes(StringRef CPU, StringRef Features, Module &M) {
+void llvm::setFunctionAttributes(StringRef CPU, StringRef Features,
+                                 const TargetOptions &Options, Module &M,
+                                 bool AlwaysRecordAttrs) {
   for (auto &F : M) {
     auto &Ctx = F.getContext();
     AttributeSet Attrs = F.getAttributes(), NewAttrs;
@@ -66,6 +71,11 @@ void llvm::setFunctionAttributes(StringRef CPU, StringRef Features, Module &M) {
       NewAttrs = NewAttrs.addAttribute(Ctx, AttributeSet::FunctionIndex,
                                        "target-features", Features);
 
+    if (Options.NoFramePointerElimOverride || AlwaysRecordAttrs)
+      NewAttrs = NewAttrs.addAttribute(
+          Ctx, AttributeSet::FunctionIndex, "no-frame-pointer-elim",
+          Options.NoFramePointerElim ? "true" : "false");
+
     // Let NewAttrs override Attrs.
     NewAttrs = Attrs.addAttributes(Ctx, AttributeSet::FunctionIndex, NewAttrs);
     F.setAttributes(NewAttrs);
diff --git a/lib/ExecutionEngine/ExecutionEngineBindings.cpp b/lib/ExecutionEngine/ExecutionEngineBindings.cpp
index 22ff3114da0..d7d3d19d0af 100644
--- a/lib/ExecutionEngine/ExecutionEngineBindings.cpp
+++ b/lib/ExecutionEngine/ExecutionEngineBindings.cpp
@@ -179,9 +179,16 @@ LLVMBool LLVMCreateMCJITCompilerForModule(
   TargetOptions targetOptions;
   targetOptions.NoFramePointerElim = options.NoFramePointerElim;
   targetOptions.EnableFastISel = options.EnableFastISel;
+  std::unique_ptr<Module> Mod(unwrap(M));
+
+  if (Mod)
+    // Set function attribute "no-frame-pointer-elim" based on
+    // NoFramePointerElim.
+    setFunctionAttributes(/* CPU */ "", /* Features */ "", targetOptions, *Mod,
+                          /* AlwaysRecordAttrs */ true);
 
   std::string Error;
-  EngineBuilder builder(std::unique_ptr<Module>(unwrap(M)));
+  EngineBuilder builder(std::move(Mod));
   builder.setEngineKind(EngineKind::JIT)
          .setErrorStr(&Error)
          .setOptLevel((CodeGenOpt::Level)options.OptLevel)
diff --git a/lib/Target/ARM/ARMFastISel.cpp b/lib/Target/ARM/ARMFastISel.cpp
index 97995d31db8..4175b4af86e 100644
--- a/lib/Target/ARM/ARMFastISel.cpp
+++ b/lib/Target/ARM/ARMFastISel.cpp
@@ -3065,23 +3065,9 @@ bool ARMFastISel::fastLowerArguments() {
 namespace llvm {
   FastISel *ARM::createFastISel(FunctionLoweringInfo &funcInfo,
                                 const TargetLibraryInfo *libInfo) {
-    const TargetMachine &TM = funcInfo.MF->getTarget();
-    const ARMSubtarget &STI =
-        static_cast<const ARMSubtarget &>(funcInfo.MF->getSubtarget());
-    // Thumb2 support on iOS; ARM support on iOS, Linux and NaCl.
-    bool UseFastISel = false;
-    UseFastISel |= STI.isTargetMachO() && !STI.isThumb1Only();
-    UseFastISel |= STI.isTargetLinux() && !STI.isThumb();
-    UseFastISel |= STI.isTargetNaCl() && !STI.isThumb();
-
-    if (UseFastISel) {
-      // iOS always has a FP for backtracking, force other targets
-      // to keep their FP when doing FastISel. The emitted code is
-      // currently superior, and in cases like test-suite's lencod
-      // FastISel isn't quite correct when FP is eliminated.
-      TM.Options.NoFramePointerElim = true;
+    if (funcInfo.MF->getSubtarget<ARMSubtarget>().useFastISel())
       return new ARMFastISel(funcInfo, libInfo);
-    }
+
     return nullptr;
   }
 }
diff --git a/lib/Target/ARM/ARMFrameLowering.cpp b/lib/Target/ARM/ARMFrameLowering.cpp
index 4eafd82af4c..a52e49780e2 100644
--- a/lib/Target/ARM/ARMFrameLowering.cpp
+++ b/lib/Target/ARM/ARMFrameLowering.cpp
@@ -43,6 +43,14 @@ ARMFrameLowering::ARMFrameLowering(const ARMSubtarget &sti)
     : TargetFrameLowering(StackGrowsDown, sti.getStackAlignment(), 0, 4),
       STI(sti) {}
 
+bool ARMFrameLowering::noFramePointerElim(const MachineFunction &MF) const {
+  // iOS always has a FP for backtracking, force other targets to keep their FP
+  // when doing FastISel. The emitted code is currently superior, and in cases
+  // like test-suite's lencod FastISel isn't quite correct when FP is eliminated.
+  return TargetFrameLowering::noFramePointerElim(MF) ||
+         MF.getSubtarget<ARMSubtarget>().useFastISel();
+}
+
 /// hasFP - Return true if the specified function should have a dedicated frame
 /// pointer register.  This is true if the function has variable sized allocas
 /// or if frame pointer elimination is disabled.
diff --git a/lib/Target/ARM/ARMFrameLowering.h b/lib/Target/ARM/ARMFrameLowering.h
index ff3425795ae..d763d17a506 100644
--- a/lib/Target/ARM/ARMFrameLowering.h
+++ b/lib/Target/ARM/ARMFrameLowering.h
@@ -43,6 +43,8 @@ public:
                                   const std::vector<CalleeSavedInfo> &CSI,
                                   const TargetRegisterInfo *TRI) const override;
 
+  bool noFramePointerElim(const MachineFunction &MF) const override;
+
   bool hasFP(const MachineFunction &MF) const override;
   bool hasReservedCallFrame(const MachineFunction &MF) const override;
   bool canSimplifyCallFramePseudos(const MachineFunction &MF) const override;
diff --git a/lib/Target/ARM/ARMSubtarget.cpp b/lib/Target/ARM/ARMSubtarget.cpp
index 89aab260366..008aeffd171 100644
--- a/lib/Target/ARM/ARMSubtarget.cpp
+++ b/lib/Target/ARM/ARMSubtarget.cpp
@@ -353,3 +353,10 @@ bool ARMSubtarget::useMovt(const MachineFunction &MF) const {
   return UseMovt && (isTargetWindows() ||
                      !MF.getFunction()->hasFnAttribute(Attribute::MinSize));
 }
+
+bool ARMSubtarget::useFastISel() const {
+  // Thumb2 support on iOS; ARM support on iOS, Linux and NaCl.
+  return TM.Options.EnableFastISel &&
+         ((isTargetMachO() && !isThumb1Only()) ||
+          (isTargetLinux() && !isThumb()) || (isTargetNaCl() && !isThumb()));
+}
diff --git a/lib/Target/ARM/ARMSubtarget.h b/lib/Target/ARM/ARMSubtarget.h
index ed1c6a0fc64..77ceb081db1 100644
--- a/lib/Target/ARM/ARMSubtarget.h
+++ b/lib/Target/ARM/ARMSubtarget.h
@@ -450,6 +450,8 @@ public:
   /// symbol.
   bool GVIsIndirectSymbol(const GlobalValue *GV, Reloc::Model RelocM) const;
 
+  /// True if fast-isel is used.
+  bool useFastISel() const;
 };
 } // End llvm namespace
 
diff --git a/lib/Target/TargetMachine.cpp b/lib/Target/TargetMachine.cpp
index 0523f04bee0..36875b47ab6 100644
--- a/lib/Target/TargetMachine.cpp
+++ b/lib/Target/TargetMachine.cpp
@@ -66,7 +66,6 @@ void TargetMachine::resetTargetOptions(const Function &F) const {
       Options.X = (F.getFnAttribute(Y).getValueAsString() == "true");          \
   } while (0)
 
-  RESET_OPTION(NoFramePointerElim, "no-frame-pointer-elim");
   RESET_OPTION(LessPreciseFPMADOption, "less-precise-fpmad");
   RESET_OPTION(UnsafeFPMath, "unsafe-fp-math");
   RESET_OPTION(NoInfsFPMath, "no-infs-fp-math");
diff --git a/test/CodeGen/ARM/disable-fp-elim.ll b/test/CodeGen/ARM/disable-fp-elim.ll
new file mode 100644
index 00000000000..dafeda2ac76
--- /dev/null
+++ b/test/CodeGen/ARM/disable-fp-elim.ll
@@ -0,0 +1,25 @@
+; RUN: llc < %s -mtriple armv7-none-linux-gnueabi -O1 | FileCheck %s --check-prefix=DISABLE-FP-ELIM
+; RUN: llc < %s -mtriple armv7-none-linux-gnueabi -disable-fp-elim -O1 | FileCheck %s --check-prefix=DISABLE-FP-ELIM
+; RUN: llc < %s -mtriple armv7-none-linux-gnueabi -disable-fp-elim=false -O1 | FileCheck %s --check-prefix=ENABLE-FP-ELIM
+; RUN: llc < %s -mtriple armv7-none-linux-gnueabi -disable-fp-elim=false -O0 | FileCheck %s --check-prefix=DISABLE-FP-ELIM
+
+; Check that command line option "-disable-fp-elim" overrides function attribute
+; "no-frame-pointer-elim". Also, check frame pointer elimination is disabled
+; when fast-isel is used.
+
+; ENABLE-FP-ELIM-NOT: .setfp
+; DISABLE-FP-ELIM: .setfp r11, sp
+
+define i32 @foo1(i32 %a, i32 %b, i32 %c, i32 %d, i32 %e) #0 {
+entry:
+  %call = tail call i32 @foo2(i32 %a)
+  %add = add i32 %c, %b
+  %add1 = add i32 %add, %d
+  %add2 = add i32 %add1, %e
+  %add3 = add i32 %add2, %call
+  ret i32 %add3
+}
+
+declare i32 @foo2(i32)
+
+attributes #0 = { nounwind "no-frame-pointer-elim"="true" }
diff --git a/tools/llc/llc.cpp b/tools/llc/llc.cpp
index 145d957890c..b7756129155 100644
--- a/tools/llc/llc.cpp
+++ b/tools/llc/llc.cpp
@@ -304,8 +304,11 @@ static int compileModule(char **argv, LLVMContext &Context) {
   if (const DataLayout *DL = Target->getDataLayout())
     M->setDataLayout(*DL);
 
-  // Override function attributes based on CPUStr and FeaturesStr.
-  setFunctionAttributes(CPUStr, FeaturesStr, *M);
+  // Override function attributes based on CPUStr, FeaturesStr, and Options.
+  // Pass AlwaysRecordAttrs=false as we want to override an attribute only when
+  // the corresponding cl::opt has been provided on llc's command line.
+  setFunctionAttributes(CPUStr, FeaturesStr, Options, *M,
+                        /* AlwaysRecordAttrs */ false);
 
   if (RelaxAll.getNumOccurrences() > 0 &&
       FileType != TargetMachine::CGFT_ObjectFile)
diff --git a/tools/opt/opt.cpp b/tools/opt/opt.cpp
index 15b24cf5072..6f3d8ca4156 100644
--- a/tools/opt/opt.cpp
+++ b/tools/opt/opt.cpp
@@ -265,7 +265,8 @@ static CodeGenOpt::Level GetCodeGenOptLevel() {
 
 // Returns the TargetMachine instance or zero if no triple is provided.
 static TargetMachine* GetTargetMachine(Triple TheTriple, StringRef CPUStr,
-                                       StringRef FeaturesStr) {
+                                       StringRef FeaturesStr,
+                                       const TargetOptions &Options) {
   std::string Error;
   const Target *TheTarget = TargetRegistry::lookupTarget(MArch, TheTriple,
                                                          Error);
@@ -275,8 +276,7 @@ static TargetMachine* GetTargetMachine(Triple TheTriple, StringRef CPUStr,
   }
 
   return TheTarget->createTargetMachine(TheTriple.getTriple(),
-                                        CPUStr, FeaturesStr,
-                                        InitTargetOptionsFromCodeGenFlags(),
+                                        CPUStr, FeaturesStr, Options,
                                         RelocModel, CMModel,
                                         GetCodeGenOptLevel());
 }
@@ -386,17 +386,21 @@ int main(int argc, char **argv) {
   Triple ModuleTriple(M->getTargetTriple());
   std::string CPUStr, FeaturesStr;
   TargetMachine *Machine = nullptr;
+  const TargetOptions Options = InitTargetOptionsFromCodeGenFlags();
 
   if (ModuleTriple.getArch()) {
     CPUStr = getCPUStr();
     FeaturesStr = getFeaturesStr();
-    Machine = GetTargetMachine(ModuleTriple, CPUStr, FeaturesStr);
+    Machine = GetTargetMachine(ModuleTriple, CPUStr, FeaturesStr, Options);
   }
 
   std::unique_ptr<TargetMachine> TM(Machine);
 
-  // Override function attributes based on CPUStr and FeaturesStr.
-  setFunctionAttributes(CPUStr, FeaturesStr, *M);
+  // Override function attributes based on CPUStr, FeaturesStr, and Options.
+  // Pass AlwaysRecordAttrs=false as we want to override an attribute only when
+  // the corresponding cl::opt has been provided on opt's command line.
+  setFunctionAttributes(CPUStr, FeaturesStr, Options, *M,
+                        /* AlwaysRecordAttrs */ false);
 
   // If the output is set to be emitted to standard out, and standard out is a
   // console, print out a warning message and refuse to do it.  We don't