This is an archive of the discontinued LLVM Phabricator instance.

[cuda] Include GPU binary into host object file and generate init/deinit code.
ClosedPublic

Authored by tra on May 5 2015, 3:46 PM.

Details

Summary

Extracted from D8463 to separate codegen and driver changes.

  • added -fcuda-include-gpubinary option to incorporate results of device-side compilation into host-side one.
  • generate code to register GPU binaries and associated kernels with CUDA runtime and clean-up on exit.
  • added test cases for init/deinit code generation.

    Complete patch and review are here: http://reviews.llvm.org/D8463

Diff Detail

Repository
rL LLVM

Event Timeline

tra updated this revision to Diff 24990.May 5 2015, 3:46 PM
tra retitled this revision from to [cuda] Include GPU binary into host object file and generate init/deinit code..
tra updated this object.
tra edited the test plan for this revision. (Show Details)
tra added reviewers: eliben, echristo.
tra added a subscriber: Unknown Object (MLST).
eliben accepted this revision.May 6 2015, 8:44 AM
eliben edited edge metadata.

LGTM

[assuming this just copies stuff from D8463 w/o modifications]

lib/CodeGen/CGCUDANV.cpp
43 ↗(On Diff #24990)

Nit: period (or semicolon) after "binaries"?

This revision is now accepted and ready to land.May 6 2015, 8:44 AM
echristo edited edge metadata.May 6 2015, 11:22 AM

Can you make all of the newly added function names lower case please? (Fixing the existing names in the CUDA stuff is preapproved).

Some more inline comments/nits.

-eric

lib/CodeGen/CGCUDANV.cpp
23 ↗(On Diff #24990)

Unused include?

240 ↗(On Diff #24990)

Comment this loop please?

test/CodeGenCUDA/device-stub.cu
5 ↗(On Diff #24990)

Reword this in some way? It's a bit oddly phrased :)

29 ↗(On Diff #24990)

"Test that we"

tra updated this revision to Diff 25078.May 6 2015, 11:55 AM
tra updated this object.
tra edited edge metadata.

Addressed echristo's comments:

  • fixed comments.
  • renamed functions according to style guide.
tra added inline comments.May 6 2015, 12:35 PM
lib/CodeGen/CGCUDANV.cpp
23 ↗(On Diff #24990)

Verifier.h is used and needed, but vector was indeed superfluous.

43 ↗(On Diff #24990)

Done.

240 ↗(On Diff #24990)

Done.

test/CodeGenCUDA/device-stub.cu
5 ↗(On Diff #24990)

Done.

29 ↗(On Diff #24990)

Fixed.

Yeah, we don't really want to run the verifier on every compile. Very slow. :)

-eric

lib/CodeGen/CGCUDANV.cpp
23 ↗(On Diff #25078)

We don't actually want to run the verifier on every compilation :)

tra updated this revision to Diff 25083.May 6 2015, 1:46 PM

Removed verifier.

echristo accepted this revision.May 6 2015, 1:53 PM
echristo edited edge metadata.

One inline comment and can you go ahead and commit the emitDeviceStubBody rename sparately please.

Thanks!

-eric

lib/CodeGen/CGCUDARuntime.h
47 ↗(On Diff #25083)

Missed a rename :)

tra updated this revision to Diff 25093.May 6 2015, 3:09 PM
tra edited edge metadata.

Removed the comment left over from early version of the patch.

tra added inline comments.May 6 2015, 3:13 PM
lib/CodeGen/CGCUDARuntime.h
47 ↗(On Diff #25083)

It was actually a leftover from the early version of the changes. EmitDeviceStubBody no longer exists in this class.

This revision was automatically updated to reflect the committed changes.