Page MenuHomePhabricator

Please use GitHub pull requests for new patches. Phabricator shutdown timeline

[Driver] Accept and ignore -fno-lifetime-dse argument
Needs RevisionPublic

Authored by thesamesam on May 18 2023, 7:58 PM.

Details

Summary

Since 47f5c54f997a59bb2c65abe6b8b811f6e7553456, we pass -fno-lifetime-dse
when building LLVM with GCC.

Unfortunately, this impacts projects which build LLVM using GCC who then try
to use e.g. clang-tidy because of the flag leaking into compile_commands.json.

While we're trying to stop adding these, given we're passing the flag ourselves,
I don't think we have much of a choice here.

Diff Detail

Event Timeline

thesamesam created this revision.May 18 2023, 7:58 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 18 2023, 7:58 PM
thesamesam requested review of this revision.May 18 2023, 7:58 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 18 2023, 7:58 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript

This is just a draft which I've only briefly tested so far. I'm wondering if this is the right place to put the option, as i've not had to write anything in tablegen before.

MaskRay requested changes to this revision.EditedMay 18 2023, 8:50 PM

I object to parse and ignore this GCC option, which appears obscure.
Clang traditionally added many clang_ignored_gcc_optimization_f_Group and IgnoredGCCCompat options to support a wide range of projects. This has been valuable in making Clang adopted in the open-source community.
Nowadays, GCC-only software has been increasingly obscure.

The original request to accept -fno-lifetime-dse is so that someone's clang-tidy or other clang tooling can accept compile_commands.json produced by GCC.
I want to highlight that this isn't a supported use.

While GCC and Clang share many options, there are also many that Clang don't recognize. For a useful option, we consider implementing it, but I think the bar is higher than appeasing such unsupported use cases. For example, I keep Linux kernel builds with both GCC and Clang. I use the Clang one for ccls (a language server) but the GCC one for debugging.
I don't mix-and-match as the GCC build has a dozen options that Clang doesn't recognize

// from 2019
ccls -index ~/Dev/Linux -init='{"clang":{"excludeArgs":[
"-falign-jumps=1","-falign-loops=1","-fconserve-stack","-fmerge-constants","-fno-code-hoisting","-fno-schedule-insns","-fno-sched-pressure","-fno-var-tracking-assignments","-fsched-pressure",
"-mhard-float","-mindirect-branch-register","-mindirect-branch=thunk-inline","-mpreferred-stack-boundary=2","-mpreferred-stack-boundary=3","-mpreferred-stack-boundary=4","-mrecord-mcount","-mindirect-branch=thunk-extern","-mno-fp-ret-in-387","-mskip-rax-setup",
"--param=allow-store-data-races=0","-Wa,arch/x86/kernel/macros.s","-Wa,-"
], "extraArgs":["--gcc-toolchain=/usr"]}}'

Note: for options controlling individual optimization behaviors, there is a large probability that they may not make sense for Clang since the two compilers' internals are so different.
Users and projects should learn to not add GCC optimization options for Clang uses.

This revision now requires changes to proceed.May 18 2023, 8:50 PM

Note: for options controlling individual optimization behaviors, there is a large probability that they may not make sense for Clang since the two compilers' internals are so different.
Users and projects should learn to not add GCC optimization options for Clang uses.

If this is the case, can we at least add a CMake option to disable LLVM from enabling this option against the user's will? As it stands https://reviews.llvm.org/rG47f5c54f997a59bb2c65abe6b8b811f6e7553456 represents a significant regression in usability. As I understand, the previous patch only had an issue with LTO builds, so it should be perfectly reasonable for users to at least disable this at the LLVM level if they do not with the LLVM build to insert an incompatible flag into their compilation database.

Note: for options controlling individual optimization behaviors, there is a large probability that they may not make sense for Clang since the two compilers' internals are so different.
Users and projects should learn to not add GCC optimization options for Clang uses.

If this is the case, can we at least add a CMake option to disable LLVM from enabling this option against the user's will? As it stands https://reviews.llvm.org/rG47f5c54f997a59bb2c65abe6b8b811f6e7553456 represents a significant regression in usability. As I understand, the previous patch only had an issue with LTO builds, so it should be perfectly reasonable for users to at least disable this at the LLVM level if they do not with the LLVM build to insert an incompatible flag into their compilation database.

I am fine with the CMake work if someone wants to do it..