[PATCH] D84364: [CUDA][HIP] Defer all diagnostics for host device functions

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl marked 4 inline comments as done.
yaxunl added a comment.

In D84364#2170769 <https://reviews.llvm.org/D84364#2170769>, @tra wrote:

> In D84364#2170244 <https://reviews.llvm.org/D84364#2170244>, @tra wrote:
>
> > I'm going to try the patch on our CUDA code and see how it fares. Stay tuned.
>
>
> The patch works well enough to build TensorFlow which is a good sign that existing working code should be OK -- we're making HD checks less strict, so already working code should not be affected.
>
> The remaining concern is whether we're going to unintentionally allow something we should not have. Let me see if I can come up with some examples, in addition to the `hdf_not_called()`one I've commented on in the test file.


I added a `Deferrable` bit to the diagnostics which can be specified in td files. This can be added to individual diagnostic defs or added to a bunch of diagnostic defs all together.

This field is used to control whether a diagnostic message can be deferred.

For now I enabled this bit for the overloading resolution diagnostics since these have been seen as false alarms in host device functions. We could let more diagnostics be deferrable if we see it is necessary.



================
Comment at: clang/test/SemaCUDA/deferred-all.cu:12-18
+// Check no diagnostics for this function since it is never
+// called.
+
+inline __host__ __device__ void hdf_not_called() {
+  callee(1);
+  bad_line
+}
----------------
tra wrote:
> I think we do need diagnostics in this scenario, regardless of whether the function is called or not.
> We would have emitted it for a regular inline function and I believe it should be the case here, too.
Limited deferred diagnostics to a set of specified diagnostics in .td files. 


================
Comment at: clang/test/SemaCUDA/deferred-all.cu:44
+
+struct A { int x; typedef int type; };
+struct B { int x; };
----------------
tra wrote:
> Nit: I'd rename `type` -> `isA` which would make its use in sfinae() more obvious.
done


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

https://reviews.llvm.org/D84364