Page MenuHomePhabricator

Clang Interface Stubs merger plumbing for Driver
Needs ReviewPublic

Authored by plotfi on Jun 30 2019, 10:11 AM.

Details

Reviewers
compnerd
cishida

Diff Detail

Event Timeline

plotfi created this revision.Jun 30 2019, 10:11 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 30 2019, 10:11 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
plotfi updated this revision to Diff 207224.Jun 30 2019, 10:13 AM

had some accidentally not-removed stray print output and/or expletives, apologies.

plotfi updated this revision to Diff 214924.Aug 13 2019, 2:39 PM
plotfi edited the summary of this revision. (Show Details)

Updated. Much better cleaner implementation.

plotfi retitled this revision from Very early work on interface stub merger plumbing. to Clang Interface Stubs merger plumbing for Driver.Aug 13 2019, 2:40 PM

Tests?

clang/lib/Driver/Driver.cpp
3420

This isn't really a link action ...

3423

I don't understand why the offload bundler is part of the interface merge phase.

clang/lib/Driver/ToolChains/InterfaceStubs.cpp
12

Why do you need GCC_INSTALL_PREFIX? GCC doesn't have interface stubs yet AFAIK.

35

debugging left overs?

37

Hmm, this seems wrong :-). You should search for it relative to the toolchain and then fallback.

clang/lib/Driver/Types.cpp
328

How does -c -emit-interface-stubs and multiple input play together?

plotfi updated this revision to Diff 219211.Fri, Sep 6, 6:17 PM
plotfi marked 2 inline comments as done.
plotfi added a reviewer: cishida.

Adding better wiring up to llvm-ifs

plotfi marked 2 inline comments as done.Fri, Sep 6, 6:19 PM
plotfi updated this revision to Diff 219226.Sat, Sep 7, 12:49 AM
plotfi marked an inline comment as done.
cishida added inline comments.Sat, Sep 7, 6:03 PM
clang/lib/Driver/ToolChains/Clang.cpp
3679–3680

nit: no need to check ArgStr if all outcomes are the same

compnerd added inline comments.Fri, Sep 13, 4:28 PM
clang/include/clang/Driver/Phases.h
24

Trailing comma please

clang/lib/Driver/Driver.cpp
3341

Bleh, this really should be something that we should be able to determine based on the input type.

clang/lib/Driver/ToolChains/Clang.cpp
3679–3680

Why have this switch and check below at all? I think that it should just be an assertion that the string is equal to that value at the very most. L3690-L3700 is dead code. Please remove, along with L3685-L3688.

clang/lib/Driver/ToolChains/InterfaceStubs.cpp
16

Why not:

namespace tools {
namespace ifstool {
27

A ternary might be easier to read

30

const auto &Input?

clang/lib/Frontend/CompilerInvocation.cpp
1736–1737

This seems entirely unnecessary, the default and case list is identical.

plotfi updated this revision to Diff 221209.Sat, Sep 21, 11:53 PM
plotfi marked 7 inline comments as done.Sun, Sep 22, 12:00 AM
plotfi added inline comments.
clang/lib/Driver/Types.cpp
328

Dropped the code for -c. For now, to get the straight up ifs I think -cc1 can be sufficient.

There are some hairy things regarding getting the correct InputType from Driver::BuildInputs that need to be sorted first.

plotfi updated this revision to Diff 221210.Sun, Sep 22, 12:00 AM
plotfi marked an inline comment as done.

fixed comment.

plotfi marked an inline comment as done.Sun, Sep 22, 12:27 AM
plotfi added inline comments.
clang/lib/Driver/ToolChains/InterfaceStubs.cpp
16

I could do that, but it would have to be:

namespace clang {
namespace driver {
namespace tools {
namespace ifstool {
...
} // namespace ifstool
} // namespace tools
} // namespace driver
} // namespace clang