jhuber6 added inline comments.
================
Comment at: clang/lib/Driver/Driver.cpp:4396-4398
+ bool SingleDeviceOutput = !llvm::any_of(OffloadActions, [](Action *A) {
+ return A->getType() == types::TY_Nothing;
+ }) && isa<CompileJobAction>(HostAction);
----------------
tra wrote:
> `any_of(A->getType() == types::TY_Nothing)` looks like a rather indirect way
> to infer that we have `-fsyntax-only`. Would there be any other legitimate
> case when we'd have `Ty_Nothing` actions?
>
>
>
This is more or less how the existing pipeline handles it. We shouldn't
propagate `TY_Nothing` attributes regardless, `-fsyntax-only` just happens to
create them. So this is more correct I believe.
================
Comment at: clang/lib/Driver/Driver.cpp:4402
/*BoundArch=*/nullptr, isa<CompileJobAction>(HostAction) ? DDep : DDeps);
- return C.MakeAction<OffloadAction>(
- HDep, isa<CompileJobAction>(HostAction) ? DDep : DDeps);
+ return C.MakeAction<OffloadAction>(HDep, SingleDeviceOutput ? DDep : DDeps);
}
----------------
tra wrote:
> Assuming that we're guaranteed that all actions produce no outputs, can we
> just pass an empty array if `-fsyntax-only` is specified?
>
>
No, this is required to make the device actions be generated. The old driver
manually added device actions to the final action list, while the new driver
adds them as host dependencies. I prefer the new method as it requires no
internal state so it can just be a simple function like here.
================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4503
for (const InputInfo &I : Inputs) {
- if (&I == &Input) {
- // This is the primary input.
+ if (&I == &Input || I.getType() == types::TY_Nothing) {
+ // This is the primary input or contains nothing.
----------------
tra wrote:
> .. and if we don't propagate no-output actions, there would be no need to
> filter them here.
Without that we would need to change the logic to append to the final action
list directly, which would be an enormous hack given the current architecture
of the new driver. FWIW I think this is a valid operation regardless, this
didn't have a good failure mode in the first place, it just crashed on some
assertion later if you did it. This at least makes sure that we'll get some "no
input" error message AFAIK.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D133161/new/
https://reviews.llvm.org/D133161
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
