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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to