[PATCH] D68587: [hip] Assume host-only compilation if the final phase is ahead of `backend`.

Mon Oct 7 14:41:55 PDT 2019

tra added a comment.

In D68587#1698102 <https://reviews.llvm.org/D68587#1698102>, @hliao wrote:

> for most compilation tools, single input and single output are expected. Without assuming `-fsyntax-only` alone is host-compilation only, that at least run syntax checking twice.

I believe the driver will not run subsequent jobs if one of the device compilations fails. You may see duplicate warnings from multiple stages, but overall the error handling works in a fairly predictable way now.

> The result may be misleading

Potentially repeated warning are still *correct*, while omitting an error is not, IMO.  I believe that did come up before and we had to make some changes to the driver to keep host compilation working even when device-side compilations produce no output.

To think of it, I'm starting to doubt that this patch is an improvement for `-M` either. You will get the dependencies for host, but they are not necessarily the same as the dependencies for the device-side compilation. Producing partial list of dependencies will be potentially incorrect. IMO we do need dependencies info from all sub-compilations.

Perhaps we should limit the scope of this patch to -E only for now?

> and there are clang-based tools (like clang-tidy) may have no legacy way to be runnable.

Tooling does get surprised by the multiple jobs created by CUDA compilation. 
The work around is to pass `--cuda-host-only`. Passing an extra flag is usually not a showstopper (at least it was not in cases I had to deal with at work and we have fair number of clang-derived tools). Usually in order to get correct CUDA compilation in this scenario you will also need to tell the tool where to find CUDA's headers, so the mechanism for passing additional options already exists.

If that's still a problem, then we should change the tooling infrastructure to use host-only compilation for HIP and CUDA by default.

> To check device-side compilation syntax, we are still able to explicitly ask that by specifying `--cuda-device-only`.

Yes, that could be done. However, as I've mentioned above the same argument applies to tooling & `--cuda-host-only`, so there's no advantage here. IMO the most common use case should be the default, so it's the clang itself which should remain working correctly without having to add extra flags.

Also, this patch makes it impossible to run -fsyntax-only on *all* sub-compilations at once. I will have to run `clang -fsyntax-only` multiple times -- once per host and once per each device. 
I do want to check all sub-compilations with `-fsyntax-only` on a regular basis (e.g. when running creduce on a cuda source), so having to do that for each sub-compilation separately does not look like an improvement to me.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68587/new/

https://reviews.llvm.org/D68587