date:20210519

[clang-tools-extra] 9f36306 - [clang-tidy] Fix a crash on invalid code for memset-usage check.

2021-05-19 Thread Haojian Wu via cfe-commits


Author: Haojian Wu
Date: 2021-05-19T09:53:18+02:00
New Revision: 9f36306cc9ac6d1d1019831d40865d0d54563379

URL: 
https://github.com/llvm/llvm-project/commit/9f36306cc9ac6d1d1019831d40865d0d54563379
DIFF: 
https://github.com/llvm/llvm-project/commit/9f36306cc9ac6d1d1019831d40865d0d54563379.diff

LOG: [clang-tidy] Fix a crash on invalid code for memset-usage check.

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

Added: 

clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-memset-usage.c

Modified: 
clang-tools-extra/clang-tidy/bugprone/SuspiciousMemsetUsageCheck.cpp

Removed: 




diff  --git 
a/clang-tools-extra/clang-tidy/bugprone/SuspiciousMemsetUsageCheck.cpp 
b/clang-tools-extra/clang-tidy/bugprone/SuspiciousMemsetUsageCheck.cpp
index 341ba6ccc09fa..1abe470682329 100644
--- a/clang-tools-extra/clang-tidy/bugprone/SuspiciousMemsetUsageCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/SuspiciousMemsetUsageCheck.cpp
@@ -32,7 +32,7 @@ void SuspiciousMemsetUsageCheck::registerMatchers(MatchFinder 
*Finder) {
   // Look for memset(x, '0', z). Probably memset(x, 0, z) was intended.
   Finder->addMatcher(
   callExpr(
-  callee(MemsetDecl),
+  callee(MemsetDecl), argumentCountIs(3),
   hasArgument(1, characterLiteral(equals(static_cast('0')))
  .bind("char-zero-fill")),
   unless(hasArgument(
@@ -43,13 +43,13 @@ void 
SuspiciousMemsetUsageCheck::registerMatchers(MatchFinder *Finder) {
   // Look for memset with an integer literal in its fill_char argument.
   // Will check if it gets truncated.
   Finder->addMatcher(
-  callExpr(callee(MemsetDecl),
+  callExpr(callee(MemsetDecl), argumentCountIs(3),
hasArgument(1, integerLiteral().bind("num-fill"))),
   this);
 
   // Look for memset(x, y, 0) as that is most likely an argument swap.
   Finder->addMatcher(
-  callExpr(callee(MemsetDecl),
+  callExpr(callee(MemsetDecl), argumentCountIs(3),
unless(hasArgument(1, anyOf(characterLiteral(equals(
static_cast('0'))),
integerLiteral()

diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-memset-usage.c 
b/clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-memset-usage.c
new file mode 100644
index 0..61e7de50953e8
--- /dev/null
+++ 
b/clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-memset-usage.c
@@ -0,0 +1,11 @@
+// RUN: %check_clang_tidy  -expect-clang-tidy-error  %s 
bugprone-suspicious-memset-usage %t
+
+void *memset(void *, int, __SIZE_TYPE__);
+void *memset(void *);
+// CHECK-MESSAGES: :[[@LINE-1]]:7: error: conflicting types for 'memset'
+
+void test() {
+  // no crash
+  memset(undefine);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: error: use of undeclared identifier
+}



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

[PATCH] D102714: [clang-tidy] Fix a crash on invalid code for memset-usage check.

2021-05-19 Thread Haojian Wu via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9f36306cc9ac: [clang-tidy] Fix a crash on invalid code for 
memset-usage check. (authored by hokein).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102714/new/

https://reviews.llvm.org/D102714

Files:
  clang-tools-extra/clang-tidy/bugprone/SuspiciousMemsetUsageCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-memset-usage.c


Index: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-memset-usage.c
===
--- /dev/null
+++ 
clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-memset-usage.c
@@ -0,0 +1,11 @@
+// RUN: %check_clang_tidy  -expect-clang-tidy-error  %s 
bugprone-suspicious-memset-usage %t
+
+void *memset(void *, int, __SIZE_TYPE__);
+void *memset(void *);
+// CHECK-MESSAGES: :[[@LINE-1]]:7: error: conflicting types for 'memset'
+
+void test() {
+  // no crash
+  memset(undefine);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: error: use of undeclared identifier
+}
Index: clang-tools-extra/clang-tidy/bugprone/SuspiciousMemsetUsageCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/SuspiciousMemsetUsageCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/SuspiciousMemsetUsageCheck.cpp
@@ -32,7 +32,7 @@
   // Look for memset(x, '0', z). Probably memset(x, 0, z) was intended.
   Finder->addMatcher(
   callExpr(
-  callee(MemsetDecl),
+  callee(MemsetDecl), argumentCountIs(3),
   hasArgument(1, characterLiteral(equals(static_cast('0')))
  .bind("char-zero-fill")),
   unless(hasArgument(
@@ -43,13 +43,13 @@
   // Look for memset with an integer literal in its fill_char argument.
   // Will check if it gets truncated.
   Finder->addMatcher(
-  callExpr(callee(MemsetDecl),
+  callExpr(callee(MemsetDecl), argumentCountIs(3),
hasArgument(1, integerLiteral().bind("num-fill"))),
   this);
 
   // Look for memset(x, y, 0) as that is most likely an argument swap.
   Finder->addMatcher(
-  callExpr(callee(MemsetDecl),
+  callExpr(callee(MemsetDecl), argumentCountIs(3),
unless(hasArgument(1, anyOf(characterLiteral(equals(
static_cast('0'))),
integerLiteral()


Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-memset-usage.c
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-memset-usage.c
@@ -0,0 +1,11 @@
+// RUN: %check_clang_tidy  -expect-clang-tidy-error  %s bugprone-suspicious-memset-usage %t
+
+void *memset(void *, int, __SIZE_TYPE__);
+void *memset(void *);
+// CHECK-MESSAGES: :[[@LINE-1]]:7: error: conflicting types for 'memset'
+
+void test() {
+  // no crash
+  memset(undefine);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: error: use of undeclared identifier
+}
Index: clang-tools-extra/clang-tidy/bugprone/SuspiciousMemsetUsageCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/SuspiciousMemsetUsageCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/SuspiciousMemsetUsageCheck.cpp
@@ -32,7 +32,7 @@
   // Look for memset(x, '0', z). Probably memset(x, 0, z) was intended.
   Finder->addMatcher(
   callExpr(
-  callee(MemsetDecl),
+  callee(MemsetDecl), argumentCountIs(3),
   hasArgument(1, characterLiteral(equals(static_cast('0')))
  .bind("char-zero-fill")),
   unless(hasArgument(
@@ -43,13 +43,13 @@
   // Look for memset with an integer literal in its fill_char argument.
   // Will check if it gets truncated.
   Finder->addMatcher(
-  callExpr(callee(MemsetDecl),
+  callExpr(callee(MemsetDecl), argumentCountIs(3),
hasArgument(1, integerLiteral().bind("num-fill"))),
   this);
 
   // Look for memset(x, y, 0) as that is most likely an argument swap.
   Finder->addMatcher(
-  callExpr(callee(MemsetDecl),
+  callExpr(callee(MemsetDecl), argumentCountIs(3),
unless(hasArgument(1, anyOf(characterLiteral(equals(
static_cast('0'))),
integerLiteral()
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D102238: [TableGen] [Clang] Clean up arm_mve.td file

2021-05-19 Thread Dave Green via Phabricator via cfe-commits

dmgreen accepted this revision.
dmgreen added a comment.

Sorry. I think we were waiting for the foldl comment to be removed. If you 
remove that, this LGTM.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102238/new/

https://reviews.llvm.org/D102238

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

[PATCH] D96033: [clang-repl] Land initial infrastructure for incremental parsing

2021-05-19 Thread Raphael Isemann via Phabricator via cfe-commits

teemperor added a comment.

In D96033#2766502 , @phosek wrote:

> In D96033#2766372 , @v.g.vassilev 
> wrote:
>
>> In D96033#2766332 , @phosek wrote:
>>
>>> We've started seeing `LLVM ERROR: out of memory` on our 2-stage LTO Linux 
>>> builders after this change landed. It looks like linking `clang-repl` 
>>> always fails on our bot, but I've also seen OOM when linking 
>>> `ClangCodeGenTests` and `FrontendTests`. Do you have any idea why this 
>>> could be happening? We'd appreciate any help since our bots have been 
>>> broken for several days now.
>>
>> Ouch. Are the bot logs public? If not maybe a stacktrace could be useful. 
>> `clang-repl` combines a lot of libraries across llvm and clang that usually 
>> are compiled separately. For instance we put in memory most of the clang 
>> frontend, the backend and the JIT. Could it be we are hitting some real 
>> limit?
>
> Yes, they are, see 
> https://luci-milo.appspot.com/p/fuchsia/builders/prod/clang-linux-x64, but 
> there isn't much information in there unfortunately. It's possible that we're 
> hitting some limit, but these bots use 32-core instances with 128GB RAM which 
> I'd hope is enough even for the LTO build.

I think the specs are fine for just building with LTO, but I am not sure if 
that's enough to for the worst case when running `ninja -j320` with an LTO 
build (which is what your job is doing). Can you try limiting your link jobs to 
something like 16 or 32 (e.g., `-DLLVM_PARALLEL_LINK_JOBS=32`)

(FWIW, your go build script also crashes with OOM errors so you really are 
running low on memory on that node)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96033/new/

https://reviews.llvm.org/D96033

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

[PATCH] D102696: [Analyzer] Find constraints that are directly attached to a BinOp

2021-05-19 Thread Gabor Marton via Phabricator via cfe-commits

martong added a comment.

> In this case the equations are $y == 0 and $x + 0 == 0 which is much easier 
> to solve.

Yes, you are right.

> This happens for the same reason that your patch is needed in the first 
> place: we're eagerly substituting a constant.

Absolutely, that's the point. On the other hand, it is very important to 
emphasize that we cannot solve this problem with a stronger solver, see my 
example with 3 variables and two equations above.

Also, another interesting question is (drawn up by @steakhal) if we can/could 
handle associativity somehow. Commutativity seems to be handled by the 
constraint manager (alas we'd search for "y + x" when we query "x + y").


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102696/new/

https://reviews.llvm.org/D102696

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

[PATCH] D102750: [clang] Fix a crash on CheckArgAlignment.

2021-05-19 Thread Haojian Wu via Phabricator via cfe-commits

hokein created this revision.
hokein added a reviewer: kadircet.
hokein requested review of this revision.
Herald added a project: clang.

We might encounter an undeduced type before calling getTypeAlignInChars.

NOTE: this retrieve the fix from
8f80c66bd2982788a8eede4419684ca72f48b9a2 
, which 
was removed in a Adam's
followup fix fbfcfdbf6828b8d36f4ec0ff5f4eac11fb1411a5 
. We 
originally
thought the crash was caused by recovery-ast, but it turns out it can
occur for other cases, e.g. typo-correction.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D102750

Files:
  clang/lib/Sema/SemaChecking.cpp
  clang/test/SemaCXX/typo-correction-crash.cpp


Index: clang/test/SemaCXX/typo-correction-crash.cpp
===
--- clang/test/SemaCXX/typo-correction-crash.cpp
+++ clang/test/SemaCXX/typo-correction-crash.cpp
@@ -42,3 +42,12 @@
   }
 };
 }
+
+namespace NoCrashOnCheckArgAlignment {
+template  void b(a &);
+void test() {
+  for (auto file_data :b(files_db_data)); // expected-error {{use of 
undeclared identifier 'files_db_data'; did you mean 'file_data'?}} \
+  // expected-note {{'file_data' 
declared here}} \
+  // expected-error {{cannot use type 
'void' as a range}}
+}
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -4571,7 +4571,8 @@
 
   // Find expected alignment, and the actual alignment of the passed object.
   // getTypeAlignInChars requires complete types
-  if (ParamTy->isIncompleteType() || ArgTy->isIncompleteType())
+  if (ParamTy->isIncompleteType() || ArgTy->isIncompleteType() ||
+  ParamTy->isUndeducedType() || ArgTy->isUndeducedType())
 return;
 
   CharUnits ParamAlign = Context.getTypeAlignInChars(ParamTy);


Index: clang/test/SemaCXX/typo-correction-crash.cpp
===
--- clang/test/SemaCXX/typo-correction-crash.cpp
+++ clang/test/SemaCXX/typo-correction-crash.cpp
@@ -42,3 +42,12 @@
   }
 };
 }
+
+namespace NoCrashOnCheckArgAlignment {
+template  void b(a &);
+void test() {
+  for (auto file_data :b(files_db_data)); // expected-error {{use of undeclared identifier 'files_db_data'; did you mean 'file_data'?}} \
+  // expected-note {{'file_data' declared here}} \
+  // expected-error {{cannot use type 'void' as a range}}
+}
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -4571,7 +4571,8 @@
 
   // Find expected alignment, and the actual alignment of the passed object.
   // getTypeAlignInChars requires complete types
-  if (ParamTy->isIncompleteType() || ArgTy->isIncompleteType())
+  if (ParamTy->isIncompleteType() || ArgTy->isIncompleteType() ||
+  ParamTy->isUndeducedType() || ArgTy->isUndeducedType())
 return;
 
   CharUnits ParamAlign = Context.getTypeAlignInChars(ParamTy);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D102576: [clang-tidy] cppcoreguidelines-avoid-do-while: a new check

2021-05-19 Thread Fabian Thurnheer via Phabricator via cfe-commits

DNS320 updated this revision to Diff 346360.
DNS320 added a comment.

I updated the check according to the last review.

It ignores now do-while statements which are inside a macro and have a false 
condition. (false condition part was taken from bugprone-terminating-continue 
)

Tests were added and the check documentation has now a hint about ignoring 
do-while statements in macros.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102576/new/

https://reviews.llvm.org/D102576

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidDoWhileCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidDoWhileCheck.h
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-avoid-do-while.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-do-while.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-do-while.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-do-while.cpp
@@ -0,0 +1,87 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-avoid-do-while %t
+
+void func1() {
+  int I{0};
+  const int Limit{10};
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: Try to avoid using do-while loops in terms of readability. Consider using a while loop instead. [cppcoreguidelines-avoid-do-while]
+  do {
+I++;
+  } while (I <= Limit);
+}
+
+void func2() {
+  int I{0};
+  const int Limit{10};
+
+  // OK
+  while (I <= Limit) {
+I++;
+  }
+}
+
+void func3() {
+#define MACRO \
+  do {\
+  } while (true)
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: Try to avoid using do-while loops in terms of readability. Consider using a while loop instead. [cppcoreguidelines-avoid-do-while]
+  MACRO;
+
+#undef MACRO
+}
+
+void func4() {
+#define MACRO \
+  do {\
+  } while (1)
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: Try to avoid using do-while loops in terms of readability. Consider using a while loop instead. [cppcoreguidelines-avoid-do-while]
+  MACRO;
+
+#undef MACRO
+}
+
+void func5() {
+#define MACRO \
+  do {\
+  } while (false)
+
+  // OK
+  MACRO;
+
+#undef MACRO
+}
+
+void func6() {
+#define MACRO \
+  do {\
+  } while (0)
+
+  // OK
+  MACRO;
+
+#undef MACRO
+}
+
+void func7() {
+#define MACRO \
+  do {\
+  } while (nullptr)
+
+  // OK
+  MACRO;
+
+#undef MACRO
+}
+
+void func8() {
+#define MACRO \
+  do {\
+  } while (__null)
+
+  // OK
+  MACRO;
+
+#undef MACRO
+}
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -144,6 +144,7 @@
`clang-analyzer-valist.Unterminated `_,
`concurrency-mt-unsafe `_,
`concurrency-thread-canceltype-asynchronous `_,
+   `cppcoreguidelines-avoid-do-while `_,
`cppcoreguidelines-avoid-goto `_,
`cppcoreguidelines-avoid-non-const-global-variables `_,
`cppcoreguidelines-init-variables `_, "Yes"
Index: clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-avoid-do-while.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-avoid-do-while.rst
@@ -0,0 +1,36 @@
+.. title:: clang-tidy - cppcoreguidelines-avoid-do-while
+
+cppcoreguidelines-avoid-do-while
+
+
+Checks if a ``do-while`` loop exists and flags it.
+
+Using a ``while`` loop instead of a ``do-while`` could improve readability and
+prevents overlooking the condition at the end.
+
+Usages of ``do-while`` loops inside a macro definition are not flagged by this
+check if the condition is either ``false``, ``0``, ``nullptr`` or ``__null``.
+
+.. code-block:: c++
+
+  void func1() {
+int I{0};
+const int Limit{10};
+
+// Consider using a while loop
+do {
+  I++;
+} while (I <= Limit);
+  }
+
+  void func2() {
+int I{0};
+const int Limit{10};
+
+// Better
+while (I <= Limit) {
+  I++;
+}
+  }
+
+This check implements rule `ES.75 `_ of the C++ Core Guidelines.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -108,6 +108,11 @@
   Finds inner loops that have not been unrolled, as well as fully unrolled
   loops with unknown loops bou

[PATCH] D96033: [clang-repl] Land initial infrastructure for incremental parsing

2021-05-19 Thread Ulrich Weigand via Phabricator via cfe-commits

uweigand added a comment.

Yes, this patch fixes the problem for me.  Thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96033/new/

https://reviews.llvm.org/D96033

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

[PATCH] D102752: [clang-offload-bundler] Delimit input/output file names by '--' for llvm-objcopy

2021-05-19 Thread Sergey Dmitriev via Phabricator via cfe-commits

sdmitriev created this revision.
sdmitriev added a reviewer: ABataev.
Herald added a reviewer: alexshap.
Herald added a subscriber: abrachet.
sdmitriev requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: cfe-commits, sstefan1.
Herald added a project: clang.

That fixes a problem of using bundler with file names starting with dash.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D102752

Files:
  clang/test/Driver/clang-offload-bundler.c
  clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp


Index: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
===
--- clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
+++ clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
@@ -621,6 +621,7 @@
 OFFLOAD_BUNDLER_MAGIC_STR + TargetNames[I] 
+
 "=readonly,exclude"));
 }
+ObjcopyArgs.push_back("--");
 ObjcopyArgs.push_back(InputFileNames[HostInputIndex]);
 ObjcopyArgs.push_back(OutputFileNames.front());
 
Index: clang/test/Driver/clang-offload-bundler.c
===
--- clang/test/Driver/clang-offload-bundler.c
+++ clang/test/Driver/clang-offload-bundler.c
@@ -274,7 +274,7 @@
 
 // RUN: clang-offload-bundler -type=o 
-targets=host-%itanium_abi_triple,openmp-powerpc64le-ibm-linux-gnu,openmp-x86_64-pc-linux-gnu
 -inputs=%t.o,%t.tgt1,%t.tgt2 -outputs=%t.bundle3.o -### 2>&1 \
 // RUN: | FileCheck %s -DHOST=%itanium_abi_triple -DINOBJ1=%t.o 
-DINOBJ2=%t.tgt1 -DINOBJ3=%t.tgt2 -DOUTOBJ=%t.bundle3.o --check-prefix 
CK-OBJ-CMD
-// CK-OBJ-CMD: llvm-objcopy{{(.exe)?}}" 
"--add-section=__CLANG_OFFLOAD_BUNDLE__host-[[HOST]]={{.*}}" 
"--set-section-flags=__CLANG_OFFLOAD_BUNDLE__host-[[HOST]]=readonly,exclude" 
"--add-section=__CLANG_OFFLOAD_BUNDLE__openmp-powerpc64le-ibm-linux-gnu=[[INOBJ2]]"
 
"--set-section-flags=__CLANG_OFFLOAD_BUNDLE__openmp-powerpc64le-ibm-linux-gnu=readonly,exclude"
 "--add-section=__CLANG_OFFLOAD_BUNDLE__openmp-x86_64-pc-linux-gnu=[[INOBJ3]]" 
"--set-section-flags=__CLANG_OFFLOAD_BUNDLE__openmp-x86_64-pc-linux-gnu=readonly,exclude"
 "[[INOBJ1]]" "[[OUTOBJ]]"
+// CK-OBJ-CMD: llvm-objcopy{{(.exe)?}}" 
"--add-section=__CLANG_OFFLOAD_BUNDLE__host-[[HOST]]={{.*}}" 
"--set-section-flags=__CLANG_OFFLOAD_BUNDLE__host-[[HOST]]=readonly,exclude" 
"--add-section=__CLANG_OFFLOAD_BUNDLE__openmp-powerpc64le-ibm-linux-gnu=[[INOBJ2]]"
 
"--set-section-flags=__CLANG_OFFLOAD_BUNDLE__openmp-powerpc64le-ibm-linux-gnu=readonly,exclude"
 "--add-section=__CLANG_OFFLOAD_BUNDLE__openmp-x86_64-pc-linux-gnu=[[INOBJ3]]" 
"--set-section-flags=__CLANG_OFFLOAD_BUNDLE__openmp-x86_64-pc-linux-gnu=readonly,exclude"
 "--" "[[INOBJ1]]" "[[OUTOBJ]]"
 
 // RUN: clang-offload-bundler -type=o 
-targets=host-%itanium_abi_triple,openmp-powerpc64le-ibm-linux-gnu,openmp-x86_64-pc-linux-gnu
 -inputs=%t.o,%t.tgt1,%t.tgt2 -outputs=%t.bundle3.o
 // RUN: clang-offload-bundler -type=o -inputs=%t.bundle3.o -list | FileCheck 
-check-prefix=CKLST %s


Index: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
===
--- clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
+++ clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
@@ -621,6 +621,7 @@
 OFFLOAD_BUNDLER_MAGIC_STR + TargetNames[I] +
 "=readonly,exclude"));
 }
+ObjcopyArgs.push_back("--");
 ObjcopyArgs.push_back(InputFileNames[HostInputIndex]);
 ObjcopyArgs.push_back(OutputFileNames.front());
 
Index: clang/test/Driver/clang-offload-bundler.c
===
--- clang/test/Driver/clang-offload-bundler.c
+++ clang/test/Driver/clang-offload-bundler.c
@@ -274,7 +274,7 @@
 
 // RUN: clang-offload-bundler -type=o -targets=host-%itanium_abi_triple,openmp-powerpc64le-ibm-linux-gnu,openmp-x86_64-pc-linux-gnu -inputs=%t.o,%t.tgt1,%t.tgt2 -outputs=%t.bundle3.o -### 2>&1 \
 // RUN: | FileCheck %s -DHOST=%itanium_abi_triple -DINOBJ1=%t.o -DINOBJ2=%t.tgt1 -DINOBJ3=%t.tgt2 -DOUTOBJ=%t.bundle3.o --check-prefix CK-OBJ-CMD
-// CK-OBJ-CMD: llvm-objcopy{{(.exe)?}}" "--add-section=__CLANG_OFFLOAD_BUNDLE__host-[[HOST]]={{.*}}" "--set-section-flags=__CLANG_OFFLOAD_BUNDLE__host-[[HOST]]=readonly,exclude" "--add-section=__CLANG_OFFLOAD_BUNDLE__openmp-powerpc64le-ibm-linux-gnu=[[INOBJ2]]" "--set-section-flags=__CLANG_OFFLOAD_BUNDLE__openmp-powerpc64le-ibm-linux-gnu=readonly,exclude" "--add-section=__CLANG_OFFLOAD_BUNDLE__openmp-x86_64-pc-linux-gnu=[[INOBJ3]]" "--set-section-flags=__CLANG_OFFLOAD_BUNDLE__openmp-x86_64-pc-linux-gnu=readonly,exclude" "[[INOBJ1]]" "[[OUTOBJ]]"
+// CK-OBJ-CMD: llvm-objcopy{{(.exe)?}}" "--add-section=__CLANG_OFFLOAD_BUNDLE__host-[[HOST]]={{.*}}" "--set-section-flags=__CLANG_OFFLOAD_BUNDLE__host-[[HOST

[PATCH] D60380: Also document -arch as -arch is mac specific

2021-05-19 Thread serge via Phabricator via cfe-commits

serge-sans-paille added inline comments.



Comment at: docs/CommandGuide/clang.rst:319
+
+  Specify the architecture to build for (Linux and others)
 

To be clear: this is probably misleading and should be `(all platforms)`


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60380/new/

https://reviews.llvm.org/D60380

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

[PATCH] D96524: [OpenCL] Add support of OpenCL C 3.0 __opencl_c_fp64

2021-05-19 Thread Anton Zabaznov via Phabricator via cfe-commits

azabaznov updated this revision to Diff 346379.
azabaznov added a comment.

Drop formatting changes from Sema.cpp


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96524/new/

https://reviews.llvm.org/D96524

Files:
  clang/docs/OpenCLSupport.rst
  clang/include/clang/Basic/DiagnosticCommonKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Basic/Targets.cpp
  clang/lib/Basic/Targets/AMDGPU.h
  clang/lib/Basic/Targets/NVPTX.h
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CodeGenOpenCL/printf.cl
  clang/test/Misc/opencl-c-3.0.incorrect_options.cl
  clang/test/SemaOpenCL/extensions.cl
  clang/test/SemaOpenCL/fp64-fp16-options.cl

Index: clang/test/SemaOpenCL/fp64-fp16-options.cl
===
--- clang/test/SemaOpenCL/fp64-fp16-options.cl
+++ clang/test/SemaOpenCL/fp64-fp16-options.cl
@@ -5,6 +5,7 @@
 
 // Test with a target not supporting fp64.
 // RUN: %clang_cc1 %s -triple r600-unknown-unknown -target-cpu r600 -verify -pedantic -fsyntax-only -DNOFP64 -DNOFP16
+// RUN: %clang_cc1 -cl-std=CL3.0 %s -triple r600-unknown-unknown -target-cpu r600 -verify -pedantic -fsyntax-only -DNOFP64 -DNOFP16
 
 // Test with some extensions enabled or disabled by cmd-line args
 //
@@ -16,12 +17,18 @@
 // RUN: %clang_cc1 %s -triple r600-unknown-unknown -target-cpu r600 -verify -pedantic -fsyntax-only -cl-ext=+all
 // RUN: %clang_cc1 %s -triple r600-unknown-unknown -target-cpu r600 -verify -pedantic -fsyntax-only -cl-ext=+all,-cl_khr_fp64 -DNOFP64
 // RUN: %clang_cc1 %s -triple r600-unknown-unknown -target-cpu r600 -verify -pedantic -fsyntax-only -cl-ext=-all,+cl_khr_fp64 -DNOFP16
+// RUN: %clang_cc1 -cl-std=CL3.0 %s -triple spir-unknown-unknown -verify -pedantic -fsyntax-only -cl-ext=-all -DNOFP64 -DNOFP16
+// RUN: %clang_cc1 -cl-std=CL3.0 %s -triple r600-unknown-unknown -target-cpu r600 -verify -pedantic -fsyntax-only -cl-ext=+all -DFP64
+// RUN: %clang_cc1 -cl-std=CL3.0 %s -triple r600-unknown-unknown -target-cpu r600 -verify -pedantic -fsyntax-only -cl-ext=+all,-__opencl_c_fp64,-cl_khr_fp64 -DNOFP64
 //
 // Concatenating
 // RUN: %clang_cc1 %s -triple spir-unknown-unknown -verify -pedantic -fsyntax-only -cl-ext=-cl_khr_fp64 -cl-ext=+cl_khr_fp64
 // RUN: %clang_cc1 %s -triple spir-unknown-unknown -verify -pedantic -fsyntax-only -cl-ext=-cl_khr_fp64,+cl_khr_fp64
 // RUN: %clang_cc1 %s -triple spir-unknown-unknown -verify -pedantic -fsyntax-only -cl-ext=-all -cl-ext=+cl_khr_fp64 -cl-ext=+cl_khr_fp16 -cl-ext=-cl_khr_fp64 -DNOFP64
 // RUN: %clang_cc1 %s -triple spir-unknown-unknown -verify -pedantic -fsyntax-only -cl-ext=-all -cl-ext=+cl_khr_fp64,-cl_khr_fp64,+cl_khr_fp16 -DNOFP64
+// RUN: %clang_cc1 -cl-std=CL3.0 %s -triple spir-unknown-unknown -verify -pedantic -fsyntax-only -cl-ext=-__opencl_c_fp64,-cl_khr_fp64 -cl-ext=+__opencl_c_fp64,+cl_khr_fp64 -DFP64
+// RUN: %clang_cc1 -cl-std=CL3.0 %s -triple spir-unknown-unknown -verify -pedantic -fsyntax-only -cl-ext=-__opencl_c_fp64,+__opencl_c_fp64,+cl_khr_fp64 -DFP64
+// RUN: %clang_cc1 -cl-std=CL3.0 %s -triple spir-unknown-unknown -verify -pedantic -fsyntax-only -cl-ext=-__opencl_c_fp64,-cl_khr_fp64 -DNOFP64
 
 // Test with -finclude-default-header, which includes opencl-c.h. opencl-c.h
 // disables all extensions by default, but supported core extensions for a
@@ -85,18 +92,30 @@
 void f2(void) {
   double d;
 #ifdef NOFP64
-// expected-error@-2{{use of type 'double' requires cl_khr_fp64 support}}
+#if (defined(__OPENCL_C_VERSION__) && __OPENCL_C_VERSION__ >= 300)
+// expected-error@-3{{use of type 'double' requires cl_khr_fp64 and __opencl_c_fp64 support}}
+#else
+// expected-error@-5{{use of type 'double' requires cl_khr_fp64 support}}
+#endif
 #endif
 
   typedef double double4 __attribute__((ext_vector_type(4)));
   double4 d4 = {0.0f, 2.0f, 3.0f, 1.0f};
 #ifdef NOFP64
-// expected-error@-3 {{use of type 'double' requires cl_khr_fp64 support}}
+#if (defined(__OPENCL_C_VERSION__) && __OPENCL_C_VERSION__ >= 300)
+// expected-error@-4 {{use of type 'double' requires cl_khr_fp64 and __opencl_c_fp64 support}}
+#else
+// expected-error@-6 {{use of type 'double' requires cl_khr_fp64 support}}
+#endif
 #endif
 
   (void) 1.0;
 #ifdef NOFP64
-// expected-warning@-2{{double precision constant requires cl_khr_fp64, casting to single precision}}
+#if (defined(__OPENCL_C_VERSION__) && __OPENCL_C_VERSION__ >= 300)
+// expected-warning@-3{{double precision constant requires cl_khr_fp64 and __opencl_c_fp64, casting to single precision}}
+#else
+// expected-warning@-5{{double precision constant requires cl_khr_fp64, casting to single precision}}
+#endif
 #endif
 }
 
Index: clang/test/Misc/opencl-c-3.0.incorrect_options.cl
===
--- /dev/null
+++ clang/test/Misc/opencl-c-3.0.incorrect_options.cl
@@ -0,0 +1,4 @@
+// RUN:

[PATCH] D96524: [OpenCL] Add support of OpenCL C 3.0 __opencl_c_fp64

2021-05-19 Thread Anton Zabaznov via Phabricator via cfe-commits

azabaznov updated this revision to Diff 346380.
azabaznov added a comment.

Drop one more formatting change from Sema.cpp


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96524/new/

https://reviews.llvm.org/D96524

Files:
  clang/docs/OpenCLSupport.rst
  clang/include/clang/Basic/DiagnosticCommonKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Basic/Targets.cpp
  clang/lib/Basic/Targets/AMDGPU.h
  clang/lib/Basic/Targets/NVPTX.h
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CodeGenOpenCL/printf.cl
  clang/test/Misc/opencl-c-3.0.incorrect_options.cl
  clang/test/SemaOpenCL/extensions.cl
  clang/test/SemaOpenCL/fp64-fp16-options.cl

Index: clang/test/SemaOpenCL/fp64-fp16-options.cl
===
--- clang/test/SemaOpenCL/fp64-fp16-options.cl
+++ clang/test/SemaOpenCL/fp64-fp16-options.cl
@@ -5,6 +5,7 @@
 
 // Test with a target not supporting fp64.
 // RUN: %clang_cc1 %s -triple r600-unknown-unknown -target-cpu r600 -verify -pedantic -fsyntax-only -DNOFP64 -DNOFP16
+// RUN: %clang_cc1 -cl-std=CL3.0 %s -triple r600-unknown-unknown -target-cpu r600 -verify -pedantic -fsyntax-only -DNOFP64 -DNOFP16
 
 // Test with some extensions enabled or disabled by cmd-line args
 //
@@ -16,12 +17,18 @@
 // RUN: %clang_cc1 %s -triple r600-unknown-unknown -target-cpu r600 -verify -pedantic -fsyntax-only -cl-ext=+all
 // RUN: %clang_cc1 %s -triple r600-unknown-unknown -target-cpu r600 -verify -pedantic -fsyntax-only -cl-ext=+all,-cl_khr_fp64 -DNOFP64
 // RUN: %clang_cc1 %s -triple r600-unknown-unknown -target-cpu r600 -verify -pedantic -fsyntax-only -cl-ext=-all,+cl_khr_fp64 -DNOFP16
+// RUN: %clang_cc1 -cl-std=CL3.0 %s -triple spir-unknown-unknown -verify -pedantic -fsyntax-only -cl-ext=-all -DNOFP64 -DNOFP16
+// RUN: %clang_cc1 -cl-std=CL3.0 %s -triple r600-unknown-unknown -target-cpu r600 -verify -pedantic -fsyntax-only -cl-ext=+all -DFP64
+// RUN: %clang_cc1 -cl-std=CL3.0 %s -triple r600-unknown-unknown -target-cpu r600 -verify -pedantic -fsyntax-only -cl-ext=+all,-__opencl_c_fp64,-cl_khr_fp64 -DNOFP64
 //
 // Concatenating
 // RUN: %clang_cc1 %s -triple spir-unknown-unknown -verify -pedantic -fsyntax-only -cl-ext=-cl_khr_fp64 -cl-ext=+cl_khr_fp64
 // RUN: %clang_cc1 %s -triple spir-unknown-unknown -verify -pedantic -fsyntax-only -cl-ext=-cl_khr_fp64,+cl_khr_fp64
 // RUN: %clang_cc1 %s -triple spir-unknown-unknown -verify -pedantic -fsyntax-only -cl-ext=-all -cl-ext=+cl_khr_fp64 -cl-ext=+cl_khr_fp16 -cl-ext=-cl_khr_fp64 -DNOFP64
 // RUN: %clang_cc1 %s -triple spir-unknown-unknown -verify -pedantic -fsyntax-only -cl-ext=-all -cl-ext=+cl_khr_fp64,-cl_khr_fp64,+cl_khr_fp16 -DNOFP64
+// RUN: %clang_cc1 -cl-std=CL3.0 %s -triple spir-unknown-unknown -verify -pedantic -fsyntax-only -cl-ext=-__opencl_c_fp64,-cl_khr_fp64 -cl-ext=+__opencl_c_fp64,+cl_khr_fp64 -DFP64
+// RUN: %clang_cc1 -cl-std=CL3.0 %s -triple spir-unknown-unknown -verify -pedantic -fsyntax-only -cl-ext=-__opencl_c_fp64,+__opencl_c_fp64,+cl_khr_fp64 -DFP64
+// RUN: %clang_cc1 -cl-std=CL3.0 %s -triple spir-unknown-unknown -verify -pedantic -fsyntax-only -cl-ext=-__opencl_c_fp64,-cl_khr_fp64 -DNOFP64
 
 // Test with -finclude-default-header, which includes opencl-c.h. opencl-c.h
 // disables all extensions by default, but supported core extensions for a
@@ -85,18 +92,30 @@
 void f2(void) {
   double d;
 #ifdef NOFP64
-// expected-error@-2{{use of type 'double' requires cl_khr_fp64 support}}
+#if (defined(__OPENCL_C_VERSION__) && __OPENCL_C_VERSION__ >= 300)
+// expected-error@-3{{use of type 'double' requires cl_khr_fp64 and __opencl_c_fp64 support}}
+#else
+// expected-error@-5{{use of type 'double' requires cl_khr_fp64 support}}
+#endif
 #endif
 
   typedef double double4 __attribute__((ext_vector_type(4)));
   double4 d4 = {0.0f, 2.0f, 3.0f, 1.0f};
 #ifdef NOFP64
-// expected-error@-3 {{use of type 'double' requires cl_khr_fp64 support}}
+#if (defined(__OPENCL_C_VERSION__) && __OPENCL_C_VERSION__ >= 300)
+// expected-error@-4 {{use of type 'double' requires cl_khr_fp64 and __opencl_c_fp64 support}}
+#else
+// expected-error@-6 {{use of type 'double' requires cl_khr_fp64 support}}
+#endif
 #endif
 
   (void) 1.0;
 #ifdef NOFP64
-// expected-warning@-2{{double precision constant requires cl_khr_fp64, casting to single precision}}
+#if (defined(__OPENCL_C_VERSION__) && __OPENCL_C_VERSION__ >= 300)
+// expected-warning@-3{{double precision constant requires cl_khr_fp64 and __opencl_c_fp64, casting to single precision}}
+#else
+// expected-warning@-5{{double precision constant requires cl_khr_fp64, casting to single precision}}
+#endif
 #endif
 }
 
Index: clang/test/Misc/opencl-c-3.0.incorrect_options.cl
===
--- /dev/null
+++ clang/test/Misc/opencl-c-3.0.incorrect_options.cl
@@ -0,0 +1,4 @@
+// RUN: not %clang_cc1 -cl

[PATCH] D102750: [clang] Fix a crash on CheckArgAlignment.

2021-05-19 Thread Kadir Cetinkaya via Phabricator via cfe-commits

kadircet accepted this revision.
kadircet added a comment.
This revision is now accepted and ready to land.

thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102750/new/

https://reviews.llvm.org/D102750

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

[PATCH] D96524: [OpenCL] Add support of OpenCL C 3.0 __opencl_c_fp64

2021-05-19 Thread Anton Zabaznov via Phabricator via cfe-commits

azabaznov marked an inline comment as done.
azabaznov added inline comments.



Comment at: clang/test/CodeGenOpenCL/printf.cl:9
 
-#ifdef cl_khr_fp64
+#if defined(cl_khr_fp64) || defined(__opencl_c_fp64)
 typedef __attribute__((ext_vector_type(2))) double double2;

Anastasia wrote:
> I think we don't technically need this change?
To be honest I am not sure. I think per OpenCL C spec extension feature macros 
should be used exactly as follows:

```
...
if defined(cl_khr_fp64) || defined(__opencl_c_fp64)
...
```

So I think this is OK to have such checks in tests as soon as we have 
functionality for simultaneous support (`opencl-c-3.0.incorrect_options.cl`). 
If we decide to change that behaviour - this test will fail.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96524/new/

https://reviews.llvm.org/D96524

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

[PATCH] D102643: [analyzer][Z3][NFC] Use GTEST_SKIP instead of hacks

2021-05-19 Thread Balázs Benics via Phabricator via cfe-commits

steakhal updated this revision to Diff 346382.
steakhal added a comment.

Introduce the test fixture, and use its `setUp()` method for implementing this 
`GTEST_SKIP` stuff.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102643/new/

https://reviews.llvm.org/D102643

Files:
  clang/unittests/StaticAnalyzer/FalsePositiveRefutationBRVisitorTest.cpp


Index: clang/unittests/StaticAnalyzer/FalsePositiveRefutationBRVisitorTest.cpp
===
--- clang/unittests/StaticAnalyzer/FalsePositiveRefutationBRVisitorTest.cpp
+++ clang/unittests/StaticAnalyzer/FalsePositiveRefutationBRVisitorTest.cpp
@@ -19,13 +19,6 @@
 #include "llvm/Config/llvm-config.h"
 #include "gtest/gtest.h"
 
-// FIXME: Use GTEST_SKIP() instead if GTest is updated to version 1.10.0
-#ifdef LLVM_WITH_Z3
-#define SKIP_WITHOUT_Z3
-#else
-#define SKIP_WITHOUT_Z3 return
-#endif
-
 namespace clang {
 namespace ento {
 namespace {
@@ -101,6 +94,15 @@
   });
 }
 
+class FalsePositiveRefutationBRVisitorTestBase : public testing::Test {
+public:
+  void SetUp() override {
+#ifndef LLVM_WITH_Z3
+GTEST_SKIP() << "Requires the LLVM_ENABLE_Z3_SOLVER cmake option.";
+#endif
+  }
+};
+
 // C++20 use constexpr below.
 const std::vector LazyAssumeArgs{
 "-Xclang", "-analyzer-config", "-Xclang", "eagerly-assume=false"};
@@ -108,8 +110,7 @@
 "-Xclang", "-analyzer-config", "-Xclang", "eagerly-assume=false",
 "-Xclang", "-analyzer-config", "-Xclang", "crosscheck-with-z3=true"};
 
-TEST(FalsePositiveRefutationBRVisitor, UnSatInTheMiddleNoReport) {
-  SKIP_WITHOUT_Z3;
+TEST_F(FalsePositiveRefutationBRVisitorTestBase, UnSatInTheMiddleNoReport) {
   constexpr auto Code = R"(
  void reachedWithContradiction();
  void reachedWithNoContradiction();
@@ -139,8 +140,8 @@
 "test.FalsePositiveGenerator:REACHED_WITH_CONTRADICTION\n");
 }
 
-TEST(FalsePositiveRefutationBRVisitor, UnSatAtErrorNodeWithNewSymbolNoReport) {
-  SKIP_WITHOUT_Z3;
+TEST_F(FalsePositiveRefutationBRVisitorTestBase,
+   UnSatAtErrorNodeWithNewSymbolNoReport) {
   constexpr auto Code = R"(
 void reportIfCanBeTrue(bool);
 void reachedWithNoContradiction();
@@ -170,9 +171,8 @@
 "test.FalsePositiveGenerator:CAN_BE_TRUE\n");
 }
 
-TEST(FalsePositiveRefutationBRVisitor,
- UnSatAtErrorNodeDueToRefinedConstraintNoReport) {
-  SKIP_WITHOUT_Z3;
+TEST_F(FalsePositiveRefutationBRVisitorTestBase,
+   UnSatAtErrorNodeDueToRefinedConstraintNoReport) {
   constexpr auto Code = R"(
 void reportIfCanBeTrue(bool);
 void reachedWithNoContradiction();


Index: clang/unittests/StaticAnalyzer/FalsePositiveRefutationBRVisitorTest.cpp
===
--- clang/unittests/StaticAnalyzer/FalsePositiveRefutationBRVisitorTest.cpp
+++ clang/unittests/StaticAnalyzer/FalsePositiveRefutationBRVisitorTest.cpp
@@ -19,13 +19,6 @@
 #include "llvm/Config/llvm-config.h"
 #include "gtest/gtest.h"
 
-// FIXME: Use GTEST_SKIP() instead if GTest is updated to version 1.10.0
-#ifdef LLVM_WITH_Z3
-#define SKIP_WITHOUT_Z3
-#else
-#define SKIP_WITHOUT_Z3 return
-#endif
-
 namespace clang {
 namespace ento {
 namespace {
@@ -101,6 +94,15 @@
   });
 }
 
+class FalsePositiveRefutationBRVisitorTestBase : public testing::Test {
+public:
+  void SetUp() override {
+#ifndef LLVM_WITH_Z3
+GTEST_SKIP() << "Requires the LLVM_ENABLE_Z3_SOLVER cmake option.";
+#endif
+  }
+};
+
 // C++20 use constexpr below.
 const std::vector LazyAssumeArgs{
 "-Xclang", "-analyzer-config", "-Xclang", "eagerly-assume=false"};
@@ -108,8 +110,7 @@
 "-Xclang", "-analyzer-config", "-Xclang", "eagerly-assume=false",
 "-Xclang", "-analyzer-config", "-Xclang", "crosscheck-with-z3=true"};
 
-TEST(FalsePositiveRefutationBRVisitor, UnSatInTheMiddleNoReport) {
-  SKIP_WITHOUT_Z3;
+TEST_F(FalsePositiveRefutationBRVisitorTestBase, UnSatInTheMiddleNoReport) {
   constexpr auto Code = R"(
  void reachedWithContradiction();
  void reachedWithNoContradiction();
@@ -139,8 +140,8 @@
 "test.FalsePositiveGenerator:REACHED_WITH_CONTRADICTION\n");
 }
 
-TEST(FalsePositiveRefutationBRVisitor, UnSatAtErrorNodeWithNewSymbolNoReport) {
-  SKIP_WITHOUT_Z3;
+TEST_F(FalsePositiveRefutationBRVisitorTestBase,
+   UnSatAtErrorNodeWithNewSymbolNoReport) {
   constexpr auto Code = R"(
 void reportIfCanBeTrue(bool);
 void reachedWithNoContradiction();
@@ -170,9 +171,8 @@
 "test.FalsePositiveGenerator:CAN_BE_TRUE\n");
 }
 
-TEST(FalsePositiveRefutationBRVisitor,
- UnSatAtErrorNodeDueToRefinedConstraintNoReport) {
-  SKIP_WITHOUT_Z3;
+TEST_F(FalsePositiveRefutationBRVisitorTestBase,
+   UnSatAtErrorNodeDueToRefinedConstraintNoReport) {
   constexpr auto Code = R"(
 void reportIfCanBeTrue(bool);
 void reachedWithNoContradiction();
___
cfe-commits mailing list
cfe-commits@lists

[clang] 479ea2a - [analyzer] Check the checker name, rather than the ProgramPointTag when silencing a checker

2021-05-19 Thread Kirstóf Umann via cfe-commits


Author: Kristóf Umann
Date: 2021-05-19T12:40:09+02:00
New Revision: 479ea2a8ed95544d2f5aaede34bfe5c298ae8bdb

URL: 
https://github.com/llvm/llvm-project/commit/479ea2a8ed95544d2f5aaede34bfe5c298ae8bdb
DIFF: 
https://github.com/llvm/llvm-project/commit/479ea2a8ed95544d2f5aaede34bfe5c298ae8bdb.diff

LOG: [analyzer] Check the checker name, rather than the ProgramPointTag when 
silencing a checker

The program point created by the checker, even if it is an error node,
might not be the same as the name under which the report is emitted.
Make sure we're checking the name of the checker, because thats what
we're silencing after all.

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

Added: 
clang/test/Analysis/silence-checkers-malloc.cpp

Modified: 
clang/lib/StaticAnalyzer/Core/BugReporter.cpp
clang/test/Analysis/malloc.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Core/BugReporter.cpp 
b/clang/lib/StaticAnalyzer/Core/BugReporter.cpp
index b64c0798d7e2..4608ee5cd40b 100644
--- a/clang/lib/StaticAnalyzer/Core/BugReporter.cpp
+++ b/clang/lib/StaticAnalyzer/Core/BugReporter.cpp
@@ -1988,12 +1988,11 @@ PathDiagnosticBuilder::generate(const 
PathDiagnosticConsumer *PDC) const {
 
   const SourceManager &SM = getSourceManager();
   const AnalyzerOptions &Opts = getAnalyzerOptions();
-  StringRef ErrorTag = ErrorNode->getLocation().getTag()->getTagDescription();
 
   // See whether we need to silence the checker/package.
   // FIXME: This will not work if the report was emitted with an incorrect tag.
   for (const std::string &CheckerOrPackage : Opts.SilencedCheckersAndPackages) 
{
-if (ErrorTag.startswith(CheckerOrPackage))
+if (R->getBugType().getCheckerName().startswith(CheckerOrPackage))
   return nullptr;
   }
 

diff  --git a/clang/test/Analysis/malloc.cpp b/clang/test/Analysis/malloc.cpp
index 21e8e79e1a89..2bbf26ac2cda 100644
--- a/clang/test/Analysis/malloc.cpp
+++ b/clang/test/Analysis/malloc.cpp
@@ -1,7 +1,32 @@
-// RUN: %clang_analyze_cc1 -w 
-analyzer-checker=core,alpha.deadcode.UnreachableCode,alpha.core.CastSize,unix.Malloc,cplusplus.NewDelete
 -analyzer-store=region -verify %s
-// RUN: %clang_analyze_cc1 -triple i386-unknown-linux-gnu -w 
-analyzer-checker=core,alpha.deadcode.UnreachableCode,alpha.core.CastSize,unix.Malloc,cplusplus.NewDelete
 -analyzer-store=region -verify %s
-// RUN: %clang_analyze_cc1 -w 
-analyzer-checker=core,alpha.deadcode.UnreachableCode,alpha.core.CastSize,unix.Malloc,cplusplus.NewDelete
 -analyzer-store=region -DTEST_INLINABLE_ALLOCATORS -verify %s
-// RUN: %clang_analyze_cc1 -triple i386-unknown-linux-gnu -w 
-analyzer-checker=core,alpha.deadcode.UnreachableCode,alpha.core.CastSize,unix.Malloc,cplusplus.NewDelete
 -analyzer-store=region -DTEST_INLINABLE_ALLOCATORS -verify %s
+// RUN: %clang_analyze_cc1 -w -verify %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=alpha.deadcode.UnreachableCode \
+// RUN:   -analyzer-checker=alpha.core.CastSize \
+// RUN:   -analyzer-checker=unix.Malloc \
+// RUN:   -analyzer-checker=cplusplus.NewDelete
+
+// RUN: %clang_analyze_cc1 -w -verify %s \
+// RUN:   -triple i386-unknown-linux-gnu \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=alpha.deadcode.UnreachableCode \
+// RUN:   -analyzer-checker=alpha.core.CastSize \
+// RUN:   -analyzer-checker=unix.Malloc \
+// RUN:   -analyzer-checker=cplusplus.NewDelete
+
+// RUN: %clang_analyze_cc1 -w -verify %s -DTEST_INLINABLE_ALLOCATORS \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=alpha.deadcode.UnreachableCode \
+// RUN:   -analyzer-checker=alpha.core.CastSize \
+// RUN:   -analyzer-checker=unix.Malloc \
+// RUN:   -analyzer-checker=cplusplus.NewDelete
+
+// RUN: %clang_analyze_cc1 -w -verify %s -DTEST_INLINABLE_ALLOCATORS \
+// RUN:   -triple i386-unknown-linux-gnu \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=alpha.deadcode.UnreachableCode \
+// RUN:   -analyzer-checker=alpha.core.CastSize \
+// RUN:   -analyzer-checker=unix.Malloc \
+// RUN:   -analyzer-checker=cplusplus.NewDelete
 
 #include "Inputs/system-header-simulator-cxx.h"
 

diff  --git a/clang/test/Analysis/silence-checkers-malloc.cpp 
b/clang/test/Analysis/silence-checkers-malloc.cpp
new file mode 100644
index ..2f6a9dd2d5b8
--- /dev/null
+++ b/clang/test/Analysis/silence-checkers-malloc.cpp
@@ -0,0 +1,40 @@
+// RUN: %clang_analyze_cc1 -verify="no-silence" %s \
+// RUN:   -triple i386-unknown-linux-gnu \
+// RUN:   -analyzer-checker=core,apiModeling \
+// RUN:   -analyzer-checker=unix.Malloc \
+// RUN:   -analyzer-checker=cplusplus.NewDelete
+
+// RUN: %clang_analyze_cc1 -verify="unix-silenced" %s \
+// RUN:   -triple i386-unknown-linux-gnu \
+// RUN:   -analyzer-checker=core,apiModeling \
+// RUN:   -analyzer-checker=unix.Malloc \
+// RUN:   -analyzer-checker=cplusplus.NewDelete\
+// RUN:   -analyzer-config silence-che

[PATCH] D102683: [analyzer] Check the checker name, rather than the ProgramPointTag when silencing a checker

2021-05-19 Thread Kristóf Umann via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG479ea2a8ed95: [analyzer] Check the checker name, rather than 
the ProgramPointTag when… (authored by Szelethus).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102683/new/

https://reviews.llvm.org/D102683

Files:
  clang/lib/StaticAnalyzer/Core/BugReporter.cpp
  clang/test/Analysis/malloc.cpp
  clang/test/Analysis/silence-checkers-malloc.cpp

Index: clang/test/Analysis/silence-checkers-malloc.cpp
===
--- /dev/null
+++ clang/test/Analysis/silence-checkers-malloc.cpp
@@ -0,0 +1,40 @@
+// RUN: %clang_analyze_cc1 -verify="no-silence" %s \
+// RUN:   -triple i386-unknown-linux-gnu \
+// RUN:   -analyzer-checker=core,apiModeling \
+// RUN:   -analyzer-checker=unix.Malloc \
+// RUN:   -analyzer-checker=cplusplus.NewDelete
+
+// RUN: %clang_analyze_cc1 -verify="unix-silenced" %s \
+// RUN:   -triple i386-unknown-linux-gnu \
+// RUN:   -analyzer-checker=core,apiModeling \
+// RUN:   -analyzer-checker=unix.Malloc \
+// RUN:   -analyzer-checker=cplusplus.NewDelete\
+// RUN:   -analyzer-config silence-checkers="unix"
+
+#include "Inputs/system-header-simulator-cxx.h"
+
+typedef __typeof(sizeof(int)) size_t;
+void *malloc(size_t);
+void free(void *);
+void *realloc(void *ptr, size_t size);
+void *calloc(size_t nmemb, size_t size);
+char *strdup(const char *s);
+
+void checkThatMallocCheckerIsRunning() {
+  malloc(4);
+} // no-silence-warning{{Potential memory leak [unix.Malloc]}}
+
+int const_ptr_and_callback_def_param_null(int, const char *, int n, void (*)(void *) = 0);
+void r11160612_no_callback() {
+  char *x = (char *)malloc(12);
+  const_ptr_and_callback_def_param_null(0, x, 12);
+} // no-silence-warning{{Potential leak of memory pointed to by 'x' [unix.Malloc]}}
+
+#define ZERO_SIZE_PTR ((void *)16)
+
+void test_delete_ZERO_SIZE_PTR() {
+  int *Ptr = (int *)ZERO_SIZE_PTR;
+  // ZERO_SIZE_PTR is specially handled but only for malloc family
+  delete Ptr; // no-silence-warning{{Argument to 'delete' is a constant address (16), which is not memory allocated by 'new' [cplusplus.NewDelete]}}
+  // unix-silenced-warning@-1{{Argument to 'delete' is a constant address (16), which is not memory allocated by 'new' [cplusplus.NewDelete]}}
+}
Index: clang/test/Analysis/malloc.cpp
===
--- clang/test/Analysis/malloc.cpp
+++ clang/test/Analysis/malloc.cpp
@@ -1,7 +1,32 @@
-// RUN: %clang_analyze_cc1 -w -analyzer-checker=core,alpha.deadcode.UnreachableCode,alpha.core.CastSize,unix.Malloc,cplusplus.NewDelete -analyzer-store=region -verify %s
-// RUN: %clang_analyze_cc1 -triple i386-unknown-linux-gnu -w -analyzer-checker=core,alpha.deadcode.UnreachableCode,alpha.core.CastSize,unix.Malloc,cplusplus.NewDelete -analyzer-store=region -verify %s
-// RUN: %clang_analyze_cc1 -w -analyzer-checker=core,alpha.deadcode.UnreachableCode,alpha.core.CastSize,unix.Malloc,cplusplus.NewDelete -analyzer-store=region -DTEST_INLINABLE_ALLOCATORS -verify %s
-// RUN: %clang_analyze_cc1 -triple i386-unknown-linux-gnu -w -analyzer-checker=core,alpha.deadcode.UnreachableCode,alpha.core.CastSize,unix.Malloc,cplusplus.NewDelete -analyzer-store=region -DTEST_INLINABLE_ALLOCATORS -verify %s
+// RUN: %clang_analyze_cc1 -w -verify %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=alpha.deadcode.UnreachableCode \
+// RUN:   -analyzer-checker=alpha.core.CastSize \
+// RUN:   -analyzer-checker=unix.Malloc \
+// RUN:   -analyzer-checker=cplusplus.NewDelete
+
+// RUN: %clang_analyze_cc1 -w -verify %s \
+// RUN:   -triple i386-unknown-linux-gnu \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=alpha.deadcode.UnreachableCode \
+// RUN:   -analyzer-checker=alpha.core.CastSize \
+// RUN:   -analyzer-checker=unix.Malloc \
+// RUN:   -analyzer-checker=cplusplus.NewDelete
+
+// RUN: %clang_analyze_cc1 -w -verify %s -DTEST_INLINABLE_ALLOCATORS \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=alpha.deadcode.UnreachableCode \
+// RUN:   -analyzer-checker=alpha.core.CastSize \
+// RUN:   -analyzer-checker=unix.Malloc \
+// RUN:   -analyzer-checker=cplusplus.NewDelete
+
+// RUN: %clang_analyze_cc1 -w -verify %s -DTEST_INLINABLE_ALLOCATORS \
+// RUN:   -triple i386-unknown-linux-gnu \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=alpha.deadcode.UnreachableCode \
+// RUN:   -analyzer-checker=alpha.core.CastSize \
+// RUN:   -analyzer-checker=unix.Malloc \
+// RUN:   -analyzer-checker=cplusplus.NewDelete
 
 #include "Inputs/system-header-simulator-cxx.h"
 
Index: clang/lib/StaticAnalyzer/Core/BugReporter.cpp
===
--- clang/lib/StaticAnalyzer/Core/BugReporter.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporter.cpp
@@ -1988,12 +1988,11 @@
 
   const SourceManager &SM = getSo

[PATCH] D96033: [clang-repl] Land initial infrastructure for incremental parsing

2021-05-19 Thread Jonas Hahnfeld via Phabricator via cfe-commits

Hahnfeld added a comment.

In D96033#2767884 , @teemperor wrote:

> In D96033#2766502 , @phosek wrote:
>
>> In D96033#2766372 , @v.g.vassilev 
>> wrote:
>>
>>> In D96033#2766332 , @phosek wrote:
>>>
 We've started seeing `LLVM ERROR: out of memory` on our 2-stage LTO Linux 
 builders after this change landed. It looks like linking `clang-repl` 
 always fails on our bot, but I've also seen OOM when linking 
 `ClangCodeGenTests` and `FrontendTests`. Do you have any idea why this 
 could be happening? We'd appreciate any help since our bots have been 
 broken for several days now.
>>>
>>> Ouch. Are the bot logs public? If not maybe a stacktrace could be useful. 
>>> `clang-repl` combines a lot of libraries across llvm and clang that usually 
>>> are compiled separately. For instance we put in memory most of the clang 
>>> frontend, the backend and the JIT. Could it be we are hitting some real 
>>> limit?
>>
>> Yes, they are, see 
>> https://luci-milo.appspot.com/p/fuchsia/builders/prod/clang-linux-x64, but 
>> there isn't much information in there unfortunately. It's possible that 
>> we're hitting some limit, but these bots use 32-core instances with 128GB 
>> RAM which I'd hope is enough even for the LTO build.
>
> I think the specs are fine for just building with LTO, but I am not sure if 
> that's enough to for the worst case when running `ninja -j320` with an LTO 
> build (which is what your job is doing). Can you try limiting your link jobs 
> to something like 16 or 32 (e.g., `-DLLVM_PARALLEL_LINK_JOBS=32`)

Right: On my system, linking `clang` with LTO takes 11.5 GB of RAM while 
`clang-repl` takes 8.4 GB. If the system has 128 GB, I agree that the issue is 
likely too many parallel links; the addition of `clang-repl` triggers this 
because it adds another large binary that the build system has to process at 
some point.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96033/new/

https://reviews.llvm.org/D96033

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

[PATCH] D102756: [clang-repl] Tell the LLJIT the exact target triple we use.

2021-05-19 Thread Vassil Vassilev via Phabricator via cfe-commits

v.g.vassilev created this revision.
v.g.vassilev added reviewers: hubert.reinterpretcast, lhames, uweigand.
v.g.vassilev requested review of this revision.

Some systems use a different data layout. For instance, s390x the layout of 
machines with vector registers is different from the ones without. In such 
cases, the JIT will automatically detect the vector registers and go out of 
sync.

This patch tells the JIT what is the target triple of the generated code so 
that both ends are in sync.

Discussion available in https://reviews.llvm.org/D96033. Thanks to @uweigand 
for helping understand the issue.


Repository:
  rC Clang

https://reviews.llvm.org/D102756

Files:
  clang/lib/Interpreter/IncrementalExecutor.cpp
  clang/lib/Interpreter/IncrementalExecutor.h
  clang/lib/Interpreter/Interpreter.cpp


Index: clang/lib/Interpreter/Interpreter.cpp
===
--- clang/lib/Interpreter/Interpreter.cpp
+++ clang/lib/Interpreter/Interpreter.cpp
@@ -16,6 +16,7 @@
 #include "IncrementalExecutor.h"
 #include "IncrementalParser.h"
 
+#include "clang/AST/ASTContext.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/CodeGen/ModuleBuilder.h"
 #include "clang/CodeGen/ObjectFilePCHContainerOperations.h"
@@ -204,8 +205,11 @@
 llvm::Error Interpreter::Execute(Transaction &T) {
   assert(T.TheModule);
   if (!IncrExecutor) {
+const llvm::Triple &Triple =
+  getCompilerInstance()->getASTContext().getTargetInfo().getTriple();
 llvm::Error Err = llvm::Error::success();
-IncrExecutor = std::make_unique(*TSCtx, Err);
+IncrExecutor = std::make_unique(*TSCtx, Err, Triple);
+
 if (Err)
   return Err;
   }
Index: clang/lib/Interpreter/IncrementalExecutor.h
===
--- clang/lib/Interpreter/IncrementalExecutor.h
+++ clang/lib/Interpreter/IncrementalExecutor.h
@@ -14,6 +14,7 @@
 #define LLVM_CLANG_LIB_INTERPRETER_INCREMENTALEXECUTOR_H
 
 #include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/Triple.h"
 #include "llvm/ExecutionEngine/Orc/ExecutionUtils.h"
 
 #include 
@@ -34,7 +35,8 @@
   llvm::orc::ThreadSafeContext &TSCtx;
 
 public:
-  IncrementalExecutor(llvm::orc::ThreadSafeContext &TSC, llvm::Error &Err);
+  IncrementalExecutor(llvm::orc::ThreadSafeContext &TSC, llvm::Error &Err,
+  const llvm::Triple &Triple);
   ~IncrementalExecutor();
 
   llvm::Error addModule(std::unique_ptr M);
Index: clang/lib/Interpreter/IncrementalExecutor.cpp
===
--- clang/lib/Interpreter/IncrementalExecutor.cpp
+++ clang/lib/Interpreter/IncrementalExecutor.cpp
@@ -26,12 +26,14 @@
 namespace clang {
 
 IncrementalExecutor::IncrementalExecutor(llvm::orc::ThreadSafeContext &TSC,
- llvm::Error &Err)
+ llvm::Error &Err,
+ const llvm::Triple &Triple)
 : TSCtx(TSC) {
   using namespace llvm::orc;
   llvm::ErrorAsOutParameter EAO(&Err);
 
-  if (auto JitOrErr = LLJITBuilder().create())
+  auto JTMB = JITTargetMachineBuilder(Triple);
+  if (auto JitOrErr = LLJITBuilder().setJITTargetMachineBuilder(JTMB).create())
 Jit = std::move(*JitOrErr);
   else {
 Err = JitOrErr.takeError();


Index: clang/lib/Interpreter/Interpreter.cpp
===
--- clang/lib/Interpreter/Interpreter.cpp
+++ clang/lib/Interpreter/Interpreter.cpp
@@ -16,6 +16,7 @@
 #include "IncrementalExecutor.h"
 #include "IncrementalParser.h"
 
+#include "clang/AST/ASTContext.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/CodeGen/ModuleBuilder.h"
 #include "clang/CodeGen/ObjectFilePCHContainerOperations.h"
@@ -204,8 +205,11 @@
 llvm::Error Interpreter::Execute(Transaction &T) {
   assert(T.TheModule);
   if (!IncrExecutor) {
+const llvm::Triple &Triple =
+  getCompilerInstance()->getASTContext().getTargetInfo().getTriple();
 llvm::Error Err = llvm::Error::success();
-IncrExecutor = std::make_unique(*TSCtx, Err);
+IncrExecutor = std::make_unique(*TSCtx, Err, Triple);
+
 if (Err)
   return Err;
   }
Index: clang/lib/Interpreter/IncrementalExecutor.h
===
--- clang/lib/Interpreter/IncrementalExecutor.h
+++ clang/lib/Interpreter/IncrementalExecutor.h
@@ -14,6 +14,7 @@
 #define LLVM_CLANG_LIB_INTERPRETER_INCREMENTALEXECUTOR_H
 
 #include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/Triple.h"
 #include "llvm/ExecutionEngine/Orc/ExecutionUtils.h"
 
 #include 
@@ -34,7 +35,8 @@
   llvm::orc::ThreadSafeContext &TSCtx;
 
 public:
-  IncrementalExecutor(llvm::orc::ThreadSafeContext &TSC, llvm::Error &Err);
+  IncrementalExecutor(llvm::orc::ThreadSafeContext &TSC, llvm::Error &Err,
+  const llvm::Triple &Triple);
   ~IncrementalExecutor();
 
   llvm::Error addModule(std::unique_ptr

[PATCH] D96033: [clang-repl] Land initial infrastructure for incremental parsing

2021-05-19 Thread Vassil Vassilev via Phabricator via cfe-commits

v.g.vassilev added a comment.

@uweigand, thanks again for confirming the patch works for you. The 
differential revision is here https://reviews.llvm.org/D102756

@teemperor and @Hahnfeld, thanks for the diagnosis. I forgot to mention that we 
had some exchange on Discord and @phosek kindly agreed to try this special flag.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96033/new/

https://reviews.llvm.org/D96033

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

[clang] 722c39f - [HIP] Tighten checks in hip-include-path.hip test case

2021-05-19 Thread Bjorn Pettersson via cfe-commits


Author: Bjorn Pettersson
Date: 2021-05-19T13:11:57+02:00
New Revision: 722c39fef5ab611b3196e964bb3177a5ab473355

URL: 
https://github.com/llvm/llvm-project/commit/722c39fef5ab611b3196e964bb3177a5ab473355
DIFF: 
https://github.com/llvm/llvm-project/commit/722c39fef5ab611b3196e964bb3177a5ab473355.diff

LOG: [HIP] Tighten checks in hip-include-path.hip test case

The checks (both positive and negative checks) in the test case
hip-include-path.hip could mistakenly end up matching the string
"clang" from the InstalledDir in case the build dir for example
was named "/home/username/build-clang/". Intention with this
patch is to tighten up the checks a bit to filter our the
part of the paths that match with InstalledDir when doing the
checks, as well as matching "/lib/clang/" rather than
just "clang/".

Problem was found when building with
  -DCLANG_DEFAULT_RTLIB=compiler-rt
  -DCLANG_DEFAULT_CXX_STDLIB=libc++
and having "clang/" in the path to the build dir.

Reviewed By: yaxunl

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

Added: 


Modified: 
clang/test/Driver/hip-include-path.hip

Removed: 




diff  --git a/clang/test/Driver/hip-include-path.hip 
b/clang/test/Driver/hip-include-path.hip
index 7af06fabe5ae..4e15e28fb276 100644
--- a/clang/test/Driver/hip-include-path.hip
+++ b/clang/test/Driver/hip-include-path.hip
@@ -16,36 +16,40 @@
 // RUN:   -std=c++11 --rocm-path=%S/Inputs/rocm -nogpuinc -nogpulib %s 2>&1 \
 // RUN:   | FileCheck -check-prefixes=COMMON,CLANG,NOHIP %s
 
+// COMMON: InstalledDir: [[ROOT:[^"]*]]/bin
+
 // COMMON-LABEL: "{{[^"]*}}clang{{[^"]*}}" "-cc1"
-// CLANG-SAME: "-internal-isystem" "{{.*}}clang/{{.*}}/include/cuda_wrappers"
-// NOCLANG-NOT: "{{.*}}clang/{{.*}}/include/cuda_wrappers"
+// CLANG-SAME: "-internal-isystem" 
"[[ROOT]]/lib/clang/{{.*}}/include/cuda_wrappers"
+// NOCLANG-NOT: "[[ROOT]]/lib/clang/{{.*}}/include/cuda_wrappers"
 // HIP-SAME: "-internal-isystem" "{{.*}}Inputs/rocm/include"
 // NOHIP-NOT: "{{.*}}Inputs/rocm/include"
 // HIP-SAME: "-include" "__clang_hip_runtime_wrapper.h"
 // NOHIP-NOT: "-include" "__clang_hip_runtime_wrapper.h"
 // skip check of standard C++ include path
-// CLANG-SAME: "-internal-isystem" "{{.*}}clang/{{.*}}/include"
-// NOCLANG-NOT: "{{.*}}clang/{{.*}}/include"
+// CLANG-SAME: "-internal-isystem" "[[ROOT]]/lib/clang/{{.*}}/include"
+// NOCLANG-NOT: "[[ROOT]]/lib/clang/{{.*}}/include"
 
 // COMMON-LABEL: "{{[^"]*}}clang{{[^"]*}}" "-cc1"
-// CLANG-SAME: "-internal-isystem" "{{.*}}clang/{{.*}}/include/cuda_wrappers"
-// NOCLANG-NOT: "{{.*}}clang/{{.*}}/include/cuda_wrappers"
+// CLANG-SAME: "-internal-isystem" 
"[[ROOT]]/lib/clang/{{.*}}/include/cuda_wrappers"
+// NOCLANG-NOT: "[[ROOT]]/lib/clang/{{.*}}/include/cuda_wrappers"
 // HIP-SAME: "-internal-isystem" "{{.*}}Inputs/rocm/include"
 // NOHIP-NOT: "{{.*}}Inputs/rocm/include"
 // HIP-SAME: "-include" "__clang_hip_runtime_wrapper.h"
 // NOHIP-NOT: "-include" "__clang_hip_runtime_wrapper.h"
 // skip check of standard C++ include path
-// CLANG-SAME: "-internal-isystem" "{{.*}}clang/{{.*}}/include"
-// NOCLANG-NOT: "{{.*}}clang/{{.*}}/include"
+// CLANG-SAME: "-internal-isystem" "[[ROOT]]/lib/clang/{{.*}}/include"
+// NOCLANG-NOT: "[[ROOT]]/lib/clang/{{.*}}/include"
 
 // RUN: %clang -c -### -target x86_64-unknown-linux-gnu --cuda-gpu-arch=gfx900 
\
 // RUN:   -std=c++11 --rocm-path=%S/Inputs/rocm -nogpulib %s 2>&1 \
 // RUN:   --hip-version=3.5 | FileCheck -check-prefixes=ROCM35 %s
 
+// ROCM35: InstalledDir: [[ROOT:[^"]*]]/bin
+
 // ROCM35-LABEL: "{{[^"]*}}clang{{[^"]*}}" "-cc1"
-// ROCM35-NOT: "{{.*}}clang/{{.*}}/include/cuda_wrappers"
-// ROCM35-SAME: "-internal-isystem" "{{[^"]*}}clang/{{[^"]*}}"
+// ROCM35-NOT: "[[ROOT]]/lib/clang/{{.*}}/include/cuda_wrappers"
+// ROCM35-SAME: "-internal-isystem" "[[ROOT]]/lib/clang/{{[^"]*}}"
 // ROCM35-SAME: "-internal-isystem" "{{[^"]*}}Inputs/rocm/include"
 // ROCM35-NOT: "-include" "__clang_hip_runtime_wrapper.h"
 // skip check of standard C++ include path
-// ROCM35-SAME: "-internal-isystem" "{{[^"]*}}clang/{{[^"]*}}/include"
+// ROCM35-SAME: "-internal-isystem" "[[ROOT]]/lib/clang/{{.*}}/include"



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

[PATCH] D102683: [analyzer] Check the checker name, rather than the ProgramPointTag when silencing a checker

2021-05-19 Thread Valeriy Savchenko via Phabricator via cfe-commits

vsavchenko added a comment.

Oh, I'm late to the party!
Glad to see you back @Szelethus !


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102683/new/

https://reviews.llvm.org/D102683

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

[PATCH] D102723: [HIP] Tighten checks in hip-include-path.hip test case

2021-05-19 Thread Bjorn Pettersson via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG722c39fef5ab: [HIP] Tighten checks in hip-include-path.hip 
test case (authored by bjope).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102723/new/

https://reviews.llvm.org/D102723

Files:
  clang/test/Driver/hip-include-path.hip


Index: clang/test/Driver/hip-include-path.hip
===
--- clang/test/Driver/hip-include-path.hip
+++ clang/test/Driver/hip-include-path.hip
@@ -16,36 +16,40 @@
 // RUN:   -std=c++11 --rocm-path=%S/Inputs/rocm -nogpuinc -nogpulib %s 2>&1 \
 // RUN:   | FileCheck -check-prefixes=COMMON,CLANG,NOHIP %s
 
+// COMMON: InstalledDir: [[ROOT:[^"]*]]/bin
+
 // COMMON-LABEL: "{{[^"]*}}clang{{[^"]*}}" "-cc1"
-// CLANG-SAME: "-internal-isystem" "{{.*}}clang/{{.*}}/include/cuda_wrappers"
-// NOCLANG-NOT: "{{.*}}clang/{{.*}}/include/cuda_wrappers"
+// CLANG-SAME: "-internal-isystem" 
"[[ROOT]]/lib/clang/{{.*}}/include/cuda_wrappers"
+// NOCLANG-NOT: "[[ROOT]]/lib/clang/{{.*}}/include/cuda_wrappers"
 // HIP-SAME: "-internal-isystem" "{{.*}}Inputs/rocm/include"
 // NOHIP-NOT: "{{.*}}Inputs/rocm/include"
 // HIP-SAME: "-include" "__clang_hip_runtime_wrapper.h"
 // NOHIP-NOT: "-include" "__clang_hip_runtime_wrapper.h"
 // skip check of standard C++ include path
-// CLANG-SAME: "-internal-isystem" "{{.*}}clang/{{.*}}/include"
-// NOCLANG-NOT: "{{.*}}clang/{{.*}}/include"
+// CLANG-SAME: "-internal-isystem" "[[ROOT]]/lib/clang/{{.*}}/include"
+// NOCLANG-NOT: "[[ROOT]]/lib/clang/{{.*}}/include"
 
 // COMMON-LABEL: "{{[^"]*}}clang{{[^"]*}}" "-cc1"
-// CLANG-SAME: "-internal-isystem" "{{.*}}clang/{{.*}}/include/cuda_wrappers"
-// NOCLANG-NOT: "{{.*}}clang/{{.*}}/include/cuda_wrappers"
+// CLANG-SAME: "-internal-isystem" 
"[[ROOT]]/lib/clang/{{.*}}/include/cuda_wrappers"
+// NOCLANG-NOT: "[[ROOT]]/lib/clang/{{.*}}/include/cuda_wrappers"
 // HIP-SAME: "-internal-isystem" "{{.*}}Inputs/rocm/include"
 // NOHIP-NOT: "{{.*}}Inputs/rocm/include"
 // HIP-SAME: "-include" "__clang_hip_runtime_wrapper.h"
 // NOHIP-NOT: "-include" "__clang_hip_runtime_wrapper.h"
 // skip check of standard C++ include path
-// CLANG-SAME: "-internal-isystem" "{{.*}}clang/{{.*}}/include"
-// NOCLANG-NOT: "{{.*}}clang/{{.*}}/include"
+// CLANG-SAME: "-internal-isystem" "[[ROOT]]/lib/clang/{{.*}}/include"
+// NOCLANG-NOT: "[[ROOT]]/lib/clang/{{.*}}/include"
 
 // RUN: %clang -c -### -target x86_64-unknown-linux-gnu --cuda-gpu-arch=gfx900 
\
 // RUN:   -std=c++11 --rocm-path=%S/Inputs/rocm -nogpulib %s 2>&1 \
 // RUN:   --hip-version=3.5 | FileCheck -check-prefixes=ROCM35 %s
 
+// ROCM35: InstalledDir: [[ROOT:[^"]*]]/bin
+
 // ROCM35-LABEL: "{{[^"]*}}clang{{[^"]*}}" "-cc1"
-// ROCM35-NOT: "{{.*}}clang/{{.*}}/include/cuda_wrappers"
-// ROCM35-SAME: "-internal-isystem" "{{[^"]*}}clang/{{[^"]*}}"
+// ROCM35-NOT: "[[ROOT]]/lib/clang/{{.*}}/include/cuda_wrappers"
+// ROCM35-SAME: "-internal-isystem" "[[ROOT]]/lib/clang/{{[^"]*}}"
 // ROCM35-SAME: "-internal-isystem" "{{[^"]*}}Inputs/rocm/include"
 // ROCM35-NOT: "-include" "__clang_hip_runtime_wrapper.h"
 // skip check of standard C++ include path
-// ROCM35-SAME: "-internal-isystem" "{{[^"]*}}clang/{{[^"]*}}/include"
+// ROCM35-SAME: "-internal-isystem" "[[ROOT]]/lib/clang/{{.*}}/include"


Index: clang/test/Driver/hip-include-path.hip
===
--- clang/test/Driver/hip-include-path.hip
+++ clang/test/Driver/hip-include-path.hip
@@ -16,36 +16,40 @@
 // RUN:   -std=c++11 --rocm-path=%S/Inputs/rocm -nogpuinc -nogpulib %s 2>&1 \
 // RUN:   | FileCheck -check-prefixes=COMMON,CLANG,NOHIP %s
 
+// COMMON: InstalledDir: [[ROOT:[^"]*]]/bin
+
 // COMMON-LABEL: "{{[^"]*}}clang{{[^"]*}}" "-cc1"
-// CLANG-SAME: "-internal-isystem" "{{.*}}clang/{{.*}}/include/cuda_wrappers"
-// NOCLANG-NOT: "{{.*}}clang/{{.*}}/include/cuda_wrappers"
+// CLANG-SAME: "-internal-isystem" "[[ROOT]]/lib/clang/{{.*}}/include/cuda_wrappers"
+// NOCLANG-NOT: "[[ROOT]]/lib/clang/{{.*}}/include/cuda_wrappers"
 // HIP-SAME: "-internal-isystem" "{{.*}}Inputs/rocm/include"
 // NOHIP-NOT: "{{.*}}Inputs/rocm/include"
 // HIP-SAME: "-include" "__clang_hip_runtime_wrapper.h"
 // NOHIP-NOT: "-include" "__clang_hip_runtime_wrapper.h"
 // skip check of standard C++ include path
-// CLANG-SAME: "-internal-isystem" "{{.*}}clang/{{.*}}/include"
-// NOCLANG-NOT: "{{.*}}clang/{{.*}}/include"
+// CLANG-SAME: "-internal-isystem" "[[ROOT]]/lib/clang/{{.*}}/include"
+// NOCLANG-NOT: "[[ROOT]]/lib/clang/{{.*}}/include"
 
 // COMMON-LABEL: "{{[^"]*}}clang{{[^"]*}}" "-cc1"
-// CLANG-SAME: "-internal-isystem" "{{.*}}clang/{{.*}}/include/cuda_wrappers"
-// NOCLANG-NOT: "{{.*}}clang/{{.*}}/include/cuda_wrappers"
+// CLANG-SAME: "-internal-isystem" "[[ROOT]]/lib/clang/{{.*}}/include/cuda_wrappers"
+// NOCLANG-NOT: "[[ROOT]]/lib/clang/{{.*}}/include/cuda_wrappers"
 // HIP-SAME: "-internal-isystem

[PATCH] D102568: [Driver] Delete -mimplicit-it=

2021-05-19 Thread Martin Storsjö via Phabricator via cfe-commits

mstorsjo added a comment.

I would request that this commit should be reverted.

> This is a GNU as

This is true

> and Clang cc1as option, not a GCC option. Users should specify 
> -Wa,-mimplicit-it= instead.

This is not true if you look at any existing stable release - Clang 12.0 
doesn't support `-Wa,-mimplicit-it=`. That was only added in D96285 
 on February 11th.

I would request that you leave both forms of the option to coexist for at least 
a couple stable releases, so that users have a reasonable way of migrating 
forward, without immediately dropping support for all older versions of the 
compiler.

In D102568#2764081 , @nickdesaulniers 
wrote:

> Seems fine to me; the only reference in all of Android that looks problematic 
> might be building libaom for windows: 
> https://android.googlesource.com/platform/external/libaom/+/refs/heads/master/libaom/build/cmake/aom_configure.cmake#158,
>  which I don't think we do for Android.

It turns out that libaom actually doesn't have any standalone gas-style 
assembly, so that option there won't ever have any effect (otherwise it would 
have broken my build).

But this change did break my build in these places:
https://code.videolan.org/videolan/x264/-/blob/b684ebe04a6f80f8207a57940a1fa00e25274f81/configure#L855
https://chromium.googlesource.com/webm/libvpx/+/66c1ff6850fd53bcf5c17247569bea1d700d6247/build/make/configure.sh#1012
https://github.com/cisco/openh264/blob/089d6c6d83ab6e5d451ef18808bb6c46faf553a2/build/platform-mingw_nt.mk#L23

> Should be an easy fix either way (replace `-mimplicit-it=always` with 
> `-Wa,-wmimplicit-it=always`).

No, it's not an easy fix as no existing releases of Clang support it.

> The linux kernel already uses `-Wa,-mimplicit-it`. Also, GCC rejects the 
> option:

Indeed.

The reason why it's common in these cases, and why the difference to GCC's 
behaviour in these cases hasn't been noticed, is that this flag is often needed 
for windows/arm, and GCC doesn't support windows/arm at all, so all those 
codepaths have only been used with Clang.

(The reason why this option often is used for that target, is that windows on 
arm only supports thumb - while many codecs with handwritten assembly is 
written to be built in arm mode only; handwritten assembly that hasn't taken 
thumb mode into account may need `-mimplicit-it=always` to work in thumb mode.)

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102568/new/

https://reviews.llvm.org/D102568

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

[PATCH] D102238: [TableGen] [Clang] Clean up arm_mve.td file

2021-05-19 Thread Paul C. Anagnostopoulos via Phabricator via cfe-commits

Paul-C-Anagnostopoulos added a comment.

Sorry, I removed it but didn't update this review. All set now.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102238/new/

https://reviews.llvm.org/D102238

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

[clang] f5b5426 - [clang] Fix a crash on CheckArgAlignment.

2021-05-19 Thread Haojian Wu via cfe-commits


Author: Haojian Wu
Date: 2021-05-19T13:29:28+02:00
New Revision: f5b5426433c9e6c24615ef1286d72c527b0b15dd

URL: 
https://github.com/llvm/llvm-project/commit/f5b5426433c9e6c24615ef1286d72c527b0b15dd
DIFF: 
https://github.com/llvm/llvm-project/commit/f5b5426433c9e6c24615ef1286d72c527b0b15dd.diff

LOG: [clang] Fix a crash on CheckArgAlignment.

We might encounter an undeduced type before calling getTypeAlignInChars.

NOTE: this retrieves the fix from
8f80c66bd2982788a8eede4419684ca72f48b9a2, which was removed in Adam's
followup fix fbfcfdbf6828b8d36f4ec0ff5f4eac11fb1411a5. We originally
thought the crash was caused by recovery-ast, but it turns out it can
occur for other cases, e.g. typo-correction.

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

Added: 


Modified: 
clang/lib/Sema/SemaChecking.cpp
clang/test/SemaCXX/typo-correction-crash.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index f3f1ccd77528..5daa88dcdbe1 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -4571,7 +4571,8 @@ void Sema::CheckArgAlignment(SourceLocation Loc, 
NamedDecl *FDecl,
 
   // Find expected alignment, and the actual alignment of the passed object.
   // getTypeAlignInChars requires complete types
-  if (ParamTy->isIncompleteType() || ArgTy->isIncompleteType())
+  if (ParamTy->isIncompleteType() || ArgTy->isIncompleteType() ||
+  ParamTy->isUndeducedType() || ArgTy->isUndeducedType())
 return;
 
   CharUnits ParamAlign = Context.getTypeAlignInChars(ParamTy);

diff  --git a/clang/test/SemaCXX/typo-correction-crash.cpp 
b/clang/test/SemaCXX/typo-correction-crash.cpp
index 49ea4c1bf16c..2a77c9df505e 100644
--- a/clang/test/SemaCXX/typo-correction-crash.cpp
+++ b/clang/test/SemaCXX/typo-correction-crash.cpp
@@ -42,3 +42,12 @@ class S {
   }
 };
 }
+
+namespace NoCrashOnCheckArgAlignment {
+template  void b(a &);
+void test() {
+  for (auto file_data :b(files_db_data)); // expected-error {{use of 
undeclared identifier 'files_db_data'; did you mean 'file_data'?}} \
+  // expected-note {{'file_data' 
declared here}} \
+  // expected-error {{cannot use type 
'void' as a range}}
+}
+}



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

[PATCH] D102750: [clang] Fix a crash on CheckArgAlignment.

2021-05-19 Thread Haojian Wu via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf5b5426433c9: [clang] Fix a crash on CheckArgAlignment. 
(authored by hokein).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102750/new/

https://reviews.llvm.org/D102750

Files:
  clang/lib/Sema/SemaChecking.cpp
  clang/test/SemaCXX/typo-correction-crash.cpp


Index: clang/test/SemaCXX/typo-correction-crash.cpp
===
--- clang/test/SemaCXX/typo-correction-crash.cpp
+++ clang/test/SemaCXX/typo-correction-crash.cpp
@@ -42,3 +42,12 @@
   }
 };
 }
+
+namespace NoCrashOnCheckArgAlignment {
+template  void b(a &);
+void test() {
+  for (auto file_data :b(files_db_data)); // expected-error {{use of 
undeclared identifier 'files_db_data'; did you mean 'file_data'?}} \
+  // expected-note {{'file_data' 
declared here}} \
+  // expected-error {{cannot use type 
'void' as a range}}
+}
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -4571,7 +4571,8 @@
 
   // Find expected alignment, and the actual alignment of the passed object.
   // getTypeAlignInChars requires complete types
-  if (ParamTy->isIncompleteType() || ArgTy->isIncompleteType())
+  if (ParamTy->isIncompleteType() || ArgTy->isIncompleteType() ||
+  ParamTy->isUndeducedType() || ArgTy->isUndeducedType())
 return;
 
   CharUnits ParamAlign = Context.getTypeAlignInChars(ParamTy);


Index: clang/test/SemaCXX/typo-correction-crash.cpp
===
--- clang/test/SemaCXX/typo-correction-crash.cpp
+++ clang/test/SemaCXX/typo-correction-crash.cpp
@@ -42,3 +42,12 @@
   }
 };
 }
+
+namespace NoCrashOnCheckArgAlignment {
+template  void b(a &);
+void test() {
+  for (auto file_data :b(files_db_data)); // expected-error {{use of undeclared identifier 'files_db_data'; did you mean 'file_data'?}} \
+  // expected-note {{'file_data' declared here}} \
+  // expected-error {{cannot use type 'void' as a range}}
+}
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -4571,7 +4571,8 @@
 
   // Find expected alignment, and the actual alignment of the passed object.
   // getTypeAlignInChars requires complete types
-  if (ParamTy->isIncompleteType() || ArgTy->isIncompleteType())
+  if (ParamTy->isIncompleteType() || ArgTy->isIncompleteType() ||
+  ParamTy->isUndeducedType() || ArgTy->isUndeducedType())
 return;
 
   CharUnits ParamAlign = Context.getTypeAlignInChars(ParamTy);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] c98833c - Reapply "[clang][deps] Support inferred modules"

2021-05-19 Thread Jan Svoboda via cfe-commits


Author: Michael Spencer
Date: 2021-05-19T13:35:51+02:00
New Revision: c98833cdaad01787ea70ecdfabb05a7e142a6671

URL: 
https://github.com/llvm/llvm-project/commit/c98833cdaad01787ea70ecdfabb05a7e142a6671
DIFF: 
https://github.com/llvm/llvm-project/commit/c98833cdaad01787ea70ecdfabb05a7e142a6671.diff

LOG: Reapply "[clang][deps] Support inferred modules"

This reapplies commit 95033eb3 that reverted commit 1d9e8e13.

The tests were failing on Windows due to spaces and backslashes in paths not 
being handled carefully.

Added: 

clang/test/ClangScanDeps/Inputs/frameworks/Inferred.framework/Frameworks/Sub.framework/Headers/Sub.h

clang/test/ClangScanDeps/Inputs/frameworks/Inferred.framework/Headers/Inferred.h
clang/test/ClangScanDeps/Inputs/frameworks/System.framework/Headers/System.h

clang/test/ClangScanDeps/Inputs/frameworks/System.framework/Modules/module.modulemap
clang/test/ClangScanDeps/Inputs/frameworks/module.modulemap
clang/test/ClangScanDeps/Inputs/modules_inferred_cdb.json
clang/test/ClangScanDeps/modules-inferred-explicit-build.m
clang/test/ClangScanDeps/modules-inferred.m
clang/utils/module-deps-to-rsp.py

Modified: 
clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
clang/test/ClangScanDeps/modules-full.cpp

Removed: 




diff  --git a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp 
b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
index 51b7fc48628c..d9e0bead13bc 100644
--- a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -33,7 +33,6 @@ makeInvocationForModuleBuildWithoutPaths(const ModuleDeps 
&Deps,
   CI.getFrontendOpts().IsSystemModule = Deps.IsSystem;
 
   CI.getLangOpts()->ImplicitModules = false;
-  CI.getHeaderSearchOpts().ImplicitModuleMaps = false;
 
   return CI;
 }
@@ -179,13 +178,22 @@ ModuleID ModuleDepCollectorPP::handleTopLevelModule(const 
Module *M) {
   const FileEntry *ModuleMap = Instance.getPreprocessor()
.getHeaderSearchInfo()
.getModuleMap()
-   .getContainingModuleMapFile(M);
+   .getModuleMapFileForUniquing(M);
   MD.ClangModuleMapFile = std::string(ModuleMap ? ModuleMap->getName() : "");
 
   serialization::ModuleFile *MF =
   MDC.Instance.getASTReader()->getModuleManager().lookup(M->getASTFile());
   MDC.Instance.getASTReader()->visitInputFiles(
   *MF, true, true, [&](const serialization::InputFile &IF, bool isSystem) {
+// __inferred_module.map is the result of the way in which an implicit
+// module build handles inferred modules. It adds an overlay VFS with
+// this file in the proper directory and relies on the rest of Clang to
+// handle it like normal. With explicitly built modules we don't need
+// to play VFS tricks, so replace it with the correct module map.
+if (IF.getFile()->getName().endswith("__inferred_module.map")) {
+  MD.FileDeps.insert(ModuleMap->getName());
+  return;
+}
 MD.FileDeps.insert(IF.getFile()->getName());
   });
 

diff  --git 
a/clang/test/ClangScanDeps/Inputs/frameworks/Inferred.framework/Frameworks/Sub.framework/Headers/Sub.h
 
b/clang/test/ClangScanDeps/Inputs/frameworks/Inferred.framework/Frameworks/Sub.framework/Headers/Sub.h
new file mode 100644
index ..e69de29bb2d1

diff  --git 
a/clang/test/ClangScanDeps/Inputs/frameworks/Inferred.framework/Headers/Inferred.h
 
b/clang/test/ClangScanDeps/Inputs/frameworks/Inferred.framework/Headers/Inferred.h
new file mode 100644
index ..1855e4fad5f8
--- /dev/null
+++ 
b/clang/test/ClangScanDeps/Inputs/frameworks/Inferred.framework/Headers/Inferred.h
@@ -0,0 +1 @@
+typedef int inferred;

diff  --git 
a/clang/test/ClangScanDeps/Inputs/frameworks/System.framework/Headers/System.h 
b/clang/test/ClangScanDeps/Inputs/frameworks/System.framework/Headers/System.h
new file mode 100644
index ..a90c62886749
--- /dev/null
+++ 
b/clang/test/ClangScanDeps/Inputs/frameworks/System.framework/Headers/System.h
@@ -0,0 +1 @@
+enum { bigger_than_int = 0x8000 };

diff  --git 
a/clang/test/ClangScanDeps/Inputs/frameworks/System.framework/Modules/module.modulemap
 
b/clang/test/ClangScanDeps/Inputs/frameworks/System.framework/Modules/module.modulemap
new file mode 100644
index ..51f72f664235
--- /dev/null
+++ 
b/clang/test/ClangScanDeps/Inputs/frameworks/System.framework/Modules/module.modulemap
@@ -0,0 +1,3 @@
+framework module System [system] {
+  umbrella header "System.h"
+}

diff  --git a/clang/test/ClangScanDeps/Inputs/frameworks/module.modulemap 
b/clang/test/ClangScanDeps/Inputs/frameworks/module.modulemap
new file mode 100644
index ..e3bad873c7e7
--- /dev/null
+++ b/clang/test/Cla

[PATCH] D102495: [clang][deps] Support inferred modules

2021-05-19 Thread Jan Svoboda via Phabricator via cfe-commits

jansvoboda11 added a comment.

Re-landed this in c98833cdaad01787ea70ecdfabb05a7e142a6671 
 after 
reproducing and fixing the Windows failure.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102495/new/

https://reviews.llvm.org/D102495

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

[PATCH] D102761: [clangd] New ParsingCallback for semantics changes

2021-05-19 Thread Kadir Cetinkaya via Phabricator via cfe-commits

kadircet created this revision.
kadircet added a reviewer: sammccall.
Herald added subscribers: usaxena95, arphaman, javed.absar.
kadircet requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

Previously notification of the Server about semantic happened strictly
before notification of the AST thread.
Hence a racy Server could make a request (like semantic tokens) after
the notification, with the assumption that it'll be served fresh
content. But it wasn't true if AST thread wasn't notified about the
change yet.

This change reverses the order of those notifications to prevent racy
interactions.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D102761

Files:
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/TUScheduler.cpp
  clang-tools-extra/clangd/TUScheduler.h


Index: clang-tools-extra/clangd/TUScheduler.h
===
--- clang-tools-extra/clangd/TUScheduler.h
+++ clang-tools-extra/clangd/TUScheduler.h
@@ -169,6 +169,10 @@
 
   /// Called whenever the TU status is updated.
   virtual void onFileUpdated(PathRef File, const TUStatus &Status) {}
+
+  /// Semantics for the TU might've changed (e.g. a new preamble), any actions
+  /// on the file are guranteed to see new semantics after the callback.
+  virtual void onSemanticsMaybeChanged(PathRef File) {}
 };
 
 /// Handles running tasks for ClangdServer and managing the resources (e.g.,
Index: clang-tools-extra/clangd/TUScheduler.cpp
===
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -909,6 +909,7 @@
 ASTPeer.updatePreamble(std::move(Req.CI), std::move(Req.Inputs),
LatestBuild, std::move(Req.CIDiags),
std::move(Req.WantDiags));
+Callbacks.onSemanticsMaybeChanged(FileName);
   });
 
   if (!LatestBuild || Inputs.ForceRebuild) {
Index: clang-tools-extra/clangd/ClangdServer.cpp
===
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -75,8 +75,6 @@
  const CanonicalIncludes &CanonIncludes) override {
 if (FIndex)
   FIndex->updatePreamble(Path, Version, Ctx, std::move(PP), CanonIncludes);
-if (ServerCallbacks)
-  ServerCallbacks->onSemanticsMaybeChanged(Path);
   }
 
   void onMainAST(PathRef Path, ParsedAST &AST, PublishFn Publish) override {
@@ -105,6 +103,11 @@
   ServerCallbacks->onFileUpdated(File, Status);
   }
 
+  void onSemanticsMaybeChanged(PathRef File) override {
+if (ServerCallbacks)
+  ServerCallbacks->onSemanticsMaybeChanged(File);
+  }
+
 private:
   FileIndex *FIndex;
   ClangdServer::Callbacks *ServerCallbacks;


Index: clang-tools-extra/clangd/TUScheduler.h
===
--- clang-tools-extra/clangd/TUScheduler.h
+++ clang-tools-extra/clangd/TUScheduler.h
@@ -169,6 +169,10 @@
 
   /// Called whenever the TU status is updated.
   virtual void onFileUpdated(PathRef File, const TUStatus &Status) {}
+
+  /// Semantics for the TU might've changed (e.g. a new preamble), any actions
+  /// on the file are guranteed to see new semantics after the callback.
+  virtual void onSemanticsMaybeChanged(PathRef File) {}
 };
 
 /// Handles running tasks for ClangdServer and managing the resources (e.g.,
Index: clang-tools-extra/clangd/TUScheduler.cpp
===
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -909,6 +909,7 @@
 ASTPeer.updatePreamble(std::move(Req.CI), std::move(Req.Inputs),
LatestBuild, std::move(Req.CIDiags),
std::move(Req.WantDiags));
+Callbacks.onSemanticsMaybeChanged(FileName);
   });
 
   if (!LatestBuild || Inputs.ForceRebuild) {
Index: clang-tools-extra/clangd/ClangdServer.cpp
===
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -75,8 +75,6 @@
  const CanonicalIncludes &CanonIncludes) override {
 if (FIndex)
   FIndex->updatePreamble(Path, Version, Ctx, std::move(PP), CanonIncludes);
-if (ServerCallbacks)
-  ServerCallbacks->onSemanticsMaybeChanged(Path);
   }
 
   void onMainAST(PathRef Path, ParsedAST &AST, PublishFn Publish) override {
@@ -105,6 +103,11 @@
   ServerCallbacks->onFileUpdated(File, Status);
   }
 
+  void onSemanticsMaybeChanged(PathRef File) override {
+if (ServerCallbacks)
+  ServerCallbacks->onSemanticsMaybeChanged(File);
+  }
+
 private:
   FileIndex *FIndex;
   ClangdServer::Callbacks *ServerCallbacks;
__

[PATCH] D95425: Implementation of global.get/set for reftypes in LLVM IR

2021-05-19 Thread Paulo Matos via Phabricator via cfe-commits

pmatos updated this revision to Diff 346411.
pmatos added a comment.

Rebase and update minor details and fix a few lint warnings...
Non-functional changes only.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D95425/new/

https://reviews.llvm.org/D95425

Files:
  clang/lib/Basic/Targets/WebAssembly.cpp
  llvm/include/llvm/CodeGen/TargetLowering.h
  llvm/include/llvm/CodeGen/ValueTypes.h
  llvm/include/llvm/Support/MachineValueType.h
  llvm/lib/CodeGen/CodeGenPrepare.cpp
  llvm/lib/CodeGen/MachineOperand.cpp
  llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
  llvm/lib/CodeGen/TargetLoweringBase.cpp
  llvm/lib/CodeGen/ValueTypes.cpp
  llvm/lib/Target/WebAssembly/Utils/WebAssemblyUtilities.cpp
  llvm/lib/Target/WebAssembly/Utils/WebAssemblyUtilities.h
  llvm/lib/Target/WebAssembly/WebAssemblyFastISel.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyISD.def
  llvm/lib/Target/WebAssembly/WebAssemblyISelDAGToDAG.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.h
  llvm/lib/Target/WebAssembly/WebAssemblyInstrTable.td
  llvm/lib/Target/WebAssembly/WebAssemblyMCInstLower.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
  llvm/test/CodeGen/WebAssembly/externref-globalget.ll
  llvm/test/CodeGen/WebAssembly/externref-globalset.ll
  llvm/test/CodeGen/WebAssembly/externref-inttoptr.ll
  llvm/test/CodeGen/WebAssembly/externref-ptrtoint.ll
  llvm/test/CodeGen/WebAssembly/externref-undef.ll
  llvm/test/CodeGen/WebAssembly/externref-unsized-load.ll
  llvm/test/CodeGen/WebAssembly/externref-unsized-store.ll
  llvm/test/CodeGen/WebAssembly/funcref-call.ll
  llvm/test/CodeGen/WebAssembly/funcref-globalget.ll
  llvm/test/CodeGen/WebAssembly/funcref-globalset.ll

Index: llvm/test/CodeGen/WebAssembly/funcref-globalset.ll
===
--- /dev/null
+++ llvm/test/CodeGen/WebAssembly/funcref-globalset.ll
@@ -0,0 +1,20 @@
+; RUN: llc < %s --mtriple=wasm32-unknown-unknown -asm-verbose=false -mattr=+reference-types | FileCheck %s
+
+%func = type opaque
+%funcref = type %func addrspace(20)* ;; addrspace 20 is nonintegral
+
+@funcref_global = local_unnamed_addr addrspace(1) global %funcref undef
+
+define void @set_funcref_global(%funcref %g) {
+  ;; this generates a global.set of @funcref_global
+  store %funcref %g, %funcref addrspace(1)* @funcref_global
+  ret void
+}
+
+; CHECK-LABEL: set_funcref_global:
+; CHECK-NEXT: functype   set_funcref_global (funcref) -> ()
+; CHECK-NEXT: local.get  0
+; CHECK-NEXT: global.set funcref_global
+; CHECK-NEXT: end_function
+
+; CHECK: .globl funcref_global
Index: llvm/test/CodeGen/WebAssembly/funcref-globalget.ll
===
--- /dev/null
+++ llvm/test/CodeGen/WebAssembly/funcref-globalget.ll
@@ -0,0 +1,19 @@
+; RUN: llc < %s --mtriple=wasm32-unknown-unknown -asm-verbose=false -mattr=+reference-types | FileCheck %s
+
+%func = type opaque
+%funcref = type %func addrspace(20)* ;; addrspace 20 is nonintegral
+
+@funcref_global = local_unnamed_addr addrspace(1) global %funcref undef
+
+define %funcref @return_funcref_global() {
+  ;; this generates a global.get of @funcref_global
+  %ref = load %funcref, %funcref addrspace(1)* @funcref_global
+  ret %funcref %ref
+}
+
+; CHECK-LABEL: return_funcref_global:
+; CHECK-NEXT: .functype   return_funcref_global () -> (funcref)
+; CHECK-NEXT: global.get funcref_global
+; CHECK-NEXT: end_function
+
+; CHECK: .globl funcref_global
Index: llvm/test/CodeGen/WebAssembly/funcref-call.ll
===
--- /dev/null
+++ llvm/test/CodeGen/WebAssembly/funcref-call.ll
@@ -0,0 +1,23 @@
+; RUN: llc < %s --mtriple=wasm32-unknown-unknown -asm-verbose=false -mattr=+reference-types | FileCheck %s
+
+%func = type void ()
+%funcref = type %func addrspace(20)* ;; addrspace 20 is nonintegral
+
+define void @call_funcref(%funcref %ref) {
+  call addrspace(20) void %ref() 
+  ret void
+}
+
+; CHECK-LABEL: call_funcref:
+; CHECK-NEXT: functype   call_funcref (funcref) -> ()
+; CHECK-NEXT: i32.const 0
+; CHECK-NEXT: local.get 0
+; CHECK-NEXT: table.set __funcref_call_table
+; CHECK-NEXT: local.get 0
+; CHECK-NEXT: call_indirect __funcref_call_table, () -> ()
+; CHECK-NEXT: i32.const 0
+; CHECK-NEXT: ref.null func
+; CHECK-NEXT: table.set __funcref_call_table
+; CHECK-NEXT: end_function
+
+; CHECK: .tabletype __funcref_call_table, funcref
Index: llvm/test/CodeGen/WebAssembly/externref-unsized-store.ll
===
--- /dev/null
+++ llvm/test/CodeGen/WebAssembly/externref-unsized-store.ll
@@ -0,0 +1,11 @@
+; RUN: not llc --mtriple=wasm32-unknown-unknown -asm-verbose=false -mattr=+reference-types < %s 2>&1 | FileCheck %s --check-prefix=CHECK-ERROR
+
+%extern = type opaque
+%externref = type %extern addrspace(10)*
+
+define void @st

[PATCH] D95425: Implementation of global.get/set for reftypes in LLVM IR

2021-05-19 Thread Paulo Matos via Phabricator via cfe-commits

pmatos added a comment.

@deadalnix @tlively @sbc100 anything missing here to get this landed?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D95425/new/

https://reviews.llvm.org/D95425

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

[PATCH] D100252: [clang] Fix for "Bug 27113 - MSVC-compat __identifier implementation incomplete"

2021-05-19 Thread Hans Wennborg via Phabricator via cfe-commits

hans added a comment.

Interesting, I hadn't seen __identifier before. It seems like a pretty esoteric 
feature.




Comment at: clang/lib/Lex/PPMacroExpansion.cpp:1817
+else if (Tok.is(tok::string_literal)) {
+  const StringRef StrData(Tok.getLiteralData() + 1, Tok.getLength() - 2);
+  Tok.setIdentifierInfo(this->getIdentifierInfo(StrData));

I'm not sure this is the right way to get the string out of a string_literal.. 
this pattern seems more common: 
https://github.com/llvm/llvm-project/blob/llvmorg-12.0.0/clang/lib/Lex/Pragma.cpp#L755

For example, what should happen with __identifier("foo" "bar")?



Comment at: clang/lib/Lex/PPMacroExpansion.cpp:1819
+  Tok.setIdentifierInfo(this->getIdentifierInfo(StrData));
+  Tok.setKind(tok::identifier);
+} else {

It says in the bug report that MSVC emits a warning for this. What does the 
warning look like, and would it make sense for Clang too warn too (it might 
not, just wondering if we should consider it).



Comment at: clang/test/Parser/MicrosoftExtensions.cpp:273
   __identifier(+) // expected-error {{cannot convert '+' token to an 
identifier}}
-  __identifier("foo") // expected-error {{cannot convert  
token to an identifier}}
   __identifier(;) // expected-error {{cannot convert ';' token to an 
identifier}}

Maybe instead of removing this, move it to the cases that work above. For 
example, I guess this should work now?

```
int __identifier("foo") = 42;
int bar = foo;
```



Comment at: clang/test/Parser/MicrosoftExtensions.cpp:276
+  __identifier("+"); // expected-error {{use of undeclared identifier '+'}}
+  __identifier(";"); // expected-error {{use of undeclared identifier ';'}}
 }

It would be interesting to see a non-error test case too, perhaps with a 
mangled name like in the PR, and to see that it makes it through to LLVM IR 
correctly..


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100252/new/

https://reviews.llvm.org/D100252

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

[PATCH] D102752: [clang-offload-bundler] Delimit input/output file names by '--' for llvm-objcopy

2021-05-19 Thread Alexey Bataev via Phabricator via cfe-commits

ABataev accepted this revision.
ABataev added a comment.
This revision is now accepted and ready to land.

LG


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102752/new/

https://reviews.llvm.org/D102752

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

[PATCH] D98798: Produce warning for performing pointer arithmetic on a null pointer.

2021-05-19 Thread Jamie Schmeiser via Phabricator via cfe-commits

jamieschmeiser reopened this revision.
jamieschmeiser added a comment.
This revision is now accepted and ready to land.

Re-opening because it was reverted.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98798/new/

https://reviews.llvm.org/D98798

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

[PATCH] D102488: [clang][deps] Prune unused header search paths

2021-05-19 Thread Jan Svoboda via Phabricator via cfe-commits

jansvoboda11 updated this revision to Diff 346418.
jansvoboda11 added a comment.

Fix test on Windows


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102488/new/

https://reviews.llvm.org/D102488

Files:
  clang/include/clang/Lex/HeaderSearch.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/include/clang/Serialization/ASTReader.h
  clang/include/clang/Serialization/ModuleFile.h
  clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
  clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
  clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp
  clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
  clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
  clang/test/ClangScanDeps/Inputs/header-search-pruning/a/a.h
  clang/test/ClangScanDeps/Inputs/header-search-pruning/b/b.h
  clang/test/ClangScanDeps/Inputs/header-search-pruning/cdb.json
  clang/test/ClangScanDeps/Inputs/header-search-pruning/mod.h
  clang/test/ClangScanDeps/Inputs/header-search-pruning/module.modulemap
  clang/test/ClangScanDeps/header-search-pruning.cpp
  clang/tools/clang-scan-deps/ClangScanDeps.cpp

Index: clang/tools/clang-scan-deps/ClangScanDeps.cpp
===
--- clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -163,6 +163,11 @@
 "'-fmodule-file=', '-o', '-fmodule-map-file='."),
 llvm::cl::init(false), llvm::cl::cat(DependencyScannerCategory));
 
+static llvm::cl::opt OptimizeArgs(
+"optimize-args",
+llvm::cl::desc("Whether to optimize command-line arguments of modules."),
+llvm::cl::init(false), llvm::cl::cat(DependencyScannerCategory));
+
 llvm::cl::opt
 NumThreads("j", llvm::cl::Optional,
llvm::cl::desc("Number of worker threads to use (default: use "
@@ -357,7 +362,26 @@
 
 private:
   StringRef lookupPCMPath(ModuleID MID) {
-return Modules[IndexedModuleID{MID, 0}].ImplicitModulePCMPath;
+auto PCMPath = PCMPaths.insert({IndexedModuleID{MID, 0}, ""});
+if (PCMPath.second)
+  PCMPath.first->second = constructPCMPath(lookupModuleDeps(MID));
+return PCMPath.first->second;
+  }
+
+  /// Construct a path where to put the explicitly built PCM - essentially the
+  /// path to implicitly built PCM with the context hash replaced by the final
+  /// (potentially modified) context hash.
+  std::string constructPCMPath(const ModuleDeps &MD) const {
+const std::string &ImplicitPCMPath = MD.ImplicitModulePCMPath;
+StringRef Filename = llvm::sys::path::filename(ImplicitPCMPath);
+StringRef ImplicitContextHashPath =
+llvm::sys::path::parent_path(ImplicitPCMPath);
+StringRef ModuleCachePath =
+llvm::sys::path::parent_path(ImplicitContextHashPath);
+
+SmallString<64> ExplicitPCMPath = ModuleCachePath;
+llvm::sys::path::append(ExplicitPCMPath, MD.ID.ContextHash, Filename);
+return std::string(ExplicitPCMPath);
   }
 
   const ModuleDeps &lookupModuleDeps(ModuleID MID) {
@@ -395,6 +419,8 @@
   std::mutex Lock;
   std::unordered_map
   Modules;
+  std::unordered_map
+  PCMPaths;
   std::vector Inputs;
 };
 
@@ -554,7 +580,7 @@
   SharedStream DependencyOS(llvm::outs());
 
   DependencyScanningService Service(ScanMode, Format, ReuseFileManager,
-SkipExcludedPPRanges);
+SkipExcludedPPRanges, OptimizeArgs);
   llvm::ThreadPool Pool(llvm::hardware_concurrency(NumThreads));
   std::vector> WorkerTools;
   for (unsigned I = 0; I < Pool.getThreadCount(); ++I)
Index: clang/test/ClangScanDeps/header-search-pruning.cpp
===
--- /dev/null
+++ clang/test/ClangScanDeps/header-search-pruning.cpp
@@ -0,0 +1,87 @@
+// RUN: rm -rf %t && mkdir -p %t
+// RUN: cp -r %S/Inputs/header-search-pruning/* %t
+// RUN: cp %S/header-search-pruning.cpp %t/header-search-pruning.cpp
+// RUN: sed -e "s|DIR|%/t|g" -e "s|DEFINES|-DINCLUDE_A|g" %S/Inputs/header-search-pruning/cdb.json > %t/cdb_a.json
+// RUN: sed -e "s|DIR|%/t|g" -e "s|DEFINES|-DINCLUDE_B|g" %S/Inputs/header-search-pruning/cdb.json > %t/cdb_b.json
+// RUN: sed -e "s|DIR|%/t|g" -e "s|DEFINES|-DINCLUDE_A -DINCLUDE_B|g" %S/Inputs/header-search-pruning/cdb.json > %t/cdb_ab.json
+//
+// RUN: clang-scan-deps -compilation-database %t/cdb_a.json -format experimental-full -optimize-args >> %t/result_a.json
+// RUN: cat %t/result_a.json | sed 's/\\/\//g' | FileCheck --check-prefixes=CHECK_A %s
+//
+// RUN: clang-scan-deps -compilation-database %t/cdb_b.json -format experimental-full -optimize-args >> %t/result_b.json
+// RUN: cat %t/result_b.json | sed 's/\\/\//g' | FileCheck --c

[PATCH] D102241: [clang] p1099 4/5: using enum EnumTag

2021-05-19 Thread Nathan Sidwell via Phabricator via cfe-commits

urnathan updated this revision to Diff 346421.
urnathan added a comment.

Added ASTmatchers for UsingEnumDecl along with unit-tests (both of which I was 
previously unaware of)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102241/new/

https://reviews.llvm.org/D102241

Files:
  clang-tools-extra/clangd/FindTarget.cpp
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/DeclCXX.h
  clang/include/clang/AST/JSONNodeDumper.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/AST/TextNodeDumper.h
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/include/clang/Basic/DeclNodes.td
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Index/IndexSymbol.h
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Sema/Template.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/Decl.cpp
  clang/lib/AST/DeclBase.cpp
  clang/lib/AST/DeclCXX.cpp
  clang/lib/AST/DeclPrinter.cpp
  clang/lib/AST/JSONNodeDumper.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/ASTMatchers/ASTMatchersInternal.cpp
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGDebugInfo.h
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Index/IndexSymbol.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Sema/SemaCXXScopeSpec.cpp
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/lib/Serialization/ASTCommon.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/AST/ast-dump-using-enum.cpp
  clang/test/SemaCXX/cxx20-using-enum.cpp
  clang/tools/libclang/CIndex.cpp
  clang/unittests/AST/ASTImporterTest.cpp
  clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -1428,6 +1428,22 @@
   usingDecl(hasAnyUsingShadowDecl(hasName("a");
 }
 
+TEST_P(ASTMatchersTest, UsingEnumDecl_MatchesUsingEnumDeclarations) {
+  if (!GetParam().isCXX20OrLater()) {
+return;
+  }
+  EXPECT_TRUE(
+  matches("namespace X { enum x {}; } using enum X::x;", usingEnumDecl()));
+}
+
+TEST_P(ASTMatchersTest, UsingEnumDecl_MatchesShadowUsingDeclarations) {
+  if (!GetParam().isCXX20OrLater()) {
+return;
+  }
+  EXPECT_TRUE(matches("namespace f { enum a {b}; } using enum f::a;",
+  usingEnumDecl(hasAnyUsingShadowDecl(hasName("b");
+}
+
 TEST_P(ASTMatchersTest, UsingDirectiveDecl_MatchesUsingNamespace) {
   if (!GetParam().isCXX()) {
 return;
Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -844,7 +844,15 @@
   testImport("namespace foo { int bar; }"
  "void declToImport() { using foo::bar; }",
  Lang_CXX03, "", Lang_CXX03, Verifier,
- functionDecl(hasDescendant(usingDecl(;
+ functionDecl(hasDescendant(usingDecl(hasName("bar");
+}
+
+TEST_P(ImportDecl, ImportUsingEnumDecl) {
+  MatchVerifier Verifier;
+  testImport("namespace foo { enum bar { baz, toto, quux }; }"
+ "void declToImport() { using enum foo::bar; }",
+ Lang_CXX20, "", Lang_CXX20, Verifier,
+ functionDecl(hasDescendant(usingEnumDecl(hasName("bar");
 }
 
 /// \brief Matches shadow declarations introduced into a scope by a
@@ -862,10 +870,16 @@
 
 TEST_P(ImportDecl, ImportUsingShadowDecl) {
   MatchVerifier Verifier;
+  // from using-decl
   testImport("namespace foo { int bar; }"
  "namespace declToImport { using foo::bar; }",
  Lang_CXX03, "", Lang_CXX03, Verifier,
- namespaceDecl(has(usingShadowDecl(;
+ namespaceDecl(has(usingShadowDecl(hasName("bar");
+  // from using-enum-decl
+  testImport("namespace foo { enum bar {baz, toto, quux }; }"
+ "namespace declToImport { using enum foo::bar; }",
+ Lang_CXX20, "", Lang_CXX20, Verifier,
+ namespaceDecl(has(usingShadowDecl(hasName("baz");
 }
 
 TEST_P(ImportExpr, ImportUnresolvedLookupExpr) {
Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -6540,6 +6540,7 @@
   }
 
   case Decl::Using:
+  case Decl::UsingEnum:
 return MakeCursorOverloadedDeclRef(cast(D), D->getLocation(),
TU);
 
Index: clang/test/SemaCXX/cxx20-using-enum.cpp
==

[PATCH] D101777: [clang] p1099 1/5: [NFC] Break out BaseUsingDecl from UsingDecl

2021-05-19 Thread Nathan Sidwell via Phabricator via cfe-commits

urnathan added inline comments.



Comment at: clang/lib/AST/ASTImporter.cpp:4623
 
-  Expected ToUsingOrErr = import(D->getUsingDecl());
-  if (!ToUsingOrErr)
-return ToUsingOrErr.takeError();
+  Expected ToIntroducerOrErr = import(D->getIntroducer());
+  if (!ToIntroducerOrErr)

shafik wrote:
> urnathan wrote:
> > shafik wrote:
> > > It would nice to expand upon the  `ImportUsingShadowDecl` in 
> > > `ASTImporterTest.cpp` as we expand this change to make sure the 
> > > ASTImporter is importing all the cases correctly. Right now we just have 
> > > a very basic test there.
> > Please clarify what you are requiring of this patch series. What you 
> > describe sounds like an orthogonal desire.
> Sure, happy to clarify. As the functionality is complete, tests should be 
> added to verify that the `ASTImporter` correctly deals with the new 
> functionality but it would be nice as we add that new test to also expand the 
> tests further but that would be optional but probably not too much more work.
I've added unit tests for UsingEnumDecl into patch 4, and extended the 
UsingDecl tests a bit further


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101777/new/

https://reviews.llvm.org/D101777

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

[PATCH] D102623: [CodeGen][AArch64][SVE] Canonicalize intrinsic rdffr{ => _z}

2021-05-19 Thread Peter Waller via Phabricator via cfe-commits

peterwaller-arm updated this revision to Diff 346423.
peterwaller-arm added a comment.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

- Move test per review comment.
- Update ACLE test.
- Rebase.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102623/new/

https://reviews.llvm.org/D102623

Files:
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_rdffr.c
  llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp
  llvm/test/CodeGen/AArch64/sve-intrinsics-ffr-manipulation.ll
  llvm/test/Transforms/InstCombine/AArch64/sve-intrinsics-rdffr-predication.ll

Index: llvm/test/Transforms/InstCombine/AArch64/sve-intrinsics-rdffr-predication.ll
===
--- /dev/null
+++ llvm/test/Transforms/InstCombine/AArch64/sve-intrinsics-rdffr-predication.ll
@@ -0,0 +1,19 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -S -instcombine < %s | FileCheck %s
+
+target triple = "aarch64-unknown-linux-gnu"
+
+; Test that rdffr is substituted with predicated form which enables ptest optimization later.
+define  @predicate_rdffr() #0 {
+; CHECK-LABEL: @predicate_rdffr(
+; CHECK-NEXT:[[TMP1:%.*]] = call  @llvm.aarch64.sve.ptrue.nxv16i1(i32 31)
+; CHECK-NEXT:[[OUT:%.*]] = call  @llvm.aarch64.sve.rdffr.z( [[TMP1]])
+; CHECK-NEXT:ret  [[OUT]]
+;
+  %out = call  @llvm.aarch64.sve.rdffr()
+  ret  %out
+}
+
+declare  @llvm.aarch64.sve.rdffr()
+
+attributes #0 = { "target-features"="+sve" }
Index: llvm/test/CodeGen/AArch64/sve-intrinsics-ffr-manipulation.ll
===
--- llvm/test/CodeGen/AArch64/sve-intrinsics-ffr-manipulation.ll
+++ llvm/test/CodeGen/AArch64/sve-intrinsics-ffr-manipulation.ll
@@ -1,33 +1,51 @@
-; RUN: llc -mtriple=aarch64-linux-gnu -mattr=+sve < %s | FileCheck %s
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s | FileCheck %s
+
+target triple = "aarch64-unknown-linux-gnu"
 
 ;
 ; RDFFR
 ;
 
-define  @rdffr() {
+define  @rdffr() #0 {
 ; CHECK-LABEL: rdffr:
-; CHECK: rdffr p0.b
-; CHECK-NEXT: ret
+; CHECK:   // %bb.0:
+; CHECK-NEXT:rdffr p0.b
+; CHECK-NEXT:ret
   %out = call  @llvm.aarch64.sve.rdffr()
   ret  %out
 }
 
-define  @rdffr_z( %pg) {
+define  @rdffr_z( %pg) #0 {
 ; CHECK-LABEL: rdffr_z:
-; CHECK: rdffr p0.b, p0/z
-; CHECK-NEXT: ret
+; CHECK:   // %bb.0:
+; CHECK-NEXT:rdffr p0.b, p0/z
+; CHECK-NEXT:ret
   %out = call  @llvm.aarch64.sve.rdffr.z( %pg)
   ret  %out
 }
 
+; Test that rdffr.z followed by ptest optimizes to flags-setting rdffrs.
+define i1 @rdffr_z_ptest( %pg) #0 {
+; CHECK-LABEL: rdffr_z_ptest:
+; CHECK:   // %bb.0:
+; CHECK-NEXT:rdffrs p0.b, p0/z
+; CHECK-NEXT:cset w0, ne
+; CHECK-NEXT:ret
+  %rdffr = call  @llvm.aarch64.sve.rdffr.z( %pg)
+  %out = call i1 @llvm.aarch64.sve.ptest.any.nxv16i1( %pg,  %rdffr)
+  ret i1 %out
+}
+
 ;
 ; SETFFR
 ;
 
-define void @set_ffr() {
+define void @set_ffr() #0 {
 ; CHECK-LABEL: set_ffr:
-; CHECK: setffr
-; CHECK-NEXT: ret
+; CHECK:   // %bb.0:
+; CHECK-NEXT:setffr
+; CHECK-NEXT:ret
   call void @llvm.aarch64.sve.setffr()
   ret void
 }
@@ -36,10 +54,11 @@
 ; WRFFR
 ;
 
-define void @wrffr( %a) {
+define void @wrffr( %a) #0 {
 ; CHECK-LABEL: wrffr:
-; CHECK: wrffr p0.b
-; CHECK-NEXT: ret
+; CHECK:   // %bb.0:
+; CHECK-NEXT:wrffr p0.b
+; CHECK-NEXT:ret
   call void @llvm.aarch64.sve.wrffr( %a)
   ret void
 }
@@ -48,3 +67,7 @@
 declare  @llvm.aarch64.sve.rdffr.z()
 declare void @llvm.aarch64.sve.setffr()
 declare void @llvm.aarch64.sve.wrffr()
+
+declare i1 @llvm.aarch64.sve.ptest.any.nxv16i1(, )
+
+attributes #0 = { "target-features"="+sve" }
Index: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp
===
--- llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp
+++ llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp
@@ -470,6 +470,23 @@
   return IC.replaceInstUsesWith(II, Extract);
 }
 
+static Optional instCombineRDFFR(InstCombiner &IC,
+IntrinsicInst &II) {
+  LLVMContext &Ctx = II.getContext();
+  IRBuilder<> Builder(Ctx);
+  Builder.SetInsertPoint(&II);
+  // Replace rdffr with predicated rdffr.z intrinsic, so that optimizePTestInstr
+  // can work with RDFFR_PP for ptest elimination.
+  auto *AllPat =
+  ConstantInt::get(Type::getInt32Ty(Ctx), AArch64SVEPredPattern::all);
+  auto *PTrue = Builder.CreateIntrinsic(Intrinsic::aarch64_sve_ptrue,
+{II.getType()}, {AllPat});
+  auto *RDFFR =
+  Builder.CreateIntrinsic(Intrinsic::aarch64_sve_rdffr_z, {}, {PTrue});
+  RDFFR->takeName(&II);
+  return IC.replaceInstUsesWith(II, RDFFR);
+}
+
 Optional
 AArch64TTIImpl::instCombineIntrinsic(InstCombiner &IC,
  IntrinsicInst &II) co

[PATCH] D102770: [clang-tidy] Fix a crash for raw-string-literal check.

2021-05-19 Thread Haojian Wu via Phabricator via cfe-commits

hokein created this revision.
hokein added a reviewer: gribozavr2.
Herald added a subscriber: xazax.hun.
hokein requested review of this revision.
Herald added a project: clang-tools-extra.

getSourceText could return an empty string for error cases (e.g. invalid
source locaiton), this patch makes the code more robust.

The crash did happen in our internal codebase, but unfortunately I
didn't manage to get a reproduce case. One thing I can confirm from
the core dump is that the crash is caused by calling isRawStringLiteral
on an empty Text.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D102770

Files:
  clang-tools-extra/clang-tidy/modernize/RawStringLiteralCheck.cpp


Index: clang-tools-extra/clang-tidy/modernize/RawStringLiteralCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/RawStringLiteralCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/RawStringLiteralCheck.cpp
@@ -56,7 +56,7 @@
   *Result.SourceManager, Result.Context->getLangOpts());
   StringRef Text = Lexer::getSourceText(CharRange, *Result.SourceManager,
 Result.Context->getLangOpts());
-  if (isRawStringLiteral(Text))
+  if (Text.empty() || isRawStringLiteral(Text))
 return false;
 
   return containsEscapes(Text, R"('\"?x01)");


Index: clang-tools-extra/clang-tidy/modernize/RawStringLiteralCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/RawStringLiteralCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/RawStringLiteralCheck.cpp
@@ -56,7 +56,7 @@
   *Result.SourceManager, Result.Context->getLangOpts());
   StringRef Text = Lexer::getSourceText(CharRange, *Result.SourceManager,
 Result.Context->getLangOpts());
-  if (isRawStringLiteral(Text))
+  if (Text.empty() || isRawStringLiteral(Text))
 return false;
 
   return containsEscapes(Text, R"('\"?x01)");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D102669: [analyzer][ctu] Fix wrong 'multiple definitions' errors caused by space characters in lookup names when parsing the ctu index file

2021-05-19 Thread Gabor Marton via Phabricator via cfe-commits

martong added a comment.

First of all, thank you for the patch!

We had a meeting with my colleges (@steakhal, @gamesh411) and we agreed in the 
following. This issue you are trying to solve here is indeed a serious problem, 
but we'd like to suggest an alternative and perhaps more durable solution. In 
`CrossTranslationUnitContext::getLookupName(const NamedDecl *ND)` it would be 
possible to extend the returned string with a prefix that encodes the length of 
the USR string.
So, instead of

  c:@F@g#I# ctu-other.cpp.ast

we'd get

  9:c:@F@g#I# ctu-other.cpp.ast

This way, we could handle even file names with spaces in them.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102669/new/

https://reviews.llvm.org/D102669

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

[PATCH] D102772: [SanitizeCoverage] Add support for NoSanitizeCoverage function attribute

2021-05-19 Thread Marco Elver via Phabricator via cfe-commits

melver created this revision.
melver added reviewers: vitalybuka, morehouse, glider, dvyukov.
Herald added subscribers: dexonsmith, jdoerfert, pengfei, hiraditya.
Herald added a reviewer: aaron.ballman.
melver requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

We really ought to support no_sanitize("coverage") in line with other
sanitizers. This came up again in discussions on the Linux-kernel
mailing lists, because we currently do workarounds using objtool to
remove coverage instrumentation. Since that support is only on x86, to
continue support coverage instrumentation on other architectures, we
must support selectively disabling coverage instrumentation via function
attributes.

Unfortunately, for SanitizeCoverage, it has not been implemented as a
sanitizer via fsanitize= and associated options in Sanitizers.def, but
rolls its own option fsanitize-coverage. This meant that we never got
"automatic" no_sanitize attribute support.

Implement no_sanitize attribute support by special-casing the string
"coverage" in the NoSanitizeAttr implementation. To keep the feature as
unintrusive to existing IR generation as possible, define a new negative
function attribute NoSanitizeCoverage to propagate the information
through to the instrumentation pass.

Fixes: https://bugs.llvm.org/show_bug.cgi?id=49035


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D102772

Files:
  clang/docs/SanitizerCoverage.rst
  clang/include/clang/Basic/Attr.td
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/sanitize-coverage.c
  llvm/include/llvm/Bitcode/LLVMBitCodes.h
  llvm/include/llvm/IR/Attributes.td
  llvm/lib/Bitcode/Reader/BitcodeReader.cpp
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/IR/Attributes.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
  llvm/lib/Transforms/Utils/CodeExtractor.cpp

Index: llvm/lib/Transforms/Utils/CodeExtractor.cpp
===
--- llvm/lib/Transforms/Utils/CodeExtractor.cpp
+++ llvm/lib/Transforms/Utils/CodeExtractor.cpp
@@ -954,6 +954,7 @@
   case Attribute::NonLazyBind:
   case Attribute::NoRedZone:
   case Attribute::NoUnwind:
+  case Attribute::NoSanitizeCoverage:
   case Attribute::NullPointerIsValid:
   case Attribute::OptForFuzzing:
   case Attribute::OptimizeNone:
Index: llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
===
--- llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
+++ llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
@@ -621,6 +621,8 @@
 return;
   if (Blocklist && Blocklist->inSection("coverage", "fun", F.getName()))
 return;
+  if (F.hasFnAttribute(Attribute::NoSanitizeCoverage))
+return;
   if (Options.CoverageType >= SanitizerCoverageOptions::SCK_Edge)
 SplitAllCriticalEdges(F, CriticalEdgeSplittingOptions().setIgnoreUnreachableDests());
   SmallVector IndirCalls;
Index: llvm/lib/IR/Verifier.cpp
===
--- llvm/lib/IR/Verifier.cpp
+++ llvm/lib/IR/Verifier.cpp
@@ -1661,6 +1661,7 @@
   case Attribute::NoCfCheck:
   case Attribute::NoUnwind:
   case Attribute::NoInline:
+  case Attribute::NoSanitizeCoverage:
   case Attribute::AlwaysInline:
   case Attribute::OptimizeForSize:
   case Attribute::StackProtect:
Index: llvm/lib/IR/Attributes.cpp
===
--- llvm/lib/IR/Attributes.cpp
+++ llvm/lib/IR/Attributes.cpp
@@ -442,6 +442,8 @@
 return "noprofile";
   if (hasAttribute(Attribute::NoUnwind))
 return "nounwind";
+  if (hasAttribute(Attribute::NoSanitizeCoverage))
+return "no_sanitize_coverage";
   if (hasAttribute(Attribute::OptForFuzzing))
 return "optforfuzzing";
   if (hasAttribute(Attribute::OptimizeNone))
Index: llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
===
--- llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
+++ llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
@@ -686,6 +686,8 @@
 return bitc::ATTR_KIND_NO_PROFILE;
   case Attribute::NoUnwind:
 return bitc::ATTR_KIND_NO_UNWIND;
+  case Attribute::NoSanitizeCoverage:
+return bitc::ATTR_KIND_NO_SANITIZE_COVERAGE;
   case Attribute::NullPointerIsValid:
 return bitc::ATTR_KIND_NULL_POINTER_IS_VALID;
   case Attribute::OptForFuzzing:
Index: llvm/lib/Bitcode/Reader/BitcodeReader.cpp
===
--- llvm/lib/Bitcode/Reader/BitcodeReader.cpp
+++ llvm/lib/Bitcode/Reader/BitcodeReader.cpp
@@ -1474,6 +1474,8 @@
 return Attribute::NoCfCheck;
   case bitc::ATTR_KIND_NO_UNWIND:
 return Attribute::NoUnwind;
+  case bitc::ATTR_KIND_NO_SANITIZE_COVERAGE:
+return Attribute::NoSanitizeCoverage;
   case bitc::

[PATCH] D102773: [clang] Invalide a non-dependent-type RecordDecl when it has any dependent-type base class specifier.

2021-05-19 Thread Haojian Wu via Phabricator via cfe-commits

hokein created this revision.
hokein added a reviewer: adamcz.
hokein requested review of this revision.
Herald added a project: clang.

This happens during the error-recovery, and it would esacpe all
dependent-type check guards in getTypeInfo/constexpr-evaluator code
paths, which lead to crashes.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D102773

Files:
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/SemaTemplate/temp_class_spec.cpp


Index: clang/test/SemaTemplate/temp_class_spec.cpp
===
--- clang/test/SemaTemplate/temp_class_spec.cpp
+++ clang/test/SemaTemplate/temp_class_spec.cpp
@@ -375,3 +375,16 @@
   Bar() : Foo() {}
 };
 } // namespace
+
+namespace Crash {
+template
+class Base {};
+
+template class Foo;
+
+template 
+class Foo : public Base {}; // expected-error{{partial specialization 
of 'Foo' does not use any of its template parameters}}
+
+// verify that getASTRecordLayout doesn't crash on the 
ClassTemplateSpecializationDecl.
+constexpr int s = sizeof(Foo);
+}
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -2508,6 +2508,14 @@
   }
 }
 
+// Make sure that we don't make an ill-formed AST where the type of the
+// Class is non-dependent and its attached base class specifier is an
+// dependent type, which violates invariants in many clang code paths (e.g.
+// constexpr evaluator). If this case happens (in errory-recovery mode), we
+// explicitly mark the Class decl invalid. The diagnostic was already
+// emitted.
+if (!Class->getTypeForDecl()->isDependentType())
+  Class->setInvalidDecl();
 return new (Context) CXXBaseSpecifier(SpecifierRange, Virtual,
   Class->getTagKind() == TTK_Class,
   Access, TInfo, EllipsisLoc);


Index: clang/test/SemaTemplate/temp_class_spec.cpp
===
--- clang/test/SemaTemplate/temp_class_spec.cpp
+++ clang/test/SemaTemplate/temp_class_spec.cpp
@@ -375,3 +375,16 @@
   Bar() : Foo() {}
 };
 } // namespace
+
+namespace Crash {
+template
+class Base {};
+
+template class Foo;
+
+template 
+class Foo : public Base {}; // expected-error{{partial specialization of 'Foo' does not use any of its template parameters}}
+
+// verify that getASTRecordLayout doesn't crash on the ClassTemplateSpecializationDecl.
+constexpr int s = sizeof(Foo);
+}
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -2508,6 +2508,14 @@
   }
 }
 
+// Make sure that we don't make an ill-formed AST where the type of the
+// Class is non-dependent and its attached base class specifier is an
+// dependent type, which violates invariants in many clang code paths (e.g.
+// constexpr evaluator). If this case happens (in errory-recovery mode), we
+// explicitly mark the Class decl invalid. The diagnostic was already
+// emitted.
+if (!Class->getTypeForDecl()->isDependentType())
+  Class->setInvalidDecl();
 return new (Context) CXXBaseSpecifier(SpecifierRange, Virtual,
   Class->getTagKind() == TTK_Class,
   Access, TInfo, EllipsisLoc);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D102149: [analyzer][ctu] Allow loading invocation list from a compilation database automatically detected from the ctu-dir

2021-05-19 Thread Gabor Marton via Phabricator via cfe-commits

martong added a comment.

Thank you for the patch!

Though, the idea is nice, there is a serious technical obstacle here: we cannot 
use the clangTooling lib as a dependency of the CTU lib because that would 
introduce a circular dependency. Actually, this was the reason for introducing 
the invocation list yaml file; we could not use the compilation database 
implementation from tooling 
(https://reviews.llvm.org/D75665?id=260238#inline-722328).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102149/new/

https://reviews.llvm.org/D102149

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

[PATCH] D101763: [analyzer][ctu] Avoid parsing invocation list again and again during on-demand parsing of CTU

2021-05-19 Thread Gabor Marton via Phabricator via cfe-commits

martong added a comment.

First of all, thank you for the patch!
We had a meeting with my colleges (@steakhal, @gamesh411) and we agreed in the 
following. This is indeed an issue and the fix is okay. About the test, we'd 
like to ask if you could create a small test lib with its own 'open' function 
that would simply call the system's open and would print 'open'. And then with 
LD_PREOLOAD you could use that lib in the lit test. (If that approach is not 
working for some reason then we may just mark the test with XFAIL.)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101763/new/

https://reviews.llvm.org/D101763

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

[PATCH] D102062: [analyzer][ctu] Append ctu-dir to ctu-invocation-list for non-absolute paths

2021-05-19 Thread Gabor Marton via Phabricator via cfe-commits

martong added a comment.

First of all, thank you for the patch!
We had a meeting with my colleges (@steakhal, @gamesh411) and we agreed in the 
following.

We'd like to keep the current behavior because this way the behavior is similar 
that we got used to with any other command line tools.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102062/new/

https://reviews.llvm.org/D102062

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

[PATCH] D102772: [SanitizeCoverage] Add support for NoSanitizeCoverage function attribute

2021-05-19 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added inline comments.



Comment at: clang/docs/SanitizerCoverage.rst:319
+It is possible to disable coverage instrumentation for select functions via the
+function attribute ``__attribute__((no_sanitize("coverage")))``.
+

I would expect to see this documentation also in AttrDocs.td within the 
`NoSanitizeDocs`.



Comment at: clang/include/clang/Basic/Attr.td:2902-2906
+  for (auto SanitizerName : sanitizers()) {
+if (SanitizerName == "coverage")
+  return true;
+  }
+  return false;




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102772/new/

https://reviews.llvm.org/D102772

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

[PATCH] D102062: [analyzer][ctu] Append ctu-dir to ctu-invocation-list for non-absolute paths

2021-05-19 Thread Ella Ma via Phabricator via cfe-commits

OikawaKirie abandoned this revision.
OikawaKirie added a comment.

In D102062#2768569 , @martong wrote:

> First of all, thank you for the patch!
> We had a meeting with my colleges (@steakhal, @gamesh411) and we agreed in 
> the following.
>
> We'd like to keep the current behavior because this way the behavior is 
> similar that we got used to with any other command line tools.

Ok, if you all agree with using `ctu-invocation-list` with a path related to 
CWD rather than `ctu-dir`, I will drop this revision here.
Although, I still believe that it would be better to make the behavior similar 
to the ctu index file, which is related to the `ctu-dir` (see the comments in 
the code).

Thanks for your suggestions on this revision.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102062/new/

https://reviews.llvm.org/D102062

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

[PATCH] D102149: [analyzer][ctu] Allow loading invocation list from a compilation database automatically detected from the ctu-dir

2021-05-19 Thread Balázs Benics via Phabricator via cfe-commits

steakhal added a comment.

There are quite a few places where the extdef mappings should be updated.
For discovering them I suggest you asserting the new file format (only for 
detecting them!). This way if you miss one, it wouldn't silently 'work' 
somehow, but raise your attention.

There are a few references to the format of this mapping in the 
`clang/docs/analyzer/user-docs/CrossTranslationUnit.rst` and probably in other 
files.
Those should be updated to match the new format.

I'm sorry for burdening you with all of this, but I think this is the way to 
make this parsing more robust. I really appreciate your work.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102149/new/

https://reviews.llvm.org/D102149

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

[PATCH] D102669: [analyzer][ctu] Fix wrong 'multiple definitions' errors caused by space characters in lookup names when parsing the ctu index file

2021-05-19 Thread Balázs Benics via Phabricator via cfe-commits

steakhal added a comment.

There are quite a few places where the extdef mappings should be updated.
For discovering them I suggest you asserting the new file format (only for 
detecting them!). This way if you miss one, it wouldn't silently 'work' 
somehow, but raise your attention.

There are a few references to the format of this mapping in the 
`clang/docs/analyzer/user-docs/CrossTranslationUnit.rst` and probably in other 
files.
Those should be updated to match the new format.

I'm sorry for burdening you with all of this, but I think this is the way to 
make this parsing more robust. I really appreciate your work.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102669/new/

https://reviews.llvm.org/D102669

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

[PATCH] D102772: [SanitizeCoverage] Add support for NoSanitizeCoverage function attribute

2021-05-19 Thread Marco Elver via Phabricator via cfe-commits

melver updated this revision to Diff 346452.
melver marked 2 inline comments as done.
melver added a comment.

Address Aaron's comments


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102772/new/

https://reviews.llvm.org/D102772

Files:
  clang/docs/SanitizerCoverage.rst
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/sanitize-coverage.c
  llvm/include/llvm/Bitcode/LLVMBitCodes.h
  llvm/include/llvm/IR/Attributes.td
  llvm/lib/Bitcode/Reader/BitcodeReader.cpp
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/IR/Attributes.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
  llvm/lib/Transforms/Utils/CodeExtractor.cpp

Index: llvm/lib/Transforms/Utils/CodeExtractor.cpp
===
--- llvm/lib/Transforms/Utils/CodeExtractor.cpp
+++ llvm/lib/Transforms/Utils/CodeExtractor.cpp
@@ -954,6 +954,7 @@
   case Attribute::NonLazyBind:
   case Attribute::NoRedZone:
   case Attribute::NoUnwind:
+  case Attribute::NoSanitizeCoverage:
   case Attribute::NullPointerIsValid:
   case Attribute::OptForFuzzing:
   case Attribute::OptimizeNone:
Index: llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
===
--- llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
+++ llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
@@ -621,6 +621,8 @@
 return;
   if (Blocklist && Blocklist->inSection("coverage", "fun", F.getName()))
 return;
+  if (F.hasFnAttribute(Attribute::NoSanitizeCoverage))
+return;
   if (Options.CoverageType >= SanitizerCoverageOptions::SCK_Edge)
 SplitAllCriticalEdges(F, CriticalEdgeSplittingOptions().setIgnoreUnreachableDests());
   SmallVector IndirCalls;
Index: llvm/lib/IR/Verifier.cpp
===
--- llvm/lib/IR/Verifier.cpp
+++ llvm/lib/IR/Verifier.cpp
@@ -1661,6 +1661,7 @@
   case Attribute::NoCfCheck:
   case Attribute::NoUnwind:
   case Attribute::NoInline:
+  case Attribute::NoSanitizeCoverage:
   case Attribute::AlwaysInline:
   case Attribute::OptimizeForSize:
   case Attribute::StackProtect:
Index: llvm/lib/IR/Attributes.cpp
===
--- llvm/lib/IR/Attributes.cpp
+++ llvm/lib/IR/Attributes.cpp
@@ -442,6 +442,8 @@
 return "noprofile";
   if (hasAttribute(Attribute::NoUnwind))
 return "nounwind";
+  if (hasAttribute(Attribute::NoSanitizeCoverage))
+return "no_sanitize_coverage";
   if (hasAttribute(Attribute::OptForFuzzing))
 return "optforfuzzing";
   if (hasAttribute(Attribute::OptimizeNone))
Index: llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
===
--- llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
+++ llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
@@ -686,6 +686,8 @@
 return bitc::ATTR_KIND_NO_PROFILE;
   case Attribute::NoUnwind:
 return bitc::ATTR_KIND_NO_UNWIND;
+  case Attribute::NoSanitizeCoverage:
+return bitc::ATTR_KIND_NO_SANITIZE_COVERAGE;
   case Attribute::NullPointerIsValid:
 return bitc::ATTR_KIND_NULL_POINTER_IS_VALID;
   case Attribute::OptForFuzzing:
Index: llvm/lib/Bitcode/Reader/BitcodeReader.cpp
===
--- llvm/lib/Bitcode/Reader/BitcodeReader.cpp
+++ llvm/lib/Bitcode/Reader/BitcodeReader.cpp
@@ -1474,6 +1474,8 @@
 return Attribute::NoCfCheck;
   case bitc::ATTR_KIND_NO_UNWIND:
 return Attribute::NoUnwind;
+  case bitc::ATTR_KIND_NO_SANITIZE_COVERAGE:
+return Attribute::NoSanitizeCoverage;
   case bitc::ATTR_KIND_NULL_POINTER_IS_VALID:
 return Attribute::NullPointerIsValid;
   case bitc::ATTR_KIND_OPT_FOR_FUZZING:
Index: llvm/include/llvm/IR/Attributes.td
===
--- llvm/include/llvm/IR/Attributes.td
+++ llvm/include/llvm/IR/Attributes.td
@@ -154,6 +154,9 @@
 /// Function doesn't unwind stack.
 def NoUnwind : EnumAttr<"nounwind">;
 
+/// No SanitizeCoverage instrumentation.
+def NoSanitizeCoverage : EnumAttr<"no_sanitize_coverage">;
+
 /// Null pointer in address space zero is valid.
 def NullPointerIsValid : EnumAttr<"null_pointer_is_valid">;
 
Index: llvm/include/llvm/Bitcode/LLVMBitCodes.h
===
--- llvm/include/llvm/Bitcode/LLVMBitCodes.h
+++ llvm/include/llvm/Bitcode/LLVMBitCodes.h
@@ -666,6 +666,7 @@
   ATTR_KIND_NO_PROFILE = 73,
   ATTR_KIND_VSCALE_RANGE = 74,
   ATTR_KIND_SWIFT_ASYNC = 75,
+  ATTR_KIND_NO_SANITIZE_COVERAGE = 76,
 };
 
 enum ComdatSelectionKindCodes {
Index: clang/test/CodeGen/sanitize-coverage.c
===
--- clang/test/Co

[PATCH] D102724: Revert "[AIX] Avoid structor alias; die before bad alias codegen"

2021-05-19 Thread Steven Wan via Phabricator via cfe-commits

stevewan added a comment.

Might be a dumb question, but I see this is a revert to rGb116ded 
, do we 
also want to revert the changes added to 
`llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp`?




Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4964
   if (!RawTriple.isOSDarwin() && !RawTriple.isNVPTX() &&
-  !RawTriple.isAMDGPU() && !RawTriple.isOSAIX())
+  !RawTriple.isAMDGPU())
 CmdArgs.push_back("-mconstructor-aliases");

jasonliu wrote:
> Format as clang-format suggest. You could run clang-format or 
> git-clang-format just for your commits by the way. 
FWIW, if `clang-format` is installed on your host (you can do this via homebrew 
on mac IIRC), arc would use it as the linter when you do `arc lint` or `arc 
diff`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102724/new/

https://reviews.llvm.org/D102724

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

[PATCH] D102779: [clang-tidy] cppcoreguidelines-explicit-constructor-and-conversion: new alias

2021-05-19 Thread Marco Gartmann via Phabricator via cfe-commits

mgartmann created this revision.
Herald added subscribers: jeroen.dobbelaere, shchenz, kbarton, xazax.hun, 
nemanjai.
mgartmann requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Since the `google-explicit-constructor` check coincides with rule

Created alias `cppcoreguidelines-explicit-constructor-and-conversion`


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D102779

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-explicit-constructor-and-conversion.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst


Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -407,6 +407,7 @@
`cppcoreguidelines-avoid-c-arrays 
`_, `modernize-avoid-c-arrays 
`_,
`cppcoreguidelines-avoid-magic-numbers 
`_, `readability-magic-numbers 
`_,
`cppcoreguidelines-c-copy-assignment-signature 
`_, 
`misc-unconventional-assign-operator 
`_,
+   `cppcoreguidelines-explicit-constructor-and-conversion 
`_, 
`google-explicit-constructor `_, "Yes"
`cppcoreguidelines-explicit-virtual-functions 
`_, `modernize-use-override 
`_, "Yes"
`cppcoreguidelines-non-private-member-variables-in-classes 
`_, 
`misc-non-private-member-variables-in-classes 
`_,
`fuchsia-header-anon-namespaces `_, 
`google-build-namespaces `_,
Index: 
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-explicit-constructor-and-conversion.rst
===
--- /dev/null
+++ 
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-explicit-constructor-and-conversion.rst
@@ -0,0 +1,14 @@
+.. title:: clang-tidy - cppcoreguidelines-explicit-constructor-and-conversion
+.. meta::
+   :http-equiv=refresh: 5;URL=google-explicit-constructor.html
+
+cppcoreguidelines-explicit-constructor
+==
+
+The cppcoreguidelines-explicit-constructor-and-conversion check is an alias,
+please see `google-explicit-constructor `_
+for more information.
+
+This check implements `C.46 
`_
+and `C.164 
`_
+from the CppCoreGuidelines.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -127,6 +127,11 @@
   :doc:`concurrency-thread-canceltype-asynchronous
   ` was added.
 
+- New alias :doc:`cppcoreguidelines-explicit-constructor-and-conversion
+  ` to
+  :doc:`google-explicit-constructor
+  ` was added.
+
 Changes in existing checks
 ^^
 
Index: 
clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
===
--- 
clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
+++ 
clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
@@ -9,6 +9,7 @@
 #include "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
+#include "../google/ExplicitConstructorCheck.h"
 #include "../misc/NonPrivateMemberVariablesInClassesCheck.h"
 #include "../misc/UnconventionalAssignOperatorCheck.h"
 #include "../modernize/AvoidCArraysCheck.h"
@@ -52,6 +53,8 @@
 "cppcoreguidelines-avoid-magic-numbers");
 CheckFactories.registerCheck(
 "cppcoreguidelines-avoid-non-const-global-variables");
+CheckFactories.registerCheck(
+"cppcoreguidelines-explicit-constructor-and-conversion");
 CheckFactories.registerCheck(
 "cppcoreguidelines-explicit-virtual-functions");
 CheckFactories.registerCheck(


Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -407,6 +407,7 @@
`cppcoreguidelines-avoid-c-arrays `_, `modernize-avoid-c-arrays `_,
`cppcoreguidelines-avoid-magic-numbers `_, `readability-magic-numbers `_,
`cppcoreguidelines-c-copy-assignment-signature `_, `misc-unconventional-assign-operator `_,
+   `cppcoreguidelines-explicit-constructor-and-conversion `_, `google-explicit-constructor `_, "Yes"
`cppcoreguidelines-explicit-virtual-functions `_, `modernize-use-override `_, "Yes"
`cppcoreguidelines-non-private-member-variables-in-classes `_, `misc-non-private-member-variables-in

[PATCH] D102724: Revert "[AIX] Avoid structor alias; die before bad alias codegen"

2021-05-19 Thread Jake Egan via Phabricator via cfe-commits

Jake-Egan added a comment.

In D102724#2768663 , @stevewan wrote:

> Might be a dumb question, but I see this is a revert to rGb116ded 
> , do we 
> also want to revert the changes added to 
> `llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp`?

The changes to that file seems to be already reverted by: 
https://reviews.llvm.org/D79127


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102724/new/

https://reviews.llvm.org/D102724

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

[PATCH] D102149: [analyzer][ctu] Allow loading invocation list from a compilation database automatically detected from the ctu-dir

2021-05-19 Thread Ella Ma via Phabricator via cfe-commits

OikawaKirie added a comment.

In D102149#2768541 , @martong wrote:

> Thank you for the patch!
>
> Though, the idea is nice, there is a serious technical obstacle here: we 
> cannot use the clangTooling lib as a dependency of the CTU lib because that 
> would introduce a circular dependency. Actually, this was the reason for 
> introducing the invocation list yaml file; we could not use the compilation 
> database implementation from tooling 
> (https://reviews.llvm.org/D75665?id=260238#inline-722328).

According to my recent experiences on using clang-check, converting the 
compilation database to an invocation list is also an acceptable solution.
Currently, a better solution may be adding a tool to handle the conversion, 
just as what has been mentioned in the revision you presented. Although, it 
would be a very simple python script such as:

  a = dict()
  for i in json.load(open("/path/to/compile_commands.json")):
  a[os.path.abspath(os.path.join(i['directory'], i['file']))] = \
  (shlex.split(i['command']) if 'command' in i else i['arguments']) + \
  ['-Xclang', '-working-directory=' + i['directory']]
  print(yaml.dump(a));

If you agree with this idea, I will follow the way to create a tool for the 
conversion and drop this revision.

Actually, I am writing a makefile to schedule the ctu-index generation as well 
as analyzing the code with the clang-check tool. And the python script snippet 
presented above is just a part of my makefile.
Maybe I can add my makefile to as a part of the analyzer.

Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:711-712
+
+  List.emplace_back("-Xclang");
+  List.emplace_back("-working-directory=" + CC.Directory);
+}

The approach of converting a compilation database to an invocation list is just 
simply adding the `-working-directory` argument for the `"directory"` entry for 
each compile command object. Therefore, a script doing the conversion can make 
it simpler and easier to use.

The main reason why this patch is submitted is I previously do not know the 
`-working-directory` argument. Without this argument, it would be very 
difficult and complex to convert a compilation database to an invocation list.

If you agree that my approach of converting the database (presented in the for 
loop) is OK, I will continue with making a tool for the conversion.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102149/new/

https://reviews.llvm.org/D102149

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

[PATCH] D102715: [AIX] Fix LIT failure on native aix

2021-05-19 Thread Xiangling Liao via Phabricator via cfe-commits

Xiangling_L updated this revision to Diff 346462.
Xiangling_L added a comment.

Adjusted the testcase so that it also tests the property well on aix;


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102715/new/

https://reviews.llvm.org/D102715

Files:
  clang/test/Sema/struct-packed-align.c


Index: clang/test/Sema/struct-packed-align.c
===
--- clang/test/Sema/struct-packed-align.c
+++ clang/test/Sema/struct-packed-align.c
@@ -146,18 +146,17 @@
 // GCC 4.4 but the change can lead to differences in the structure layout.
 // See the documentation of -Wpacked-bitfield-compat for more information.
 struct packed_chars {
-  char a:4;
+  char a : 8, b : 8, c : 8, d : 4;
 #ifdef __ORBIS__
   // Test for pre-r254596 clang behavior on the PS4 target. PS4 must maintain
   // ABI backwards compatibility.
-  char b:8 __attribute__ ((packed));
+  char e : 8 __attribute__((packed));
   // expected-warning@-1 {{'packed' attribute ignored for field of type 
'char'}}
-  char c:4;
 #else
-  char b:8 __attribute__ ((packed));
+  char e : 8 __attribute__((packed));
   // expected-warning@-1 {{'packed' attribute was ignored on bit-fields with 
single-byte alignment in older versions of GCC and Clang}}
-  char c:4;
 #endif
+  char f : 4, g : 8, h : 8, i : 8;
 };
 
 #if (defined(_WIN32) || defined(__ORBIS__)) && !defined(__declspec) // 
_MSC_VER is unavailable in cc1.
@@ -165,9 +164,14 @@
 //
 // Additionally, test for pre-r254596 clang behavior on the PS4 target. PS4
 // must maintain ABI backwards compatibility.
-extern int o1[sizeof(struct packed_chars) == 3 ? 1 : -1];
+extern int o1[sizeof(struct packed_chars) == 9 ? 1 : -1];
 extern int o2[__alignof(struct packed_chars) == 1 ? 1 : -1];
+#elif defined(_AIX)
+// On AIX, [bool, char, short] bitfields have the same alignment as unsigned
+// int.
+extern int o1[sizeof(struct packed_chars) == 8 ? 1 : -1];
+extern int o2[__alignof(struct packed_chars) == 4 ? 1 : -1];
 #else
-extern int o1[sizeof(struct packed_chars) == 2 ? 1 : -1];
+extern int o1[sizeof(struct packed_chars) == 8 ? 1 : -1];
 extern int o2[__alignof(struct packed_chars) == 1 ? 1 : -1];
 #endif


Index: clang/test/Sema/struct-packed-align.c
===
--- clang/test/Sema/struct-packed-align.c
+++ clang/test/Sema/struct-packed-align.c
@@ -146,18 +146,17 @@
 // GCC 4.4 but the change can lead to differences in the structure layout.
 // See the documentation of -Wpacked-bitfield-compat for more information.
 struct packed_chars {
-  char a:4;
+  char a : 8, b : 8, c : 8, d : 4;
 #ifdef __ORBIS__
   // Test for pre-r254596 clang behavior on the PS4 target. PS4 must maintain
   // ABI backwards compatibility.
-  char b:8 __attribute__ ((packed));
+  char e : 8 __attribute__((packed));
   // expected-warning@-1 {{'packed' attribute ignored for field of type 'char'}}
-  char c:4;
 #else
-  char b:8 __attribute__ ((packed));
+  char e : 8 __attribute__((packed));
   // expected-warning@-1 {{'packed' attribute was ignored on bit-fields with single-byte alignment in older versions of GCC and Clang}}
-  char c:4;
 #endif
+  char f : 4, g : 8, h : 8, i : 8;
 };
 
 #if (defined(_WIN32) || defined(__ORBIS__)) && !defined(__declspec) // _MSC_VER is unavailable in cc1.
@@ -165,9 +164,14 @@
 //
 // Additionally, test for pre-r254596 clang behavior on the PS4 target. PS4
 // must maintain ABI backwards compatibility.
-extern int o1[sizeof(struct packed_chars) == 3 ? 1 : -1];
+extern int o1[sizeof(struct packed_chars) == 9 ? 1 : -1];
 extern int o2[__alignof(struct packed_chars) == 1 ? 1 : -1];
+#elif defined(_AIX)
+// On AIX, [bool, char, short] bitfields have the same alignment as unsigned
+// int.
+extern int o1[sizeof(struct packed_chars) == 8 ? 1 : -1];
+extern int o2[__alignof(struct packed_chars) == 4 ? 1 : -1];
 #else
-extern int o1[sizeof(struct packed_chars) == 2 ? 1 : -1];
+extern int o1[sizeof(struct packed_chars) == 8 ? 1 : -1];
 extern int o2[__alignof(struct packed_chars) == 1 ? 1 : -1];
 #endif
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] d30dfa8 - [clang][patch] Add support for option -fextend-arguments={32,64}: widen integer arguments to int64 in unprototyped function calls

2021-05-19 Thread Melanie Blower via cfe-commits


Author: Melanie Blower
Date: 2021-05-19T10:59:56-04:00
New Revision: d30dfa86760ced9ac57f676340b34f2247898102

URL: 
https://github.com/llvm/llvm-project/commit/d30dfa86760ced9ac57f676340b34f2247898102
DIFF: 
https://github.com/llvm/llvm-project/commit/d30dfa86760ced9ac57f676340b34f2247898102.diff

LOG: [clang][patch] Add support for option -fextend-arguments={32,64}: widen 
integer arguments to int64 in unprototyped function calls

Reviewed By: Aaron Ballman

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

Added: 
clang/test/CodeGen/extend-arg-64.c
clang/test/Driver/fextend-args.c

Modified: 
clang/include/clang/Basic/LangOptions.def
clang/include/clang/Basic/LangOptions.h
clang/include/clang/Basic/TargetInfo.h
clang/include/clang/Driver/Options.td
clang/lib/Basic/Targets/X86.h
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Sema/SemaExpr.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/LangOptions.def 
b/clang/include/clang/Basic/LangOptions.def
index 20c03987bd02..fff5fe23dc80 100644
--- a/clang/include/clang/Basic/LangOptions.def
+++ b/clang/include/clang/Basic/LangOptions.def
@@ -415,6 +415,10 @@ LANGOPT(RelativeCXXABIVTables, 1, 0,
 
 LANGOPT(ArmSveVectorBits, 32, 0, "SVE vector size in bits")
 
+ENUM_LANGOPT(ExtendIntArgs, ExtendArgsKind, 1, ExtendArgsKind::ExtendTo32, 
+ "Controls how scalar integer arguments are extended in calls "
+ "to unprototyped and varargs functions")
+
 #undef LANGOPT
 #undef COMPATIBLE_LANGOPT
 #undef BENIGN_LANGOPT

diff  --git a/clang/include/clang/Basic/LangOptions.h 
b/clang/include/clang/Basic/LangOptions.h
index 454c101b2142..7fd4bf0eb206 100644
--- a/clang/include/clang/Basic/LangOptions.h
+++ b/clang/include/clang/Basic/LangOptions.h
@@ -267,6 +267,13 @@ class LangOptions : public LangOptionsBase {
 Single
   };
 
+  enum class ExtendArgsKind {
+/// Integer arguments are sign or zero extended to 32/64 bits
+/// during default argument promotions.
+ExtendTo32,
+ExtendTo64
+  };
+
 public:
   /// The used language standard.
   LangStandard::Kind LangStd;

diff  --git a/clang/include/clang/Basic/TargetInfo.h 
b/clang/include/clang/Basic/TargetInfo.h
index b55a99051cfb..29b957607f3e 100644
--- a/clang/include/clang/Basic/TargetInfo.h
+++ b/clang/include/clang/Basic/TargetInfo.h
@@ -1415,6 +1415,9 @@ class TargetInfo : public virtual TransferrableTargetInfo,
   bool isBigEndian() const { return BigEndian; }
   bool isLittleEndian() const { return !BigEndian; }
 
+  /// Whether the option -fextend-arguments={32,64} is supported on the target.
+  virtual bool supportsExtendIntArgs() const { return false; }
+
   /// Gets the default calling convention for the given target and
   /// declaration context.
   virtual CallingConv getDefaultCallingConv() const {

diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index a876f48c5596..1b78810bf352 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -1451,6 +1451,14 @@ defm math_errno : BoolFOption<"math-errno",
   PosFlag,
   NegFlag>,
   ShouldParseIf;
+def fextend_args_EQ : Joined<["-"], "fextend-arguments=">, Group,
+  Flags<[CC1Option, NoArgumentUnused]>,
+  HelpText<"Controls how scalar integer arguments are extended in calls "
+   "to unprototyped and varargs functions">,
+  Values<"32,64">,
+  NormalizedValues<["ExtendTo32", "ExtendTo64"]>,
+  NormalizedValuesScope<"LangOptions::ExtendArgsKind">,
+  MarshallingInfoEnum,"ExtendTo32">;
 def fbracket_depth_EQ : Joined<["-"], "fbracket-depth=">, Group, 
Flags<[CoreOption]>;
 def fsignaling_math : Flag<["-"], "fsignaling-math">, Group;
 def fno_signaling_math : Flag<["-"], "fno-signaling-math">, Group;

diff  --git a/clang/lib/Basic/Targets/X86.h b/clang/lib/Basic/Targets/X86.h
index 58b48ffd6328..0ba2fd6ba024 100644
--- a/clang/lib/Basic/Targets/X86.h
+++ b/clang/lib/Basic/Targets/X86.h
@@ -338,6 +338,10 @@ class LLVM_LIBRARY_VISIBILITY X86TargetInfo : public 
TargetInfo {
 
   bool setFPMath(StringRef Name) override;
 
+  bool supportsExtendIntArgs() const override {
+return getTriple().getArch() != llvm::Triple::x86;
+  }
+
   CallingConvCheckResult checkCallingConvention(CallingConv CC) const override 
{
 // Most of the non-ARM calling conventions are i386 conventions.
 switch (CC) {

diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index 47552dcb6fdf..add62e3ab4ab 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -4903,6 +4903,20 @@ void Clang::ConstructJob(Compilation &C, const JobAction 
&JA,
 
   RenderFloatingPointOptions(TC, D, OFastEnabled, Args, CmdArgs, JA);
 
+  if (Arg *A = Args.getLastArg(options::OPT_fextend_args_EQ)) {
+const llvm::Triple::ArchType Arch = TC.getArch();
+if (A

[PATCH] D101640: [clang][patch] Add support for option -fextend-arguments={32,64}: widen integer arguments to int64 in unprototyped function calls

2021-05-19 Thread Melanie Blower via Phabricator via cfe-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGd30dfa86760c: [clang][patch] Add support for option 
-fextend-arguments={32,64}: widen integer… (authored by mibintc).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101640/new/

https://reviews.llvm.org/D101640

Files:
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Basic/TargetInfo.h
  clang/include/clang/Driver/Options.td
  clang/lib/Basic/Targets/X86.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/CodeGen/extend-arg-64.c
  clang/test/Driver/fextend-args.c

Index: clang/test/Driver/fextend-args.c
===
--- /dev/null
+++ clang/test/Driver/fextend-args.c
@@ -0,0 +1,17 @@
+// Options for intel arch
+// RUN: %clang -### -target x86_64-apple-darwin -fextend-arguments=32 %s 2>&1 \
+// RUN: | FileCheck --implicit-check-not "-fextend-arguments=32"  %s
+// RUN: %clang -### -target x86_64-apple-darwin -fextend-arguments=64 %s 2>&1 \
+// RUN: | FileCheck -check-prefix=CHECK-64 %s
+
+// Unsupported target
+// RUN: not %clang -target aarch64-unknown-windows-msvc -fextend-arguments=32 %s 2>&1 \
+// RUN: | FileCheck -check-prefix=UNSUPPORTED-TARGET %s
+
+// Invalid option value
+// RUN: not %clang -target x86_64-apple-darwin -fextend-arguments=0 %s 2>&1 \
+// RUN: | FileCheck -check-prefix=INVALID-VALUE %s
+
+// CHECK-64: "-cc1" {{.*}}"-fextend-arguments=64"
+// UNSUPPORTED-TARGET: error: unsupported option
+// INVALID-VALUE: error: invalid argument '0' to -fextend-arguments
Index: clang/test/CodeGen/extend-arg-64.c
===
--- /dev/null
+++ clang/test/CodeGen/extend-arg-64.c
@@ -0,0 +1,103 @@
+// RUN: %clang_cc1 -DD128 -triple x86_64-apple-darwin -fextend-arguments=64  \
+// RUN:%s -emit-llvm -o - | FileCheck %s -check-prefix=CHECKEXT
+
+// When the option isn't selected, no effect
+// RUN: %clang_cc1 -DD128 -triple x86_64-apple-darwin  \
+// RUN: %s -emit-llvm -o - | FileCheck %s \
+// RUN:--implicit-check-not "ext {{.*}}to i64"
+
+// The option isn't supported on x86, no effect
+// RUN: %clang_cc1 -triple i386-pc-linux-gnu -fextend-arguments=64 \
+// RUN: %s -emit-llvm -o - | FileCheck %s \
+// RUN:--implicit-check-not "ext {{.*}}to i64"
+
+// The option isn't supported on ppc, no effect
+// RUN: %clang_cc1 -triple ppc64le -fextend-arguments=64 \
+// RUN: %s -emit-llvm -o - | FileCheck %s \
+// RUN:--implicit-check-not "ext {{.*}}to i64"
+
+int vararg(int, ...);
+void knr();
+
+unsigned int u32;
+int s32;
+unsigned short u16;
+short s16;
+unsigned char u8;
+signed char s8;
+long long ll;
+_ExtInt(23) ei23;
+float ff;
+double dd;
+#ifdef D128
+__int128 i128;
+#endif
+
+int test() {
+  // CHECK: define{{.*}} i32 @test{{.*}}
+
+  // CHECKEXT:  [[TAG_u32:%.*]] = load i32, i32* @u32{{.*}}
+  // CHECKEXT: [[CONV_u32:%.*]] = zext i32 [[TAG_u32]] to i64
+
+  // CHECKEXT:  [[TAG_s32:%.*]] = load i32, i32* @s32
+  // CHECKEXT: [[CONV_s32:%.*]] = sext i32 [[TAG_s32]] to i64
+
+  // CHECKEXT:  [[TAG_u16:%.*]] = load i16, i16* @u16
+  // CHECKEXT: [[CONV_u16:%.*]] = zext i16 [[TAG_u16]] to i64
+
+  // CHECKEXT:  [[TAG_s16:%.*]] = load i16, i16* @s16
+  // CHECKEXT: [[CONV_s16:%.*]] = sext i16 [[TAG_s16]] to i64
+
+  // CHECKEXT:  [[TAG_u8:%.*]] = load i8, i8* @u8
+  // CHECKEXT: [[CONV_u8:%.*]] = zext i8 [[TAG_u8]] to i64
+
+  // CHECKEXT:  [[TAG_s8:%.*]] = load i8, i8* @s8
+  // CHECKEXT: [[CONV_s8:%.*]] = sext i8 [[TAG_s8]] to i64
+  // CHECKEXT: call{{.*}} @vararg(i32 %0, i64 [[CONV_u32]], i64 [[CONV_s32]], i64 [[CONV_u16]], i64 [[CONV_s16]], i64 [[CONV_u8]], i64 [[CONV_s8]]
+
+  int sum = 0;
+  sum = vararg(sum, u32, s32, u16, s16, u8, s8);
+  knr(ll);
+  // CHECKEXT: load i64, i64* @ll
+  // CHECKEXT-NEXT: call void (i64, ...) bitcast {{.*}} @knr
+
+  knr(ei23);
+  // CHECKEXT: load i23, i23* @ei23
+  // CHECKEXT-NEXT: call void (i23, ...) bitcast{{.*}} @knr
+
+  knr(ff);
+  // CHECKEXT: load float
+  // CHECKEXT-NEXT: fpext float {{.*}} to double
+  // CHECKEXT-NEXT: call{{.*}} void (double, ...) bitcast{{.*}} @knr
+
+  knr(dd);
+  // CHECKEXT: load double
+  // CHECKEXT-NEXT: call{{.*}} void (double, ...) bitcast{{.*}} @knr
+
+#ifdef D128
+  knr(i128);
+  // CHECKEXT: load i128
+  // CHECKEXT: call{{.*}} void (i64, i64, ...) bitcast{{.*}} @knr
+#endif
+
+  knr(u32, s32, u16, s16, u8, s8);
+  // CHECKEXT:  [[TAg_u32:%.*]] = load i32, i32* @u32{{.*}}
+  // CHECKEXT: [[CONv_u32:%.*]] = zext i32 [[TAg_u32]] to i64
+
+  // CHECKEXT:  [[TAg_s32:%.*]] = load i32, i32* @s32
+  // CHECKEXT: [[CONv_s32:%.*]] = sext i32 [[TAg_s32]] to i64
+
+  // CHECKEXT:  [[TAg_u16:%.*]] = load i16, i16* @u16
+  // CHECKEXT: [[CONv_u16:%.*]] = zext i16 [[TA

[PATCH] D102623: [CodeGen][AArch64][SVE] Canonicalize intrinsic rdffr{ => _z}

2021-05-19 Thread Bradley Smith via Phabricator via cfe-commits

bsmith accepted this revision.
bsmith added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102623/new/

https://reviews.llvm.org/D102623

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

[PATCH] D98895: [X86][Draft] Disable long double type for -mno-x87 option

2021-05-19 Thread Andrew Savonichev via Phabricator via cfe-commits

asavonic added inline comments.



Comment at: clang/test/Sema/x86-no-x87.c:48-61
+void assign2() {
+  struct st_long_double st;
+#ifndef NOERROR
+  // expected-error@+2{{long double is not supported on this target}}
+#endif
+  st.ld = 0.42;
+}

pengfei wrote:
> These seems pass with GCC. https://godbolt.org/z/qM4nWhThx
Right. Assignment of a literal is compiled to just `mov` without any x87 
instructions, so it is not diagnosed by GCC. On the other hand, assignment of a 
variable does trigger the error:

void assign4(double d) {
  struct st_long_double st;
  st.ld = d; // error: long double is not supported on this target
}

We can update the patch to do the same for some cases, but it does not look 
very consistent, and makes assumptions on how the code is optimized and 
compiled.

GCC has an advantage here, because it emits the diagnostic at a lower level 
after at lease some optimizations are done. For example, the following code 
does not trigger an error for GCC because of inlining:

double get_const() {
  return 0.42;
}
void assign5(struct st_long_double *st) {
  st->ld = get_const();
}



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98895/new/

https://reviews.llvm.org/D98895

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

[PATCH] D102134: [docs]Updated the AMD GPU Attributes documentation

2021-05-19 Thread PoojaYadav via Phabricator via cfe-commits

pooja2299 added inline comments.



Comment at: clang/include/clang/Basic/AttrDocs.td:2244-2247
+and is an optimization hint. It is mandatory to use this attribute in some 
+situations. Because when the attribute is absent, the compiler assumes the 
+default maximum workgroup size of 256 but nowadays the workgroup size can 
legally go 
+to 1024. 

arsenm wrote:
> aaron.ballman wrote:
> > 
> You're updating this with outdated information. In general functions should 
> be conservatively correct by default with no attribute specified. This was 
> broken at one point in the past. The default assumed workgroup size is now 
> 1024, but for opencl clang will always default to a max of 256
Ohh. Thanks for your feedback. Will update it 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102134/new/

https://reviews.llvm.org/D102134

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

[PATCH] D97085: [OpenMP] libomp: implement OpenMP 5.1 inoutset task dependence type

2021-05-19 Thread Joachim Protze via Phabricator via cfe-commits

protze.joachim added inline comments.



Comment at: openmp/runtime/src/kmp_taskdeps.cpp:344-346
+// link node as successor of all nodes in the prev_set if any
+npredecessors +=
+__kmp_depnode_link_successor(gtid, thread, task, node, prev_set);

AndreyChurbanov wrote:
> protze.joachim wrote:
> > AndreyChurbanov wrote:
> > > protze.joachim wrote:
> > > > If I understand this code right, this has O(n^2) complexity for two 
> > > > sets of size n?
> > > > 
> > > > Consider:
> > > > ```
> > > > for (int i=0; i > > > #pragma omp task depend(in:A)
> > > > work(A,i);
> > > > }
> > > > for (int i=0; i > > > #pragma omp task depend(inoutset:A)
> > > > work(A,i);
> > > > }
> > > > ```
> > > > All n tasks in the first loop would be predecessor for each of the 
> > > > tasks in the second loop. This will also result in n^2 
> > > > releases/notifications.
> > > > 
> > > > 
> > > > I'd suggest to model the dependencies like:
> > > > ```
> > > > for (int i=0; i > > > #pragma omp task depend(in:A)
> > > > work(A,i);
> > > > }
> > > > #pragma omp taskwait depend(inout:A) nowait
> > > > for (int i=0; i > > > #pragma omp task depend(inoutset:A)
> > > > work(A,i);
> > > > }
> > > > ```
> > > > I.e., add a dummy dependency node between the two sets, which has n 
> > > > predecessors and n successors. You probably understand the depnode code 
> > > > better, than I do, but I think you would only need to add some code in 
> > > > line 357, where `last_set` is moved to `prev_set`.
> > > > This dummy node would reduce linking and releasing complexity to O(n).
> > > This could be a separate research project.  Because such change may cause 
> > > performance regressions on some real codes, so it needs thorough 
> > > investigation.  I mean insertion of an auxiliary dummy node into 
> > > dependency graph is definitely an overhead, which may or may not be 
> > > overridden by reduction in number of edges in the graph.  Or it can be 
> > > made optional optimization under an external control, if there is a 
> > > significant gain in some particular case.
> > I don't suggest to insert the auxiliary node in the general case. Just for 
> > the case that you see a transition of `in` -> `inoutset` or vice versa.
> > 
> > With current task dependencies, you always have 1 node notifying n nodes 
> > (`out` -> `in`) or n nodes notifying one node (`in` -> `out`). You can see 
> > this as an amortized O(1) operation per task in the graph.
> > 
> > Here you introduce a code pattern, where n nodes each will need to notify m 
> > other nodes. This leads to an O(n) operation per task. I'm really worried 
> > about the complexity of this pattern.
> > If there is no use case with large n, I don't understand, why `inoutset` 
> > was introduced in the first place.
> > I don't suggest to insert the auxiliary node in the general case. Just for 
> > the case that you see a transition of `in` -> `inoutset` or vice versa.
> > 
> > With current task dependencies, you always have 1 node notifying n nodes 
> > (`out` -> `in`) or n nodes notifying one node (`in` -> `out`). You can see 
> > this as an amortized O(1) operation per task in the graph.
> > 
> > Here you introduce a code pattern, where n nodes each will need to notify m 
> > other nodes. This leads to an O(n) operation per task. I'm really worried 
> > about the complexity of this pattern.
> No, I don't introduce the pattern, as it already worked for sets of tasks 
> with `in` dependency following set of tasks with `mutexinoutset` dependency 
> or vice versa.
> 
> > If there is no use case with large n, I don't understand, why `inoutset` 
> > was introduced in the first place.
> I didn't like that `inoutset` was introduced as the clone of "in" in order to 
> separate two (big) sets of mutually independent tasks, but this has already 
> been added to specification, so we have to implement it. Of cause your 
> suggestion can dramatically speed up dependency processing of some trivial 
> case with two huge sets of tasks one wit `in` dependency another with 
> `inoutset`; but it slightly changes the semantics of user's code, and 
> actually can slowdown other cases.  I would let users do such optimizations.  
> Or compiler can do this once it sees big sets of tasks those don't have 
> barrier-like "taskwait nowait".  For user this is one line of code, for 
> compiler this is dozen lines of code, for runtime library this is pretty big 
> change which does not worth efforts to implement, I think.  Especially given 
> that such "optimization" will definitely slowdown some cases.  E.g. small set 
> of tasks with `in` dependency can be followed single task with `inoutset` in 
> a loop. Why should we slowdown this case?  Library has no idea of the 
> structure of user's code.
> 
> In general, I don't like the idea when runtime library tries to optimize 
> user's code.  Especially along with other changes in the same patch.
> 
> No, I don't introduce

[PATCH] D102696: [Analyzer] Find constraints that are directly attached to a BinOp

2021-05-19 Thread Valeriy Savchenko via Phabricator via cfe-commits

vsavchenko added a comment.

My take on this: for whatever approach, we definitely will be able to construct 
a more nested/complex example so that it doesn't work.
For this patch, I'm wondering about something like this:

  int foo() {
if (z != 0)
  return 0;
if (x + y + z != 0)
  return 0;
clang_analyzer_eval(x + y + z == 0); // expected-warning{{TRUE}} OK.
if (y != 0)
  return 0;
clang_analyzer_eval(y == 0); // expected-warning{{TRUE}} OK.
clang_analyzer_eval(z == 0); // expected-warning{{TRUE}} OK.
clang_analyzer_eval(x + y + z == 0); // expected-warning{{TRUE}} ?
  }

As for the solver, it is something that tormented me for a long time.  Is there 
a way to avoid a full-blown brute force check of all existing constraints and 
get some knowledge about symbolic expressions by constraints these symbolic 
expressions are actually part of (right now we can reason about expressions if 
we know some information about its own parts aka operands)?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102696/new/

https://reviews.llvm.org/D102696

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

[PATCH] D102782: Add support for Warning Flag "-Wstack-usage="

2021-05-19 Thread Ryan Santhirarajan via Phabricator via cfe-commits

rsanthir.quic created this revision.
Herald added subscribers: dexonsmith, dang.
rsanthir.quic requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

With this implementation "-Wstack-usage" acts as an alias to
"-Wframe-larger-than"


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D102782

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Basic/Warnings.cpp
  clang/test/Frontend/backend-stack-usage-diagnostic.c


Index: clang/test/Frontend/backend-stack-usage-diagnostic.c
===
--- /dev/null
+++ clang/test/Frontend/backend-stack-usage-diagnostic.c
@@ -0,0 +1,17 @@
+// RUN: %clang -Wstack-usage=0 -o /dev/null -c %s 2> %t.err
+// RUN: FileCheck < %t.err %s --check-prefix=WARN
+// RUN: %clang -Wno-stack-usage= -w -o /dev/null -c %s 2> %t.err
+// RUN: FileCheck < %t.err %s --check-prefix=IGNORE --allow-empty
+// RUN: %clang -Wstack-usage=0 -w -o /dev/null -c %s 2> %t.err
+// RUN: FileCheck < %t.err %s --check-prefix=IGNORE --allow-empty
+// RUN: %clang -Wstack-usage=3 -o /dev/null -c %s 2> %t.err
+// RUN: FileCheck < %t.err %s --check-prefix=WARN
+// RUN: %clang -Wstack-usage=100 -o /dev/null -c %s 2> %t.err
+// RUN: FileCheck < %t.err %s --check-prefix=IGNORE --allow-empty
+
+// WARN: warning: stack frame size of {{[0-9]+}} bytes in function 
'test_square'
+// IGNORE-NOT:  stack frame size of {{[0-9]+}} bytes in function 'test_square'
+int test_square(int num) {
+  return num * num;
+}
+
Index: clang/lib/Basic/Warnings.cpp
===
--- clang/lib/Basic/Warnings.cpp
+++ clang/lib/Basic/Warnings.cpp
@@ -94,6 +94,11 @@
   if (Opt == "format=0")
 Opt = "no-format";
 
+  // -Wstack-usage is aliased to -Wframe-larger-than, this handles
+  // the negative case, as table gen does not.
+  if (Opt == "no-stack-usage=")
+Opt = "no-frame-larger-than=";
+
   // Check to see if this warning starts with "no-", if so, this is a
   // negative form of the option.
   bool isPositive = true;
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -2500,6 +2500,8 @@
 def Wlarger_than_EQ : Joined<["-"], "Wlarger-than=">, 
Group;
 def Wlarger_than_ : Joined<["-"], "Wlarger-than-">, Alias;
 def Wframe_larger_than_EQ : Joined<["-"], "Wframe-larger-than=">, 
Group, Flags<[NoXarchOption]>;
+def Wstack_stack_usage_EQ : Joined<["-"], "Wstack-usage=">, 
Flags<[NoXarchOption]>,
+  Alias;
 
 def : Flag<["-"], "fterminated-vtables">, Alias;
 defm threadsafe_statics : BoolFOption<"threadsafe-statics",


Index: clang/test/Frontend/backend-stack-usage-diagnostic.c
===
--- /dev/null
+++ clang/test/Frontend/backend-stack-usage-diagnostic.c
@@ -0,0 +1,17 @@
+// RUN: %clang -Wstack-usage=0 -o /dev/null -c %s 2> %t.err
+// RUN: FileCheck < %t.err %s --check-prefix=WARN
+// RUN: %clang -Wno-stack-usage= -w -o /dev/null -c %s 2> %t.err
+// RUN: FileCheck < %t.err %s --check-prefix=IGNORE --allow-empty
+// RUN: %clang -Wstack-usage=0 -w -o /dev/null -c %s 2> %t.err
+// RUN: FileCheck < %t.err %s --check-prefix=IGNORE --allow-empty
+// RUN: %clang -Wstack-usage=3 -o /dev/null -c %s 2> %t.err
+// RUN: FileCheck < %t.err %s --check-prefix=WARN
+// RUN: %clang -Wstack-usage=100 -o /dev/null -c %s 2> %t.err
+// RUN: FileCheck < %t.err %s --check-prefix=IGNORE --allow-empty
+
+// WARN: warning: stack frame size of {{[0-9]+}} bytes in function 'test_square'
+// IGNORE-NOT:  stack frame size of {{[0-9]+}} bytes in function 'test_square'
+int test_square(int num) {
+  return num * num;
+}
+
Index: clang/lib/Basic/Warnings.cpp
===
--- clang/lib/Basic/Warnings.cpp
+++ clang/lib/Basic/Warnings.cpp
@@ -94,6 +94,11 @@
   if (Opt == "format=0")
 Opt = "no-format";
 
+  // -Wstack-usage is aliased to -Wframe-larger-than, this handles
+  // the negative case, as table gen does not.
+  if (Opt == "no-stack-usage=")
+Opt = "no-frame-larger-than=";
+
   // Check to see if this warning starts with "no-", if so, this is a
   // negative form of the option.
   bool isPositive = true;
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -2500,6 +2500,8 @@
 def Wlarger_than_EQ : Joined<["-"], "Wlarger-than=">, Group;
 def Wlarger_than_ : Joined<["-"], "Wlarger-than-">, Alias;
 def Wframe_larger_than_EQ : Joined<["-"], "Wframe-larger-than=">, Group, Flags<[NoXarchOption]>;
+def Wstack_stack_usage_EQ : Joined<["-"], "Wstack-usage=">, Flags<[NoXarchOption]>,
+  Alias;
 
 def : Flag<["-"]

[PATCH] D102772: [SanitizeCoverage] Add support for NoSanitizeCoverage function attribute

2021-05-19 Thread Marco Elver via Phabricator via cfe-commits

melver updated this revision to Diff 346484.
melver added a comment.

Test that always_inline works as expected with no_sanitize("coverage")


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102772/new/

https://reviews.llvm.org/D102772

Files:
  clang/docs/SanitizerCoverage.rst
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/sanitize-coverage.c
  llvm/include/llvm/Bitcode/LLVMBitCodes.h
  llvm/include/llvm/IR/Attributes.td
  llvm/lib/Bitcode/Reader/BitcodeReader.cpp
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/IR/Attributes.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
  llvm/lib/Transforms/Utils/CodeExtractor.cpp

Index: llvm/lib/Transforms/Utils/CodeExtractor.cpp
===
--- llvm/lib/Transforms/Utils/CodeExtractor.cpp
+++ llvm/lib/Transforms/Utils/CodeExtractor.cpp
@@ -954,6 +954,7 @@
   case Attribute::NonLazyBind:
   case Attribute::NoRedZone:
   case Attribute::NoUnwind:
+  case Attribute::NoSanitizeCoverage:
   case Attribute::NullPointerIsValid:
   case Attribute::OptForFuzzing:
   case Attribute::OptimizeNone:
Index: llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
===
--- llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
+++ llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
@@ -621,6 +621,8 @@
 return;
   if (Blocklist && Blocklist->inSection("coverage", "fun", F.getName()))
 return;
+  if (F.hasFnAttribute(Attribute::NoSanitizeCoverage))
+return;
   if (Options.CoverageType >= SanitizerCoverageOptions::SCK_Edge)
 SplitAllCriticalEdges(F, CriticalEdgeSplittingOptions().setIgnoreUnreachableDests());
   SmallVector IndirCalls;
Index: llvm/lib/IR/Verifier.cpp
===
--- llvm/lib/IR/Verifier.cpp
+++ llvm/lib/IR/Verifier.cpp
@@ -1661,6 +1661,7 @@
   case Attribute::NoCfCheck:
   case Attribute::NoUnwind:
   case Attribute::NoInline:
+  case Attribute::NoSanitizeCoverage:
   case Attribute::AlwaysInline:
   case Attribute::OptimizeForSize:
   case Attribute::StackProtect:
Index: llvm/lib/IR/Attributes.cpp
===
--- llvm/lib/IR/Attributes.cpp
+++ llvm/lib/IR/Attributes.cpp
@@ -442,6 +442,8 @@
 return "noprofile";
   if (hasAttribute(Attribute::NoUnwind))
 return "nounwind";
+  if (hasAttribute(Attribute::NoSanitizeCoverage))
+return "no_sanitize_coverage";
   if (hasAttribute(Attribute::OptForFuzzing))
 return "optforfuzzing";
   if (hasAttribute(Attribute::OptimizeNone))
Index: llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
===
--- llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
+++ llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
@@ -686,6 +686,8 @@
 return bitc::ATTR_KIND_NO_PROFILE;
   case Attribute::NoUnwind:
 return bitc::ATTR_KIND_NO_UNWIND;
+  case Attribute::NoSanitizeCoverage:
+return bitc::ATTR_KIND_NO_SANITIZE_COVERAGE;
   case Attribute::NullPointerIsValid:
 return bitc::ATTR_KIND_NULL_POINTER_IS_VALID;
   case Attribute::OptForFuzzing:
Index: llvm/lib/Bitcode/Reader/BitcodeReader.cpp
===
--- llvm/lib/Bitcode/Reader/BitcodeReader.cpp
+++ llvm/lib/Bitcode/Reader/BitcodeReader.cpp
@@ -1474,6 +1474,8 @@
 return Attribute::NoCfCheck;
   case bitc::ATTR_KIND_NO_UNWIND:
 return Attribute::NoUnwind;
+  case bitc::ATTR_KIND_NO_SANITIZE_COVERAGE:
+return Attribute::NoSanitizeCoverage;
   case bitc::ATTR_KIND_NULL_POINTER_IS_VALID:
 return Attribute::NullPointerIsValid;
   case bitc::ATTR_KIND_OPT_FOR_FUZZING:
Index: llvm/include/llvm/IR/Attributes.td
===
--- llvm/include/llvm/IR/Attributes.td
+++ llvm/include/llvm/IR/Attributes.td
@@ -154,6 +154,9 @@
 /// Function doesn't unwind stack.
 def NoUnwind : EnumAttr<"nounwind">;
 
+/// No SanitizeCoverage instrumentation.
+def NoSanitizeCoverage : EnumAttr<"no_sanitize_coverage">;
+
 /// Null pointer in address space zero is valid.
 def NullPointerIsValid : EnumAttr<"null_pointer_is_valid">;
 
Index: llvm/include/llvm/Bitcode/LLVMBitCodes.h
===
--- llvm/include/llvm/Bitcode/LLVMBitCodes.h
+++ llvm/include/llvm/Bitcode/LLVMBitCodes.h
@@ -666,6 +666,7 @@
   ATTR_KIND_NO_PROFILE = 73,
   ATTR_KIND_VSCALE_RANGE = 74,
   ATTR_KIND_SWIFT_ASYNC = 75,
+  ATTR_KIND_NO_SANITIZE_COVERAGE = 76,
 };
 
 enum ComdatSelectionKindCodes {
Index: clang/test/CodeGen/sanitize-coverage.c
===
--- clang/te

[PATCH] D96033: [clang-repl] Land initial infrastructure for incremental parsing

2021-05-19 Thread Petr Hosek via Phabricator via cfe-commits

phosek added a comment.

In D96033#2767884 , @teemperor wrote:

> In D96033#2766502 , @phosek wrote:
>
>> In D96033#2766372 , @v.g.vassilev 
>> wrote:
>>
>>> In D96033#2766332 , @phosek wrote:
>>>
 We've started seeing `LLVM ERROR: out of memory` on our 2-stage LTO Linux 
 builders after this change landed. It looks like linking `clang-repl` 
 always fails on our bot, but I've also seen OOM when linking 
 `ClangCodeGenTests` and `FrontendTests`. Do you have any idea why this 
 could be happening? We'd appreciate any help since our bots have been 
 broken for several days now.
>>>
>>> Ouch. Are the bot logs public? If not maybe a stacktrace could be useful. 
>>> `clang-repl` combines a lot of libraries across llvm and clang that usually 
>>> are compiled separately. For instance we put in memory most of the clang 
>>> frontend, the backend and the JIT. Could it be we are hitting some real 
>>> limit?
>>
>> Yes, they are, see 
>> https://luci-milo.appspot.com/p/fuchsia/builders/prod/clang-linux-x64, but 
>> there isn't much information in there unfortunately. It's possible that 
>> we're hitting some limit, but these bots use 32-core instances with 128GB 
>> RAM which I'd hope is enough even for the LTO build.
>
> I think the specs are fine for just building with LTO, but I am not sure if 
> that's enough to for the worst case when running `ninja -j320` with an LTO 
> build (which is what your job is doing). Can you try limiting your link jobs 
> to something like 16 or 32 (e.g., `-DLLVM_PARALLEL_LINK_JOBS=32`)
>
> (FWIW, your go build script also crashes with OOM errors so you really are 
> running low on memory on that node)`

`-j320` is only used for the first stage compiler which uses distributed 
compilation and no LTO, the second stage which uses LTO and where we see this 
issue uses Ninja default, so `-j32` in this case.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96033/new/

https://reviews.llvm.org/D96033

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

[PATCH] D102715: [AIX] Fix LIT failure on native aix

2021-05-19 Thread Hubert Tong via Phabricator via cfe-commits

hubert.reinterpretcast accepted this revision.
hubert.reinterpretcast added a comment.
This revision is now accepted and ready to land.

This LGTM. I've added additional reviewers based on the history of the file in 
case they will have comments. Please hold on committing this until tomorrow.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102715/new/

https://reviews.llvm.org/D102715

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

[PATCH] D96033: [clang-repl] Land initial infrastructure for incremental parsing

2021-05-19 Thread Raphael Isemann via Phabricator via cfe-commits

teemperor added a comment.

In D96033#2768940 , @phosek wrote:

> In D96033#2767884 , @teemperor wrote:
>
>> In D96033#2766502 , @phosek wrote:
>>
>>> In D96033#2766372 , @v.g.vassilev 
>>> wrote:
>>>
 In D96033#2766332 , @phosek wrote:

> We've started seeing `LLVM ERROR: out of memory` on our 2-stage LTO Linux 
> builders after this change landed. It looks like linking `clang-repl` 
> always fails on our bot, but I've also seen OOM when linking 
> `ClangCodeGenTests` and `FrontendTests`. Do you have any idea why this 
> could be happening? We'd appreciate any help since our bots have been 
> broken for several days now.

 Ouch. Are the bot logs public? If not maybe a stacktrace could be useful. 
 `clang-repl` combines a lot of libraries across llvm and clang that 
 usually are compiled separately. For instance we put in memory most of the 
 clang frontend, the backend and the JIT. Could it be we are hitting some 
 real limit?
>>>
>>> Yes, they are, see 
>>> https://luci-milo.appspot.com/p/fuchsia/builders/prod/clang-linux-x64, but 
>>> there isn't much information in there unfortunately. It's possible that 
>>> we're hitting some limit, but these bots use 32-core instances with 128GB 
>>> RAM which I'd hope is enough even for the LTO build.
>>
>> I think the specs are fine for just building with LTO, but I am not sure if 
>> that's enough to for the worst case when running `ninja -j320` with an LTO 
>> build (which is what your job is doing). Can you try limiting your link jobs 
>> to something like 16 or 32 (e.g., `-DLLVM_PARALLEL_LINK_JOBS=32`)
>>
>> (FWIW, your go build script also crashes with OOM errors so you really are 
>> running low on memory on that node)`
>
> `-j320` is only used for the first stage compiler which uses distributed 
> compilation and no LTO, the second stage which uses LTO and where we see this 
> issue uses Ninja default, so `-j32` in this case.

I admit I don't really know the CI system on your node, but I assumed you're 
using `-j320` from this output which I got by clicking on "execution details" 
on the aborted stage of this build 
:

  Executing command [
'/b/s/w/ir/x/w/cipd/ninja',
'-j320',
'stage2-check-clang',
'stage2-check-lld',
'stage2-check-llvm',
'stage2-check-polly',
  ]
  escaped for shell: /b/s/w/ir/x/w/cipd/ninja -j320 stage2-check-clang 
stage2-check-lld stage2-check-llvm stage2-check-polly
  in dir /b/s/w/ir/x/w/staging/llvm_build
  at time 2021-05-18T20:53:37.215574


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96033/new/

https://reviews.llvm.org/D96033

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

[PATCH] D96033: [clang-repl] Land initial infrastructure for incremental parsing

2021-05-19 Thread Raphael Isemann via Phabricator via cfe-commits

teemperor added a comment.

(Tabbed on the Submit button instead of finishing that quote block...)

So I assume the stage2- targets are just invoking some ninja invocations in 
sequence?

Anyway, what I think it would be helpful to see what link jobs were in 
progress. But I guess even with 32 jobs you could end up with enough memory in 
32 linker invocations that processes start get OOM killed.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96033/new/

https://reviews.llvm.org/D96033

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

[PATCH] D102715: [AIX] Fix LIT failure on native aix

2021-05-19 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added inline comments.



Comment at: clang/test/Sema/struct-packed-align.c:170
+#elif defined(_AIX)
+// On AIX, [bool, char, short] bitfields have the same alignment as unsigned
+// int.

We're not really testing the behavior of `bool` or `short` anywhere and it'd be 
nice to verify that. Perhaps instead of modifying an existing test to add more 
fields, it'd make sense to make a new test structure?

While thinking of other potentially smaller-than-int types, I wondered whether 
`wchar_t` has special behavior here as well (I have no idea how that type is 
defined for AIX, so it's entirely possible it's size and alignment already 
match `int`).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102715/new/

https://reviews.llvm.org/D102715

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

[PATCH] D102772: [SanitizeCoverage] Add support for NoSanitizeCoverage function attribute

2021-05-19 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added a comment.

The clang parts look good to me, but someone else should do the final sign-off.




Comment at: clang/lib/Sema/SemaDeclAttr.cpp:7276-7278
 if (parseSanitizerValue(SanitizerName, /*AllowGroups=*/true) ==
-SanitizerMask())
+SanitizerMask() &&
+SanitizerName != "coverage")

The formatting looks rather off here to my eyes, but if this is what 
clang-format produces for you, then I'm fine with it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102772/new/

https://reviews.llvm.org/D102772

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

[clang] 76b8754 - Revert "Reapply "[clang][deps] Support inferred modules""

2021-05-19 Thread Frederik Gossen via cfe-commits


Author: Frederik Gossen
Date: 2021-05-19T19:19:37+02:00
New Revision: 76b8754d1bba6a8490c0f7e8a9e2fb3d181f0b03

URL: 
https://github.com/llvm/llvm-project/commit/76b8754d1bba6a8490c0f7e8a9e2fb3d181f0b03
DIFF: 
https://github.com/llvm/llvm-project/commit/76b8754d1bba6a8490c0f7e8a9e2fb3d181f0b03.diff

LOG: Revert "Reapply "[clang][deps] Support inferred modules""

This reverts commit c98833cdaad01787ea70ecdfabb05a7e142a6671.
The test `ClangScanDeps/modules-inferred-explicit-build.m` creates files
in the current directory.

Added: 


Modified: 
clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
clang/test/ClangScanDeps/modules-full.cpp

Removed: 

clang/test/ClangScanDeps/Inputs/frameworks/Inferred.framework/Frameworks/Sub.framework/Headers/Sub.h

clang/test/ClangScanDeps/Inputs/frameworks/Inferred.framework/Headers/Inferred.h
clang/test/ClangScanDeps/Inputs/frameworks/System.framework/Headers/System.h

clang/test/ClangScanDeps/Inputs/frameworks/System.framework/Modules/module.modulemap
clang/test/ClangScanDeps/Inputs/frameworks/module.modulemap
clang/test/ClangScanDeps/Inputs/modules_inferred_cdb.json
clang/test/ClangScanDeps/modules-inferred-explicit-build.m
clang/test/ClangScanDeps/modules-inferred.m
clang/utils/module-deps-to-rsp.py



diff  --git a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp 
b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
index d9e0bead13bc..51b7fc48628c 100644
--- a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -33,6 +33,7 @@ makeInvocationForModuleBuildWithoutPaths(const ModuleDeps 
&Deps,
   CI.getFrontendOpts().IsSystemModule = Deps.IsSystem;
 
   CI.getLangOpts()->ImplicitModules = false;
+  CI.getHeaderSearchOpts().ImplicitModuleMaps = false;
 
   return CI;
 }
@@ -178,22 +179,13 @@ ModuleID ModuleDepCollectorPP::handleTopLevelModule(const 
Module *M) {
   const FileEntry *ModuleMap = Instance.getPreprocessor()
.getHeaderSearchInfo()
.getModuleMap()
-   .getModuleMapFileForUniquing(M);
+   .getContainingModuleMapFile(M);
   MD.ClangModuleMapFile = std::string(ModuleMap ? ModuleMap->getName() : "");
 
   serialization::ModuleFile *MF =
   MDC.Instance.getASTReader()->getModuleManager().lookup(M->getASTFile());
   MDC.Instance.getASTReader()->visitInputFiles(
   *MF, true, true, [&](const serialization::InputFile &IF, bool isSystem) {
-// __inferred_module.map is the result of the way in which an implicit
-// module build handles inferred modules. It adds an overlay VFS with
-// this file in the proper directory and relies on the rest of Clang to
-// handle it like normal. With explicitly built modules we don't need
-// to play VFS tricks, so replace it with the correct module map.
-if (IF.getFile()->getName().endswith("__inferred_module.map")) {
-  MD.FileDeps.insert(ModuleMap->getName());
-  return;
-}
 MD.FileDeps.insert(IF.getFile()->getName());
   });
 

diff  --git 
a/clang/test/ClangScanDeps/Inputs/frameworks/Inferred.framework/Frameworks/Sub.framework/Headers/Sub.h
 
b/clang/test/ClangScanDeps/Inputs/frameworks/Inferred.framework/Frameworks/Sub.framework/Headers/Sub.h
deleted file mode 100644
index e69de29bb2d1..

diff  --git 
a/clang/test/ClangScanDeps/Inputs/frameworks/Inferred.framework/Headers/Inferred.h
 
b/clang/test/ClangScanDeps/Inputs/frameworks/Inferred.framework/Headers/Inferred.h
deleted file mode 100644
index 1855e4fad5f8..
--- 
a/clang/test/ClangScanDeps/Inputs/frameworks/Inferred.framework/Headers/Inferred.h
+++ /dev/null
@@ -1 +0,0 @@
-typedef int inferred;

diff  --git 
a/clang/test/ClangScanDeps/Inputs/frameworks/System.framework/Headers/System.h 
b/clang/test/ClangScanDeps/Inputs/frameworks/System.framework/Headers/System.h
deleted file mode 100644
index a90c62886749..
--- 
a/clang/test/ClangScanDeps/Inputs/frameworks/System.framework/Headers/System.h
+++ /dev/null
@@ -1 +0,0 @@
-enum { bigger_than_int = 0x8000 };

diff  --git 
a/clang/test/ClangScanDeps/Inputs/frameworks/System.framework/Modules/module.modulemap
 
b/clang/test/ClangScanDeps/Inputs/frameworks/System.framework/Modules/module.modulemap
deleted file mode 100644
index 51f72f664235..
--- 
a/clang/test/ClangScanDeps/Inputs/frameworks/System.framework/Modules/module.modulemap
+++ /dev/null
@@ -1,3 +0,0 @@
-framework module System [system] {
-  umbrella header "System.h"
-}

diff  --git a/clang/test/ClangScanDeps/Inputs/frameworks/module.modulemap 
b/clang/test/ClangScanDeps/Inputs/frameworks/module.modulemap
deleted file mode 100644
index e3bad873c7e7..
--- a/clan

[PATCH] D102772: [SanitizeCoverage] Add support for NoSanitizeCoverage function attribute

2021-05-19 Thread Marco Elver via Phabricator via cfe-commits

melver added inline comments.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:7276-7278
 if (parseSanitizerValue(SanitizerName, /*AllowGroups=*/true) ==
-SanitizerMask())
+SanitizerMask() &&
+SanitizerName != "coverage")

aaron.ballman wrote:
> The formatting looks rather off here to my eyes, but if this is what 
> clang-format produces for you, then I'm fine with it.
Yeah, I had something else here, but then clang-format suggested this and I 
don't like it either.

Could remove the /*AllowGroups=*/ bit, for the sake of formatting better?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102772/new/

https://reviews.llvm.org/D102772

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

[PATCH] D102772: [SanitizeCoverage] Add support for NoSanitizeCoverage function attribute

2021-05-19 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added inline comments.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:7276-7278
 if (parseSanitizerValue(SanitizerName, /*AllowGroups=*/true) ==
-SanitizerMask())
+SanitizerMask() &&
+SanitizerName != "coverage")

melver wrote:
> aaron.ballman wrote:
> > The formatting looks rather off here to my eyes, but if this is what 
> > clang-format produces for you, then I'm fine with it.
> Yeah, I had something else here, but then clang-format suggested this and I 
> don't like it either.
> 
> Could remove the /*AllowGroups=*/ bit, for the sake of formatting better?
I'd rather have the comment + ugly formatting than pretty formatting + 
mysterious boolean literal, so I'd say let's just leave it as-is.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102772/new/

https://reviews.llvm.org/D102772

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

[PATCH] D102568: [Driver] Delete -mimplicit-it=

2021-05-19 Thread Nick Desaulniers via Phabricator via cfe-commits

nickdesaulniers added a comment.

In D102568#2768224 , @mstorsjo wrote:

> I would request that this commit should be reverted.

Sure, @MaskRay would you mind reverting?

> But this change did break my build in these places:
> https://code.videolan.org/videolan/x264/-/blob/b684ebe04a6f80f8207a57940a1fa00e25274f81/configure#L855
> https://chromium.googlesource.com/webm/libvpx/+/66c1ff6850fd53bcf5c17247569bea1d700d6247/build/make/configure.sh#1012
> https://github.com/cisco/openh264/blob/089d6c6d83ab6e5d451ef18808bb6c46faf553a2/build/platform-mingw_nt.mk#L23

Thanks for the links; @MaskRay I think you should file some issues in those 
projects to help them move to the assembler option before we try to remove it 
again.

>> Should be an easy fix either way (replace `-mimplicit-it=always` with 
>> `-Wa,-wmimplicit-it=always`).
>
> No, it's not an easy fix as no existing releases of Clang support it.

Well, these projects linked above will need to implement either feature 
detection for the command line flag options before hard coding them, or 
compiler version checks.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102568/new/

https://reviews.llvm.org/D102568

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

[PATCH] D102495: [clang][deps] Support inferred modules

2021-05-19 Thread Frederik Gossen via Phabricator via cfe-commits

frgossen added inline comments.



Comment at: clang/test/ClangScanDeps/modules-inferred-explicit-build.m:13-15
+// RUN: %clang @%t.inferred.cc1.rsp -pedantic -Werror
+// RUN: %clang @%t.system.cc1.rsp -pedantic -Werror
+// RUN: %clang -x objective-c -fsyntax-only %t.dir/modules_cdb_input.cpp \

These three commands rely on file creations in the current working directory.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102495/new/

https://reviews.llvm.org/D102495

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

[PATCH] D102715: [AIX] Fix LIT failure on native aix

2021-05-19 Thread Xiangling Liao via Phabricator via cfe-commits

Xiangling_L marked an inline comment as done.
Xiangling_L added inline comments.

Comment at: clang/test/Sema/struct-packed-align.c:170
+#elif defined(_AIX)
+// On AIX, [bool, char, short] bitfields have the same alignment as unsigned
+// int.

aaron.ballman wrote:
> We're not really testing the behavior of `bool` or `short` anywhere and it'd 
> be nice to verify that. Perhaps instead of modifying an existing test to add 
> more fields, it'd make sense to make a new test structure?
> 
> While thinking of other potentially smaller-than-int types, I wondered 
> whether `wchar_t` has special behavior here as well (I have no idea how that 
> type is defined for AIX, so it's entirely possible it's size and alignment 
> already match `int`).
> We're not really testing the behavior of bool or short anywhere and it'd be 
> nice to verify that. 

The comment is to explain why char has 4-byte alignment mainly. And the 
testcase here is, as comments mentioned, to test `Packed attribute shouldn't be 
ignored for bit-field of char types`.  Perhaps I should remove `bool` and 
`short` so that people wouldn't be confused.  

And the special alignment regarding bool, short etc. has been tested when the 
special rule introduced on aix here: https://reviews.llvm.org/D87029.

> Perhaps instead of modifying an existing test to add more fields, it'd make 
> sense to make a new test structure?

I don't think it's necessary to make a new test structure. The modified 
testcase test the same property as the original one. And it is more capable as 
it can also test the property for AIX target.

> I wondered whether wchar_t has special behavior here as well 

I think `wchar_t` has the same special behavior. Basically any type smaller 
than 4 bytes will be aligned to 4 when it comes to bitfield. Please correct me 
if I am wrong @hubert.reinterpretcast 

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102715/new/

https://reviews.llvm.org/D102715

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

[PATCH] D102715: Fix LIT failure on native aix

2021-05-19 Thread Xiangling Liao via Phabricator via cfe-commits

Xiangling_L updated this revision to Diff 346503.
Xiangling_L marked an inline comment as done.
Xiangling_L edited the summary of this revision.
Xiangling_L added a comment.

Adjust the comment;


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102715/new/

https://reviews.llvm.org/D102715

Files:
  clang/test/Sema/struct-packed-align.c


Index: clang/test/Sema/struct-packed-align.c
===
--- clang/test/Sema/struct-packed-align.c
+++ clang/test/Sema/struct-packed-align.c
@@ -146,18 +146,17 @@
 // GCC 4.4 but the change can lead to differences in the structure layout.
 // See the documentation of -Wpacked-bitfield-compat for more information.
 struct packed_chars {
-  char a:4;
+  char a : 8, b : 8, c : 8, d : 4;
 #ifdef __ORBIS__
   // Test for pre-r254596 clang behavior on the PS4 target. PS4 must maintain
   // ABI backwards compatibility.
-  char b:8 __attribute__ ((packed));
+  char e : 8 __attribute__((packed));
   // expected-warning@-1 {{'packed' attribute ignored for field of type 
'char'}}
-  char c:4;
 #else
-  char b:8 __attribute__ ((packed));
+  char e : 8 __attribute__((packed));
   // expected-warning@-1 {{'packed' attribute was ignored on bit-fields with 
single-byte alignment in older versions of GCC and Clang}}
-  char c:4;
 #endif
+  char f : 4, g : 8, h : 8, i : 8;
 };
 
 #if (defined(_WIN32) || defined(__ORBIS__)) && !defined(__declspec) // 
_MSC_VER is unavailable in cc1.
@@ -165,9 +164,13 @@
 //
 // Additionally, test for pre-r254596 clang behavior on the PS4 target. PS4
 // must maintain ABI backwards compatibility.
-extern int o1[sizeof(struct packed_chars) == 3 ? 1 : -1];
+extern int o1[sizeof(struct packed_chars) == 9 ? 1 : -1];
 extern int o2[__alignof(struct packed_chars) == 1 ? 1 : -1];
+#elif defined(_AIX)
+// On AIX, char bitfields have the same alignment as unsigned int.
+extern int o1[sizeof(struct packed_chars) == 8 ? 1 : -1];
+extern int o2[__alignof(struct packed_chars) == 4 ? 1 : -1];
 #else
-extern int o1[sizeof(struct packed_chars) == 2 ? 1 : -1];
+extern int o1[sizeof(struct packed_chars) == 8 ? 1 : -1];
 extern int o2[__alignof(struct packed_chars) == 1 ? 1 : -1];
 #endif


Index: clang/test/Sema/struct-packed-align.c
===
--- clang/test/Sema/struct-packed-align.c
+++ clang/test/Sema/struct-packed-align.c
@@ -146,18 +146,17 @@
 // GCC 4.4 but the change can lead to differences in the structure layout.
 // See the documentation of -Wpacked-bitfield-compat for more information.
 struct packed_chars {
-  char a:4;
+  char a : 8, b : 8, c : 8, d : 4;
 #ifdef __ORBIS__
   // Test for pre-r254596 clang behavior on the PS4 target. PS4 must maintain
   // ABI backwards compatibility.
-  char b:8 __attribute__ ((packed));
+  char e : 8 __attribute__((packed));
   // expected-warning@-1 {{'packed' attribute ignored for field of type 'char'}}
-  char c:4;
 #else
-  char b:8 __attribute__ ((packed));
+  char e : 8 __attribute__((packed));
   // expected-warning@-1 {{'packed' attribute was ignored on bit-fields with single-byte alignment in older versions of GCC and Clang}}
-  char c:4;
 #endif
+  char f : 4, g : 8, h : 8, i : 8;
 };
 
 #if (defined(_WIN32) || defined(__ORBIS__)) && !defined(__declspec) // _MSC_VER is unavailable in cc1.
@@ -165,9 +164,13 @@
 //
 // Additionally, test for pre-r254596 clang behavior on the PS4 target. PS4
 // must maintain ABI backwards compatibility.
-extern int o1[sizeof(struct packed_chars) == 3 ? 1 : -1];
+extern int o1[sizeof(struct packed_chars) == 9 ? 1 : -1];
 extern int o2[__alignof(struct packed_chars) == 1 ? 1 : -1];
+#elif defined(_AIX)
+// On AIX, char bitfields have the same alignment as unsigned int.
+extern int o1[sizeof(struct packed_chars) == 8 ? 1 : -1];
+extern int o2[__alignof(struct packed_chars) == 4 ? 1 : -1];
 #else
-extern int o1[sizeof(struct packed_chars) == 2 ? 1 : -1];
+extern int o1[sizeof(struct packed_chars) == 8 ? 1 : -1];
 extern int o2[__alignof(struct packed_chars) == 1 ? 1 : -1];
 #endif
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D102715: Fix LIT failure on native aix

2021-05-19 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added inline comments.



Comment at: clang/test/Sema/struct-packed-align.c:170
+#elif defined(_AIX)
+// On AIX, [bool, char, short] bitfields have the same alignment as unsigned
+// int.

Xiangling_L wrote:
> aaron.ballman wrote:
> > We're not really testing the behavior of `bool` or `short` anywhere and 
> > it'd be nice to verify that. Perhaps instead of modifying an existing test 
> > to add more fields, it'd make sense to make a new test structure?
> > 
> > While thinking of other potentially smaller-than-int types, I wondered 
> > whether `wchar_t` has special behavior here as well (I have no idea how 
> > that type is defined for AIX, so it's entirely possible it's size and 
> > alignment already match `int`).
> > We're not really testing the behavior of bool or short anywhere and it'd be 
> > nice to verify that. 
> 
> The comment is to explain why char has 4-byte alignment mainly. And the 
> testcase here is, as comments mentioned, to test `Packed attribute shouldn't 
> be ignored for bit-field of char types`.  Perhaps I should remove `bool` and 
> `short` so that people wouldn't be confused.  
> 
> And the special alignment regarding bool, short etc. has been tested when the 
> special rule introduced on aix here: https://reviews.llvm.org/D87029.
> 
> 
> 
> > Perhaps instead of modifying an existing test to add more fields, it'd make 
> > sense to make a new test structure?
> 
> I don't think it's necessary to make a new test structure. The modified 
> testcase test the same property as the original one. And it is more capable 
> as it can also test the property for AIX target.
> 
> 
> 
> 
> > I wondered whether wchar_t has special behavior here as well 
> 
> I think `wchar_t` has the same special behavior. Basically any type smaller 
> than 4 bytes will be aligned to 4 when it comes to bitfield. Please correct 
> me if I am wrong @hubert.reinterpretcast 
> 
> 
> Perhaps I should remove bool and short so that people wouldn't be confused.

That might not be a bad idea. I saw the comment and went to look for the 
declarations of `bool` and `short` type to verify they were behaving the same 
way, hence the confusion.

> The modified testcase test the same property as the original one.

The part that worries me is that it shifts the offset for `e`. Before, the 
packed field could be packed into the previous allocation unit (4 bits + 8 bits 
fit comfortably within a 32-bit allocation unit), but now the packed field is 
in an awkward spot (28 bits + 8 bits no longer fits into a 32-bit allocation 
unit). So I think it could be subtly changing the behavior of the test, but 
perhaps not in an observable way that matters (I admit that I don't know all 
the ins and outs of our packing strategies).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102715/new/

https://reviews.llvm.org/D102715

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

[PATCH] D102784: [Diagnostics] Allow emitting analysis and missed remarks on functions

2021-05-19 Thread Joseph Huber via Phabricator via cfe-commits

jhuber6 updated this revision to Diff 346516.
jhuber6 added a comment.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Fixing Clang tests.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102784/new/

https://reviews.llvm.org/D102784

Files:
  clang/test/OpenMP/remarks_parallel_in_multiple_target_state_machines.c
  clang/test/OpenMP/remarks_parallel_in_target_state_machine.c
  llvm/include/llvm/IR/DiagnosticInfo.h
  llvm/lib/IR/DiagnosticInfo.cpp
  llvm/lib/Transforms/IPO/OpenMPOpt.cpp
  llvm/test/Transforms/OpenMP/deduplication_remarks.ll
  llvm/test/Transforms/OpenMP/gpu_kernel_detection_remarks.ll
  llvm/test/Transforms/OpenMP/icv_remarks.ll

Index: llvm/test/Transforms/OpenMP/icv_remarks.ll
===
--- llvm/test/Transforms/OpenMP/icv_remarks.ll
+++ llvm/test/Transforms/OpenMP/icv_remarks.ll
@@ -1,5 +1,5 @@
-; RUN: opt -passes=openmp-opt-cgscc -pass-remarks=openmp-opt -openmp-print-icv-values -disable-output < %s 2>&1 | FileCheck %s
-; RUN: opt -openmp-opt-cgscc -pass-remarks=openmp-opt -openmp-print-icv-values -disable-output < %s 2>&1 | FileCheck %s
+; RUN: opt -passes=openmp-opt-cgscc -pass-remarks-analysis=openmp-opt -openmp-print-icv-values -disable-output < %s 2>&1 | FileCheck %s
+; RUN: opt -openmp-opt-cgscc -pass-remarks-analysis=openmp-opt -openmp-print-icv-values -disable-output < %s 2>&1 | FileCheck %s
 
 ; ModuleID = 'icv_remarks.c'
 source_filename = "icv_remarks.c"
Index: llvm/test/Transforms/OpenMP/gpu_kernel_detection_remarks.ll
===
--- llvm/test/Transforms/OpenMP/gpu_kernel_detection_remarks.ll
+++ llvm/test/Transforms/OpenMP/gpu_kernel_detection_remarks.ll
@@ -1,5 +1,5 @@
-; RUN: opt -passes=openmp-opt-cgscc -pass-remarks=openmp-opt -openmp-print-gpu-kernels -disable-output < %s 2>&1 | FileCheck %s --implicit-check-not=non_kernel
-; RUN: opt-openmp-opt-cgscc -pass-remarks=openmp-opt -openmp-print-gpu-kernels -disable-output < %s 2>&1 | FileCheck %s --implicit-check-not=non_kernel
+; RUN: opt -passes=openmp-opt-cgscc -pass-remarks-analysis=openmp-opt -openmp-print-gpu-kernels -disable-output < %s 2>&1 | FileCheck %s --implicit-check-not=non_kernel
+; RUN: opt-openmp-opt-cgscc -pass-remarks-analysis=openmp-opt -openmp-print-gpu-kernels -disable-output < %s 2>&1 | FileCheck %s --implicit-check-not=non_kernel
 
 ; CHECK-DAG: remark: :0:0: OpenMP GPU kernel kernel1
 ; CHECK-DAG: remark: :0:0: OpenMP GPU kernel kernel2
Index: llvm/test/Transforms/OpenMP/deduplication_remarks.ll
===
--- llvm/test/Transforms/OpenMP/deduplication_remarks.ll
+++ llvm/test/Transforms/OpenMP/deduplication_remarks.ll
@@ -10,9 +10,9 @@
 @0 = private unnamed_addr global %struct.ident_t { i32 0, i32 34, i32 0, i32 0, i8* getelementptr inbounds ([23 x i8], [23 x i8]* @.str0, i32 0, i32 0) }, align 8
 @.str0 = private unnamed_addr constant [23 x i8] c";unknown;unknown;0;0;;\00", align 1
 
-; CHECK: remark: deduplication_remarks.c:9:10: OpenMP runtime call __kmpc_global_thread_num moved to deduplication_remarks.c:5:10
-; CHECK: remark: deduplication_remarks.c:7:10: OpenMP runtime call __kmpc_global_thread_num deduplicated
-; CHECK: remark: deduplication_remarks.c:5:10: OpenMP runtime call __kmpc_global_thread_num deduplicated
+; CHECK: remark: deduplication_remarks.c:9:10: OpenMP runtime call __kmpc_global_thread_num moved to beginning of OpenMP region
+; CHECK: remark: deduplication_remarks.c:4:0: OpenMP runtime call __kmpc_global_thread_num deduplicated
+; CHECK: remark: deduplication_remarks.c:4:0: OpenMP runtime call __kmpc_global_thread_num deduplicated
 define dso_local void @deduplicate() local_unnamed_addr !dbg !14 {
   %1 = tail call i32 @__kmpc_global_thread_num(%struct.ident_t* nonnull @0), !dbg !21
   call void @useI32(i32 %1), !dbg !23
Index: llvm/lib/Transforms/IPO/OpenMPOpt.cpp
===
--- llvm/lib/Transforms/IPO/OpenMPOpt.cpp
+++ llvm/lib/Transforms/IPO/OpenMPOpt.cpp
@@ -578,15 +578,15 @@
 for (Function *F : OMPInfoCache.ModuleSlice) {
   for (auto ICV : ICVs) {
 auto ICVInfo = OMPInfoCache.ICVs[ICV];
-auto Remark = [&](OptimizationRemark OR) {
-  return OR << "OpenMP ICV " << ore::NV("OpenMPICV", ICVInfo.Name)
-<< " Value: "
-<< (ICVInfo.InitValue
-? ICVInfo.InitValue->getValue().toString(10, true)
-: "IMPLEMENTATION_DEFINED");
+auto Remark = [&](OptimizationRemarkAnalysis ORA) {
+  return ORA << "OpenMP ICV " << ore::NV("OpenMPICV", ICVInfo.Name)
+ << " Value: "
+ << (ICVInfo.InitValue
+ ? ICVInfo.InitValue->getValue().toString(10, true)
+

[PATCH] D102715: Fix LIT failure on native aix

2021-05-19 Thread Xiangling Liao via Phabricator via cfe-commits

Xiangling_L added inline comments.



Comment at: clang/test/Sema/struct-packed-align.c:170
+#elif defined(_AIX)
+// On AIX, [bool, char, short] bitfields have the same alignment as unsigned
+// int.

aaron.ballman wrote:
> Xiangling_L wrote:
> > aaron.ballman wrote:
> > > We're not really testing the behavior of `bool` or `short` anywhere and 
> > > it'd be nice to verify that. Perhaps instead of modifying an existing 
> > > test to add more fields, it'd make sense to make a new test structure?
> > > 
> > > While thinking of other potentially smaller-than-int types, I wondered 
> > > whether `wchar_t` has special behavior here as well (I have no idea how 
> > > that type is defined for AIX, so it's entirely possible it's size and 
> > > alignment already match `int`).
> > > We're not really testing the behavior of bool or short anywhere and it'd 
> > > be nice to verify that. 
> > 
> > The comment is to explain why char has 4-byte alignment mainly. And the 
> > testcase here is, as comments mentioned, to test `Packed attribute 
> > shouldn't be ignored for bit-field of char types`.  Perhaps I should remove 
> > `bool` and `short` so that people wouldn't be confused.  
> > 
> > And the special alignment regarding bool, short etc. has been tested when 
> > the special rule introduced on aix here: https://reviews.llvm.org/D87029.
> > 
> > 
> > 
> > > Perhaps instead of modifying an existing test to add more fields, it'd 
> > > make sense to make a new test structure?
> > 
> > I don't think it's necessary to make a new test structure. The modified 
> > testcase test the same property as the original one. And it is more capable 
> > as it can also test the property for AIX target.
> > 
> > 
> > 
> > 
> > > I wondered whether wchar_t has special behavior here as well 
> > 
> > I think `wchar_t` has the same special behavior. Basically any type smaller 
> > than 4 bytes will be aligned to 4 when it comes to bitfield. Please correct 
> > me if I am wrong @hubert.reinterpretcast 
> > 
> > 
> > Perhaps I should remove bool and short so that people wouldn't be confused.
> 
> That might not be a bad idea. I saw the comment and went to look for the 
> declarations of `bool` and `short` type to verify they were behaving the same 
> way, hence the confusion.
> 
> > The modified testcase test the same property as the original one.
> 
> The part that worries me is that it shifts the offset for `e`. Before, the 
> packed field could be packed into the previous allocation unit (4 bits + 8 
> bits fit comfortably within a 32-bit allocation unit), but now the packed 
> field is in an awkward spot (28 bits + 8 bits no longer fits into a 32-bit 
> allocation unit). So I think it could be subtly changing the behavior of the 
> test, but perhaps not in an observable way that matters (I admit that I don't 
> know all the ins and outs of our packing strategies).
> but now the packed field is in an awkward spot (28 bits + 8 bits no longer 
> fits into a 32-bit allocation unit)


I think this is exactly the purpose of the test. We'd like to tell if the 
`packed` attribute has effect or not.

Before the modification, on AIX, no matter the packed works or not, you will 
see the size = 4, align = 4 since char has 4-byte alignment.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102715/new/

https://reviews.llvm.org/D102715

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

[PATCH] D102715: Fix LIT failure on native aix

2021-05-19 Thread Hubert Tong via Phabricator via cfe-commits

hubert.reinterpretcast added inline comments.



Comment at: clang/test/Sema/struct-packed-align.c:170
+#elif defined(_AIX)
+// On AIX, [bool, char, short] bitfields have the same alignment as unsigned
+// int.

Xiangling_L wrote:
> aaron.ballman wrote:
> > Xiangling_L wrote:
> > > aaron.ballman wrote:
> > > > We're not really testing the behavior of `bool` or `short` anywhere and 
> > > > it'd be nice to verify that. Perhaps instead of modifying an existing 
> > > > test to add more fields, it'd make sense to make a new test structure?
> > > > 
> > > > While thinking of other potentially smaller-than-int types, I wondered 
> > > > whether `wchar_t` has special behavior here as well (I have no idea how 
> > > > that type is defined for AIX, so it's entirely possible it's size and 
> > > > alignment already match `int`).
> > > > We're not really testing the behavior of bool or short anywhere and 
> > > > it'd be nice to verify that. 
> > > 
> > > The comment is to explain why char has 4-byte alignment mainly. And the 
> > > testcase here is, as comments mentioned, to test `Packed attribute 
> > > shouldn't be ignored for bit-field of char types`.  Perhaps I should 
> > > remove `bool` and `short` so that people wouldn't be confused.  
> > > 
> > > And the special alignment regarding bool, short etc. has been tested when 
> > > the special rule introduced on aix here: https://reviews.llvm.org/D87029.
> > > 
> > > 
> > > 
> > > > Perhaps instead of modifying an existing test to add more fields, it'd 
> > > > make sense to make a new test structure?
> > > 
> > > I don't think it's necessary to make a new test structure. The modified 
> > > testcase test the same property as the original one. And it is more 
> > > capable as it can also test the property for AIX target.
> > > 
> > > 
> > > 
> > > 
> > > > I wondered whether wchar_t has special behavior here as well 
> > > 
> > > I think `wchar_t` has the same special behavior. Basically any type 
> > > smaller than 4 bytes will be aligned to 4 when it comes to bitfield. 
> > > Please correct me if I am wrong @hubert.reinterpretcast 
> > > 
> > > 
> > > Perhaps I should remove bool and short so that people wouldn't be 
> > > confused.
> > 
> > That might not be a bad idea. I saw the comment and went to look for the 
> > declarations of `bool` and `short` type to verify they were behaving the 
> > same way, hence the confusion.
> > 
> > > The modified testcase test the same property as the original one.
> > 
> > The part that worries me is that it shifts the offset for `e`. Before, the 
> > packed field could be packed into the previous allocation unit (4 bits + 8 
> > bits fit comfortably within a 32-bit allocation unit), but now the packed 
> > field is in an awkward spot (28 bits + 8 bits no longer fits into a 32-bit 
> > allocation unit). So I think it could be subtly changing the behavior of 
> > the test, but perhaps not in an observable way that matters (I admit that I 
> > don't know all the ins and outs of our packing strategies).
> > but now the packed field is in an awkward spot (28 bits + 8 bits no longer 
> > fits into a 32-bit allocation unit)
> 
> 
> I think this is exactly the purpose of the test. We'd like to tell if the 
> `packed` attribute has effect or not.
> 
> Before the modification, on AIX, no matter the packed works or not, you will 
> see the size = 4, align = 4 since char has 4-byte alignment.
My understanding is that the "awkward spot" was always the intention of the 
test. It's just that the assumption around "allocation unit" size being 1 for 
`char` was encoded into the test.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102715/new/

https://reviews.llvm.org/D102715

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

[PATCH] D98798: Produce warning for performing pointer arithmetic on a null pointer.

2021-05-19 Thread Jamie Schmeiser via Phabricator via cfe-commits

jamieschmeiser updated this revision to Diff 346515.
jamieschmeiser edited the summary of this revision.
jamieschmeiser added a comment.

As requested, I have added a new warning option -Wnull-pointer-subtraction (and 
added it to -Wextra) that does not trigger on system headers.  In addition, I 
changed the message used such that it states that it is undefined behaviour for 
C but may be undefined behaviour in C++ to address the concerns that performing 
the subtraction with a pointer that dynamically becomes null is not undefined 
behaviour.  Expanded the testing to also test that the warning does not fire on 
system headers.  Also, updated the release notes to indicate the new option.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98798/new/

https://reviews.llvm.org/D98798

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/Inputs/pointer-subtraction.h
  clang/test/Sema/pointer-subtraction.c
  clang/test/Sema/pointer-subtraction.cpp

Index: clang/test/Sema/pointer-subtraction.cpp
===
--- /dev/null
+++ clang/test/Sema/pointer-subtraction.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 %s -fsyntax-only -verify -pedantic -Wextra -std=c++11 -isystem %S/Inputs
+// RUN: %clang_cc1 %s -fsyntax-only -verify -pedantic -Wnull-pointer-subtraction -std=c++11 -isystem %S/Inputs
+// RUN: %clang_cc1 %s -fsyntax-only -verify -pedantic -Wextra -std=c++11 -isystem %S/Inputs -Wsystem-headers -DSYSTEM_WARNINGS
+// RUN: %clang_cc1 %s -fsyntax-only -verify -pedantic -Wnull-pointer-subtraction -std=c++11 -isystem %S/Inputs -Wsystem-headers -DSYSTEM_WARNINGS
+
+#include 
+
+void a() {
+  char *f = (char*)0;
+  f = (char*)((char*)0 - f); // expected-warning {{performing pointer subtraction with a null pointer may have undefined behavior}}
+  f = (char*)(f - (char*)0); // expected-warning {{performing pointer subtraction with a null pointer may have undefined behavior}}
+  f = (char*)((char*)0 - (char*)0); // valid in C++
+
+#ifndef SYSTEM_WARNINGS
+  SYSTEM_MACRO(f);
+#else
+  SYSTEM_MACRO(f); // expected-warning {{performing pointer subtraction with a null pointer may have undefined behavior}}
+#endif
+}
Index: clang/test/Sema/pointer-subtraction.c
===
--- /dev/null
+++ clang/test/Sema/pointer-subtraction.c
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 %s -fsyntax-only -verify -pedantic -Wextra -std=c11 -isystem %S/Inputs
+// RUN: %clang_cc1 %s -fsyntax-only -verify -pedantic -Wnull-pointer-subtraction -std=c11 -isystem %S/Inputs
+// RUN: %clang_cc1 %s -fsyntax-only -verify -pedantic -Wextra -std=c11 -isystem %S/Inputs -Wsystem-headers -DSYSTEM_WARNINGS
+// RUN: %clang_cc1 %s -fsyntax-only -verify -pedantic -Wnull-pointer-subtraction -std=c11 -isystem %S/Inputs -Wsystem-headers -DSYSTEM_WARNINGS
+
+#include 
+
+void a() {
+  char *f = (char*)0;
+  f = (char*)((char*)0 - f); // expected-warning {{performing pointer subtraction with a null pointer has undefined behavior}}
+  f = (char*)(f - (char*)0); // expected-warning {{performing pointer subtraction with a null pointer has undefined behavior}}
+  f = (char*)((char*)0 - (char*)0); // expected-warning {{performing pointer subtraction with a null pointer has undefined behavior}} expected-warning {{performing pointer subtraction with a null pointer has undefined behavior}}
+
+#ifndef SYSTEM_WARNINGS
+  SYSTEM_MACRO(f);
+#else
+  SYSTEM_MACRO(f); // expected-warning {{performing pointer subtraction with a null pointer has undefined behavior}}
+#endif
+}
Index: clang/test/Sema/Inputs/pointer-subtraction.h
===
--- /dev/null
+++ clang/test/Sema/Inputs/pointer-subtraction.h
@@ -0,0 +1 @@
+#define SYSTEM_MACRO(x) (x) = (char*)((char*)0 - (x))
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -10366,6 +10366,22 @@
   << S.getLangOpts().CPlusPlus << Pointer->getSourceRange();
 }
 
+/// Diagnose invalid subraction on a null pointer.
+///
+static void diagnoseSubtractionOnNullPointer(Sema &S, SourceLocation Loc,
+	 Expr *Pointer, bool BothNull) {
+  // Null - null is valid in C++ [expr.add]p7
+  if (BothNull && S.getLangOpts().CPlusPlus)
+return;
+
+  // Is this s a macro from a system header?
+  if (S.Diags.getSuppressSystemWarnings() && S.SourceMgr.isInSystemMacro(Loc))
+return;
+
+  S.Diag(Loc, diag::warn_pointer_sub_null_ptr)
+<< S.getLangOpts().CPlusPlus << Pointer->getSourceRange();
+}
+
 /// Diagnose invalid arithmetic on two function pointers.
 static void diagnoseArithmeticOnTwoFunctionPointers(Sema &S, SourceLocation Loc,
 Expr *LHS, Expr *RHS) {
@@ -10797,7 +10813,16 @@

[PATCH] D98798: Produce warning for performing pointer arithmetic on a null pointer.

2021-05-19 Thread Jamie Schmeiser via Phabricator via cfe-commits

jamieschmeiser requested review of this revision.
jamieschmeiser added a comment.

Significant changes made since previously accepted.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98798/new/

https://reviews.llvm.org/D98798

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

[PATCH] D102791: [WebAssembly] Warn on exception spec for Emscripten EH

2021-05-19 Thread Heejin Ahn via Phabricator via cfe-commits

aheejin created this revision.
aheejin added a reviewer: dschuff.
Herald added subscribers: wingo, ecnelises, sunfish, jgravelle-google, sbc100.
aheejin requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

It turns out we have not correctly supported exception spec all along in
Emscripten EH. Emscripten EH supports `throw()` but not `throw` with
types. See https://bugs.llvm.org/show_bug.cgi?id=50396.

Wasm EH also only supports `throw()` but not `throw` with types, and we
have been printing a warning message for the latter. This prints the
same warning message for `throw` with types when Emscripten EH is used,
or more precisely, when Wasm EH is not used. (So this will print the
warning messsage even when `-fno-exceptions` is used but I think that
should be fine. It's cumbersome to do a complilcated option checking in
CGException.cpp and options checkings are mostly done in elsewhere.)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D102791

Files:
  clang/lib/CodeGen/CGException.cpp
  clang/test/CodeGenCXX/wasm-eh.cpp


Index: clang/test/CodeGenCXX/wasm-eh.cpp
===
--- clang/test/CodeGenCXX/wasm-eh.cpp
+++ clang/test/CodeGenCXX/wasm-eh.cpp
@@ -381,7 +381,7 @@
 // RUN: %clang_cc1 %s -triple wasm32-unknown-unknown -fms-extensions 
-fexceptions -fcxx-exceptions -exception-model=wasm -target-feature 
+exception-handling -emit-llvm -o - -std=c++11 2>&1 | FileCheck %s 
--check-prefix=WARNING-DEFAULT
 // RUN: %clang_cc1 %s -triple wasm32-unknown-unknown -fms-extensions 
-fexceptions -fcxx-exceptions -exception-model=wasm -target-feature 
+exception-handling -Wwasm-exception-spec -emit-llvm -o - -std=c++11 2>&1 | 
FileCheck %s --check-prefix=WARNING-ON
 // RUN: %clang_cc1 %s -triple wasm32-unknown-unknown -fms-extensions 
-fexceptions -fcxx-exceptions -exception-model=wasm -target-feature 
+exception-handling -Wno-wasm-exception-spec -emit-llvm -o - -std=c++11 2>&1 | 
FileCheck %s --check-prefix=WARNING-OFF
-// RUN: %clang_cc1 %s -triple wasm32-unknown-unknown -fexceptions 
-fcxx-exceptions -emit-llvm -o - -std=c++11 2>&1 | FileCheck %s 
--check-prefix=NOT-WASM-EH
+// RUN: %clang_cc1 %s -triple wasm32-unknown-unknown -fexceptions 
-fcxx-exceptions -emit-llvm -o - -std=c++11 2>&1 | FileCheck %s 
--check-prefix=EM-EH-WARNING
 
 // Wasm EH ignores dynamic exception specifications with types at the moment.
 // This is controlled by -Wwasm-exception-spec, which is on by default. This
@@ -392,7 +392,7 @@
 // WARNING-DEFAULT: warning: dynamic exception specifications with types are 
currently ignored in wasm
 // WARNING-ON: warning: dynamic exception specifications with types are 
currently ignored in wasm
 // WARNING-OFF-NOT: warning: dynamic exception specifications with types are 
currently ignored in wasm
-// NOT-WASM-EH-NOT: warning: dynamic exception specifications with types are 
currently ignored in wasm
+// EM-EH-WARNING: warning: dynamic exception specifications with types are 
currently ignored in wasm
 
 // Wasm curremtly treats 'throw()' in the same way as 'noexept'. Check if the
 // same warning message is printed as if when a 'noexcept' function throws.
Index: clang/lib/CodeGen/CGException.cpp
===
--- clang/lib/CodeGen/CGException.cpp
+++ clang/lib/CodeGen/CGException.cpp
@@ -473,9 +473,9 @@
 // encode these in an object file but MSVC doesn't do anything with it.
 if (getTarget().getCXXABI().isMicrosoft())
   return;
-// In wasm we currently treat 'throw()' in the same way as 'noexcept'. In
+// In Wasm EH we currently treat 'throw()' in the same way as 'noexcept'. 
In
 // case of throw with types, we ignore it and print a warning for now.
-// TODO Correctly handle exception specification in wasm
+// TODO Correctly handle exception specification in Wasm EH
 if (CGM.getLangOpts().hasWasmExceptions()) {
   if (EST == EST_DynamicNone)
 EHStack.pushTerminate();
@@ -485,6 +485,19 @@
 << FD->getExceptionSpecSourceRange();
   return;
 }
+// Currently Emscripten EH only handles 'throw()' but not 'throw' with
+// types. 'throw()' handling will be done in JS glue code so we don't need
+// to do anything in that case. Just print a warning message in case of
+// throw with types.
+// TODO Correctly handle exception specification in Emscripten EH
+if (getTarget().getCXXABI() == TargetCXXABI::WebAssembly &&
+CGM.getLangOpts().getExceptionHandling() ==
+LangOptions::ExceptionHandlingKind::None &&
+EST == EST_Dynamic)
+  CGM.getDiags().Report(D->getLocation(),
+diag::warn_wasm_dynamic_exception_spec_ignored)
+  << FD->getExceptionSpecSourceRange();
+
 unsigned NumExceptions = Proto->getNumExceptions();
 EHFilterScope *Filter = EHStack.pushFilter(NumExceptions);

[PATCH] D102568: [Driver] Delete -mimplicit-it=

2021-05-19 Thread Martin Storsjö via Phabricator via cfe-commits

mstorsjo added a comment.

In D102568#2769053 , @nickdesaulniers 
wrote:

>> But this change did break my build in these places:
>> https://code.videolan.org/videolan/x264/-/blob/b684ebe04a6f80f8207a57940a1fa00e25274f81/configure#L855
>> https://chromium.googlesource.com/webm/libvpx/+/66c1ff6850fd53bcf5c17247569bea1d700d6247/build/make/configure.sh#1012
>> https://github.com/cisco/openh264/blob/089d6c6d83ab6e5d451ef18808bb6c46faf553a2/build/platform-mingw_nt.mk#L23
>
> Thanks for the links; @MaskRay I think you should file some issues in those 
> projects to help them move to the assembler option before we try to remove it 
> again.

Well I already started doing that, I had made one PR, when CI tests there 
informed me that the new form of the option didn't work out with the version of 
Clang in use there (10.0 fwiw).

>>> Should be an easy fix either way (replace `-mimplicit-it=always` with 
>>> `-Wa,-wmimplicit-it=always`).
>>
>> No, it's not an easy fix as no existing releases of Clang support it.
>
> Well, these projects linked above will need to implement either feature 
> detection for the command line flag options before hard coding them, or 
> compiler version checks.

I really think it's silly to demand multiple projects to implement detection 
for this option. Just let's revert this change, let both option forms coexist 
in a couple public stable releases, until it's acceptable to raise the minimum 
required tool version for those build configurations to Clang 13 (it's a quite 
niche configuration, but upgrading CI systems always takes some time), and then 
after that drop the old form (e.g. in Clang 15). Maybe we could add a 
deprecation warning to the one that we're going to remove in the future?

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102568/new/

https://reviews.llvm.org/D102568

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

[PATCH] D102782: Add support for Warning Flag "-Wstack-usage="

2021-05-19 Thread David Blaikie via Phabricator via cfe-commits

dblaikie added a comment.

Generally looks reasonable to me - but I'll leave signoff to some other folks 
who have been more involved in the discussion so far.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102782/new/

https://reviews.llvm.org/D102782

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

[PATCH] D102715: Fix LIT failure on native aix

2021-05-19 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman accepted this revision.
aaron.ballman added a comment.

LGTM!




Comment at: clang/test/Sema/struct-packed-align.c:170
+#elif defined(_AIX)
+// On AIX, [bool, char, short] bitfields have the same alignment as unsigned
+// int.

hubert.reinterpretcast wrote:
> Xiangling_L wrote:
> > aaron.ballman wrote:
> > > Xiangling_L wrote:
> > > > aaron.ballman wrote:
> > > > > We're not really testing the behavior of `bool` or `short` anywhere 
> > > > > and it'd be nice to verify that. Perhaps instead of modifying an 
> > > > > existing test to add more fields, it'd make sense to make a new test 
> > > > > structure?
> > > > > 
> > > > > While thinking of other potentially smaller-than-int types, I 
> > > > > wondered whether `wchar_t` has special behavior here as well (I have 
> > > > > no idea how that type is defined for AIX, so it's entirely possible 
> > > > > it's size and alignment already match `int`).
> > > > > We're not really testing the behavior of bool or short anywhere and 
> > > > > it'd be nice to verify that. 
> > > > 
> > > > The comment is to explain why char has 4-byte alignment mainly. And the 
> > > > testcase here is, as comments mentioned, to test `Packed attribute 
> > > > shouldn't be ignored for bit-field of char types`.  Perhaps I should 
> > > > remove `bool` and `short` so that people wouldn't be confused.  
> > > > 
> > > > And the special alignment regarding bool, short etc. has been tested 
> > > > when the special rule introduced on aix here: 
> > > > https://reviews.llvm.org/D87029.
> > > > 
> > > > 
> > > > 
> > > > > Perhaps instead of modifying an existing test to add more fields, 
> > > > > it'd make sense to make a new test structure?
> > > > 
> > > > I don't think it's necessary to make a new test structure. The modified 
> > > > testcase test the same property as the original one. And it is more 
> > > > capable as it can also test the property for AIX target.
> > > > 
> > > > 
> > > > 
> > > > 
> > > > > I wondered whether wchar_t has special behavior here as well 
> > > > 
> > > > I think `wchar_t` has the same special behavior. Basically any type 
> > > > smaller than 4 bytes will be aligned to 4 when it comes to bitfield. 
> > > > Please correct me if I am wrong @hubert.reinterpretcast 
> > > > 
> > > > 
> > > > Perhaps I should remove bool and short so that people wouldn't be 
> > > > confused.
> > > 
> > > That might not be a bad idea. I saw the comment and went to look for the 
> > > declarations of `bool` and `short` type to verify they were behaving the 
> > > same way, hence the confusion.
> > > 
> > > > The modified testcase test the same property as the original one.
> > > 
> > > The part that worries me is that it shifts the offset for `e`. Before, 
> > > the packed field could be packed into the previous allocation unit (4 
> > > bits + 8 bits fit comfortably within a 32-bit allocation unit), but now 
> > > the packed field is in an awkward spot (28 bits + 8 bits no longer fits 
> > > into a 32-bit allocation unit). So I think it could be subtly changing 
> > > the behavior of the test, but perhaps not in an observable way that 
> > > matters (I admit that I don't know all the ins and outs of our packing 
> > > strategies).
> > > but now the packed field is in an awkward spot (28 bits + 8 bits no 
> > > longer fits into a 32-bit allocation unit)
> > 
> > 
> > I think this is exactly the purpose of the test. We'd like to tell if the 
> > `packed` attribute has effect or not.
> > 
> > Before the modification, on AIX, no matter the packed works or not, you 
> > will see the size = 4, align = 4 since char has 4-byte alignment.
> My understanding is that the "awkward spot" was always the intention of the 
> test. It's just that the assumption around "allocation unit" size being 1 for 
> `char` was encoded into the test.
Ah, thank you both for pointing that out to me. This just shifts the awkward 
spot to make the test scenario more obvious. That makes a lot more sense to me.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102715/new/

https://reviews.llvm.org/D102715

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

[PATCH] D102568: [Driver] Delete -mimplicit-it=

2021-05-19 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay added a comment.

In D102568#2769237 , @mstorsjo wrote:

> In D102568#2769053 , 
> @nickdesaulniers wrote:
>
>>> But this change did break my build in these places:
>>> https://code.videolan.org/videolan/x264/-/blob/b684ebe04a6f80f8207a57940a1fa00e25274f81/configure#L855
>>> https://chromium.googlesource.com/webm/libvpx/+/66c1ff6850fd53bcf5c17247569bea1d700d6247/build/make/configure.sh#1012
>>> https://github.com/cisco/openh264/blob/089d6c6d83ab6e5d451ef18808bb6c46faf553a2/build/platform-mingw_nt.mk#L23
>>
>> Thanks for the links; @MaskRay I think you should file some issues in those 
>> projects to help them move to the assembler option before we try to remove 
>> it again.
>
> Well I already started doing that, I had made one PR, when CI tests there 
> informed me that the new form of the option didn't work out with the version 
> of Clang in use there (10.0 fwiw).

Thanks for noticing the teams.

 Should be an easy fix either way (replace `-mimplicit-it=always` with 
 `-Wa,-wmimplicit-it=always`).
>>>
>>> No, it's not an easy fix as no existing releases of Clang support it.
>>
>> Well, these projects linked above will need to implement either feature 
>> detection for the command line flag options before hard coding them, or 
>> compiler version checks.
>
> I really think it's silly to demand multiple projects to implement detection 
> for this option. Just let's revert this change, let both option forms coexist 
> in a couple public stable releases, until it's acceptable to raise the 
> minimum required tool version for those build configurations to Clang 13 
> (it's a quite niche configuration, but upgrading CI systems always takes some 
> time), and then after that drop the old form (e.g. in Clang 15). Maybe we 
> could add a deprecation warning to the one that we're going to remove in the 
> future?

I don't mind reverting this temporarily.
However, reverting this would break musl build.
musl detects both options and will add both if available: 
`-Wa,-mimplicit-it=never -mimplicit-it=always` will cause a duplicate option 
failure.
Can't there be other projects which do similar detection and be broken by 
having both options?

I think "waiting for a few releases" is too much and doesn't improve things. I 
can accept "waiting for one major release".


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102568/new/

https://reviews.llvm.org/D102568

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

[PATCH] D102568: [Driver] Delete -mimplicit-it=

2021-05-19 Thread David Blaikie via Phabricator via cfe-commits

dblaikie added a comment.

> I think "waiting for a few releases" is too much and doesn't improve things 
> (they will notice issues until you remove the option). I can accept "waiting 
> for one major release".

Having fairly broad windows of not breaking backwards compatibility is a fairly 
reasonable request. Take LLVM itself, for instance - which supports building 
with Clang back as far as 3.5. (not suggesting we need to have a window that 
large for every feature/piece of surface area - but that there is scope for 
fairly wide windows)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102568/new/

https://reviews.llvm.org/D102568

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

[PATCH] D101793: [clang][AST] Improve AST Reader/Writer memory footprint

2021-05-19 Thread Wei Wang via Phabricator via cfe-commits

weiwang added a comment.

Tried to make `Sema::DeclsToCheckForDeferredDiags` `llvm::SmallSetVector`. The 
heap RSS did drop significantly (from peak 100GB to 59GB) , but not as good as 
the current fix (peak 26GB), which makes 
`ASTReader::DeclsToCheckForDeferredDiags` `llvm::SmallSetVector`.

I think the reason is that the duplicated decls are read from multiple module 
file sources (`ASTReader::ReadAST()` -> `ASTReader::ReadASTBlock()`), then 
stored into `ASTReader::DeclsToCheckForDeferredDiags`, then goes into 
`Sema::DeclsToCheckForDeferredDiags` in 
`ASTReader::ReadDeclsToCheckForDeferredDiags()`. Doing dedup at the early stage 
when the decls were just read in `ASTReader` is more effective at reducing RSS.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101793/new/

https://reviews.llvm.org/D101793

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

[PATCH] D102568: [Driver] Delete -mimplicit-it=

2021-05-19 Thread Martin Storsjö via Phabricator via cfe-commits

mstorsjo added a comment.

In D102568#2769267 , @MaskRay wrote:

> I don't mind reverting this temporarily.
> However, reverting this would break musl build.
> musl (since 2018-09) detects both options and will add both if available: 
> `-Wa,-mimplicit-it=never -mimplicit-it=always` will cause a duplicate option 
> failure.
> Can't there be other projects which do similar detection and be broken by 
> having both options?

I'm curious - if it detects both forms and adds both options, why does it add 
them with contradicting option values? If I understand things correctly, adding 
both options with matching values isn't an error?

Ok, so now I checked, and it does seem to error out even if they have matching 
values - that sounds like the real issue to me.

In the meantime, wouldn't it be possible to detect the presence of the other 
one and check if they match or not, to avoid passing duplicate options to the 
backend? I can give that a try.

> I think "waiting for a few releases" is too much and doesn't improve things 
> (they will notice issues until you remove the option). I can accept "waiting 
> for one major release".

Ok, that's at least some sort of middle ground. Would fixing the duplicate 
option issue (as long as they have matching values) open up for keeping both a 
little bit longer?

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102568/new/

https://reviews.llvm.org/D102568

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

[PATCH] D102568: [Driver] Delete -mimplicit-it=

2021-05-19 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay added a comment.

I think libvpx's ASFLAGS usage is about GNU as, not the driver option.

In D102568#2769296 , @dblaikie wrote:

>> I think "waiting for a few releases" is too much and doesn't improve things 
>> (they will notice issues until you remove the option). I can accept "waiting 
>> for one major release".
>
> Having fairly broad windows of not breaking backwards compatibility is a 
> fairly reasonable request. Take LLVM itself, for instance - which supports 
> building with Clang back as far as 3.5. (not suggesting we need to have a 
> window that large for every feature/piece of surface area - but that there is 
> scope for fairly wide windows)

I understand the broad Windows and I also strive for this portability.

However, reverting this patch is now a trade-off. It would make x264/openh264's 
mingw build happy but break musl's arm build (2018-09 ~ 2021-02).
There can be other projects doing detection on both -mimplicit-it and 
-Wa,-mimplicit-it and will be broken by reverting this patch.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102568/new/

https://reviews.llvm.org/D102568

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

[PATCH] D102568: [Driver] Delete -mimplicit-it=

2021-05-19 Thread Martin Storsjö via Phabricator via cfe-commits

mstorsjo added a comment.

In D102568#2769341 , @MaskRay wrote:

> I think libvpx's ASFLAGS usage is about GNU as, not the driver option.

It is indeed for calling a GNU as-like tool, but at 
https://chromium.googlesource.com/webm/libvpx/+/66c1ff6850fd53bcf5c17247569bea1d700d6247/build/make/configure.sh#1003
 it sets `AS="$CC -c"`, which is for cases when using clang-based tools where 
there's no standalone `as`-like tool to use.

My initial patch for that case looked like this:

  diff --git a/build/make/configure.sh b/build/make/configure.sh 
  index 81d30a16c..526c08d3a 100644 
  --- a/build/make/configure.sh 
  +++ b/build/make/configure.sh 
  @@ -1009,7 +1009,16 @@ EOF 
 if enabled thumb; then 
   asm_conversion_cmd="$asm_conversion_cmd -thumb" 
   check_add_cflags -mthumb 
  -check_add_asflags -mthumb -mimplicit-it=always 
  +check_add_asflags -mthumb 
  +case ${tgt_os} in 
  +  win*) 
  +# If $AS is $CC, this flag needs to be passed with -Wa. 
  +check_add_asflags -Wa,-mimplicit-it=always 
  +;; 
  +  *) 
  +check_add_asflags -mimplicit-it=always 
  +;; 
  +esac 
 fi 
 ;; 
   vs*) 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102568/new/

https://reviews.llvm.org/D102568

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

[PATCH] D102736: Fix tmp files being left on Windows builds.

2021-05-19 Thread Adrian McCarthy via Phabricator via cfe-commits

amccarth added a comment.

At some point, the duplicate handle must be closed.  I don't see that 
happening.  I've added an inline comment where I think it should be done.

(I find it weird that duplicating the handle seems necessary.)

At a high level, it seems a shame that `llvm::support::fs` doesn't have 
create-temporary-file and keep-temporary-file operations to hide all this 
detail from the frontend.




Comment at: clang/lib/Frontend/CompilerInstance.cpp:721
+llvm::sys::fs::UnmarkFileForDeletion(OF.Handle);
+
 // If '-working-directory' was passed, the output filename should be

IIUC, OF.Handle is the duplicate handle, and we're done with it at this point.  
It should be closed, before doing things like trying to rename/move the file.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102736/new/

https://reviews.llvm.org/D102736

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

[PATCH] D102736: Fix tmp files being left on Windows builds.

2021-05-19 Thread Adrian McCarthy via Phabricator via cfe-commits

amccarth requested changes to this revision.
amccarth added a comment.
This revision now requires changes to proceed.

Sorry, forgot to Request Changes.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102736/new/

https://reviews.llvm.org/D102736

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

[clang] 2db182f - [Diagnostics] Allow emitting analysis and missed remarks on functions

2021-05-19 Thread Joseph Huber via cfe-commits


Author: Joseph Huber
Date: 2021-05-19T15:10:20-04:00
New Revision: 2db182ff8d0c0d50155bf70d1db60b4e78c348ca

URL: 
https://github.com/llvm/llvm-project/commit/2db182ff8d0c0d50155bf70d1db60b4e78c348ca
DIFF: 
https://github.com/llvm/llvm-project/commit/2db182ff8d0c0d50155bf70d1db60b4e78c348ca.diff

LOG: [Diagnostics] Allow emitting analysis and missed remarks on functions

Summary:
Currently, only `OptimizationRemarks` can be emitted using a Function.
Add constructors to allow this for `OptimizationRemarksAnalysis` and
`OptimizationRemarkMissed` as well.

Reviewed By: jdoerfert thegameg

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

Added: 


Modified: 
clang/test/OpenMP/remarks_parallel_in_multiple_target_state_machines.c
clang/test/OpenMP/remarks_parallel_in_target_state_machine.c
llvm/include/llvm/IR/DiagnosticInfo.h
llvm/lib/IR/DiagnosticInfo.cpp
llvm/lib/Transforms/IPO/OpenMPOpt.cpp
llvm/test/Transforms/OpenMP/deduplication_remarks.ll
llvm/test/Transforms/OpenMP/gpu_kernel_detection_remarks.ll
llvm/test/Transforms/OpenMP/icv_remarks.ll

Removed: 




diff  --git 
a/clang/test/OpenMP/remarks_parallel_in_multiple_target_state_machines.c 
b/clang/test/OpenMP/remarks_parallel_in_multiple_target_state_machines.c
index 8a8b55686d4c3..72593b96bd1ba 100644
--- a/clang/test/OpenMP/remarks_parallel_in_multiple_target_state_machines.c
+++ b/clang/test/OpenMP/remarks_parallel_in_multiple_target_state_machines.c
@@ -1,6 +1,6 @@
-// RUN: %clang_cc1 -verify=host
  -Rpass=openmp -fopenmp -x c++ 
-triple powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda 
-emit-llvm-bc %s -o %t-ppc-host.bc
-// RUN: %clang_cc1 -verify=all,safe
  -Rpass=openmp -fopenmp -O2 -x c++ 
-triple nvptx64-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm 
%s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o %t.out
-// RUN: %clang_cc1 -fexperimental-new-pass-manager -verify=all,safe
  -Rpass=openmp -fopenmp -O2 -x c++ 
-triple nvptx64-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm 
%s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o %t.out
+// RUN: %clang_cc1 -verify=host  
-Rpass=openmp-opt -Rpass-analysis=openmp -fopenmp -x c++ -triple 
powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm-bc 
%s -o %t-ppc-host.bc
+// RUN: %clang_cc1 -verify=all,safe  
-Rpass=openmp-opt -Rpass-analysis=openmp -fopenmp -O2 -x c++ -triple 
nvptx64-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm %s 
-fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o %t.out
+// RUN: %clang_cc1 -fexperimental-new-pass-manager -verify=all,safe  
-Rpass=openmp-opt -Rpass-analysis=openmp -fopenmp -O2 -x c++ -triple 
nvptx64-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm %s 
-fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o %t.out
 
 // host-no-diagnostics
 
@@ -96,5 +96,5 @@ void spmd(void) {
   }
 }
 
-// all-remark@* 5 {{OpenMP runtime call __kmpc_global_thread_num moved to}}
+// all-remark@* 5 {{OpenMP runtime call __kmpc_global_thread_num moved to 
beginning of OpenMP region}}
 // all-remark@* 12 {{OpenMP runtime call __kmpc_global_thread_num 
deduplicated}}

diff  --git a/clang/test/OpenMP/remarks_parallel_in_target_state_machine.c 
b/clang/test/OpenMP/remarks_parallel_in_target_state_machine.c
index 3d0d527dc42fd..604c2f2abfc25 100644
--- a/clang/test/OpenMP/remarks_parallel_in_target_state_machine.c
+++ b/clang/test/OpenMP/remarks_parallel_in_target_state_machine.c
@@ -1,6 +1,6 @@
-// RUN: %clang_cc1 -verify=host -Rpass=openmp 
-fopenmp -x c++ -triple powerpc64le-unknown-unknown 
-fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm-bc %s -o %t-ppc-host.bc
-// RUN: %clang_cc1 -verify  -Rpass=openmp 
-fopenmp -O2 -x c++ -triple nvptx64-unknown-unknown 
-fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm %s -fopenmp-is-device 
-fopenmp-host-ir-file-path %t-ppc-host.bc -o %t.out
-// RUN: %clang_cc1 -fexperimental-new-pass-manager -verify  -Rpass=openmp 
-fopenmp -O2 -x c++ -triple nvptx64-unknown-unknown 
-fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm %s -fopenmp-is-device 
-fopenmp-host-ir-file-path %t-ppc-host.bc -o %t.out
+// RUN: %clang_cc1 -verify=host -Rpass=openmp 
-Rpass-analysis=openmp-opt -fopenmp -x c++ -triple powerpc64le-unknown-unknown 
-fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm-bc %s -o %t-ppc-host.bc
+// RUN: %clang_cc1 -verify  -Rpass=openmp 
-Rpass-ana

[PATCH] D102784: [Diagnostics] Allow emitting analysis and missed remarks on functions

2021-05-19 Thread Joseph Huber via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG2db182ff8d0c: [Diagnostics] Allow emitting analysis and 
missed remarks on functions (authored by jhuber6).

Changed prior to commit:
  https://reviews.llvm.org/D102784?vs=346516&id=346531#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102784/new/

https://reviews.llvm.org/D102784

Files:
  clang/test/OpenMP/remarks_parallel_in_multiple_target_state_machines.c
  clang/test/OpenMP/remarks_parallel_in_target_state_machine.c
  llvm/include/llvm/IR/DiagnosticInfo.h
  llvm/lib/IR/DiagnosticInfo.cpp
  llvm/lib/Transforms/IPO/OpenMPOpt.cpp
  llvm/test/Transforms/OpenMP/deduplication_remarks.ll
  llvm/test/Transforms/OpenMP/gpu_kernel_detection_remarks.ll
  llvm/test/Transforms/OpenMP/icv_remarks.ll

Index: llvm/test/Transforms/OpenMP/icv_remarks.ll
===
--- llvm/test/Transforms/OpenMP/icv_remarks.ll
+++ llvm/test/Transforms/OpenMP/icv_remarks.ll
@@ -1,5 +1,5 @@
-; RUN: opt -passes=openmp-opt-cgscc -pass-remarks=openmp-opt -openmp-print-icv-values -disable-output < %s 2>&1 | FileCheck %s
-; RUN: opt -openmp-opt-cgscc -pass-remarks=openmp-opt -openmp-print-icv-values -disable-output < %s 2>&1 | FileCheck %s
+; RUN: opt -passes=openmp-opt-cgscc -pass-remarks-analysis=openmp-opt -openmp-print-icv-values -disable-output < %s 2>&1 | FileCheck %s
+; RUN: opt -openmp-opt-cgscc -pass-remarks-analysis=openmp-opt -openmp-print-icv-values -disable-output < %s 2>&1 | FileCheck %s
 
 ; ModuleID = 'icv_remarks.c'
 source_filename = "icv_remarks.c"
Index: llvm/test/Transforms/OpenMP/gpu_kernel_detection_remarks.ll
===
--- llvm/test/Transforms/OpenMP/gpu_kernel_detection_remarks.ll
+++ llvm/test/Transforms/OpenMP/gpu_kernel_detection_remarks.ll
@@ -1,5 +1,5 @@
-; RUN: opt -passes=openmp-opt-cgscc -pass-remarks=openmp-opt -openmp-print-gpu-kernels -disable-output < %s 2>&1 | FileCheck %s --implicit-check-not=non_kernel
-; RUN: opt-openmp-opt-cgscc -pass-remarks=openmp-opt -openmp-print-gpu-kernels -disable-output < %s 2>&1 | FileCheck %s --implicit-check-not=non_kernel
+; RUN: opt -passes=openmp-opt-cgscc -pass-remarks-analysis=openmp-opt -openmp-print-gpu-kernels -disable-output < %s 2>&1 | FileCheck %s --implicit-check-not=non_kernel
+; RUN: opt-openmp-opt-cgscc -pass-remarks-analysis=openmp-opt -openmp-print-gpu-kernels -disable-output < %s 2>&1 | FileCheck %s --implicit-check-not=non_kernel
 
 ; CHECK-DAG: remark: :0:0: OpenMP GPU kernel kernel1
 ; CHECK-DAG: remark: :0:0: OpenMP GPU kernel kernel2
Index: llvm/test/Transforms/OpenMP/deduplication_remarks.ll
===
--- llvm/test/Transforms/OpenMP/deduplication_remarks.ll
+++ llvm/test/Transforms/OpenMP/deduplication_remarks.ll
@@ -10,9 +10,9 @@
 @0 = private unnamed_addr global %struct.ident_t { i32 0, i32 34, i32 0, i32 0, i8* getelementptr inbounds ([23 x i8], [23 x i8]* @.str0, i32 0, i32 0) }, align 8
 @.str0 = private unnamed_addr constant [23 x i8] c";unknown;unknown;0;0;;\00", align 1
 
-; CHECK: remark: deduplication_remarks.c:9:10: OpenMP runtime call __kmpc_global_thread_num moved to deduplication_remarks.c:5:10
-; CHECK: remark: deduplication_remarks.c:7:10: OpenMP runtime call __kmpc_global_thread_num deduplicated
-; CHECK: remark: deduplication_remarks.c:5:10: OpenMP runtime call __kmpc_global_thread_num deduplicated
+; CHECK: remark: deduplication_remarks.c:4:0: OpenMP runtime call __kmpc_global_thread_num moved to beginning of OpenMP region
+; CHECK: remark: deduplication_remarks.c:4:0: OpenMP runtime call __kmpc_global_thread_num deduplicated
+; CHECK: remark: deduplication_remarks.c:4:0: OpenMP runtime call __kmpc_global_thread_num deduplicated
 define dso_local void @deduplicate() local_unnamed_addr !dbg !14 {
   %1 = tail call i32 @__kmpc_global_thread_num(%struct.ident_t* nonnull @0), !dbg !21
   call void @useI32(i32 %1), !dbg !23
Index: llvm/lib/Transforms/IPO/OpenMPOpt.cpp
===
--- llvm/lib/Transforms/IPO/OpenMPOpt.cpp
+++ llvm/lib/Transforms/IPO/OpenMPOpt.cpp
@@ -581,15 +581,15 @@
 for (Function *F : OMPInfoCache.ModuleSlice) {
   for (auto ICV : ICVs) {
 auto ICVInfo = OMPInfoCache.ICVs[ICV];
-auto Remark = [&](OptimizationRemark OR) {
-  return OR << "OpenMP ICV " << ore::NV("OpenMPICV", ICVInfo.Name)
-<< " Value: "
-<< (ICVInfo.InitValue
-? ICVInfo.InitValue->getValue().toString(10, true)
-: "IMPLEMENTATION_DEFINED");
+auto Remark = [&](OptimizationRemarkAnalysis ORA) {
+  return ORA << "OpenMP ICV " << ore::NV("OpenMPICV", ICVInfo.Name)
+ << " Value: "
+ << (ICVInfo.

[PATCH] D102568: [Driver] Delete -mimplicit-it=

2021-05-19 Thread David Blaikie via Phabricator via cfe-commits

dblaikie added a comment.

In D102568#2769341 , @MaskRay wrote:

> I think libvpx's ASFLAGS usage is about GNU as, not the driver option.
>
> In D102568#2769296 , @dblaikie 
> wrote:
>
>>> I think "waiting for a few releases" is too much and doesn't improve things 
>>> (they will notice issues until you remove the option). I can accept 
>>> "waiting for one major release".
>>
>> Having fairly broad windows of not breaking backwards compatibility is a 
>> fairly reasonable request. Take LLVM itself, for instance - which supports 
>> building with Clang back as far as 3.5. (not suggesting we need to have a 
>> window that large for every feature/piece of surface area - but that there 
>> is scope for fairly wide windows)
>
> I understand the broad Windows and I also strive for this portability.
>
> However, reverting this patch is now a trade-off. It would make 
> x264/openh264's mingw build happy but break musl's arm build (2018-09 ~ ).
> There can be other projects doing detection on both -mimplicit-it and 
> -Wa,-mimplicit-it and will be broken by reverting this patch.

Not sure I follow - presumably musl's arm build wasn't working with clang 
before this patch? Or something changed there after this patch landed?

It's not OK to say "this patch fixed a bug for someone, so now reverting it is 
on equal footing with the regressions the patch caused" - if a patch broke an 
existing use case that's different from fixing a use case that wasn't working 
to begin with & reverting to the known/existing failures would be where things 
tend to lean (not absolute by any means - there are cases where we tradeoff 
adding a new bug to fix an existing bug) if all other things are equal.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102568/new/

https://reviews.llvm.org/D102568

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

[PATCH] D102568: [Driver] Delete -mimplicit-it=

2021-05-19 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay added a comment.

In D102568#2769340 , @mstorsjo wrote:

> In the meantime, wouldn't it be possible to detect the presence of the other 
> one and check if they match or not, to avoid passing duplicate options to the 
> backend? I can give that a try.

Should be possible! Thanks for offering the help. I think it is too much to 
test contradicting values like `// NEVER_ALWAYS: "-mllvm" 
"-arm-implicit-it=never" "-mllvm" "-arm-implicit-it=always"`.
We can just assume having different values is undefined behavior. (No project 
should specify different values.)

>> I think "waiting for a few releases" is too much and doesn't improve things 
>> (they will notice issues until you remove the option). I can accept "waiting 
>> for one major release".

I said this because this would break musl arm build.

> Ok, that's at least some sort of middle ground. Would fixing the duplicate 
> option issue (as long as they have matching values) open up for keeping both 
> a little bit longer?

If you can make -mimplicit-it= -Wa,-mimplicit-it= work, keeping the driver 
option bit longer LG.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102568/new/

https://reviews.llvm.org/D102568

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

1 2 >

1 - 100 of 187 matches

Mail list logo