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

[Nico Weber via cfe-commits]

On Sat, Apr 22, 2017 at 8:40 PM, Duncan P. N. Exon Smith via cfe-commits <
cfe-commits@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
>
> 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 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@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=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
>
> 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=261773=261774=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 >
> FailingCommandList;
> -
> -static bool ActionFailed(const Action *A,
> - const FailingCommandList ) {
> -
> -  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 ,
> - const FailingCommandList ) {
> -  return !ActionFailed((), FailingCommands);
> -}
> -
> -void Compilation::ExecuteJobs(const JobList ,
> -  FailingCommandList ) const
> {
> +void Compilation::ExecuteJobs(
> +const JobList ,
> +SmallVectorImpl> )
> const {
>   for (const auto  : 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=261773=261774=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 

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

[Duncan P. N. Exon Smith via cfe-commits]

[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 

> 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?

> On Feb 24, 2016, at 13:49, Justin Lebar via cfe-commits 
>  wrote:
> 
> Author: jlebar
> Date: Wed Feb 24 15:49:28 2016
> New Revision: 261774
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=261774=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
> 
> 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=261773=261774=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 > 
> FailingCommandList;
> -
> -static bool ActionFailed(const Action *A,
> - const FailingCommandList ) {
> -
> -  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 ,
> - const FailingCommandList ) {
> -  return !ActionFailed((), FailingCommands);
> -}
> -
> -void Compilation::ExecuteJobs(const JobList ,
> -  FailingCommandList ) const {
> +void Compilation::ExecuteJobs(
> +const JobList ,
> +SmallVectorImpl> ) const 
> {
>   for (const auto  : 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=261773=261774=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 

[PATCH] D30465: [mips] Set the Int64Type / IntMaxType types correctly for OpenBSD/mips64

[Simon Dardis via Phabricator via cfe-commits]

sdardis added a comment.

FYI, you should close this revision manually and cite the commit 
(https://reviews.llvm.org/rL297098). Also please cite the review and all the 
reviewers when you commit. E.g. the commit message for that should be:

[mips] Set the Int64Type / IntMaxType types correctly for OpenBSD/mips64

Set the Int64Type / IntMaxType types correctly for OpenBSD/mips64.

Reviewers: sdardis

Differential Revision: https://reviews.llvm.org/D30465

Doing all this helps with auditing commits and future maintenance. Phab will 
pick up on commit messages in that form and close the review automatically. The 
arcanist tool will automate adding the necessary lines to the commit message.


Repository:
  rL LLVM

https://reviews.llvm.org/D30465



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

[PATCH] D32309: [libcxx] [test] Resolve compiler warnings in LCM/GCD tests

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal added inline comments.



Comment at: test/std/numerics/numeric.ops/numeric.ops.gcd/gcd.pass.cpp:46
+std::gcd(static_cast(0), static_cast(0)))>::value, "");
+const bool result = static_cast>(out) ==
+std::gcd(static_cast(in1), static_cast(in2));

rsmith wrote:
> Is there a reason to recompute the type here rather than using `Output`?
Hmm... I think I was concerned about it looking like the cast o the output type 
was an assertion that the return type was in fact of that type (which isn't 
true). But that should be taken care of by the previous static_asserts.


https://reviews.llvm.org/D32309



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

[PATCH] D31856: Headers: Make the type of SIZE_MAX the same as size_t

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added inline comments.



Comment at: clang/test/Headers/stdint-typeof-MINMAX.cpp:1
+// RUN: %clang_cc1 %s -ffreestanding -std=c++1z -fsyntax-only 
-triple=aarch64-none-none
+// RUN: %clang_cc1 %s -ffreestanding -std=c++1z -fsyntax-only 
-triple=arm-none-none

efriedma wrote:
> dexonsmith wrote:
> > I copied the `-ffreestanding` out of test/Preprocessor/stdint.c, but I'm 
> > wondering if it even serves a purpose here... if not, we could make the RUN 
> > lines shorter (and fit 80 columns without breaking in two).
> Without -ffreestanding, you're testing the contents of /usr/include/stdint.h 
> rather than the compiler's builtin header.
Ah; I found no difference locally because I have no /usr/include.  `-v` shows 
me:

ignoring nonexistent directory "/usr/include"

`-nostdsysteminc` has the same effect as `-ffreestanding`... but I guess I may 
as well leave it as is.


https://reviews.llvm.org/D31856



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

[PATCH] D32389: [libclang] Expose some target information via the C API.

[Emilio Cobos Álvarez via Phabricator via cfe-commits]

emilio added a comment.

I'd appreciate if anyone could point me to an appropriate reviewer for this.

Thanks in advance! :)


Repository:
  rL LLVM

https://reviews.llvm.org/D32389



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

[PATCH] D32389: [libclang] Expose some target information via the C API.

[Emilio Cobos Álvarez via Phabricator via cfe-commits]

emilio created this revision.
emilio added a project: clang-c.

This allows users to query the target triple and target pointer width, which
would make me able to fix https://github.com/servo/rust-bindgen/issues/593 and
other related bugs in an elegant way (without having to manually parse the
target triple in the command line arguments).


Repository:
  rL LLVM

https://reviews.llvm.org/D32389

Files:
  clang/include/clang-c/Index.h
  clang/test/Index/target-info.c
  clang/tools/c-index-test/c-index-test.c
  clang/tools/libclang/CIndex.cpp
  clang/tools/libclang/libclang.exports

Index: clang/tools/libclang/libclang.exports
===
--- clang/tools/libclang/libclang.exports
+++ clang/tools/libclang/libclang.exports
@@ -250,6 +250,8 @@
 clang_getTokenSpelling
 clang_getTranslationUnitCursor
 clang_getTranslationUnitSpelling
+clang_getTranslationUnitTargetTriple
+clang_getTranslationUnitTargetPointerWidth
 clang_getTypeDeclaration
 clang_getTypeKindSpelling
 clang_getTypeSpelling
Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -26,6 +26,7 @@
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/DiagnosticCategories.h"
 #include "clang/Basic/DiagnosticIDs.h"
+#include "clang/Basic/TargetInfo.h"
 #include "clang/Basic/Version.h"
 #include "clang/Frontend/ASTUnit.h"
 #include "clang/Frontend/CompilerInstance.h"
@@ -4005,6 +4006,28 @@
   return cxstring::createDup(CXXUnit->getOriginalSourceFileName());
 }
 
+CXString clang_getTranslationUnitTargetTriple(CXTranslationUnit CTUnit) {
+  if (isNotUsableTU(CTUnit)) {
+LOG_BAD_TU(CTUnit);
+return cxstring::createEmpty();
+  }
+
+  ASTUnit *CXXUnit = cxtu::getASTUnit(CTUnit);
+  std::string Triple =
+CXXUnit->getASTContext().getTargetInfo().getTriple().normalize();
+  return cxstring::createDup(Triple);
+}
+
+int clang_getTranslationUnitTargetPointerWidth(CXTranslationUnit CTUnit) {
+  if (isNotUsableTU(CTUnit)) {
+LOG_BAD_TU(CTUnit);
+return -1;
+  }
+
+  ASTUnit *CXXUnit = cxtu::getASTUnit(CTUnit);
+  return CXXUnit->getASTContext().getTargetInfo().getMaxPointerWidth();
+}
+
 CXCursor clang_getTranslationUnitCursor(CXTranslationUnit TU) {
   if (isNotUsableTU(TU)) {
 LOG_BAD_TU(TU);
Index: clang/tools/c-index-test/c-index-test.c
===
--- clang/tools/c-index-test/c-index-test.c
+++ clang/tools/c-index-test/c-index-test.c
@@ -1560,6 +1560,47 @@
 }
 
 /**/
+/* Target information testing.*/
+/**/
+
+static int print_target_info(int argc, const char **argv) {
+  CXIndex Idx;
+  CXTranslationUnit TU;
+  const char *FileName;
+  enum CXErrorCode Err;
+  CXString Target;
+  int PointerWidth;
+
+  if (argc == 0) {
+fprintf(stderr, "No filename specified\n");
+return 1;
+  }
+
+  FileName = argv[1];
+
+  Idx = clang_createIndex(0, 1);
+  Err = clang_parseTranslationUnit2(Idx, FileName, argv, argc, NULL, 0,
+getDefaultParsingOptions(), );
+  if (Err != CXError_Success) {
+fprintf(stderr, "Couldn't parse translation unit!\n");
+describeLibclangFailure(Err);
+clang_disposeIndex(Idx);
+return 1;
+  }
+
+  Target = clang_getTranslationUnitTargetTriple(TU);
+  printf("TargetTriple: %s\n", clang_getCString(Target));
+  clang_disposeString(Target);
+
+  PointerWidth = clang_getTranslationUnitTargetPointerWidth(TU);
+  printf("PointerWidth: %d\n", PointerWidth);
+
+  clang_disposeTranslationUnit(TU);
+  clang_disposeIndex(Idx);
+  return 0;
+}
+
+/**/
 /* Loading ASTs/source.   */
 /**/
 
@@ -4297,11 +4338,12 @@
 "   c-index-test -test-print-type {}*\n"
 "   c-index-test -test-print-type-size {}*\n"
 "   c-index-test -test-print-bitwidth {}*\n"
+"   c-index-test -test-print-target-info {}*\n"
 "   c-index-test -test-print-type-declaration {}*\n"
 "   c-index-test -print-usr [ {}]*\n"
-"   c-index-test -print-usr-file \n"
-"   c-index-test -write-pch  \n");
+"   c-index-test -print-usr-file \n");
   fprintf(stderr,
+"   c-index-test -write-pch  \n"
 "   c-index-test -compilation-db [lookup ] database\n");
   fprintf(stderr,
 "   c-index-test -print-build-session-timestamp\n");
@@ -4407,6 +4449,8 @@
 return perform_test_load_tu(argv[2], "all", NULL, PrintMangledName, NULL);
   else if (argc > 2 && strcmp(argv[1], "-test-print-manglings") == 0)
 

[PATCH] D32341: Fix a bug that warnings generated with -M or -MM flags

[Yuka Takahashi via Phabricator via cfe-commits]

yamaguchi updated this revision to Diff 96283.
yamaguchi added a comment.

Fixed typo.


https://reviews.llvm.org/D32341

Files:
  lib/Driver/ToolChains/Clang.cpp
  test/Driver/m_and_mm.c


Index: test/Driver/m_and_mm.c
===
--- test/Driver/m_and_mm.c
+++ test/Driver/m_and_mm.c
@@ -1,3 +1,7 @@
 // RUN: %clang -### \
 // RUN:   -M -MM %s 2> %t
 // RUN: not grep '"-sys-header-deps"' %t
+
+// RUN: %clang -M -MM %s 2> %t
+// RUN: not grep "warning" %t
+#warning "This warning shouldn't show up with -M and -MM"
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -980,6 +980,7 @@
 DepTarget = Args.MakeArgString(llvm::sys::path::filename(P));
   }
 
+  CmdArgs.push_back("-w");
   CmdArgs.push_back("-MT");
   SmallString<128> Quoted;
   QuoteTarget(DepTarget, Quoted);


Index: test/Driver/m_and_mm.c
===
--- test/Driver/m_and_mm.c
+++ test/Driver/m_and_mm.c
@@ -1,3 +1,7 @@
 // RUN: %clang -### \
 // RUN:   -M -MM %s 2> %t
 // RUN: not grep '"-sys-header-deps"' %t
+
+// RUN: %clang -M -MM %s 2> %t
+// RUN: not grep "warning" %t
+#warning "This warning shouldn't show up with -M and -MM"
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -980,6 +980,7 @@
 DepTarget = Args.MakeArgString(llvm::sys::path::filename(P));
   }
 
+  CmdArgs.push_back("-w");
   CmdArgs.push_back("-MT");
   SmallString<128> Quoted;
   QuoteTarget(DepTarget, Quoted);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32341: Fix a bug that warnings generated with -M or -MM flags

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added a comment.

LGTM modulo the comments.




Comment at: test/Driver/m_and_mm.c:7
+// RUN: not grep "warning" %t
+#warning "This warning shouldn't shop up with -M and -MM"

Typo.


https://reviews.llvm.org/D32341



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