Skip to content

[CIR][CodeGen] Enable -fno-PIE #940

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

Merged
merged 5 commits into from
Oct 9, 2024
Merged
Show file tree
Hide file tree
Changes from 4 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
19 changes: 18 additions & 1 deletion clang/lib/CIR/CodeGen/CIRGenModule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -449,7 +449,24 @@ static bool shouldAssumeDSOLocal(const CIRGenModule &CGM,
return false;

if (CGOpts.DirectAccessExternalData) {
llvm_unreachable("-fdirect-access-external-data not supported");
// If -fdirect-access-external-data (default for -fno-pic), set dso_local
// for non-thread-local variables. If the symbol is not defined in the
// executable, a copy relocation will be needed at link time. dso_local is
// excluded for thread-local variables because they generally don't support
// copy relocations.
if (auto gv = dyn_cast<mlir::cir::GlobalOp>(GV.getOperation()))
if (!gv.getTlsModelAttr())
return true;

// -fno-pic sets dso_local on a function declaration to allow direct
// accesses when taking its address (similar to a data symbol). If the
// function is not defined in the executable, a canonical PLT entry will be
// needed at link time. -fno-direct-access-external-data can avoid the
// canonical PLT entry. We don't generalize this condition to -fpie/-fpic as
// it could just cause trouble without providing perceptible benefits.
if (isa<mlir::cir::FuncOp>(GV) && !CGOpts.NoPLT &&
RM == llvm::Reloc::Static)
return true;
}

// If we can use copy relocations we can assume it is local.
Expand Down
3 changes: 3 additions & 0 deletions clang/test/CIR/CodeGen/no-pie.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
// RUN: %clang --target=x86_64-unknown-linux-gnu -fclangir %s -fno-PIE -c

void empty(void) {}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'd be nice for the test to show the code that's generated for accessing global variables and calling functions with -fno-PIE (ideally we can cover all the new conditions that were added).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't find a difference in the produced CIR with fno-PIE enabled, or maybe I am wrong? It begs the question of if enabling the flag is enough to add support for this option.

Copy link
Member

@bcardosolopes bcardosolopes Oct 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since your change is on shouldAssumeDSOLocal, checking whether the CIR and LLVM output contains or doesn't contain the dso stuff is probably the best thing to do here. See clang/test/CIR/CodeGen/abstract-cond.c for an example on how to craft such tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated the test!