[LLVM] Automatically strip -fno-lifetime-dse from compile_commands.json

[jhuber6]

Summary:
This flag is always passed by LLVM to bypass some LTO related bug with
GCC. The problem is that this is not supported by clang and shows up in
the compile_commands.json file. That means it will always show at least
one error complaining about this file. Since I already have a script to
merge the runtimes stuff, just strip this out there for convenience.

There was a discussion a long time ago about not adding this as a no-op
flag to clang, so this is the next best thing.

[github-actions]

✅ With the latest revision this PR passed the Python code formatter.

[jplehr]

LGTM, though I don't feel confident enough to accept.

[Meinersbur]

If I understand correctly, you are compiling LLVM with gcc, so the compile_commands.json will have entries of the form

"command": "/usr/bin/g++ ... -fno-lifetime-dse
"

Seems legitimate to me. I would want the file to accuratelty represent the command line the files were compiled with.

[petrhosek]

I don't understand the issue, the command field for each file in the compilation database should include a full compiler invocation, so if you merge compilation database from both the LLVM and runtime builds, so you should end up with something like:

[
  ...
  {
    "command": "/usr/bin/g++ ... -fno-lifetime-dse",
    ...
  },
  ...
  {
    "command": "/path/to/build/bin/clang ...",
    ...
  },
  ...
]

It sounds like you also end up with /path/to/build/bin/clang ... -fno-lifetime-dse entries in the database. Where do those come from?

[jhuber6]

I don't understand the issue, the command field for each file in the compilation database should include a full compiler invocation, so if you merge compilation database from both the LLVM and runtime builds, so you should end up with something like:

[
  ...
  {
    "command": "/usr/bin/g++ ... -fno-lifetime-dse",
    ...
  },
  ...
  {
    "command": "/path/to/build/bin/clang ...",
    ...
  },
  ...
]

It sounds like you also end up with /path/to/build/bin/clang ... -fno-lifetime-dse entries in the database. Where do those come from?

They're added by the LLVM build itself, you cannot turn them off. It was done in the past as a fix for some GNU LTO bug.

llvm-project/llvm/cmake/modules/HandleLLVMOptions.cmake

Line 698 in 3812189

append("-fno-lifetime-dse" CMAKE_C_FLAGS CMAKE_CXX_FLAGS)

.

I suppose in my case I could just force building with system clang all the time instead, but it's annoying in general.

[petrhosek]

I see, so if I understand correctly this has nothing to do with compilation database merging. Rather the issue is when we build with GCC, the LLVM build would set -fno-lifetime-dse but if you then try to use the generated compilation databse with a Clang-based tool like clangd, you'll get an error because the flag is unsupported. Is that correct? If so I don't think this is the right solution, rather I think we should accept and ignore the flag in the Clang driver. Do you have a link to the past discussion?

[MaskRay]

I believe that the past discussion is that we don't take as a ground to add an ignored option to clangDriver.

clangDriver would become really messy if we add such GCC optimization specific options to Clang.
I am fine with adding this to convenience scripts, not user-facing products.

Perhaps language servers could strict such -f options. Most don't change indexing or completion behaviors anyway.

[Meinersbur]

Just encountered the problem with CMAKE_CXX_INCLUDE_WHAT_YOU_USE=include-what-you-use. include-what-you-use use clang internally, so when using gcc as main compiler, the LLVM build system will add -fno-lifetime-dse which is rejected by clang/iwyu. This is independent of compile_commands.json.

[jhuber6]

Was there ever an issue to track the original bug that this flag works around?

[nikic]

Was there ever an issue to track the original bug that this flag works around?

The issue is #24952.

Fixing it probably needs something along the lines of #105714, but for operator delete instead of operator new.

[jhuber6]

Ping.

[jhuber6]

Ping

[jhuber6]

Ping

[MaskRay]

@thesamesam Is it feasible to add -fno-lifetime-dse (currently for all GCC builds
47f5c54) only when GCC-specific -flto is used?

I don't think any developer uses GCC LTO for compile_commands.json. So preventing -fno-lifetime-dse for non-LTO gcc builds suffices.

Then we don't need a change to this Python script.

[jhuber6]

@thesamesam Is it feasible to add -fno-lifetime-dse (currently for all GCC builds 47f5c54) only when GCC-specific -flto is used?

I don't think any developer uses GCC LTO for compile_commands.json. So preventing -fno-lifetime-dse for non-LTO gcc builds suffices.

Then we don't need a change to this Python script.

I would heavily prefer that solution.

[thesamesam]

It's not ideal (as the issue could arise without LTO) but I'm fine with it as a compromise. It's also an easy thing for us to investigate if an issue pops up building with GCC again.