Page MenuHomePhabricator

[Tooling] Allow -flto flags and filter out -Wa, flags
ClosedPublic

Authored by chh on Aug 20 2018, 3:15 PM.

Details

Summary

This change fixes the problem in https://bugs.llvm.org/show_bug.cgi?id=38332
by allowing driver::Action::BackendJobClass to run with the analyzer.
Otherwise, such jobs will look up the non-existing compilation database and then run without flags.
Also filter out the -Wa,* flags that could be passed to and ignored by the clang compiler.
Clang-tidy gives warnings about unused -Wa,* flags.

Diff Detail

Repository
rL LLVM

Event Timeline

chh created this revision.Aug 20 2018, 3:15 PM
chh added a subscriber: chapuni.Aug 20 2018, 3:20 PM
chh added subscribers: alexfh, djasper, klimek, pirama.
chh added a reviewer: klimek.
chh removed a subscriber: klimek.
ioeric added inline comments.
lib/Tooling/CompilationDatabase.cpp
304 ↗(On Diff #161578)

It seems to me that BackendJob will always be preceded by a CompileJob (according to https://github.com/llvm-mirror/clang/blob/master/lib/Driver/Driver.cpp#L3506), so at least of one job would be CompileJobClass here. Not sure if I understood it correctly though. Would you mind clarify?

chh added a comment.Aug 21 2018, 1:17 PM

I tested with clang-tidy test.cpp -- -c -Iinc -flto,
and found that in Driver.cpp:getTool Inputs has one action equal to
BackendJobClass (without -flto) or CompileJobClass (with -flto).

When in CompilationDatabase:stripPositionalArgs, the Jobs has only one
AssembleJobClass (without -flto) or BackendJobClass (with -flto).
It is confusing to change the Backend/Compile to Assemble/Backend.
However, I have not seen multiple jobs yet.
Do you have a test case that could trigger multiple jobs with or without -flto?

srhines added inline comments.Aug 21 2018, 3:12 PM
lib/Tooling/CompilationDatabase.cpp
304 ↗(On Diff #161578)

Is the difference here because clang driver != clang-tidy driver?

chh added inline comments.Aug 21 2018, 6:00 PM
lib/Tooling/CompilationDatabase.cpp
304 ↗(On Diff #161578)

The drivers are different but share some logic. This part is not used by clang driver.
For clang-tidy and clang-check, it looks like trying to skip Link job, but to handle Assemble, Compile, and Backend jobs.

ioeric accepted this revision.Aug 22 2018, 2:29 AM
In D51002#1208152, @chh wrote:

I tested with clang-tidy test.cpp -- -c -Iinc -flto,
and found that in Driver.cpp:getTool Inputs has one action equal to
BackendJobClass (without -flto) or CompileJobClass (with -flto).

When in CompilationDatabase:stripPositionalArgs, the Jobs has only one
AssembleJobClass (without -flto) or BackendJobClass (with -flto).
It is confusing to change the Backend/Compile to Assemble/Backend.
However, I have not seen multiple jobs yet.
Do you have a test case that could trigger multiple jobs with or without -flto?

Not really. I'm not familiar with backend jobs and just wanted to ask for clarification. Thanks!

lib/Tooling/CompilationDatabase.cpp
172 ↗(On Diff #161578)

Do we also need to something for BackendJob here?

This revision is now accepted and ready to land.Aug 22 2018, 2:29 AM
This revision was automatically updated to reflect the committed changes.