Re: [PATCH] D16514: Add -stop-on-failure driver option, and enable it by default for CUDA compiles.

jlebar added a comment. espindola reverted this in r260522 because of test failures in Driver/output-file-cleanup.c. The reason I didn't catch this locally is that the test is non-hermetic -- if it passed once in an objdir, this patch does not make it fail again. You have to nuke (part of)

Re: [PATCH] D16514: Add -stop-on-failure driver option, and enable it by default for CUDA compiles.

This revision was automatically updated to reflect the committed changes. Closed by commit rL260448: Bail on compilation as soon as a job fails. (authored by jlebar). Changed prior to commit: http://reviews.llvm.org/D16514?vs=47520=47525#toc Repository: rL LLVM

Re: [PATCH] D16514: Add -stop-on-failure driver option, and enable it by default for CUDA compiles.

echristo accepted this revision. echristo added a reviewer: echristo. echristo added a comment. This works for me and I can't think of anything it's going to break so LGTM. Thanks! -eric http://reviews.llvm.org/D16514 ___ cfe-commits mailing list

Re: [PATCH] D16514: Add -stop-on-failure driver option, and enable it by default for CUDA compiles.

jlebar added a comment. Okay, I see why things don't work as expected without this patch but do work for e.g. macos universal binaries. The reason is, we build a completely separate set of actions for each invocation of cc1 -- one for the host compilation, and one for each device arch. Then

Re: [PATCH] D16514: Add -stop-on-failure driver option, and enable it by default for CUDA compiles.

jlebar added a comment. Talking to echristo irl, he would like to know why we don't have this problem with mac universal binaries -- or, do we? He would like to be consistent; I'm onboard with that. http://reviews.llvm.org/D16514 ___ cfe-commits

Re: [PATCH] D16514: Add -stop-on-failure driver option, and enable it by default for CUDA compiles.

jlebar added a comment. Eric, are you OK with this going in, or do you want to consider alternatives? http://reviews.llvm.org/D16514 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D16514: Add -stop-on-failure driver option, and enable it by default for CUDA compiles.

tra added inline comments. Comment at: include/clang/Driver/Options.td:1807 @@ +1806,3 @@ +"CUDA compilation without --save-temps.">; +def nostop_on_failure : Flag<["-"], "nostop-on-failure">, Flags<[DriverOption]>; + I'd use 'no-' prefix.

Re: [PATCH] D16514: Add -stop-on-failure driver option, and enable it by default for CUDA compiles.

jlebar updated this revision to Diff 46300. jlebar marked an inline comment as done. jlebar added a comment. Address tra's review comment (rename flag). http://reviews.llvm.org/D16514 Files: include/clang/Driver/Compilation.h include/clang/Driver/Driver.h include/clang/Driver/Options.td

Re: [PATCH] D16514: Add -stop-on-failure driver option, and enable it by default for CUDA compiles.

jlebar added inline comments. Comment at: lib/Driver/Driver.cpp:650 @@ -638,3 +649,3 @@ SmallVector, 4> FailingCommands; - C.ExecuteJobs(C.getJobs(), FailingCommands); + C.ExecuteJobs(C.getJobs(), /* StopOnFailure = */ false, FailingCommands);

Re: [PATCH] D16514: Add -stop-on-failure driver option, and enable it by default for CUDA compiles.

jlebar added inline comments. Comment at: lib/Driver/Driver.cpp:652 @@ -640,3 +651,3 @@ SmallVector, 4> FailingCommands; - C.ExecuteJobs(C.getJobs(), FailingCommands); + C.ExecuteJobs(C.getJobs(), /* StopOnFailure = */ false, FailingCommands);

Re: [PATCH] D16514: Add -stop-on-failure driver option, and enable it by default for CUDA compiles.

jlebar updated this revision to Diff 46315. jlebar marked 3 inline comments as done. jlebar added a comment. Pass StopOnFailure = true when running the preprocessor after an ICE. http://reviews.llvm.org/D16514 Files: include/clang/Driver/Compilation.h include/clang/Driver/Driver.h

Re: [PATCH] D16514: Add -stop-on-failure driver option, and enable it by default for CUDA compiles.

echristo added a comment. > The other reason, which is less important, is that when you have one arch and > ptxas fails -- which, it shouldn't, but we're not good enough to catch > everything yet, and likely won't be for some time -- the error you get is > > ptxas: foo is not defined >

Re: [PATCH] D16514: Add -stop-on-failure driver option, and enable it by default for CUDA compiles.

echristo added a comment. In general it feels like keeping 2 errors might make the most sense: (Using a multiarch build rather than a cuda command line, but it should still be the same behavior for consistency) t.c: #if _NOT_ARCH4_ #error "aiee!" #endif clang -arch arch1 -arch arch2 -arch

Re: [PATCH] D16514: Add -stop-on-failure driver option, and enable it by default for CUDA compiles.

tra accepted this revision. tra added a comment. This revision is now accepted and ready to land. LGTM. Comment at: lib/Driver/Driver.cpp:652 @@ -640,3 +651,3 @@ SmallVector, 4> FailingCommands; - C.ExecuteJobs(C.getJobs(), FailingCommands);

Re: [PATCH] D16514: Add -stop-on-failure driver option, and enable it by default for CUDA compiles.

jlebar added a comment. In http://reviews.llvm.org/D16514#338631, @echristo wrote: > In general it feels like keeping 2 errors might make the most sense: > > #if _NOT_ARCH4_ > #error "aiee!" > #endif > > clang -arch arch1 -arch arch2 -arch arch3 -arch arch4 t.c > > seems like it might be nice

Re: [PATCH] D16514: Add -stop-on-failure driver option, and enable it by default for CUDA compiles.

tra added inline comments. Comment at: lib/Driver/Driver.cpp:652 @@ -640,3 +651,3 @@ SmallVector, 4> FailingCommands; - C.ExecuteJobs(C.getJobs(), FailingCommands); + C.ExecuteJobs(C.getJobs(), /* StopOnFailure = */ false, FailingCommands);

Re: [PATCH] D16514: Add -stop-on-failure driver option, and enable it by default for CUDA compiles.

jlebar added a comment. Friendly ping. http://reviews.llvm.org/D16514 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits