steven_wu added inline comments.

================
Comment at: test/Driver/cuda-bail-out.cu:47
+// CHECK-HOST-ERROR: Error during compilation for host
+// CHECK-HOST-ERROR-NOT: Error during compilation for sm_35
----------------
tra wrote:
> steven_wu wrote:
> > tra wrote:
> > > steven_wu wrote:
> > > > tra wrote:
> > > > > To make it more robust, I'd add another copy of the last line before 
> > > > > CHECK-HOST-ERROR: so it would catch device-side errors if they were 
> > > > > to happen ahead of the host.
> > > > > 
> > > > > Generally speaking, expected behavior is "I want to see an error from 
> > > > > one compilation only". We don't really care which CUDA compilation 
> > > > > phase produces it. Perhaps all we need in all test cases is:
> > > > > 
> > > > > ```
> > > > > CHECK: Error during compilation
> > > > > CHECK-NOT:  Error during compilation
> > > > > ```
> > > > > 
> > > > > This way we don't need to depend on specific phase order.
> > > > That will be a design choice for CUDA driver. I have no preference 
> > > > going either direction. Just let me know so I will update the test case.
> > > > 
> > > > Speaking of "-fsyntax-only", this is another interesting behavior of 
> > > > clang cuda driver:
> > > > ```
> > > > $ clang -x cuda /dev/null -x c /dev/null -ccc-print-phases
> > > > 14: input, "/dev/null", c, (host-cuda)
> > > > $ clang -fsyntax-only -x cuda /dev/null -x c /dev/null -ccc-print-phases
> > > > 9: input, "/dev/null", c
> > > > ```
> > > > So depending on if -fsyntax-only is used or not, the c language part 
> > > > can be either offloading or not offloading.
> > > > This is a corner case that the driver behavior will change after this 
> > > > patch.
> > > OK. Let's just check for one error only. 
> > > 
> > > As for the second, it is, IMO a problem. The file after ```-x c```  
> > > should have been treated as plain C input, regardless of -fsyntax-only.  
> > > It's reproducible in clean clang tree, so it's not due to this patch. 
> > The corner case I am talking about is after this patch, "-x c" will be 
> > syntax checked even if "-x cuda" failed the syntax check (which sounds 
> > good) but only when using "-fsyntax-only"
> > 
> > I am updating the test case.
> Sorry, I should've phrased my suggestion better. I didn't mean to remove 
> multiple sources of errors. Quite the opposite. There should be multiple test 
> runs with errors produced by combination of phases. E.g. host, host+sm_35, 
> host+sm_60, sm_35+sm+60, host+sm_35+sm_60.
> 
> The check itself is fine, you just need multiple runs that trigger errors 
> during particular phase.
> 
> ```
> #if defined(ERROR_HOST) && !defined(__CUDA_ARCH__)
> #error Host error
> #endif
> 
> #if defined(ERROR_SM35) && (__CUDA_ARCH__ == 350)
> #error sm_35 error
> #endif
> ...
> 
> ```
> 
> and then do multiple runs with various combinations of -DERROR_xxx
> 
Hmm, I might still understand you incorrectly. If the error message is 
different for host and different devices, checking the error message requires 
the compilation being executed in a certain order. Otherwise, you won't know 
which error is going to be emitted.
On the other hand, if you don't care about the order and don't want to check 
for any ordering, the best we can do is run all the combinations and check the 
error only happens once. In my current test, I have host + default_sm, host + 
sm_35 + sim_60. I can add --cude-device-only but is there anything else I need 
to do?


https://reviews.llvm.org/D39502



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

Reply via email to