Page MenuHomePhabricator

Always have clang pass -pie-level and -pic-level values to the code generator
ClosedPublic

Authored by tmsriram on Apr 6 2016, 3:12 PM.

Details

Summary

clang does not pass pie-level and pic-level option values to the code generator with "-x ir" due to the following code in CompilerInvocation.cpp:

if (DashX == IK_AST || DashX == IK_LLVM_IR) {
    ....;
 } else {
    ParseLangArgs(*Res.getLangOpts(), Args, DashX, Res.getTargetOpts(), Diags);
 }

pie-level and pic-level are LangArgs and are not parsed with "-x ir". They define macros PIC and PIE_ when set, see lib/Frontend/InitPreprocessor.cpp

Not setting LangOpts.PIELevel means that Options.PositionIndependentExecutable is always false even when -fPIE is used. This patch fixes that.

I considered an alternative of making pie-level and pic-level CodeGen Options. However, since these options are used to set macros PIE, PIC, etc. this looked better as LangOpts. Please let me know what you think.

For a broader context, I am working on optimizing accesses to global variables with -fPIE on x86-64 and this patch is the first step to get there. In particular, I noticed the following code generated with clang:

hello.cpp:

int a = 0;

int main() {

return a;

}

$ clang -O2 -fPIE hello.cc -S
$ cat hello.s

main: # @main
movq a@GOTPCREL(%rip), %rax
movl (%rax), %eax
retq

Creating a GOT entry for global 'a' is unnecessary as 'a' is defined in hello.cpp which will be linked into a position independent executable (fPIE). Hence, the definition of 'a' cannot be overridden and we can remove a load. The efficient access is this:

main: # @main
movl a(%rip), %eax
retq

I plan to address this in a separate patch.

Event Timeline

tmsriram updated this revision to Diff 52853.Apr 6 2016, 3:12 PM
tmsriram retitled this revision from to Always have clang pass -pie-level and -pic-level values to the code generator.
tmsriram updated this object.
tmsriram added reviewers: rnk, davidxl.
tmsriram added a subscriber: cfe-commits.
rnk accepted this revision.Apr 6 2016, 4:34 PM
rnk edited edge metadata.

lgtm

There are similar flags, like -O, which define preprocessor macros and get fed to the backend. In that case, we actually have duplicate LangOpts and CodeGenOpts. I don't think we need to do that in this case, though.

lib/Frontend/CompilerInvocation.cpp
2140 ↗(On Diff #52945)

This code would probably be cleaner with a local variable LanguageOptions &LangOpts = *Res.getLangOpts()

This revision is now accepted and ready to land.Apr 6 2016, 4:34 PM
tmsriram updated this revision to Diff 52945.Apr 7 2016, 12:46 PM
tmsriram edited edge metadata.

Cleanup the code to use a local variable to store *Res.getLangOpts()

This revision was automatically updated to reflect the committed changes.