Skip to content

Revert "IRGen: Always eliminate frame pointers of leaf functions" #31993

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions include/swift/AST/IRGenOptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,6 @@ class IRGenOptions {
unsigned DisableLLVMSLPVectorizer : 1;

/// Disable frame pointer elimination?
unsigned DisableFPElimLeaf : 1;
unsigned DisableFPElim : 1;

/// Special codegen for playgrounds.
Expand Down Expand Up @@ -320,7 +319,6 @@ class IRGenOptions {
DisableClangModuleSkeletonCUs(false), UseJIT(false),
DisableLLVMOptzns(false),
DisableSwiftSpecificLLVMOptzns(false), DisableLLVMSLPVectorizer(false),
DisableFPElimLeaf(false),
DisableFPElim(true), Playground(false), EmitStackPromotionChecks(false),
FunctionSections(false), PrintInlineTree(false), EmbedMode(IRGenEmbedMode::None),
HasValueNamesSetting(false), ValueNames(false),
Expand Down
4 changes: 0 additions & 4 deletions include/swift/Option/Options.td
Original file line number Diff line number Diff line change
Expand Up @@ -1010,10 +1010,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.">;
Expand Down
2 changes: 0 additions & 2 deletions lib/Driver/ToolChains.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -288,8 +288,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);
Expand Down
3 changes: 0 additions & 3 deletions lib/Frontend/CompilerInvocation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1514,9 +1514,6 @@ static bool ParseIRGenArgs(IRGenOptions &Opts, ArgList &Args,
getRuntimeCompatVersion();
}

if (Args.hasArg(OPT_disable_leaf_frame_pointer_elim))
Opts.DisableFPElimLeaf = true;

return false;
}

Expand Down
58 changes: 16 additions & 42 deletions lib/IRGen/IRGenModule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -90,34 +90,21 @@ 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.setFramePointer(Opts.DisableFPElim
? clang::CodeGenOptions::FramePointerKind::All
: clang::CodeGenOptions::FramePointerKind::None);
CGO.DiscardValueNames = !Opts.shouldProvideValueNames();
switch (Opts.DebugInfoLevel) {
case IRGenDebugInfoLevel::None:
Expand Down Expand Up @@ -207,17 +194,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)),
Expand Down Expand Up @@ -1001,20 +988,7 @@ bool swift::irgen::shouldRemoveTargetFeature(StringRef feature) {

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);
Attrs.addAttribute("frame-pointer", HasFramePointer ? "all" : "none");
}

void IRGenModule::setHasFramePointer(llvm::Function *F,
Expand Down
64 changes: 0 additions & 64 deletions test/IRGen/framepointer.sil

This file was deleted.

73 changes: 0 additions & 73 deletions test/IRGen/framepointer_arm64.sil

This file was deleted.

6 changes: 6 additions & 0 deletions test/Misc/tbi.sil
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,19 @@

// NO_TBI-LABEL: .globl _testTBI
// NO_TBI: _testTBI
// NO_TBI-NEXT: stp
// NO_TBI-NEXT: mov
// NO_TBI-NEXT: and
// NO_TBI-NEXT: ldr
// NO_TBI-NEXT: ldp
// NO_TBI-NEXT: ret

// TBI-LABEL: .globl _testTBI
// TBI: _testTBI:
// TBI-NEXT: stp
// TBI-NEXT: mov
// TBI-NEXT: ldr
// TBI-NEXT: ldp
// TBI-NEXT: ret

sil_stage canonical
Expand Down