r261774 - Bail on compilation as soon as a job fails.

Steven Wu via cfe-commits cfe-commits at lists.llvm.org
Mon May 15 15:26:04 PDT 2017


> On May 15, 2017, at 3:03 PM, Nico Weber <thakis at chromium.org> wrote:
> 
> test/Driver/cl-pch-errorhandling.cpp tests the clang-cl pch bits.

I don't think reverting r261774 will break any cl-pch tests (didn't try windows). That is why I wondering if there is any test case for CUDA so I don't break them when fixing UNIX conformance.

Steven

> 
> On Mon, May 15, 2017 at 5:42 PM, Steven Wu <stevenwu at apple.com <mailto:stevenwu at apple.com>> wrote:
> The other option is to make this behavior configurable so that clang on UNIX behaves differently than clang-cl or CUDA. I am not sure what problem CUDA is hitting. Is there a test case for that?
> 
> Steven
> 
>> On May 15, 2017, at 12:42 PM, Nico Weber <thakis at chromium.org <mailto:thakis at chromium.org>> wrote:
>> 
>> r262420 landed in a way adapted to Justin's change. An earlier version of https://reviews.llvm.org/D17695 <https://reviews.llvm.org/D17695> had a different approach. I don't know if that approach could be used for CUDA. Justin should reply here.
>> 
>> On Mon, May 15, 2017 at 1:32 PM, Steven Wu <stevenwu at apple.com <mailto:stevenwu at apple.com>> wrote:
>> Hi Nico
>> 
>> Now that r262420 is landed. Is there any plan to move CUDA to the new approach so we can fix the UNIX conformance test?
>> 
>> Thanks
>> 
>> Steven
>> 
>>> On Apr 22, 2017, at 8:08 PM, Nico Weber via cfe-commits <cfe-commits at lists.llvm.org <mailto:cfe-commits at lists.llvm.org>> wrote:
>>> 
>>> On Sat, Apr 22, 2017 at 8:40 PM, Duncan P. N. Exon Smith via cfe-commits <cfe-commits at lists.llvm.org <mailto:cfe-commits at lists.llvm.org>> wrote:
>>> [Some necromancy here...]
>>> 
>>> This commit effectively reverted r173361 and r173825, breaking UNIX conformance for our c99 wrapper.
>>> 
>>> See:
>>> http://pubs.opengroup.org/onlinepubs/9699919799/utilities/c99.html <http://pubs.opengroup.org/onlinepubs/9699919799/utilities/c99.html>
>>>> When c99 encounters a compilation error that causes an object file not to be created, it shall write a diagnostic to standard error and continue to compile other source code operands, but it shall not perform the link phase and it shall return a non-zero exit status.
>>> 
>>> 
>>> We had a test, but this commit changed that as well (I suppose it could have been better documented).
>>> 
>>> How easily could this be restricted to only affect CUDA jobs?
>>> 
>>> If this gets reverted, the clang-cl PCH code needs to use the approach in https://reviews.llvm.org/D17695 <https://reviews.llvm.org/D17695> before I rebased that patch over this revision here, so that a PCH compilation failure stops the compilation of the main TU.
>>>  
>>> 
>>>> On Feb 24, 2016, at 13:49, Justin Lebar via cfe-commits <cfe-commits at lists.llvm.org <mailto:cfe-commits at lists.llvm.org>> wrote:
>>>> 
>>>> Author: jlebar
>>>> Date: Wed Feb 24 15:49:28 2016
>>>> New Revision: 261774
>>>> 
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=261774&view=rev <http://llvm.org/viewvc/llvm-project?rev=261774&view=rev>
>>>> Log:
>>>> Bail on compilation as soon as a job fails.
>>>> 
>>>> Summary:
>>>> (Re-land of r260448, which was reverted in r260522 due to a test failure
>>>> in Driver/output-file-cleanup.c that only showed up in fresh builds.)
>>>> 
>>>> Previously we attempted to be smart; if one job failed, we'd run all
>>>> jobs that didn't depend on the failing job.
>>>> 
>>>> Problem is, this doesn't work well for e.g. CUDA compilation without
>>>> -save-temps.  In this case, the device-side and host-side Assemble
>>>> actions (which actually are responsible for preprocess, compile,
>>>> backend, and assemble, since we're not saving temps) are necessarily
>>>> distinct.  So our clever heuristic doesn't help us, and we repeat every
>>>> error message once for host and once for each device arch.
>>>> 
>>>> The main effect of this change, other than fixing CUDA, is that if you
>>>> pass multiple cc files to one instance of clang and you get a compile
>>>> error, we'll stop when the first cc1 job fails.
>>>> 
>>>> Reviewers: echristo
>>>> 
>>>> Subscribers: cfe-commits, jhen, echristo, tra, rafael
>>>> 
>>>> Differential Revision: http://reviews.llvm.org/D17217 <http://reviews.llvm.org/D17217>
>>>> 
>>>> Modified:
>>>>    cfe/trunk/lib/Driver/Compilation.cpp
>>>>    cfe/trunk/test/Driver/output-file-cleanup.c
>>>> 
>>>> Modified: cfe/trunk/lib/Driver/Compilation.cpp
>>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Compilation.cpp?rev=261774&r1=261773&r2=261774&view=diff <http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Compilation.cpp?rev=261774&r1=261773&r2=261774&view=diff>
>>>> ==============================================================================
>>>> --- cfe/trunk/lib/Driver/Compilation.cpp (original)
>>>> +++ cfe/trunk/lib/Driver/Compilation.cpp Wed Feb 24 15:49:28 2016
>>>> @@ -163,39 +163,17 @@ int Compilation::ExecuteCommand(const Co
>>>>   return ExecutionFailed ? 1 : Res;
>>>> }
>>>> 
>>>> -typedef SmallVectorImpl< std::pair<int, const Command *> > FailingCommandList;
>>>> -
>>>> -static bool ActionFailed(const Action *A,
>>>> -                         const FailingCommandList &FailingCommands) {
>>>> -
>>>> -  if (FailingCommands.empty())
>>>> -    return false;
>>>> -
>>>> -  for (FailingCommandList::const_iterator CI = FailingCommands.begin(),
>>>> -         CE = FailingCommands.end(); CI != CE; ++CI)
>>>> -    if (A == &(CI->second->getSource()))
>>>> -      return true;
>>>> -
>>>> -  for (const Action *AI : A->inputs())
>>>> -    if (ActionFailed(AI, FailingCommands))
>>>> -      return true;
>>>> -
>>>> -  return false;
>>>> -}
>>>> -
>>>> -static bool InputsOk(const Command &C,
>>>> -                     const FailingCommandList &FailingCommands) {
>>>> -  return !ActionFailed(&C.getSource(), FailingCommands);
>>>> -}
>>>> -
>>>> -void Compilation::ExecuteJobs(const JobList &Jobs,
>>>> -                              FailingCommandList &FailingCommands) const {
>>>> +void Compilation::ExecuteJobs(
>>>> +    const JobList &Jobs,
>>>> +    SmallVectorImpl<std::pair<int, const Command *>> &FailingCommands) const {
>>>>   for (const auto &Job : Jobs) {
>>>> -    if (!InputsOk(Job, FailingCommands))
>>>> -      continue;
>>>>     const Command *FailingCommand = nullptr;
>>>> -    if (int Res = ExecuteCommand(Job, FailingCommand))
>>>> +    if (int Res = ExecuteCommand(Job, FailingCommand)) {
>>>>       FailingCommands.push_back(std::make_pair(Res, FailingCommand));
>>>> +      // Bail as soon as one command fails, so we don't output duplicate error
>>>> +      // messages if we die on e.g. the same file.
>>>> +      return;
>>>> +    }
>>>>   }
>>>> }
>>>> 
>>>> 
>>>> Modified: cfe/trunk/test/Driver/output-file-cleanup.c
>>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/output-file-cleanup.c?rev=261774&r1=261773&r2=261774&view=diff <http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/output-file-cleanup.c?rev=261774&r1=261773&r2=261774&view=diff>
>>>> ==============================================================================
>>>> --- cfe/trunk/test/Driver/output-file-cleanup.c (original)
>>>> +++ cfe/trunk/test/Driver/output-file-cleanup.c Wed Feb 24 15:49:28 2016
>>>> @@ -38,6 +38,9 @@ invalid C code
>>>> // RUN: test -f %t1.s
>>>> // RUN: test ! -f %t2.s
>>>> 
>>>> +// When given multiple .c files to compile, clang compiles them in order until
>>>> +// it hits an error, at which point it stops.
>>>> +//
>>>> // RUN: touch %t1.c
>>>> // RUN: echo "invalid C code" > %t2.c
>>>> // RUN: touch %t3.c
>>>> @@ -46,6 +49,6 @@ invalid C code
>>>> // RUN: cd %T && not %clang -S %t1.c %t2.c %t3.c %t4.c %t5.c
>>>> // RUN: test -f %t1.s
>>>> // RUN: test ! -f %t2.s
>>>> -// RUN: test -f %t3.s
>>>> +// RUN: test ! -f %t3.s
>>>> // RUN: test ! -f %t4.s
>>>> -// RUN: test -f %t5.s
>>>> +// RUN: test ! -f %t5.s
>>>> 
>>>> 
>>>> _______________________________________________
>>>> cfe-commits mailing list
>>>> cfe-commits at lists.llvm.org <mailto:cfe-commits at lists.llvm.org>
>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits <http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits>
>>> 
>>> 
>>> _______________________________________________
>>> cfe-commits mailing list
>>> cfe-commits at lists.llvm.org <mailto:cfe-commits at lists.llvm.org>
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits <http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits>
>>> 
>>> 
>>> _______________________________________________
>>> cfe-commits mailing list
>>> cfe-commits at lists.llvm.org <mailto:cfe-commits at lists.llvm.org>
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits <http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits>
>> 
>> 
> 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170515/b19fab3c/attachment-0001.html>


More information about the cfe-commits mailing list