This is an archive of the discontinued LLVM Phabricator instance.

[CUDA] Mark device functions as nounwind.
ClosedPublic

Authored by jlebar on Oct 2 2016, 4:03 PM.

Details

Summary

This prevents clang from emitting 'invoke's and catch statements.

Things previously mostly worked thanks to TryToMarkNoThrow() in
CodeGenFunction. But this is not a proper IPO, and it doesn't properly
handle cases like mutual recursion.

Fixes bug 30593.

Diff Detail

Repository
rL LLVM

Event Timeline

jlebar updated this revision to Diff 73222.Oct 2 2016, 4:03 PM
jlebar retitled this revision from to [CUDA] Mark device functions as nounwind..
jlebar updated this object.
jlebar added a reviewer: tra.
jlebar added a subscriber: cfe-commits.
rnk added a subscriber: rnk.Oct 3 2016, 8:49 AM

It feels like the right thing is to disable EH in device side compilation, but obviously that won't work because it would reject try/throw in host code. I think instead of doing that, we should make sure that CUDA diagnoses try / catch / throw in device functions, and then do what you've done here.

Also, take a look at CodeGenFunction::getInvokeDestImpl(). I think you should add some checks in there to return nullptr if we're doing a device-side CUDA compilation. That's a much more direct way to ensure we never generate invokes on the device side.

clang/lib/Sema/SemaDecl.cpp
12082 ↗(On Diff #73222)

Can we use a noexcept exception specification instead of this GCC attribute?

jlebar added a comment.Oct 3 2016, 9:19 AM
In D25166#559117, @rnk wrote:

It feels like the right thing is to disable EH in device side compilation, but obviously that won't work because it would reject try/throw in host code.

Exactly.

I think instead of doing that, we should make sure that CUDA diagnoses try / catch / throw in device functions, > and then do what you've done here.

Disallowing try/throw is https://reviews.llvm.org/D25036.

Also, take a look at CodeGenFunction::getInvokeDestImpl(). I think you should add some checks in there to return nullptr if we're doing a device-side CUDA compilation. That's a much more direct way to ensure we never generate invokes on the device side.

Hm...if we did this but didn't mark functions as "this never throws", would that really take care of everything? For example, if we were to call an external function which is itself not marked as noexcept, llvm wouldn't be able to infer that we don't throw.

Are you saying we should do both? I'm happy to do that, but I am not sure, if we keep the nounwind attribute in, whether it's possible to write a testcase.

Can we use a noexcept exception specification instead of this GCC attribute?

I have no attachment to this specific attribute, but wouldn't adding noexcept change the semantics of the program (because traits can detect whether or not noexcept is present)?

rnk added a comment.Oct 3 2016, 11:21 AM

Also, take a look at CodeGenFunction::getInvokeDestImpl(). I think you should add some checks in there to return nullptr if we're doing a device-side CUDA compilation. That's a much more direct way to ensure we never generate invokes on the device side.

Hm...if we did this but didn't mark functions as "this never throws", would that really take care of everything? For example, if we were to call an external function which is itself not marked as noexcept, llvm wouldn't be able to infer that we don't throw.

Are you saying we should do both? I'm happy to do that, but I am not sure, if we keep the nounwind attribute in, whether it's possible to write a testcase.

I think you want both: you want all device CUDA function definitions (and declarations!) to be marked nounwind to help the optimizer, but you also want to directly make sure clang codegen knows that exceptions are turned off on the device side by changing getInvokeDestImpl. It's essentially the key place we check LangOpts.Exceptions, so if we can't turn that off, then we probably want to inject our CUDA device checks there.

Can we use a noexcept exception specification instead of this GCC attribute?

I have no attachment to this specific attribute, but wouldn't adding noexcept change the semantics of the program (because traits can detect whether or not noexcept is present)?

Hm, yeah, that's no good because then you'll have host vs. device conflicts in those traits. This should probably be a CodeGen-only change anyway, so you might as well add nounwind to function declarations in CGCall.cpp.

jlebar updated this revision to Diff 73577.Oct 4 2016, 4:29 PM

Move everything into codegen.

rnk accepted this revision.Oct 4 2016, 4:38 PM
rnk added a reviewer: rnk.

lgtm

clang/test/CodeGenCUDA/nothrow.cu
16 ↗(On Diff #73577)

I would check for call void @_Z1fv here, and make foo noexcept. The noexcept is useful because it forces us to essentially emit try { ... } catch (...) { std::terminate(); } around the whole function, but we probably allow noexcept.

This revision is now accepted and ready to land.Oct 4 2016, 4:38 PM
jlebar updated this revision to Diff 73579.Oct 4 2016, 4:43 PM
jlebar marked an inline comment as done.
jlebar edited edge metadata.

Update tests.

This revision was automatically updated to reflect the committed changes.