This is an archive of the discontinued LLVM Phabricator instance.

[CUDA] Emit deferred diagnostics during Sema rather than during codegen.
ClosedPublic

Authored by jlebar on Oct 12 2016, 6:42 PM.

Details

Summary

Emitting deferred diagnostics during codegen was a hack. It did work,
but usability was poor, both for us as compiler devs and for users. We
don't codegen if there are any sema errors, so for users this meant that
they wouldn't see deferred errors if there were any non-deferred errors.
For devs, this meant that we had to carefully split up our tests so that
when we tested deferred errors, we didn't emit any non-deferred errors.

This change moves checking for deferred errors into Sema. See the big
comment in SemaCUDA.cpp for an overview of the idea.

This checking adds overhead to compilation, because we have to maintain
a partial call graph. As a result, this change makes deferred errors a
CUDA-only concept (whereas before they were a general concept). If
anyone else wants to use this framework for something other than CUDA,
we can generalize at that time.

This patch makes the minimal set of test changes -- after this lands,
I'll go back through and do a cleanup of the tests that we no longer
have to split up.

Diff Detail

Repository
rL LLVM

Event Timeline

jlebar updated this revision to Diff 74466.Oct 12 2016, 6:42 PM
jlebar retitled this revision from to [CUDA] Emit deferred diagnostics during Sema rather than during codegen..
jlebar updated this object.
jlebar added a reviewer: rnk.
jlebar added subscribers: tra, rsmith, cfe-commits.
rnk edited edge metadata.Oct 13 2016, 9:58 AM

Nice! Looks like this wasn't too bad.

clang/lib/Sema/SemaCUDA.cpp
546 ↗(On Diff #74466)

There are two other instances of conditions equivalent to this in ASTContext::DeclMustBeEmitted. You should add a predicate like isDiscardableGVALinkage(GVALinkage) to Linkage.h and call it in DeclMustBeEmitted.

654 ↗(On Diff #74466)

This is iterating a DenseSet of pointers, so it'll be non-determinstic. Use SetVector to get a determinstically ordered set.

jlebar marked 2 inline comments as done.Oct 13 2016, 11:42 AM
In D25541#569360, @rnk wrote:

Nice! Looks like this wasn't too bad.

Like many things in my life lately, it wasn't after Richard explained to me how to do it. :)

Thank you for the review.

clang/lib/Sema/SemaCUDA.cpp
546 ↗(On Diff #74466)

Sure, let me do that in a separate patch, https://reviews.llvm.org/D25571.

rnk accepted this revision.Oct 13 2016, 1:45 PM
rnk edited edge metadata.

lgtm

This revision is now accepted and ready to land.Oct 13 2016, 1:45 PM
This revision was automatically updated to reflect the committed changes.
jlebar marked an inline comment as done.