[CIR][CodeGen] Enable -fno-PIE #940
Conversation
| @@ -449,7 +449,11 @@ static bool shouldAssumeDSOLocal(const CIRGenModule &CGM, | |||
| return false; | |||
|
|
|||
| if (CGOpts.DirectAccessExternalData) { | |||
| llvm_unreachable("-fdirect-access-external-data not supported"); | |||
| if (auto gv = dyn_cast<mlir::cir::GlobalOp>(GV.getOperation())) | |||
| return !gv.getTlsModel().has_value(); | |||
There was a problem hiding this comment.
I don't think this is equivalent to the CodeGen code? That one only returns if the var isn't thread-local, but this one always returns. It's equivalent right now because of the way the function is laid out, but if things change in the future, it's probably easier to mirror the existing structure as closely as possible.
I'm linking the corresponding CodeGen code here for reference for everyone: https://github.com/llvm/llvm-project/blob/b2c615fc792c3f75af66aac99c05ffa85ef50354/clang/lib/CodeGen/CodeGenModule.cpp#L1726-L1744
There was a problem hiding this comment.
We should probably incorporate the OG cloning assumptions + asserts into the webpage docs, which I was never able to do, just in case you find yourself wanting to update some docs, this could be a good one!
clang/test/CIR/CodeGen/no-pie.c
Outdated
| @@ -0,0 +1,3 @@ | |||
| // RUN: %clang --target=x86_64-unknown-linux-gnu -fclangir %s -fno-PIE -c | |||
|
|
|||
| void empty(void) {} | |||
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
updated the test!
The title describes the purpose of the PR. The logic was gotten from the original CodeGen, and I added a test to check that `-fno-PIE` is indeed enabled.
The title describes the purpose of the PR. The logic was gotten from the original CodeGen, and I added a test to check that `-fno-PIE` is indeed enabled.
The title describes the purpose of the PR. The logic was gotten from the original CodeGen, and I added a test to check that `-fno-PIE` is indeed enabled.
The title describes the purpose of the PR. The logic was gotten from the original CodeGen, and I added a test to check that `-fno-PIE` is indeed enabled.
The title describes the purpose of the PR.
The logic was gotten from the original CodeGen, and I added a test to check that
-fno-PIEis indeed enabled.