tra 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 ---------------- 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 https://reviews.llvm.org/D39502 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
