Merge pull request swiftlang#31986 from aschwaighofer/irgen_inherit_clangs_fp_elim

IRGen: Default to clang's frame pointer elimination settings

Author: aschwaighofer

include/swift/AST/IRGenOptions.h

@@ -190,10 +190,6 @@ class IRGenOptions {
   /// Whether we should run LLVM SLP vectorizer.
   unsigned DisableLLVMSLPVectorizer : 1;
 
-  /// Disable frame pointer elimination?
-  unsigned DisableFPElimLeaf : 1;
-  unsigned DisableFPElim : 1;
-  
   /// Special codegen for playgrounds.
   unsigned Playground : 1;
 
@@ -320,8 +316,7 @@ class IRGenOptions {
         DisableClangModuleSkeletonCUs(false), UseJIT(false),
         DisableLLVMOptzns(false),
         DisableSwiftSpecificLLVMOptzns(false), DisableLLVMSLPVectorizer(false),
-        DisableFPElimLeaf(false),
-        DisableFPElim(true), Playground(false), EmitStackPromotionChecks(false),
+        Playground(false), EmitStackPromotionChecks(false),
         FunctionSections(false), PrintInlineTree(false), EmbedMode(IRGenEmbedMode::None),
         HasValueNamesSetting(false), ValueNames(false),
         EnableReflectionMetadata(true), EnableReflectionNames(true),

include/swift/Option/Options.td

@@ -1017,10 +1017,6 @@ def enable_private_imports : Flag<["-"], "enable-private-imports">,
   Flags<[FrontendOption, NoInteractiveOption, HelpHidden]>,
   HelpText<"Allows this module's internal and private API to be accessed">;
 
-def disable_leaf_frame_pointer_elim : Flag<["-"], "no-omit-leaf-frame-pointer">,
-  Flags<[FrontendOption, NoInteractiveOption, HelpHidden]>,
-  HelpText<"Don't omit the frame pointer for leaf functions">;
-
 def sanitize_EQ : CommaJoined<["-"], "sanitize=">,
   Flags<[FrontendOption, NoInteractiveOption]>, MetaVarName<"<check>">,
   HelpText<"Turn on runtime checks for erroneous behavior.">;

lib/Driver/ToolChains.cpp

@@ -290,8 +290,6 @@ void ToolChain::addCommonFrontendArgs(const OutputInfo &OI,
       arguments.push_back("-enable-anonymous-context-mangled-names");
   }
 
-  inputArgs.AddLastArg(arguments, options::OPT_disable_leaf_frame_pointer_elim);
-
   // Pass through any subsystem flags.
   inputArgs.AddAllArgs(arguments, options::OPT_Xllvm);
   inputArgs.AddAllArgs(arguments, options::OPT_Xcc);

lib/Frontend/CompilerInvocation.cpp

@@ -1533,9 +1533,6 @@ static bool ParseIRGenArgs(IRGenOptions &Opts, ArgList &Args,
         getRuntimeCompatVersion();
   }
 
-  if (Args.hasArg(OPT_disable_leaf_frame_pointer_elim))
-    Opts.DisableFPElimLeaf = true;
-
   return false;
 }
 

lib/IRGen/GenMeta.cpp

@@ -180,7 +180,7 @@ static void emitMetadataCompletionFunction(IRGenModule &IGM,
     IGM.getAddrOfTypeMetadataCompletionFunction(typeDecl, ForDefinition);
   f->setAttributes(IGM.constructInitialAttributes());
   f->setDoesNotThrow();
-  IGM.setHasFramePointer(f, false);
+  IGM.setHasNoFramePointer(f);
 
   IRGenFunction IGF(IGM, f);
 
@@ -2270,7 +2270,7 @@ namespace {
         IGM.getAddrOfTypeMetadataInstantiationFunction(Target, ForDefinition);
       f->setAttributes(IGM.constructInitialAttributes());
       f->setDoesNotThrow();
-      IGM.setHasFramePointer(f, false);
+      IGM.setHasNoFramePointer(f);
 
       IRGenFunction IGF(IGM, f);
 

lib/IRGen/IRGenModule.cpp

@@ -90,34 +90,19 @@ static llvm::PointerType *createStructPointerType(IRGenModule &IGM,
   return createStructType(IGM, name, types)->getPointerTo(DefaultAS);
 };
 
-static clang::CodeGenOptions::FramePointerKind
-shouldUseFramePointer(const IRGenOptions &Opts, const llvm::Triple &triple) {
-  if (Opts.DisableFPElim) {
-    // General frame pointer elimination is disabled.
-    // Should we at least eliminate in leaf functions?
-    // Currently we only do that on arm64 (this matches the behavior of clang).
-    return Opts.DisableFPElimLeaf
-               ? clang::CodeGenOptions::FramePointerKind::All
-               : triple.isAArch64()
-                     ? clang::CodeGenOptions::FramePointerKind::NonLeaf
-                     : clang::CodeGenOptions::FramePointerKind::All;
-  }
-
-  return clang::CodeGenOptions::FramePointerKind::None;
-}
-
-static clang::CodeGenerator *
-createClangCodeGenerator(ASTContext &Context, llvm::LLVMContext &LLVMContext,
-                         const IRGenOptions &Opts, StringRef ModuleName,
-                         StringRef PD, const llvm::Triple &triple) {
+static clang::CodeGenerator *createClangCodeGenerator(ASTContext &Context,
+                                                 llvm::LLVMContext &LLVMContext,
+                                                      const IRGenOptions &Opts,
+                                                      StringRef ModuleName,
+                                                      StringRef PD) {
   auto Loader = Context.getClangModuleLoader();
   auto *Importer = static_cast<ClangImporter*>(&*Loader);
   assert(Importer && "No clang module loader!");
   auto &ClangContext = Importer->getClangASTContext();
 
   auto &CGO = Importer->getClangCodeGenOpts();
   CGO.OptimizationLevel = Opts.shouldOptimize() ? 3 : 0;
-  CGO.setFramePointer(shouldUseFramePointer(Opts, triple));
+
   CGO.DiscardValueNames = !Opts.shouldProvideValueNames();
   switch (Opts.DebugInfoLevel) {
   case IRGenDebugInfoLevel::None:
@@ -207,17 +192,17 @@ static void sanityCheckStdlib(IRGenModule &IGM) {
 
 IRGenModule::IRGenModule(IRGenerator &irgen,
                          std::unique_ptr<llvm::TargetMachine> &&target,
-                         SourceFile *SF, StringRef ModuleName,
-                         StringRef OutputFilename,
+                         SourceFile *SF,
+                         StringRef ModuleName, StringRef OutputFilename,
                          StringRef MainInputFilenameForDebugInfo,
                          StringRef PrivateDiscriminator)
-    : LLVMContext(new llvm::LLVMContext()), IRGen(irgen),
-      Context(irgen.SIL.getASTContext()),
+    : LLVMContext(new llvm::LLVMContext()),
+      IRGen(irgen), Context(irgen.SIL.getASTContext()),
       // The LLVMContext (and the IGM itself) will get deleted by the IGMDeleter
       // as long as the IGM is registered with the IRGenerator.
-      ClangCodeGen(createClangCodeGenerator(Context, *LLVMContext, irgen.Opts,
-                                            ModuleName, PrivateDiscriminator,
-                                            irgen.getEffectiveClangTriple())),
+      ClangCodeGen(createClangCodeGenerator(Context, *LLVMContext,
+                                            irgen.Opts,
+                                            ModuleName, PrivateDiscriminator)),
       Module(*ClangCodeGen->GetModule()),
       DataLayout(irgen.getClangDataLayout()),
       Triple(irgen.getEffectiveClangTriple()), TargetMachine(std::move(target)),
@@ -999,28 +984,13 @@ bool swift::irgen::shouldRemoveTargetFeature(StringRef feature) {
   return feature == "+thumb-mode";
 }
 
-void IRGenModule::setHasFramePointer(llvm::AttrBuilder &Attrs,
-                                     bool HasFramePointer) {
-  if (!HasFramePointer) {
-    Attrs.addAttribute("frame-pointer", "none");
-    return;
-  }
-  if (IRGen.Opts.DisableFPElimLeaf) {
-    Attrs.addAttribute("frame-pointer", "all");
-    return;
-  }
-
-  // We omit frame pointers for leaf functions only for arm64 for now (matching
-  // clang's behavior).
-  auto framePointer =
-      IRGen.getEffectiveClangTriple().isAArch64() ? "non-leaf" : "all";
-  Attrs.addAttribute("frame-pointer", framePointer);
+void IRGenModule::setHasNoFramePointer(llvm::AttrBuilder &Attrs) {
+  Attrs.addAttribute("frame-pointer", "none");
 }
 
-void IRGenModule::setHasFramePointer(llvm::Function *F,
-                                     bool HasFramePointer) {
+void IRGenModule::setHasNoFramePointer(llvm::Function *F) {
   llvm::AttrBuilder b;
-  setHasFramePointer(b, HasFramePointer);
+  setHasNoFramePointer(b);
   F->addAttributes(llvm::AttributeList::FunctionIndex, b);
 }
 
@@ -1030,10 +1000,6 @@ void IRGenModule::constructInitialFnAttributes(llvm::AttrBuilder &Attrs,
   // Add the default attributes for the Clang configuration.
   clang::CodeGen::addDefaultFunctionDefinitionAttributes(getClangCGM(), Attrs);
 
-  // Add frame pointer attributes.
-  // FIXME: why are we doing this?
-  setHasFramePointer(Attrs, IRGen.Opts.DisableFPElim);
-  
   // Add/remove MinSize based on the appropriate setting.
   if (FuncOptMode == OptimizationMode::NotSet)
     FuncOptMode = IRGen.Opts.OptMode;

lib/IRGen/IRGenModule.h

@@ -1313,8 +1313,8 @@ private:                            \
   void constructInitialFnAttributes(llvm::AttrBuilder &Attrs,
                                     OptimizationMode FuncOptMode =
                                       OptimizationMode::NotSet);
-  void setHasFramePointer(llvm::AttrBuilder &Attrs, bool HasFP);
-  void setHasFramePointer(llvm::Function *F, bool HasFP);
+  void setHasNoFramePointer(llvm::AttrBuilder &Attrs);
+  void setHasNoFramePointer(llvm::Function *F);
   llvm::AttributeList constructInitialAttributes();
 
   void emitProtocolDecl(ProtocolDecl *D);

lib/IRGen/MetadataRequest.cpp

@@ -1684,7 +1684,7 @@ void irgen::emitCacheAccessFunction(IRGenModule &IGM,
   accessor->addAttribute(llvm::AttributeList::FunctionIndex,
                          llvm::Attribute::NoInline);
   // Accessor functions don't need frame pointers.
-  IGM.setHasFramePointer(accessor, false);
+  IGM.setHasNoFramePointer(accessor);
 
   // This function is logically 'readnone': the caller does not need
   // to reason about any side effects or stores it might perform.
@@ -2094,8 +2094,8 @@ MetadataResponse irgen::emitGenericTypeMetadataAccessFunction(
       thunkFn->setCallingConv(IGM.SwiftCC);
       thunkFn->addAttribute(llvm::AttributeList::FunctionIndex,
                             llvm::Attribute::NoInline);
-      IGM.setHasFramePointer(thunkFn, false);
-      
+      IGM.setHasNoFramePointer(thunkFn);
+
       [&IGM, thunkFn]{
         IRGenFunction subIGF(IGM, thunkFn);
 
@@ -2631,7 +2631,7 @@ emitMetadataAccessByMangledName(IRGenFunction &IGF, CanType type,
     instantiationFn->setDoesNotThrow();
     instantiationFn->addAttribute(llvm::AttributeList::FunctionIndex,
                                   llvm::Attribute::NoInline);
-    IGM.setHasFramePointer(instantiationFn, false);
+    IGM.setHasNoFramePointer(instantiationFn);
 
     [&IGM, instantiationFn, request]{
       IRGenFunction subIGF(IGM, instantiationFn);

test/IRGen/framepointer.sil

@@ -1,6 +1,8 @@
 // RUN: %target-swift-frontend -primary-file %s -emit-ir | %FileCheck %s --check-prefix=CHECK
-// RUN: %target-swift-frontend -primary-file %s -emit-ir -no-omit-leaf-frame-pointer| %FileCheck %s --check-prefix=CHECK-ALL
+// RUN: %target-swift-frontend -primary-file %s -emit-ir -Xcc -mno-omit-leaf-frame-pointer | %FileCheck %s --check-prefix=CHECK-ALL
 // RUN: %target-swift-frontend -primary-file %s -S | %FileCheck %s  --check-prefix=CHECKASM --check-prefix=CHECKASM-%target-os-%target-cpu
+// RUN: %target-swift-frontend -primary-file %s -emit-ir -Xcc -momit-leaf-frame-pointer | %FileCheck %s --check-prefix=LEAF
+// RUN: %target-swift-frontend -primary-file %s -emit-ir -Xcc -fomit-frame-pointer | %FileCheck %s --check-prefix=NOFP
 
 // REQUIRES: CPU=x86_64
 
@@ -46,6 +48,32 @@ entry(%i : $Int32):
 
 // CHECK-ALL: attributes [[ATTR]] = {{{.*}}"frame-pointer"="all"
 
+// LEAF: define{{.*}} swiftcc i32 @leaf_function_no_frame_pointer(i32 %0) [[ATTR:#.*]] {
+// LEAF: entry:
+// LEAF:   ret i32 %0
+// LEAF: }
+
+// LEAF: define{{.*}} swiftcc i32 @non_leaf_function_with_frame_pointer(i32 %0) [[ATTR]] {
+// LEAF: entry:
+// LEAF:   %1 = call swiftcc i32 @leaf_function_no_frame_pointer(i32 %0)
+// LEAF:   ret i32 %1
+// LEAF: }
+
+// LEAF: attributes [[ATTR]] = {{{.*}}"frame-pointer"="non-leaf"
+
+// NOFP: define{{.*}} swiftcc i32 @leaf_function_no_frame_pointer(i32 %0) [[ATTR:#.*]] {
+// NOFP: entry:
+// NOFP:   ret i32 %0
+// NOFP: }
+
+// NOFP: define{{.*}} swiftcc i32 @non_leaf_function_with_frame_pointer(i32 %0) [[ATTR]] {
+// NOFP: entry:
+// NOFP:   %1 = call swiftcc i32 @leaf_function_no_frame_pointer(i32 %0)
+// NOFP:   ret i32 %1
+// NOFP: }
+
+// NOFP: attributes [[ATTR]] = {{{.*}}"frame-pointer"="none"
+
 // Silence other os-archs.
 // CHECKASM: {{.*}}
 

test/IRGen/framepointer_arm64.sil

@@ -1,7 +1,7 @@
 // RUN: %target-swift-frontend -target arm64-apple-ios8.0 -primary-file %s -emit-ir | %FileCheck %s --check-prefix=CHECK
-// RUN: %target-swift-frontend -target arm64-apple-ios8.0 -primary-file %s -emit-ir -no-omit-leaf-frame-pointer| %FileCheck %s --check-prefix=CHECK-ALL
+// RUN: %target-swift-frontend -target arm64-apple-ios8.0 -primary-file %s -emit-ir -Xcc -mno-omit-leaf-frame-pointer| %FileCheck %s --check-prefix=CHECK-ALL
 // RUN: %target-swift-frontend -target arm64-apple-ios8.0 -primary-file %s -S | %FileCheck %s --check-prefix=CHECKASM
-// RUN: %target-swift-frontend -target arm64-apple-ios8.0 -primary-file %s -S -no-omit-leaf-frame-pointer | %FileCheck %s --check-prefix=CHECKASM-ALL
+// RUN: %target-swift-frontend -target arm64-apple-ios8.0 -primary-file %s -S -Xcc -mno-omit-leaf-frame-pointer | %FileCheck %s --check-prefix=CHECKASM-ALL
 
 // REQUIRES: CODEGENERATOR=AArch64
 // REQUIRES: OS=ios