From 626316f0c373627f6f414ab141e54e92b8b11dbe Mon Sep 17 00:00:00 2001 From: Akira Hatanaka Date: Sat, 23 May 2015 01:14:08 +0000 Subject: [PATCH] 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". llvm-svn: 238080 --- include/llvm/CodeGen/CommandFlags.h | 1 + include/llvm/Target/TargetFrameLowering.h | 3 ++ include/llvm/Target/TargetOptions.h | 16 +++++++++-- lib/CodeGen/TargetFrameLoweringImpl.cpp | 7 +++++ lib/CodeGen/TargetOptionsImpl.cpp | 28 +++++++++++++------ .../ExecutionEngineBindings.cpp | 9 +++++- lib/Target/ARM/ARMFastISel.cpp | 18 ++---------- lib/Target/ARM/ARMFrameLowering.cpp | 8 ++++++ lib/Target/ARM/ARMFrameLowering.h | 2 ++ lib/Target/ARM/ARMSubtarget.cpp | 7 +++++ lib/Target/ARM/ARMSubtarget.h | 2 ++ lib/Target/TargetMachine.cpp | 1 - test/CodeGen/ARM/disable-fp-elim.ll | 25 +++++++++++++++++ tools/llc/llc.cpp | 7 +++-- tools/opt/opt.cpp | 16 +++++++---- 15 files changed, 112 insertions(+), 38 deletions(-) create mode 100644 test/CodeGen/ARM/disable-fp-elim.ll 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 @@ -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; - return NoFramePointerElim; + // 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 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 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(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(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().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().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 &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 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