Page MenuHomePhabricator

[CUDA] propagate to CUDA sub-compilations target triple of opposite side.
ClosedPublic

Authored by tra on Sep 24 2015, 1:04 PM.

Details

Summary

Propagates AuxTriple throughout job pipeline construction in driver and on to Tool::ConstructJob().
It in turn passes target triple of opposite side of CUDA compilation as -aux-triple option to sub-compilations.

Diff Detail

Repository
rL LLVM

Event Timeline

tra updated this revision to Diff 35658.Sep 24 2015, 1:04 PM
tra retitled this revision from to [CUDA] propagate to CUDA sub-compilations target triple of opposite side..
tra updated this object.
tra added a reviewer: echristo.
tra added a subscriber: cfe-commits.
echristo edited edge metadata.Oct 21 2015, 12:39 PM

Would this make more sense to put in the Compilation class rather than on the Action? It seems like it could go next to DefaultToolchain or could even be a separate toolchain itself so that all of the bits could be looked up?

Thanks!

-eric

tra updated this revision to Diff 38269.Oct 23 2015, 3:25 PM
tra edited edge metadata.

Instead of passing AuxTriple around as an argument, store ToolChain info in Compilation and retrieve it from there.

Some inline comments for discussion.

Thanks!

-eric

lib/Driver/Driver.cpp
503 ↗(On Diff #38269)

Can pass one or the other here? I don't think you need both a reference to C and one of its members?

1285–1291 ↗(On Diff #38269)

Can't you now do all of this in BuildCompilation?

tra added inline comments.Oct 28 2015, 4:05 PM
lib/Driver/Driver.cpp
503 ↗(On Diff #38269)

I can fold getArgs and getActions for BuildUniversalActions(), but not toolchain as it's called with different ones.

1285–1291 ↗(On Diff #38269)

I could, but i'd prefer to avoid putting hardcoded cuda logic into otherwise generic BuildCompilation for no good reason.

Here is where we start creating device-side actions and have all relevant info for that so it appears to be a sensible place to pick an appropriate toolchain for that as well.

When we have better support for multiple compilation targets toolchain selection would probably be more generic and would migrate towards BuildCompilation, but we're not there yet. :-)

tra updated this revision to Diff 38694.Oct 28 2015, 4:14 PM

Folded some arguments of BuildUniversalActions.

tra updated this revision to Diff 39017.Nov 2 2015, 5:55 PM

Moved CUDA toolchain selection to Compilation.
Removed DeviceTriple info from CudaHostActions and CudaDeviceActions.
Removed few now-unnecessary parameters and code that used them.

tra updated this revision to Diff 39019.Nov 2 2015, 6:06 PM

Moved CUDA toolchain selection to Driver::BuildCompilation() where Compilation is set up.

tra marked an inline comment as done.Nov 2 2015, 6:07 PM
jingyue added a subscriber: jingyue.Nov 6 2015, 8:57 AM
echristo accepted this revision.Nov 12 2015, 2:22 PM
echristo edited edge metadata.

One inline comment then OK for now.

-eric

lib/Driver/Tools.cpp
3227–3240 ↗(On Diff #39019)

This is pretty heinous. I don't have a better way of doing it offhand, but please document this with a rather large FIXME and continue on the path to generic compilation support we've been talking about.

This revision is now accepted and ready to land.Nov 12 2015, 2:22 PM
tra updated this revision to Diff 40163.Nov 13 2015, 10:42 AM
tra edited edge metadata.

Added a FIXME note regarding figuring out which compilation pass we're constructing.
Undone the change to make getToolchain public as it's no longer accessed from outside of Driver.
Converted the if(AuxToolchain) into assert.

This revision was automatically updated to reflect the committed changes.