[PATCH] D85276: [PGO][CUDA][HIP] Skip generating profile on the device stub and wrong-side functions.

2020-08-05 Thread Michael Liao via Phabricator via cfe-commits
hliao created this revision.
hliao added reviewers: tra, yaxunl, bogner.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
hliao requested review of this revision.

- Skip generating profile data on `__global__` function in the host 
compilation. It's a host-side stub function only and don't have profile 
instrumentation generated on the real function body. The extra profile data 
results in the malformed instrumentation profile data.
- Skip generating region mapping on functions in the wrong-side, i.e., + For 
the device compilation, skip host-only functions; and, + For the host 
compilation, skip device-only functions (including `__global__` functions.)
- As the device-side profiling is not ready yet, only host-side profile code 
generation is checked.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85276

Files:
  clang/lib/CodeGen/CodeGenPGO.cpp
  clang/test/CodeGenCUDA/profile-coverage-mapping.cu


Index: clang/test/CodeGenCUDA/profile-coverage-mapping.cu
===
--- /dev/null
+++ clang/test/CodeGenCUDA/profile-coverage-mapping.cu
@@ -0,0 +1,20 @@
+// RUN: echo "GPU binary would be here" > %t
+// RUN: %clang_cc1 -fprofile-instrument=clang -triple x86_64-linux-gnu 
-target-sdk-version=8.0 -fcuda-include-gpubinary %t -emit-llvm -o - %s | 
FileCheck --check-prefix=PGOGEN %s
+// RUN: %clang_cc1 -fprofile-instrument=clang -fcoverage-mapping -triple 
x86_64-linux-gnu -target-sdk-version=8.0 -fcuda-include-gpubinary %t -emit-llvm 
-o - %s | FileCheck --check-prefix=COVMAP %s
+// RUN: %clang_cc1 -fprofile-instrument=clang -fcoverage-mapping 
-dump-coverage-mapping -triple x86_64-linux-gnu -target-sdk-version=8.0 
-fcuda-include-gpubinary %t -emit-llvm-only -o - %s | FileCheck 
--check-prefix=MAPPING %s
+
+#include "Inputs/cuda.h"
+
+// PGOGEN-NOT: @__profn_{{.*kernel.*}} =
+// COVMAP-COUNT-2: section "__llvm_covfun", comdat
+// COVMAP-NOT: section "__llvm_covfun", comdat
+// MAPPING-NOT: {{.*dfn.*}}:
+// MAPPING-NOT: {{.*kernel.*}}:
+
+__device__ void dfn(int i) {}
+
+__global__ void kernel(int i) { dfn(i); }
+
+void host(void) {
+  kernel<<<1, 1>>>(1);
+}
Index: clang/lib/CodeGen/CodeGenPGO.cpp
===
--- clang/lib/CodeGen/CodeGenPGO.cpp
+++ clang/lib/CodeGen/CodeGenPGO.cpp
@@ -773,6 +773,11 @@
   if (!D->hasBody())
 return;
 
+  // Skip CUDA/HIP kernel launch stub functions.
+  if (CGM.getLangOpts().CUDA && !CGM.getLangOpts().CUDAIsDevice &&
+  D->hasAttr())
+return;
+
   bool InstrumentRegions = CGM.getCodeGenOpts().hasProfileClangInstr();
   llvm::IndexedInstrProfReader *PGOReader = CGM.getPGOReader();
   if (!InstrumentRegions && !PGOReader)
@@ -831,6 +836,16 @@
   if (!D->getBody())
 return true;
 
+  // Skip host-only functions in the CUDA device compilation and device-only
+  // functions in the host compilation.
+  if (CGM.getLangOpts().CUDA &&
+  ((CGM.getLangOpts().CUDAIsDevice && !D->hasAttr() &&
+!D->hasAttr()) ||
+   (!CGM.getLangOpts().CUDAIsDevice &&
+(D->hasAttr() ||
+ (!D->hasAttr() && D->hasAttr())
+return true;
+
   // Don't map the functions in system headers.
   const auto &SM = CGM.getContext().getSourceManager();
   auto Loc = D->getBody()->getBeginLoc();


Index: clang/test/CodeGenCUDA/profile-coverage-mapping.cu
===
--- /dev/null
+++ clang/test/CodeGenCUDA/profile-coverage-mapping.cu
@@ -0,0 +1,20 @@
+// RUN: echo "GPU binary would be here" > %t
+// RUN: %clang_cc1 -fprofile-instrument=clang -triple x86_64-linux-gnu -target-sdk-version=8.0 -fcuda-include-gpubinary %t -emit-llvm -o - %s | FileCheck --check-prefix=PGOGEN %s
+// RUN: %clang_cc1 -fprofile-instrument=clang -fcoverage-mapping -triple x86_64-linux-gnu -target-sdk-version=8.0 -fcuda-include-gpubinary %t -emit-llvm -o - %s | FileCheck --check-prefix=COVMAP %s
+// RUN: %clang_cc1 -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping -triple x86_64-linux-gnu -target-sdk-version=8.0 -fcuda-include-gpubinary %t -emit-llvm-only -o - %s | FileCheck --check-prefix=MAPPING %s
+
+#include "Inputs/cuda.h"
+
+// PGOGEN-NOT: @__profn_{{.*kernel.*}} =
+// COVMAP-COUNT-2: section "__llvm_covfun", comdat
+// COVMAP-NOT: section "__llvm_covfun", comdat
+// MAPPING-NOT: {{.*dfn.*}}:
+// MAPPING-NOT: {{.*kernel.*}}:
+
+__device__ void dfn(int i) {}
+
+__global__ void kernel(int i) { dfn(i); }
+
+void host(void) {
+  kernel<<<1, 1>>>(1);
+}
Index: clang/lib/CodeGen/CodeGenPGO.cpp
===
--- clang/lib/CodeGen/CodeGenPGO.cpp
+++ clang/lib/CodeGen/CodeGenPGO.cpp
@@ -773,6 +773,11 @@
   if (!D->hasBody())
 return;
 
+  // Skip CUDA/HIP kernel launch stub functions.
+  if (CGM.getLangOpts().CUDA && !CGM.getLangOpts().CUDAIsDevice &&
+  D->hasAttr())
+return;
+
   bool InstrumentRegions =

[PATCH] D83242: [clang][BPF] support type exist/size and enum exist/value relocations

2020-08-05 Thread Yonghong Song via Phabricator via cfe-commits
yonghong-song added a comment.

> Hm.. This root type is used by libbpf to find candidate in kernel BTF. Name 
> and kind of that type is what is used. When you have typedef t and struct t, 
> their names match, but their kinds don't. So it could lead to inability to 
> find a candidate.

The inproper CSE can only happen if the declaration in user program like 
`struct t { ... }; typedef struct t t;` and user try to find existence of both 
`struct t` and typedef `t`. We could have issue if kernel does not have `struct 
t { ...}` and only has `typedef t`. Theoretically this could happen, but as you 
mentioned the chance is really slim. So will not change current implementation 
right now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83242

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


[PATCH] D67422: [analyzer] NFC: Move path diagnostic consumer implementations to libAnalysis.

2020-08-05 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment.

Other than parameter names, it looks totally reasonable to me.




Comment at: clang/lib/Analysis/HTMLPathDiagnosticConsumer.cpp:70
+ const std::string &OutputDir,
+ const Preprocessor &pp, bool 
supportsMultipleFiles)
   : DiagOpts(std::move(DiagOpts)), Directory(OutputDir), PP(pp),

Maybe while we are here, we can change the names to match the code style (aka 
capitalize)?
It seems like all other functions have similar problems.  It's no biggie, but 
still...

NOTE: I really wanted to try this new feature of Phabricator 😊 



Comment at: clang/lib/Analysis/PlistHTMLPathDiagnosticConsumer.cpp:25
+PathDiagnosticConsumerOptions DiagOpts, PathDiagnosticConsumers &C,
+const std::string &prefix, const Preprocessor &PP,
+const cross_tu::CrossTranslationUnitContext &CTU) {

As long as it is a new function, I guess we should definitely keep names 
according to the **Coding Standards**


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

https://reviews.llvm.org/D67422

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


[PATCH] D85185: [SyntaxTree] Add test coverage for `->*` operator

2020-08-05 Thread Eduardo Caldas via Phabricator via cfe-commits
eduucaldas updated this revision to Diff 283144.
eduucaldas added a comment.

Answer comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85185

Files:
  clang/unittests/Tooling/Syntax/TreeTest.cpp


Index: clang/unittests/Tooling/Syntax/TreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/TreeTest.cpp
+++ clang/unittests/Tooling/Syntax/TreeTest.cpp
@@ -2322,16 +2322,17 @@
   friend bool operator<(const X&, const X&);
   friend X operator<<(X&, const X&);
   X operator,(X&);
-  // TODO: Fix crash on member function pointer and add a test for `->*`
-  // TODO: Unbox operators in syntax tree. 
+  X operator->*(int);
+  // TODO: Unbox operators in syntax tree.
   // Represent operators by `+` instead of `IdExpression-UnqualifiedId-+`
 };
-void test(X x, X y) {
+void test(X x, X y, X* xp, int X::* pmi) {
   x = y;
   x + y;
   x < y;
   x << y;
   x, y;
+  xp->*pmi;
 }
 )cpp",
   R"txt(
@@ -2430,6 +2431,17 @@
 | | |   |   `-&
 | | |   `-)
 | | `-;
+| |-SimpleDeclaration
+| | |-X
+| | |-SimpleDeclarator
+| | | |-operator
+| | | |-->*
+| | | `-ParametersAndQualifiers
+| | |   |-(
+| | |   |-SimpleDeclaration
+| | |   | `-int
+| | |   `-)
+| | `-;
 | |-}
 | `-;
 `-SimpleDeclaration
@@ -2447,6 +2459,21 @@
   |   | |-X
   |   | `-SimpleDeclarator
   |   |   `-y
+  |   |-,
+  |   |-SimpleDeclaration
+  |   | |-X
+  |   | `-SimpleDeclarator
+  |   |   |-*
+  |   |   `-xp
+  |   |-,
+  |   |-SimpleDeclaration
+  |   | |-int
+  |   | `-SimpleDeclarator
+  |   |   |-MemberPointer
+  |   |   | |-X
+  |   |   | |-::
+  |   |   | `-*
+  |   |   `-pmi
   |   `-)
   `-CompoundStatement
 |-{
@@ -2511,6 +2538,16 @@
 | |   `-UnqualifiedId
 | | `-y
 | `-;
+|-ExpressionStatement
+| |-BinaryOperatorExpression
+| | |-IdExpression
+| | | `-UnqualifiedId
+| | |   `-xp
+| | |-->*
+| | `-IdExpression
+| |   `-UnqualifiedId
+| | `-pmi
+| `-;
 `-}
 )txt"));
 }


Index: clang/unittests/Tooling/Syntax/TreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/TreeTest.cpp
+++ clang/unittests/Tooling/Syntax/TreeTest.cpp
@@ -2322,16 +2322,17 @@
   friend bool operator<(const X&, const X&);
   friend X operator<<(X&, const X&);
   X operator,(X&);
-  // TODO: Fix crash on member function pointer and add a test for `->*`
-  // TODO: Unbox operators in syntax tree. 
+  X operator->*(int);
+  // TODO: Unbox operators in syntax tree.
   // Represent operators by `+` instead of `IdExpression-UnqualifiedId-+`
 };
-void test(X x, X y) {
+void test(X x, X y, X* xp, int X::* pmi) {
   x = y;
   x + y;
   x < y;
   x << y;
   x, y;
+  xp->*pmi;
 }
 )cpp",
   R"txt(
@@ -2430,6 +2431,17 @@
 | | |   |   `-&
 | | |   `-)
 | | `-;
+| |-SimpleDeclaration
+| | |-X
+| | |-SimpleDeclarator
+| | | |-operator
+| | | |-->*
+| | | `-ParametersAndQualifiers
+| | |   |-(
+| | |   |-SimpleDeclaration
+| | |   | `-int
+| | |   `-)
+| | `-;
 | |-}
 | `-;
 `-SimpleDeclaration
@@ -2447,6 +2459,21 @@
   |   | |-X
   |   | `-SimpleDeclarator
   |   |   `-y
+  |   |-,
+  |   |-SimpleDeclaration
+  |   | |-X
+  |   | `-SimpleDeclarator
+  |   |   |-*
+  |   |   `-xp
+  |   |-,
+  |   |-SimpleDeclaration
+  |   | |-int
+  |   | `-SimpleDeclarator
+  |   |   |-MemberPointer
+  |   |   | |-X
+  |   |   | |-::
+  |   |   | `-*
+  |   |   `-pmi
   |   `-)
   `-CompoundStatement
 |-{
@@ -2511,6 +2538,16 @@
 | |   `-UnqualifiedId
 | | `-y
 | `-;
+|-ExpressionStatement
+| |-BinaryOperatorExpression
+| | |-IdExpression
+| | | `-UnqualifiedId
+| | |   `-xp
+| | |-->*
+| | `-IdExpression
+| |   `-UnqualifiedId
+| | `-pmi
+| `-;
 `-}
 )txt"));
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] c5cdc3e - [SyntaxTree] Add test coverage for `->*` operator

2020-08-05 Thread Eduardo Caldas via cfe-commits

Author: Eduardo Caldas
Date: 2020-08-05T07:36:39Z
New Revision: c5cdc3e801ad1b0aceaf220d78a3ff3fab1e0fdb

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

LOG: [SyntaxTree] Add test coverage for `->*` operator

This was the last binary operator that we supported but didn't have any
test coverage. The recent fix in a crash in member pointers allowed us
to add this test.

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

Added: 


Modified: 
clang/unittests/Tooling/Syntax/TreeTest.cpp

Removed: 




diff  --git a/clang/unittests/Tooling/Syntax/TreeTest.cpp 
b/clang/unittests/Tooling/Syntax/TreeTest.cpp
index 3ccfabb95da9..e696be3dae7c 100644
--- a/clang/unittests/Tooling/Syntax/TreeTest.cpp
+++ b/clang/unittests/Tooling/Syntax/TreeTest.cpp
@@ -2329,16 +2329,17 @@ struct X {
   friend bool operator<(const X&, const X&);
   friend X operator<<(X&, const X&);
   X operator,(X&);
-  // TODO: Fix crash on member function pointer and add a test for `->*`
-  // TODO: Unbox operators in syntax tree. 
+  X operator->*(int);
+  // TODO: Unbox operators in syntax tree.
   // Represent operators by `+` instead of `IdExpression-UnqualifiedId-+`
 };
-void test(X x, X y) {
+void test(X x, X y, X* xp, int X::* pmi) {
   x = y;
   x + y;
   x < y;
   x << y;
   x, y;
+  xp->*pmi;
 }
 )cpp",
   R"txt(
@@ -2437,6 +2438,17 @@ void test(X x, X y) {
 | | |   |   `-&
 | | |   `-)
 | | `-;
+| |-SimpleDeclaration
+| | |-X
+| | |-SimpleDeclarator
+| | | |-operator
+| | | |-->*
+| | | `-ParametersAndQualifiers
+| | |   |-(
+| | |   |-SimpleDeclaration
+| | |   | `-int
+| | |   `-)
+| | `-;
 | |-}
 | `-;
 `-SimpleDeclaration
@@ -2454,6 +2466,21 @@ void test(X x, X y) {
   |   | |-X
   |   | `-SimpleDeclarator
   |   |   `-y
+  |   |-,
+  |   |-SimpleDeclaration
+  |   | |-X
+  |   | `-SimpleDeclarator
+  |   |   |-*
+  |   |   `-xp
+  |   |-,
+  |   |-SimpleDeclaration
+  |   | |-int
+  |   | `-SimpleDeclarator
+  |   |   |-MemberPointer
+  |   |   | |-X
+  |   |   | |-::
+  |   |   | `-*
+  |   |   `-pmi
   |   `-)
   `-CompoundStatement
 |-{
@@ -2518,6 +2545,16 @@ void test(X x, X y) {
 | |   `-UnqualifiedId
 | | `-y
 | `-;
+|-ExpressionStatement
+| |-BinaryOperatorExpression
+| | |-IdExpression
+| | | `-UnqualifiedId
+| | |   `-xp
+| | |-->*
+| | `-IdExpression
+| |   `-UnqualifiedId
+| | `-pmi
+| `-;
 `-}
 )txt"));
 }



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


[PATCH] D85185: [SyntaxTree] Add test coverage for `->*` operator

2020-08-05 Thread Eduardo Caldas 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 rGc5cdc3e801ad: [SyntaxTree] Add test coverage for `->*` 
operator (authored by eduucaldas).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85185

Files:
  clang/unittests/Tooling/Syntax/TreeTest.cpp


Index: clang/unittests/Tooling/Syntax/TreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/TreeTest.cpp
+++ clang/unittests/Tooling/Syntax/TreeTest.cpp
@@ -2329,16 +2329,17 @@
   friend bool operator<(const X&, const X&);
   friend X operator<<(X&, const X&);
   X operator,(X&);
-  // TODO: Fix crash on member function pointer and add a test for `->*`
-  // TODO: Unbox operators in syntax tree. 
+  X operator->*(int);
+  // TODO: Unbox operators in syntax tree.
   // Represent operators by `+` instead of `IdExpression-UnqualifiedId-+`
 };
-void test(X x, X y) {
+void test(X x, X y, X* xp, int X::* pmi) {
   x = y;
   x + y;
   x < y;
   x << y;
   x, y;
+  xp->*pmi;
 }
 )cpp",
   R"txt(
@@ -2437,6 +2438,17 @@
 | | |   |   `-&
 | | |   `-)
 | | `-;
+| |-SimpleDeclaration
+| | |-X
+| | |-SimpleDeclarator
+| | | |-operator
+| | | |-->*
+| | | `-ParametersAndQualifiers
+| | |   |-(
+| | |   |-SimpleDeclaration
+| | |   | `-int
+| | |   `-)
+| | `-;
 | |-}
 | `-;
 `-SimpleDeclaration
@@ -2454,6 +2466,21 @@
   |   | |-X
   |   | `-SimpleDeclarator
   |   |   `-y
+  |   |-,
+  |   |-SimpleDeclaration
+  |   | |-X
+  |   | `-SimpleDeclarator
+  |   |   |-*
+  |   |   `-xp
+  |   |-,
+  |   |-SimpleDeclaration
+  |   | |-int
+  |   | `-SimpleDeclarator
+  |   |   |-MemberPointer
+  |   |   | |-X
+  |   |   | |-::
+  |   |   | `-*
+  |   |   `-pmi
   |   `-)
   `-CompoundStatement
 |-{
@@ -2518,6 +2545,16 @@
 | |   `-UnqualifiedId
 | | `-y
 | `-;
+|-ExpressionStatement
+| |-BinaryOperatorExpression
+| | |-IdExpression
+| | | `-UnqualifiedId
+| | |   `-xp
+| | |-->*
+| | `-IdExpression
+| |   `-UnqualifiedId
+| | `-pmi
+| `-;
 `-}
 )txt"));
 }


Index: clang/unittests/Tooling/Syntax/TreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/TreeTest.cpp
+++ clang/unittests/Tooling/Syntax/TreeTest.cpp
@@ -2329,16 +2329,17 @@
   friend bool operator<(const X&, const X&);
   friend X operator<<(X&, const X&);
   X operator,(X&);
-  // TODO: Fix crash on member function pointer and add a test for `->*`
-  // TODO: Unbox operators in syntax tree. 
+  X operator->*(int);
+  // TODO: Unbox operators in syntax tree.
   // Represent operators by `+` instead of `IdExpression-UnqualifiedId-+`
 };
-void test(X x, X y) {
+void test(X x, X y, X* xp, int X::* pmi) {
   x = y;
   x + y;
   x < y;
   x << y;
   x, y;
+  xp->*pmi;
 }
 )cpp",
   R"txt(
@@ -2437,6 +2438,17 @@
 | | |   |   `-&
 | | |   `-)
 | | `-;
+| |-SimpleDeclaration
+| | |-X
+| | |-SimpleDeclarator
+| | | |-operator
+| | | |-->*
+| | | `-ParametersAndQualifiers
+| | |   |-(
+| | |   |-SimpleDeclaration
+| | |   | `-int
+| | |   `-)
+| | `-;
 | |-}
 | `-;
 `-SimpleDeclaration
@@ -2454,6 +2466,21 @@
   |   | |-X
   |   | `-SimpleDeclarator
   |   |   `-y
+  |   |-,
+  |   |-SimpleDeclaration
+  |   | |-X
+  |   | `-SimpleDeclarator
+  |   |   |-*
+  |   |   `-xp
+  |   |-,
+  |   |-SimpleDeclaration
+  |   | |-int
+  |   | `-SimpleDeclarator
+  |   |   |-MemberPointer
+  |   |   | |-X
+  |   |   | |-::
+  |   |   | `-*
+  |   |   `-pmi
   |   `-)
   `-CompoundStatement
 |-{
@@ -2518,6 +2545,16 @@
 | |   `-UnqualifiedId
 | | `-y
 | `-;
+|-ExpressionStatement
+| |-BinaryOperatorExpression
+| | |-IdExpression
+| | | `-UnqualifiedId
+| | |   `-xp
+| | |-->*
+| | `-IdExpression
+| |   `-UnqualifiedId
+| | `-pmi
+| `-;
 `-}
 )txt"));
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D84736: [analyzer][RFC] Handle pointer difference of ElementRegion and SymbolicRegion

2020-08-05 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment.

Great job, I'm on board with this change!

I think that our code is a bit under commented when it comes to the logic 
inside of functions, so I would really appreciate if we can add a good chunk of 
comments here!

Also, I believe integration tests would be pretty much straightforward with 
`clang_analyzer_eval`.




Comment at: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:1035-1038
+return evalBinOpNN(state, BO_Mul, Index,
+   makeArrayIndex(SingleElementSize.getQuantity()),
+   ArrayIndexTy)
+.castAs();

I would prefer having a comment for this line with a very simple formula of the 
thing we are calculating.  I think it can increase the readability of the code 
because when there is a call accepting a whole bunch of arguments it is never 
obvious and it always takes time to parse through.



Comment at: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:1041
+
+  // If both has the same memory region, and the index has a concrete 
value,
+  // we can evaluate equality operators.

This comment is a bit deceiving IMO.  It returns a concrete value even when 
`HasSameMemRegions` is false, but from the comment it seems like we evaluate 
the operator ONLY when `HasSameMemRegions` is true.



Comment at: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:1068
+}
+  } else if (LeftER && !RightER) {
+NonLoc LeftIndex = ByteOffsetOfElement(LeftER);

I think it's better to add comments for each case describing what is the 
situation we handle here in a simplified form.
And for each return within the case - a short comment with the reason.


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

https://reviews.llvm.org/D84736

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


[PATCH] D85282: [Concepts] Dump template arguments for immediately declared constraint.

2020-08-05 Thread Haojian Wu via Phabricator via cfe-commits
hokein created this revision.
hokein added a reviewer: nridge.
Herald added a subscriber: kristof.beyls.
Herald added a project: clang.
hokein requested review of this revision.

The template arguments were dumped as part of the TemplateTypeParmDecl, which
was incorrect.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85282

Files:
  clang/include/clang/AST/ASTNodeTraverser.h
  clang/lib/AST/TextNodeDumper.cpp
  clang/test/AST/ast-dump-concepts.cpp


Index: clang/test/AST/ast-dump-concepts.cpp
===
--- clang/test/AST/ast-dump-concepts.cpp
+++ clang/test/AST/ast-dump-concepts.cpp
@@ -15,8 +15,12 @@
 template 
 struct Foo {
   // CHECK:  TemplateTypeParmDecl {{.*}} referenced Concept {{.*}} 
'binary_concept'
-  // CHECK-NEXT: |-ConceptSpecializationExpr {{.*}}  'bool' 
Concept {{.*}} 'binary_concept'
-  // CHECK-NEXT: `-TemplateArgument {{.*}} type 'int'
+  // CHECK-NEXT: `-ConceptSpecializationExpr {{.*}} 'bool' Concept {{.*}} 
'binary_concept'
+  // CHECK-NEXT:   |-TemplateArgument {{.*}} type 'R'
+  // CHECK-NEXT:   | `-TemplateTypeParmType {{.*}} 'R'
+  // CHECK-NEXT:   |   `-TemplateTypeParm {{.*}} 'R'
+  // CHECK-NEXT:   `-TemplateArgument {{.*}} type 'int'
+  // CHECK-NEXT: `-BuiltinType {{.*}} 'int'
   template  R>
   Foo(R);
 
@@ -25,11 +29,11 @@
   template 
   Foo(R);
 
-  // CHECK:  FunctionTemplateDecl {{.*}}  {{.*}} 
Foo
+  // CHECK:  FunctionTemplateDecl {{.*}}  {{.*}} Foo
   template 
   Foo(R, int) requires unary_concept;
 
-  // CHECK:  FunctionTemplateDecl {{.*}}  {{.*}} 
Foo
+  // CHECK:  FunctionTemplateDecl {{.*}}  {{.*}} Foo
   template 
   Foo(R, char) requires unary_concept {
   }
Index: clang/lib/AST/TextNodeDumper.cpp
===
--- clang/lib/AST/TextNodeDumper.cpp
+++ clang/lib/AST/TextNodeDumper.cpp
@@ -2000,7 +2000,6 @@
   dumpBareDeclRef(TC->getFoundDecl());
   OS << ")";
 }
-AddChild([=] { Visit(TC->getImmediatelyDeclaredConstraint()); });
   } else if (D->wasDeclaredWithTypename())
 OS << " typename";
   else
Index: clang/include/clang/AST/ASTNodeTraverser.h
===
--- clang/include/clang/AST/ASTNodeTraverser.h
+++ clang/include/clang/AST/ASTNodeTraverser.h
@@ -543,9 +543,7 @@
 
   void VisitTemplateTypeParmDecl(const TemplateTypeParmDecl *D) {
 if (const auto *TC = D->getTypeConstraint())
-  if (TC->hasExplicitTemplateArgs())
-for (const auto &ArgLoc : TC->getTemplateArgsAsWritten()->arguments())
-  dumpTemplateArgumentLoc(ArgLoc);
+  Visit(TC->getImmediatelyDeclaredConstraint());
 if (D->hasDefaultArgument())
   Visit(D->getDefaultArgument(), SourceRange(),
 D->getDefaultArgStorage().getInheritedFrom(),
@@ -574,6 +572,12 @@
 Visit(D->getConstraintExpr());
   }
 
+  void VisitConceptSpecializationExpr(const ConceptSpecializationExpr *CSE) {
+if (CSE->hasExplicitTemplateArgs())
+  for (const auto &ArgLoc : CSE->getTemplateArgsAsWritten()->arguments())
+dumpTemplateArgumentLoc(ArgLoc);
+  }
+
   void VisitUsingShadowDecl(const UsingShadowDecl *D) {
 if (auto *TD = dyn_cast(D->getUnderlyingDecl()))
   Visit(TD->getTypeForDecl());


Index: clang/test/AST/ast-dump-concepts.cpp
===
--- clang/test/AST/ast-dump-concepts.cpp
+++ clang/test/AST/ast-dump-concepts.cpp
@@ -15,8 +15,12 @@
 template 
 struct Foo {
   // CHECK:  TemplateTypeParmDecl {{.*}} referenced Concept {{.*}} 'binary_concept'
-  // CHECK-NEXT: |-ConceptSpecializationExpr {{.*}}  'bool' Concept {{.*}} 'binary_concept'
-  // CHECK-NEXT: `-TemplateArgument {{.*}} type 'int'
+  // CHECK-NEXT: `-ConceptSpecializationExpr {{.*}} 'bool' Concept {{.*}} 'binary_concept'
+  // CHECK-NEXT:   |-TemplateArgument {{.*}} type 'R'
+  // CHECK-NEXT:   | `-TemplateTypeParmType {{.*}} 'R'
+  // CHECK-NEXT:   |   `-TemplateTypeParm {{.*}} 'R'
+  // CHECK-NEXT:   `-TemplateArgument {{.*}} type 'int'
+  // CHECK-NEXT: `-BuiltinType {{.*}} 'int'
   template  R>
   Foo(R);
 
@@ -25,11 +29,11 @@
   template 
   Foo(R);
 
-  // CHECK:  FunctionTemplateDecl {{.*}}  {{.*}} Foo
+  // CHECK:  FunctionTemplateDecl {{.*}}  {{.*}} Foo
   template 
   Foo(R, int) requires unary_concept;
 
-  // CHECK:  FunctionTemplateDecl {{.*}}  {{.*}} Foo
+  // CHECK:  FunctionTemplateDecl {{.*}}  {{.*}} Foo
   template 
   Foo(R, char) requires unary_concept {
   }
Index: clang/lib/AST/TextNodeDumper.cpp
===
--- clang/lib/AST/TextNodeDumper.cpp
+++ clang/lib/AST/TextNodeDumper.cpp
@@ -2000,7 +2000,6 @@
   dumpBareDeclRef(TC->getFoundDecl());
   OS << ")";
 }
-AddChild([=] { Visit(TC->getImmediatelyDeclaredConstraint()); });
   } else if (D->wasDeclaredWithTypename())
 OS << " typen

[PATCH] D85272: [clangd] Semantic highlighting for dependent template name in template argument

2020-08-05 Thread Haojian Wu via Phabricator via cfe-commits
hokein accepted this revision.
hokein added inline comments.
This revision is now accepted and ready to land.



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:305
+  break;
+default:;
+}

nit: move the trailing `;` to a new line.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85272

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


[PATCH] D85285: [clangd] WIP experimentation with finding static grpc++ libraries

2020-08-05 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev created this revision.
Herald added subscribers: llvm-commits, cfe-commits, usaxena95, kadircet, 
arphaman, jkorous, mgorny.
Herald added projects: clang, LLVM.
kbobyrev requested review of this revision.
Herald added subscribers: MaskRay, ilya-biryukov.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85285

Files:
  clang-tools-extra/clangd/index/remote/server/CMakeLists.txt
  llvm/cmake/modules/FindGRPC.cmake


Index: llvm/cmake/modules/FindGRPC.cmake
===
--- llvm/cmake/modules/FindGRPC.cmake
+++ llvm/cmake/modules/FindGRPC.cmake
@@ -26,9 +26,9 @@
   if (GRPC_CPP_PLUGIN-NOTFOUND OR PROTOC-NOTFOUND)
 message(FATAL_ERROR "gRPC C++ Plugin and Protoc must be on $PATH for 
Clangd remote index build")
   endif()
-  # On macOS the libraries are typically installed via Homebrew and are not on
-  # the system path.
   if (${APPLE})
+# On macOS the libraries are typically installed via Homebrew and are not 
on
+# the system path.
 find_program(HOMEBREW brew)
 # If Homebrew is not found, the user might have installed libraries
 # manually. Fall back to the system path.
@@ -66,6 +66,51 @@
   IMPORTED_LOCATION ${PROTOBUF_LIBRARY})
   endif()
 endif()
+  elseif(${UNIX} AND NOT BUILD_SHARED_LIBS)
+# Use dpkg on Debian-like Linux distributions to find static libraries.
+find_program(DPKG dpkg)
+if (NOT DPKG-NOTFOUND)
+  execute_process(COMMAND ${DPKG} -L libgrpc++-dev
+OUTPUT_VARIABLE GRPC_DPKG_PATHS
+RESULT_VARIABLE GRPC_DPKG_RETURN_CODE
+OUTPUT_STRIP_TRAILING_WHITESPACE)
+  if (GRPC_DPKG_RETURN_CODE EQUAL "0")
+# Split the output into CMake list.
+string(REPLACE "\n" ";" GRPC_DPKG_LINES ${GRPC_DPKG_PATHS})
+foreach(GRPC_PATH ${GRPC_DPKG_LINES})
+  string(FIND ${GRPC_PATH} "libgrpc++.a" LIBGRPCPP_SUBSTR_IDX)
+  if (NOT LIBGRPCPP_SUBSTR_IDX EQUAL "-1")
+add_library(grpc++ STATIC IMPORTED GLOBAL)
+set_target_properties(grpc++ PROPERTIES
+  IMPORTED_LOCATION ${GRPC_PATH})
+message("grpc++ lib ${GRPC_PATH}")
+  endif()
+  string(FIND ${GRPC_PATH} "libgrpc++_unsecure.a" 
LIBGRRPC_UNSECURE_SUBSTR_IDX)
+  if (NOT LIBGRRPC_UNSECURE_SUBSTR_IDX EQUAL "-1")
+add_library(libgrpc++_unsecure STATIC IMPORTED GLOBAL)
+set_target_properties(libgrpc++_unsecure PROPERTIES
+  IMPORTED_LOCATION ${GRPC_PATH})
+message("grpc+++_unsecure lib ${GRPC_PATH}")
+  endif()
+endforeach()
+  endif()
+  execute_process(COMMAND ${DPKG} -L libprotobuf-dev
+OUTPUT_VARIABLE PROTOBUF_DPKG_PATHS
+RESULT_VARIABLE PROTOBUF_DPKG_RETURN_CODE
+OUTPUT_STRIP_TRAILING_WHITESPACE)
+  if (PROTOBUF_DPKG_RETURN_CODE EQUAL "0")
+# Split the output into CMake list.
+string(REPLACE "\n" ";" PROTOBUF_DPKG_LINES ${PROTOBUF_DPKG_PATHS})
+foreach(PROTOBUF_PATH ${PROTOBUF_DPKG_LINES})
+  string(FIND ${PROTOBUF_PATH} "libprotobuf.a" LIBPROTOBUF_SUBSTR_IDX)
+  if (NOT LIBPROTOBUF_SUBSTR_IDX EQUAL "-1")
+add_library(protobuf STATIC IMPORTED GLOBAL)
+set_target_properties(protobuf PROPERTIES
+  IMPORTED_LOCATION ${PROTOBUF_PATH})
+  endif()
+endforeach()
+  endif()
+endif()
   endif()
 endif()
 
Index: clang-tools-extra/clangd/index/remote/server/CMakeLists.txt
===
--- clang-tools-extra/clangd/index/remote/server/CMakeLists.txt
+++ clang-tools-extra/clangd/index/remote/server/CMakeLists.txt
@@ -15,5 +15,8 @@
   RemoteIndexProtos
   clangdRemoteMarshalling
 
-  grpc++
+  /usr/local/google/home/kbobyrev/software/grpc-latest/lib/libgrpc++.a
+  /usr/local/google/home/kbobyrev/software/grpc-latest/lib/libgrpc.a
+  /usr/local/google/home/kbobyrev/software/grpc-latest/lib/libgrpc++_unsecure.a
+  /usr/local/google/home/kbobyrev/software/grpc-latest/lib/libgrpc_unsecure.a
   )


Index: llvm/cmake/modules/FindGRPC.cmake
===
--- llvm/cmake/modules/FindGRPC.cmake
+++ llvm/cmake/modules/FindGRPC.cmake
@@ -26,9 +26,9 @@
   if (GRPC_CPP_PLUGIN-NOTFOUND OR PROTOC-NOTFOUND)
 message(FATAL_ERROR "gRPC C++ Plugin and Protoc must be on $PATH for Clangd remote index build")
   endif()
-  # On macOS the libraries are typically installed via Homebrew and are not on
-  # the system path.
   if (${APPLE})
+# On macOS the libraries are typically installed via Homebrew and are not on
+# the system path.
 find_program(HOMEBREW brew)
 # If Homebrew is not found, the user might have installed libraries
 # manually. Fall back to the system path.
@@ -66,6 +66,51 @@
   IMPORTED_LO

[PATCH] D85128: [Prototype][SVE] Support arm_sve_vector_bits attribute

2020-08-05 Thread Cullen Rhodes via Phabricator via cfe-commits
c-rhodes added a comment.

In D85128#2193309 , @tschuett wrote:

> Sorry. I meant ABI. Can link GCC .o files with Clang .o files using the 
> attributes?

Yes they should be compatible. The machine-level ABI distinguishes 4 types of 
SVE vector [1]:

- VG×64-bit vector of 8-bit elements
- VG×64-bit vector of 16-bit elements
- VG×64-bit vector of 32-bit elements
- VG×64-bit vector of 64-bit elements

The VLS and VLA types are defined by the ACLE to map to the same machine-level 
SVE vectors (in both compilers). Hence the mangling changes in this patch.

[1] 
https://github.com/ARM-software/abi-aa/blob/master/aapcs64/aapcs64.rst#fundamental-data-types


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85128

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


[PATCH] D85287: Extend -Wtautological-bitwise-compare "bitwise or with non-zero value" warnings

2020-08-05 Thread Stephan Bergmann via Phabricator via cfe-commits
sberg created this revision.
sberg added reviewers: jordan_rose, rtrieu.
Herald added a project: clang.
sberg requested review of this revision.

...to C++ constant expression operands.

(I had been puzzled why Clang had not found  "cid#1465512 Wrong operator used"
in the LibreOffice source code, only found by Coverity Scan.  At least building
LibreOffice with this change caused no false positives.)

In C, values of const-qualified variables are never constant expressions, so
nothing should change for C code.

To avoid potential further false positives, restrict this change only to the
"bitwise or with non-zero value" warnings while keeping all other
-Wtautological-bitwise-compare warnings as-is, at least for now.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85287

Files:
  clang/lib/Analysis/CFG.cpp
  clang/test/Sema/warn-bitwise-compare.c
  clang/test/SemaCXX/warn-bitwise-compare.cpp

Index: clang/test/SemaCXX/warn-bitwise-compare.cpp
===
--- clang/test/SemaCXX/warn-bitwise-compare.cpp
+++ clang/test/SemaCXX/warn-bitwise-compare.cpp
@@ -1,6 +1,12 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -Wtautological-bitwise-compare %s
 // RUN: %clang_cc1 -fsyntax-only -verify -Wall -Wno-unused %s
 
+int f() { return 3; }
+
+const int const1 = 1;
+constexpr int const2 = 2;
+const int const3 = f();
+
 void test(int x) {
   bool b1 = (8 & x) == 3;
   // expected-warning@-1 {{bitwise comparison always evaluates to false}}
@@ -10,4 +16,15 @@
   // expected-warning@-1 {{bitwise or with non-zero value always evaluates to true}}
   bool b4 = !!(x | 5);
   // expected-warning@-1 {{bitwise or with non-zero value always evaluates to true}}
+  bool b5 = (x | const1);
+  // expected-warning@-1 {{bitwise or with non-zero value always evaluates to true}}
+  bool b6 = (x | const2);
+  // expected-warning@-1 {{bitwise or with non-zero value always evaluates to true}}
+  bool b7 = (x | const3);
+
+  // No warnings at least for now:
+  bool b8 = (x & const1) == 8;
+  bool b9 = (x | const1) == 4;
+  bool b10 = (x & const2) == 8;
+  bool b11 = (x | const2) == 4;
 }
Index: clang/test/Sema/warn-bitwise-compare.c
===
--- clang/test/Sema/warn-bitwise-compare.c
+++ clang/test/Sema/warn-bitwise-compare.c
@@ -8,6 +8,8 @@
   ONE,
 };
 
+const int const1 = 1;
+
 void f(int x) {
   if ((8 & x) == 3) {}  // expected-warning {{bitwise comparison always evaluates to false}}
   if ((x & 8) == 4) {}  // expected-warning {{bitwise comparison always evaluates to false}}
@@ -34,6 +36,9 @@
 
   if ((x & mydefine) == 8) {}
   if ((x | mydefine) == 4) {}
+
+  if ((x & const1) == 8) {}
+  if ((x | const1) == 4) {}
 }
 
 void g(int x) {
@@ -45,4 +50,6 @@
   if (x | ONE) {}  // expected-warning {{bitwise or with non-zero value always evaluates to true}}
 
   if (x | ZERO) {}
+
+  if (x | const1) {}
 }
Index: clang/lib/Analysis/CFG.cpp
===
--- clang/lib/Analysis/CFG.cpp
+++ clang/lib/Analysis/CFG.cpp
@@ -93,15 +93,22 @@
   return isa(E);
 }
 
-/// Helper for tryNormalizeBinaryOperator. Attempts to extract an IntegerLiteral
-/// constant expression or EnumConstantDecl from the given Expr. If it fails,
+/// Helper for tryNormalizeBinaryOperator. Attempts to extract a suitable
+/// integer or enum constant expression from the given Expr. If it fails,
 /// returns nullptr.
-static const Expr *tryTransformToIntOrEnumConstant(const Expr *E) {
+static const Expr *tryTransformToIntOrEnumConstant(const Expr *E,
+   ASTContext &Ctx) {
   E = E->IgnoreParens();
   if (IsIntegerLiteralConstantExpr(E))
 return E;
-  if (auto *DR = dyn_cast(E->IgnoreParenImpCasts()))
-return isa(DR->getDecl()) ? DR : nullptr;
+  if (auto *DR = dyn_cast(E->IgnoreParenImpCasts())) {
+auto *D = DR->getDecl();
+if (isa(D))
+  return DR;
+if (auto *VD = dyn_cast(D))
+  if (VD->isUsableInConstantExpressions(Ctx))
+return DR;
+  }
   return nullptr;
 }
 
@@ -111,11 +118,11 @@
 /// If this fails, at least one of the returned DeclRefExpr or Expr will be
 /// null.
 static std::tuple
-tryNormalizeBinaryOperator(const BinaryOperator *B) {
+tryNormalizeBinaryOperator(const BinaryOperator *B, ASTContext &Ctx) {
   BinaryOperatorKind Op = B->getOpcode();
 
   const Expr *MaybeDecl = B->getLHS();
-  const Expr *Constant = tryTransformToIntOrEnumConstant(B->getRHS());
+  const Expr *Constant = tryTransformToIntOrEnumConstant(B->getRHS(), Ctx);
   // Expr looked like `0 == Foo` instead of `Foo == 0`
   if (Constant == nullptr) {
 // Flip the operator
@@ -129,7 +136,7 @@
   Op = BO_GE;
 
 MaybeDecl = B->getRHS();
-Constant = tryTransformToIntOrEnumConstant(B->getLHS());
+Constant = tryTransformToIntOrEnumConstant(B

[PATCH] D80791: [AArch64] Generate .note.gnu.property based on module flags.

2020-08-05 Thread Daniel Kiss via Phabricator via cfe-commits
danielkiss updated this revision to Diff 283172.
danielkiss edited the summary of this revision.
danielkiss added a comment.

This version of the patch behaves as `gcc` for case when no function present 
and when function has `-mbranch-protection` attribute without compiler flag.
The logic should be in clang because in llvm we won't have enough information 
to handle these things. (see D75181 )


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

https://reviews.llvm.org/D80791

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp
  llvm/test/CodeGen/AArch64/note-gnu-property-pac-bti-0.ll
  llvm/test/CodeGen/AArch64/note-gnu-property-pac-bti-1.ll
  llvm/test/CodeGen/AArch64/note-gnu-property-pac-bti-2.ll
  llvm/test/CodeGen/AArch64/note-gnu-property-pac-bti-3.ll
  llvm/test/CodeGen/AArch64/note-gnu-property-pac-bti-4.ll
  llvm/test/CodeGen/AArch64/note-gnu-property-pac-bti-5.ll
  llvm/test/CodeGen/AArch64/note-gnu-property-pac-bti-6.ll
  llvm/test/CodeGen/AArch64/note-gnu-property-pac-bti-7.ll
  llvm/test/CodeGen/AArch64/note-gnu-property-pac-bti-8.ll

Index: llvm/test/CodeGen/AArch64/note-gnu-property-pac-bti-8.ll
===
--- llvm/test/CodeGen/AArch64/note-gnu-property-pac-bti-8.ll
+++ /dev/null
@@ -1,21 +0,0 @@
-; RUN: llc -mtriple=aarch64-linux %s   -o - | \
-; RUN:   FileCheck %s --check-prefix=ASM
-; RUN: llc -mtriple=aarch64-linux %s -filetype=obj -o - | \
-; RUN:   llvm-readelf --notes - | FileCheck %s --check-prefix=OBJ
-
-define dso_local i32 @f() #0 {
-entry:
-  %r = tail call i32 @g()
-  ret i32 %r
-}
-
-declare dso_local i32 @g()
-
-attributes #0 = { "branch-target-enforcement" }
-
-; Declarations don't prevent setting BTI
-; ASM:	.word	3221225472
-; ASM-NEXT:	.word	4
-; ASM-NEXT:	.word	1
-
-; OBJ: Properties: aarch64 feature: BTI
Index: llvm/test/CodeGen/AArch64/note-gnu-property-pac-bti-7.ll
===
--- llvm/test/CodeGen/AArch64/note-gnu-property-pac-bti-7.ll
+++ /dev/null
@@ -1,23 +0,0 @@
-; RUN: llc -mtriple=aarch64-linux %s   -o - 2>&1 | \
-; RUN:   FileCheck %s --check-prefix=ASM
-; RUN: llc -mtriple=aarch64-linux %s -filetype=obj -o -  |  \
-; RUN:   llvm-readelf -S - | FileCheck %s --check-prefix=OBJ
-
-define dso_local i32 @f() #0 {
-entry:
-  ret i32 0
-}
-
-define dso_local i32 @g() #1 {
-entry:
-  ret i32 0
-}
-
-attributes #0 = { "sign-return-address"="non-leaf" }
-
-attributes #1 = { "branch-target-enforcement" }
-
-; No common attribute, no note section
-; ASM: warning: not setting BTI in feature flags
-; ASM-NOT: .note.gnu.property
-; OBJ-NOT: .note.gnu.property
Index: llvm/test/CodeGen/AArch64/note-gnu-property-pac-bti-6.ll
===
--- llvm/test/CodeGen/AArch64/note-gnu-property-pac-bti-6.ll
+++ /dev/null
@@ -1,22 +0,0 @@
-; RUN: llc -mtriple=aarch64-linux %s   -o - | \
-; RUN:   FileCheck %s --check-prefix=ASM
-; RUN: llc -mtriple=aarch64-linux %s -filetype=obj -o - |  \
-; RUN:   llvm-readelf -S - | FileCheck %s --check-prefix=OBJ
-
-define dso_local i32 @f() #0 {
-entry:
-  ret i32 0
-}
-
-define dso_local i32 @g() #1 {
-entry:
-  ret i32 0
-}
-
-attributes #0 = { "sign-return-address"="non-leaf" }
-
-attributes #1 = { "sign-return-address"="none" }
-
-; No common attribute, no note section
-; ASM-NOT: .note.gnu.property
-; OBJ-NOT: .note.gnu.property
Index: llvm/test/CodeGen/AArch64/note-gnu-property-pac-bti-5.ll
===
--- llvm/test/CodeGen/AArch64/note-gnu-property-pac-bti-5.ll
+++ /dev/null
@@ -1,26 +0,0 @@
-; RUN: llc -mtriple=aarch64-linux %s   -o - 2>&1 | \
-; RUN:   FileCheck %s --check-prefix=ASM
-; RUN: llc -mtriple=aarch64-linux %s -filetype=obj -o -  |  \
-; RUN:   llvm-readelf --notes - | FileCheck %s --check-prefix=OBJ
-
-define dso_local i32 @f() #0 {
-entry:
-  ret i32 0
-}
-
-define dso_local i32 @g() #1 {
-entry:
-  ret i32 0
-}
-
-attributes #0 = { "branch-target-enforcement" "sign-return-address"="non-leaf" }
-
-attributes #1 = { "sign-return-address"="all" }
-
-; Only the common atttribute (PAC)
-; ASM: warning: not setting BTI in feature flags
-; ASM:	.word	3221225472
-; ASM-NEXT:	.word	4
-; ASM-NEXT:	.word	2
-
-; OBJ: Properties: aarch64 feature: PAC
Index: llvm/test/CodeGen/AArch64/note-gnu-property-pac-bti-4.ll
===
--- llvm/test/CodeGen/AArch64/note-gnu-property-pac-bti-4.ll
+++ llvm/test/CodeGen/AArch64/note-gnu-property-pac-bti-4.ll
@@ -1,25 +1,12 @@
 ; RUN: llc -mtriple=aarch64-linux %s   -o - | \
 ; RUN:   FileCheck %s --check-prefix=ASM
-; RUN: llc -mtriple=aarch64-linux %s -filetype=obj -o - |  \
-; RUN:   llvm-readelf --notes - | FileCheck %s --chec

[PATCH] D85253: [clangd] Show correct hover tooltip for non-preamble macro definition.

2020-08-05 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment.

Sigh, (I think) this is working for macros defined in preamble region as a side 
effect of preamble being loaded separately and before anything in the main 
file. E.g.

  #define FO^O 1
  #define FOO 2
  
  void foo() { int x = FOO; };

hovering over the first definition of FOO (at carrot) will yield `#define FOO 
2`, because that's the latest definition before `main file`.

That being said, I don't really think there's much value in showing the 
hovercard at definition of a macro, as that's what we show anyways. (showing 
the surrounding namespace is even worse, but that's a different problem and i 
thought we've solved it recently, are you sure you are seeing that behaviour 
with a build from head?).
So I believe you are rather trying to fix the issue of displaying misleading 
hover info, and if that's the case I think we should rather fix that by not 
displaying hover infos at the macro definition locations.

WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85253

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


[clang] 3ab0155 - Revert "[CMake] Simplify CMake handling for zlib"

2020-08-05 Thread Hans Wennborg via cfe-commits

Author: Hans Wennborg
Date: 2020-08-05T12:31:44+02:00
New Revision: 3ab01550b632dad46f9595d74855749557ffd25c

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

LOG: Revert "[CMake] Simplify CMake handling for zlib"

This quietly disabled use of zlib on Windows even when building with
-DLLVM_ENABLE_ZLIB=FORCE_ON.

> Rather than handling zlib handling manually, use find_package from CMake
> to find zlib properly. Use this to normalize the LLVM_ENABLE_ZLIB,
> HAVE_ZLIB, HAVE_ZLIB_H. Furthermore, require zlib if LLVM_ENABLE_ZLIB is
> set to YES, which requires the distributor to explicitly select whether
> zlib is enabled or not. This simplifies the CMake handling and usage in
> the rest of the tooling.
>
> This is a reland of abb0075 with all followup changes and fixes that
> should address issues that were reported in PR44780.
>
> Differential Revision: https://reviews.llvm.org/D79219

This reverts commit 10b1b4a231a485f1711d576e6131f6755e008abe and follow-ups
64d99cc6abed78c00a2a7863b02ce54911a5264f and
f9fec0447e12da9e8cf4b628f6d45f4941e7d182.

Added: 


Modified: 
clang/test/CMakeLists.txt
clang/test/lit.site.cfg.py.in
compiler-rt/test/lit.common.configured.in
lld/test/CMakeLists.txt
lld/test/lit.site.cfg.py.in
lldb/cmake/modules/LLDBStandalone.cmake
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
llvm/cmake/config-ix.cmake
llvm/cmake/modules/LLVMConfig.cmake.in
llvm/include/llvm/Config/config.h.cmake
llvm/lib/Support/CMakeLists.txt
llvm/lib/Support/CRC.cpp
llvm/lib/Support/Compression.cpp
llvm/test/CMakeLists.txt
llvm/test/lit.site.cfg.py.in
llvm/unittests/Support/CompressionTest.cpp
mlir/examples/standalone/CMakeLists.txt

Removed: 




diff  --git a/clang/test/CMakeLists.txt b/clang/test/CMakeLists.txt
index 334a90498d0d..38bbc5be90d5 100644
--- a/clang/test/CMakeLists.txt
+++ b/clang/test/CMakeLists.txt
@@ -9,6 +9,15 @@ endif ()
 
 string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} CLANG_TOOLS_DIR 
${LLVM_RUNTIME_OUTPUT_INTDIR})
 
+if(CLANG_BUILT_STANDALONE)
+  # Set HAVE_LIBZ according to recorded LLVM_ENABLE_ZLIB value. This
+  # value is forced to 0 if zlib was not found, so it is fine to use it
+  # instead of HAVE_LIBZ (not recorded).
+  if(LLVM_ENABLE_ZLIB)
+set(HAVE_LIBZ 1)
+  endif()
+endif()
+
 llvm_canonicalize_cmake_booleans(
   CLANG_BUILD_EXAMPLES
   CLANG_ENABLE_ARCMT
@@ -16,7 +25,7 @@ llvm_canonicalize_cmake_booleans(
   CLANG_SPAWN_CC1
   ENABLE_BACKTRACES
   ENABLE_EXPERIMENTAL_NEW_PASS_MANAGER
-  LLVM_ENABLE_ZLIB
+  HAVE_LIBZ
   LLVM_ENABLE_PER_TARGET_RUNTIME_DIR
   LLVM_ENABLE_PLUGINS
   LLVM_ENABLE_THREADS)

diff  --git a/clang/test/lit.site.cfg.py.in b/clang/test/lit.site.cfg.py.in
index 286ea06d798c..d9b5b2f2592e 100644
--- a/clang/test/lit.site.cfg.py.in
+++ b/clang/test/lit.site.cfg.py.in
@@ -16,7 +16,7 @@ config.host_triple = "@LLVM_HOST_TRIPLE@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.host_cxx = "@CMAKE_CXX_COMPILER@"
 config.llvm_use_sanitizer = "@LLVM_USE_SANITIZER@"
-config.have_zlib = @LLVM_ENABLE_ZLIB@
+config.have_zlib = @HAVE_LIBZ@
 config.clang_arcmt = @CLANG_ENABLE_ARCMT@
 config.clang_default_cxx_stdlib = "@CLANG_DEFAULT_CXX_STDLIB@"
 config.clang_staticanalyzer = @CLANG_ENABLE_STATIC_ANALYZER@

diff  --git a/compiler-rt/test/lit.common.configured.in 
b/compiler-rt/test/lit.common.configured.in
index 000bf9b98470..1f746c067b84 100644
--- a/compiler-rt/test/lit.common.configured.in
+++ b/compiler-rt/test/lit.common.configured.in
@@ -57,7 +57,7 @@ elif config.android:
 else:
   set_default("target_suffix", "-%s" % config.target_arch)
 
-set_default("have_zlib", "@LLVM_ENABLE_ZLIB@")
+set_default("have_zlib", "@HAVE_LIBZ@")
 set_default("libcxx_used", "@LLVM_LIBCXX_USED@")
 
 # LLVM tools dir can be passed in lit parameters, so try to

diff  --git a/lld/test/CMakeLists.txt b/lld/test/CMakeLists.txt
index 52e6118ba876..74b29f5d65b8 100644
--- a/lld/test/CMakeLists.txt
+++ b/lld/test/CMakeLists.txt
@@ -4,8 +4,17 @@ set(LLVM_BUILD_MODE "%(build_mode)s")
 set(LLVM_TOOLS_DIR "${LLVM_TOOLS_BINARY_DIR}/%(build_config)s")
 set(LLVM_LIBS_DIR 
"${LLVM_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX}/%(build_config)s")
 
+if(LLD_BUILT_STANDALONE)
+  # Set HAVE_LIBZ according to recorded LLVM_ENABLE_ZLIB value. This
+  # value is forced to 0 if zlib was not found, so it is fine to use it
+  # instead of HAVE_LIBZ (not recorded).
+  if(LLVM_ENABLE_ZLIB)
+set(HAVE_LIBZ 1)
+  endif()
+endif()
+
 llvm_canonicalize_cmake_booleans(
-  LLVM_ENABLE_ZLIB
+  HAVE_LIBZ
   LLVM_LIBXML2_ENABLED
   )
 

diff  --git a/lld/test/lit.site.cfg.py.in b/lld/test/lit.site.cfg.py.in
index 3d4c51f4ab64..4aa2fcda7

[PATCH] D84736: [analyzer][RFC] Handle pointer difference of ElementRegion and SymbolicRegion

2020-08-05 Thread Balázs Benics via Phabricator via cfe-commits
steakhal planned changes to this revision.
steakhal marked an inline comment as done.
steakhal added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:1035-1038
+return evalBinOpNN(state, BO_Mul, Index,
+   makeArrayIndex(SingleElementSize.getQuantity()),
+   ArrayIndexTy)
+.castAs();

vsavchenko wrote:
> I would prefer having a comment for this line with a very simple formula of 
> the thing we are calculating.  I think it can increase the readability of the 
> code because when there is a call accepting a whole bunch of arguments it is 
> never obvious and it always takes time to parse through.
Sure, I will add something like this:
```
// T buf[n];   Element{buf,2} --> 2 * sizeof(T)
```



Comment at: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:1041
+
+  // If both has the same memory region, and the index has a concrete 
value,
+  // we can evaluate equality operators.

vsavchenko wrote:
> This comment is a bit deceiving IMO.  It returns a concrete value even when 
> `HasSameMemRegions` is false, but from the comment it seems like we evaluate 
> the operator ONLY when `HasSameMemRegions` is true.
No, the comment is up to date.
This lambda handles both equality and inequality operators, and this is why its 
called EvaluateEqualityOperators.

EQResult will be true only if the two memory regions are the same and the index 
is zero.
The result of the **inequality**  is just the negated version of the result of 
equality.



Comment at: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:1068
+}
+  } else if (LeftER && !RightER) {
+NonLoc LeftIndex = ByteOffsetOfElement(LeftER);

vsavchenko wrote:
> I think it's better to add comments for each case describing what is the 
> situation we handle here in a simplified form.
> And for each return within the case - a short comment with the reason.
I'm not sure if it's necessary.
I have already described these 3 cases at the beginning.
Quote:
```
// Handle: \forall MemRegion X, \forall NonLoc n, m:
//  - Element{X,n} OP Element{X,m}
//  - Element{X,n} OP X
//  -X OP Element{X,n}
// We don't handle here nested ElementRegions like in the this expression:
// Element{Element{x,3,int [10]},5,int}  ==  Element{x,35,int}
```
If you still insist, I will defenately add more comments though.


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

https://reviews.llvm.org/D84736

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


[PATCH] D82756: Port some floating point options to new option marshalling infrastructure

2020-08-05 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia added inline comments.



Comment at: clang/test/CodeGen/fp-function-attrs.cpp:2
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -ffast-math -ffinite-math-only 
-menable-unsafe-fp-math \
+// RUN:   -menable-no-infs -menable-no-nans -fno-signed-zeros 
-freciprocal-math \
+// RUN:   -fapprox-func -mreassociate -ffp-contract=fast -emit-llvm -o - %s | 
FileCheck %s

dang wrote:
> Anastasia wrote:
> > dang wrote:
> > > Anastasia wrote:
> > > > Not clear why do you need to pass these extra flags now?
> > > Previously passing -ffast-math to CC1 implied all these other flags. I am 
> > > trying to make CC1 option parsing as simple as possible, so that we can 
> > > then make it easy to generate a command line from a CompilerInvocation 
> > > instance. You can refer to [[ 
> > > http://lists.llvm.org/pipermail/cfe-dev/2020-May/065421.html | 
> > > http://lists.llvm.org/pipermail/cfe-dev/2020-May/065421.html ]] for more 
> > > details on why we want to be able to do this
> > Just to understand, there used to be implied flags and it made the manual 
> > command line use of clang more compact and easy... Is the idea now to 
> > change those compound flags such that individul flags always need to be 
> > passed?
> > 
> > Although I thought you are still adding the implicit flags:
> > 
> >   {options::OPT_cl_fast_relaxed_math,
> >[&](const Arg *Arg) {
> >  RenderArg(Arg);
> > 
> >  CmdArgs.push_back(GetArgString(options::OPT_cl_mad_enable));
> >  CmdArgs.push_back(GetArgString(options::OPT_ffast_math));
> >  
> > CmdArgs.push_back(GetArgString(options::OPT_ffinite_math_only));
> >  CmdArgs.push_back(
> >  GetArgString(options::OPT_menable_unsafe_fp_math));
> >  CmdArgs.push_back(GetArgString(options::OPT_mreassociate));
> >  CmdArgs.push_back(GetArgString(options::OPT_menable_no_nans));
> >  CmdArgs.push_back(
> >  GetArgString(options::OPT_menable_no_infinities));
> >  CmdArgs.push_back(GetArgString(options::OPT_fno_signed_zeros));
> >  CmdArgs.push_back(GetArgString(options::OPT_freciprocal_math));
> >  CmdArgs.push_back(GetArgString(options::OPT_fapprox_func));
> >}}
> > 
> > Do I just misunderstand something?
> The command line of the driver doesn't change. This patch only affects what 
> CC1 understands, now CC1 doesn't know anymore that `-cl-fast-relaxed-math` 
> implies all these other options so the driver is responsible for specifying 
> them when it constructs the CC1 command line.
> 
> To summarize, the clang driver command line isn't affected by this patch and 
> it shouldn't be so let me know if something is wrong there. However, manually 
> constructed `clang -cc1` invocations need to specify the all the implied 
> flags manually now.
Yes I understand, however, I am wondering whether this is intuitive because it 
seems the behavior of clang with `-cc1` and without will be different if the 
same `-cl-fast-relaxed-math` flag is passed.

I also find adding all the flags manually is too verbode if 
`-cl-fast-relaxed-math` assumes to enable all the extra setting.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82756

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


[clang-tools-extra] a441616 - Support member expressions in bugprone-bool-pointer-implicit-conversion.

2020-08-05 Thread Aaron Ballman via cfe-commits

Author: Alex Cameron
Date: 2020-08-05T07:14:28-04:00
New Revision: a44161692ae879068d4086a7e568a348800ba01d

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

LOG: Support member expressions in bugprone-bool-pointer-implicit-conversion.

This addresses PR45189.

Added: 


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

clang-tools-extra/test/clang-tidy/checkers/bugprone-bool-pointer-implicit-conversion.cpp

Removed: 




diff  --git 
a/clang-tools-extra/clang-tidy/bugprone/BoolPointerImplicitConversionCheck.cpp 
b/clang-tools-extra/clang-tidy/bugprone/BoolPointerImplicitConversionCheck.cpp
index b764bdbf7c4c..17dab1b0f73e 100644
--- 
a/clang-tools-extra/clang-tidy/bugprone/BoolPointerImplicitConversionCheck.cpp
+++ 
b/clang-tools-extra/clang-tidy/bugprone/BoolPointerImplicitConversionCheck.cpp
@@ -20,53 +20,68 @@ void 
BoolPointerImplicitConversionCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(
   traverse(
   ast_type_traits::TK_AsIs,
-  ifStmt(hasCondition(findAll(implicitCastExpr(
- unless(hasParent(unaryOperator(hasOperatorName("!",
- hasSourceExpression(expr(
- hasType(pointerType(pointee(booleanType(,
- ignoringParenImpCasts(declRefExpr().bind("expr",
- hasCastKind(CK_PointerToBoolean,
- unless(isInTemplateInstantiation()))
+  ifStmt(
+  hasCondition(findAll(implicitCastExpr(
+  unless(hasParent(unaryOperator(hasOperatorName("!",
+  hasSourceExpression(expr(
+  hasType(pointerType(pointee(booleanType(,
+  ignoringParenImpCasts(anyOf(declRefExpr().bind("expr"),
+  
memberExpr().bind("expr"),
+  hasCastKind(CK_PointerToBoolean,
+  unless(isInTemplateInstantiation()))
   .bind("if")),
   this);
 }
 
-void BoolPointerImplicitConversionCheck::check(
-const MatchFinder::MatchResult &Result) {
-  auto *If = Result.Nodes.getNodeAs("if");
-  auto *Var = Result.Nodes.getNodeAs("expr");
-
+static void checkImpl(const MatchFinder::MatchResult &Result, const Expr *Ref,
+  const IfStmt *If,
+  const ast_matchers::internal::Matcher &RefMatcher,
+  ClangTidyCheck &Check) {
   // Ignore macros.
-  if (Var->getBeginLoc().isMacroID())
+  if (Ref->getBeginLoc().isMacroID())
 return;
 
-  // Only allow variable accesses for now, no function calls or member exprs.
+  // Only allow variable accesses and member exprs for now, no function calls.
   // Check that we don't dereference the variable anywhere within the if. This
   // avoids false positives for checks of the pointer for nullptr before it is
   // dereferenced. If there is a dereferencing operator on this variable don't
   // emit a diagnostic. Also ignore array subscripts.
-  const Decl *D = Var->getDecl();
-  auto DeclRef = ignoringParenImpCasts(declRefExpr(to(equalsNode(D;
-  if (!match(findAll(
- unaryOperator(hasOperatorName("*"), 
hasUnaryOperand(DeclRef))),
+  if (!match(findAll(unaryOperator(hasOperatorName("*"),
+   hasUnaryOperand(RefMatcher))),
  *If, *Result.Context)
.empty() ||
-  !match(findAll(arraySubscriptExpr(hasBase(DeclRef))), *If,
+  !match(findAll(arraySubscriptExpr(hasBase(RefMatcher))), *If,
  *Result.Context)
.empty() ||
   // FIXME: We should still warn if the paremater is implicitly converted 
to
   // bool.
-  !match(findAll(callExpr(hasAnyArgument(ignoringParenImpCasts(DeclRef,
- *If, *Result.Context)
+  !match(
+   
findAll(callExpr(hasAnyArgument(ignoringParenImpCasts(RefMatcher,
+   *If, *Result.Context)
.empty() ||
-  !match(findAll(cxxDeleteExpr(has(ignoringParenImpCasts(expr(DeclRef),
- *If, *Result.Context)
+  !match(
+   
findAll(cxxDeleteExpr(has(ignoringParenImpCasts(expr(RefMatcher),
+   *If, *Result.Context)
.empty())
 return;
 
-  diag(Var->getBeginLoc(), "dubious check of 'bool *' against 'nullptr', did "
-   "you mean to dereference it?")
-  << FixItHint::CreateInsertion(Var->getBeginLoc(), "*");
+  Check.diag(Ref->getBeginLoc(),
+ "dubious check of 'bool *' against 'nullptr', did "
+ "you mean to dereference it?")
+  << FixItHint::CreateInsertion(Ref->getBeginLoc(), "*");
+}
+
+void BoolPointerImplicitConversionCheck::check(

[PATCH] D85191: [AST] Get field size in chars rather than bits in RecordLayoutBuilder.

2020-08-05 Thread Bevin Hansson via Phabricator via cfe-commits
ebevhan added a comment.

In D85191#2193645 , @rsmith wrote:

>> This is not ideal, since it makes the calculations char size dependent, and 
>> breaks for sizes that are not a multiple of the char size.
>
> How can we have a non-bitfield member whose size is not a multiple of the 
> size of a char?

Downstream, we have fixed-point types that are 24 bits large, but where the 
char size is 16. The type then takes up 2 chars, where 8 of the bits are 
padding. The only way in Clang to express that the width of the bit 
representation of a type should be smaller than the number of chars it takes up 
in memory -- and consequently, produce an `i24` in IR -- is to return a 
non-charsize multiple from getTypeInfo.

We did it this way because it was possible. If the intent is for getTypeInfo to 
always return sizes that are multiples of the char size, then the design should 
be inverted and getTypeInfo should simply be calling getTypeInfoInChars and 
multiply that result by the char size. But that isn't how it works.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85191

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


[PATCH] D83188: [clang-tidy] bugprone-bool-pointer-implicit-conversion doesn't handle members

2020-08-05 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

Re-accepting the changes.


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

https://reviews.llvm.org/D83188

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


[PATCH] D83188: [clang-tidy] bugprone-bool-pointer-implicit-conversion doesn't handle members

2020-08-05 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman closed this revision.
aaron.ballman added a comment.

Thank you for the patch, I've gone ahead and commit on your behalf in 
a44161692ae879068d4086a7e568a348800ba01d 
.


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

https://reviews.llvm.org/D83188

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


[PATCH] D85191: [AST] Get field size in chars rather than bits in RecordLayoutBuilder.

2020-08-05 Thread Bevin Hansson via Phabricator via cfe-commits
ebevhan added inline comments.



Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1841
   auto setDeclInfo = [&](bool IsIncompleteArrayType) {
-TypeInfo TI = Context.getTypeInfo(D->getType());
-FieldAlign = Context.toCharUnitsFromBits(TI.Align);
+auto TI = Context.getTypeInfoInChars(D->getType());
+FieldAlign = TI.second;

ebevhan wrote:
> Xiangling_L wrote:
> > In most cases, `getTypeInfoInChars` invokes `getTypeInfo` underneath. So to 
> > make people be careful about this, I would suggest to leave a comment 
> > explaining/claiming we have to call `getTypeInfoInChars` here. And also 
> > maybe adding a testcase to guard the scenario you were talking about would 
> > be helpful to prevent someone to use `getTypeInfo` here in the future.
> I can do that.
> 
> I honestly don't think it would be a bad idea to add an assertion to 
> toCharUnitsFromBits that checks for non-bytesize-multiple amounts. I wonder 
> how much would fail if I did that, though.
Oh, I guess I only really replied to the first part about adding a comment 
here... I'm not sure I can make a test case for this, since I don't think 
there's anything that triggers this upstream.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85191

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


[PATCH] D85033: [clang] Provide a better pretty-printed name for unnamed parameters, lambda classes and lambda captures.

2020-08-05 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM aside from a minor nit.




Comment at: clang/lib/AST/Decl.cpp:2130
+  // the pretty-printed name of the capture instead.
+  if (isa(DD) &&
+  maybePrintFieldForLambdaCapture(OS, Policy, cast(DD)))

Rather than `isa<>` followed by a `cast<>`, how about:
```
if (const auto *FD = dyn_cast(DD)) {
  if (maybePrintFieldForLambdaCapture(...)
return;
}
```
Alternatively, you could sink the `dyn_cast<>` down into 
`maybePrintFieldForLambdaCapture()` since that already has to handle case where 
it's not printing a field capture.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85033

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


[PATCH] D85291: [clangd] Fix a crash in DefineInline

2020-08-05 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision.
kadircet added a reviewer: hokein.
Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous.
Herald added a project: clang.
kadircet requested review of this revision.
Herald added subscribers: MaskRay, ilya-biryukov.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85291

Files:
  clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
  clang-tools-extra/clangd/unittests/TweakTests.cpp


Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -1093,6 +1093,11 @@
 template<> void f^oo() {
   bar();
 })cpp");
+  EXPECT_UNAVAILABLE(R"cpp(
+namespace bar {
+  template  void f^oo() {}
+  template void foo();
+})cpp");
 }
 
 TEST_F(DefineInlineTest, CheckForCanonDecl) {
Index: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
@@ -339,7 +339,7 @@
 // specialization.
 const FunctionDecl *findTarget(const FunctionDecl *FD) {
   auto CanonDecl = FD->getCanonicalDecl();
-  if (!FD->isFunctionTemplateSpecialization())
+  if (!FD->isFunctionTemplateSpecialization() || CanonDecl == FD)
 return CanonDecl;
   // For specializations CanonicalDecl is the TemplatedDecl, which is not the
   // target we want to inline into. Instead we traverse previous decls to find


Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -1093,6 +1093,11 @@
 template<> void f^oo() {
   bar();
 })cpp");
+  EXPECT_UNAVAILABLE(R"cpp(
+namespace bar {
+  template  void f^oo() {}
+  template void foo();
+})cpp");
 }
 
 TEST_F(DefineInlineTest, CheckForCanonDecl) {
Index: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
@@ -339,7 +339,7 @@
 // specialization.
 const FunctionDecl *findTarget(const FunctionDecl *FD) {
   auto CanonDecl = FD->getCanonicalDecl();
-  if (!FD->isFunctionTemplateSpecialization())
+  if (!FD->isFunctionTemplateSpecialization() || CanonDecl == FD)
 return CanonDecl;
   // For specializations CanonicalDecl is the TemplatedDecl, which is not the
   // target we want to inline into. Instead we traverse previous decls to find
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D83717: [clang-tidy] Add check fo SEI CERT item ENV32-C

2020-08-05 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang-tools-extra/clang-tidy/cert/ExitHandlerCheck.cpp:33
+bool isExitFunction(StringRef FnName) {
+  return FnName == "_Exit" || FnName == "exit" || FnName == "quick_exit";
+}

njames93 wrote:
> whisperity wrote:
> > aaron.ballman wrote:
> > > Because this rule applies in C++ as well as C, you should protect these 
> > > names so that calling something like this doesn't trigger the check:
> > > ```
> > > namespace menu_items {
> > > void exit(int);
> > > }
> > > ```
> > > I think we want `::_Exit` and `::std::_Exit` to check the fully qualified 
> > > names (I believe this will still work in C, but should be tested). The 
> > > same goes for the other names (including `atexit` and `at_quick_exit` 
> > > above).
> > > I think we want `::_Exit` and `::std::_Exit` to check the fully qualified 
> > > names (I believe this will still work in C, but should be tested).
> > 
> > Tested with Clang-Query:
> > 
> > ```
> > clang-query> m functionDecl(hasName("::atexit"))
> > 
> > Match #1:
> > 
> > /home/whisperity/LLVM/Build/foo.c:1:1: note: "root" binds here
> > int atexit(int) {}
> > ^~
> > 1 match.
> > ```
> That only works because the logic inside the `hasName`matcher 
That's the bit I was worried about, too, thanks for confirming @njames93. 



Comment at: clang-tools-extra/clang-tidy/cert/ExitHandlerCheck.cpp:64
+/// argument.
+void ExitHandlerCheck::registerMatchers(MatchFinder *Finder) {
+  const auto IsRegisterFunction =

whisperity wrote:
> aaron.ballman wrote:
> > whisperity wrote:
> > > What happens if this test is run on C++ code calling the same functions? 
> > > I see the rule is only applicable to C, for some reason... Should it be 
> > > disabled from registering if by accident the check is enabled on a C++ 
> > > source file?
> > The CERT C++ rules inherit many of the C rules, including this one. It's 
> > listed towards the bottom of the set of inherited rules here: 
> > https://wiki.sei.cmu.edu/confluence/pages/viewpage.action?pageId=88046336
> Right, thanks for the heads-up. This should be somewhat more indicative on 
> the Wiki on the page for the rule (especially because people won't 
> immediately understand why a `-c` check reports on their cpp code, assuming 
> they understand `-c` means C.)
I would imagine people shouldn't be confused by a C check triggering in C++ 
given that C++ incorporates much of C so there's a considerable amount of 
overlap (for instance, this hasn't been an issue with `cert-env33-c` which 
applies in both C and C++).

That said, what do you think should be indicated on the wiki (I assume you mean 
the CERT wiki and not the clang-tidy documentation)? I'm happy to pass the 
suggestion along to folks I still know at CERT.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83717

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


[PATCH] D85239: [DOCS] Add more detail to stack protector documentation

2020-08-05 Thread Peter Smith via Phabricator via cfe-commits
psmith added a comment.

In D85239#2194604 , @MaskRay wrote:

> This file is auto generated. The real documentation should go to 
> `clang/include/clang/Driver/Options.td`

Thanks for pointing that out! I regenerated the ClangCommandLineReference.rst 
from Options.td but it looks like I'm not the only one that missed the comment 
on the top line. There are 27 differences in the file including some that are 
not in Options.td. I'm loathe to lose work by regenerating the whole file so 
I've included just the diff created by the change to Options.td. At least if 
the file is synched up again then we'll keep the edits.

Assuming this is still OK I'll commit tomorrow.


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

https://reviews.llvm.org/D85239

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


[PATCH] D85239: [DOCS] Add more detail to stack protector documentation

2020-08-05 Thread Peter Smith via Phabricator via cfe-commits
psmith updated this revision to Diff 283201.
psmith added a comment.
Herald added a subscriber: dang.

uploaded diff with both Options.td and ClangCommandLineReference.rst


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

https://reviews.llvm.org/D85239

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/include/clang/Driver/Options.td


Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1801,10 +1801,15 @@
"as well as any calls to alloca or the taking of an address from a 
local variable">;
 def fstack_protector : Flag<["-"], "fstack-protector">, Group,
   HelpText<"Enable stack protectors for some functions vulnerable to stack 
smashing. "
-   "This uses a loose heuristic which considers functions vulnerable "
-   "if they contain a char (or 8bit integer) array or constant sized 
calls to "
-   "alloca, which are of greater size than ssp-buffer-size (default: 8 
bytes). "
-   "All variable sized calls to alloca are considered vulnerable">;
+   "This uses a loose heuristic which considers functions vulnerable 
if they "
+   "contain a char (or 8bit integer) array or constant sized calls to 
alloca "
+   ", which are of greater size than ssp-buffer-size (default: 8 
bytes). All "
+   "variable sized calls to alloca are considered vulnerable. A 
function with"
+   "a stack protector has a guard value added to the stack frame that 
is "
+   "checked on function exit. The guard value must be positioned in 
the "
+   "stack frame such that a buffer overflow from a vulnerable variable 
will "
+   "overwrite the guard value before overwriting the function's return 
"
+   "address. The reference stack guard value is stored in a global 
variable.">;
 def ftrivial_auto_var_init : Joined<["-"], "ftrivial-auto-var-init=">, 
Group,
   Flags<[CC1Option, CoreOption]>, HelpText<"Initialize trivial automatic stack 
variables: uninitialized (default)"
   " | pattern">, Values<"uninitialized,pattern">;
Index: clang/docs/ClangCommandLineReference.rst
===
--- clang/docs/ClangCommandLineReference.rst
+++ clang/docs/ClangCommandLineReference.rst
@@ -2136,7 +2136,7 @@
 
 .. option:: -fstack-protector, -fno-stack-protector
 
-Enable stack protectors for some functions vulnerable to stack smashing. This 
uses a loose heuristic which considers functions vulnerable if they contain a 
char (or 8bit integer) array or constant sized calls to alloca, which are of 
greater size than ssp-buffer-size (default: 8 bytes). All variable sized calls 
to alloca are considered vulnerable
+Enable stack protectors for some functions vulnerable to stack smashing. This 
uses a loose heuristic which considers functions vulnerable if they contain a 
char (or 8bit integer) array or constant sized calls to alloca , which are of 
greater size than ssp-buffer-size (default: 8 bytes). All variable sized calls 
to alloca are considered vulnerable. A function witha stack protector has a 
guard value added to the stack frame that is checked on function exit. The 
guard value must be positioned in the stack frame such that a buffer overflow 
from a vulnerable variable will overwrite the guard value before overwriting 
the function's return address. The reference stack guard value is stored in a 
global variable.
 
 .. option:: -fstack-protector-all
 


Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1801,10 +1801,15 @@
"as well as any calls to alloca or the taking of an address from a local variable">;
 def fstack_protector : Flag<["-"], "fstack-protector">, Group,
   HelpText<"Enable stack protectors for some functions vulnerable to stack smashing. "
-   "This uses a loose heuristic which considers functions vulnerable "
-   "if they contain a char (or 8bit integer) array or constant sized calls to "
-   "alloca, which are of greater size than ssp-buffer-size (default: 8 bytes). "
-   "All variable sized calls to alloca are considered vulnerable">;
+   "This uses a loose heuristic which considers functions vulnerable if they "
+   "contain a char (or 8bit integer) array or constant sized calls to alloca "
+   ", which are of greater size than ssp-buffer-size (default: 8 bytes). All "
+   "variable sized calls to alloca are considered vulnerable. A function with"
+   "a stack protector has a guard value added to the stack frame that is "
+   "checked on function exit. The guard value must be positioned in the "
+   "stack frame such that a buffer overflow from a vu

[clang] 45f2a56 - [CUDA][HIP] Support accessing static device variable in host code for -fno-gpu-rdc

2020-08-05 Thread Yaxun Liu via cfe-commits

Author: Yaxun (Sam) Liu
Date: 2020-08-05T07:57:38-04:00
New Revision: 45f2a56856e29b8cb038b2e559289b91fb98fedf

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

LOG: [CUDA][HIP] Support accessing static device variable in host code for 
-fno-gpu-rdc

nvcc supports accessing file-scope static device variables in host code by host 
APIs
like cudaMemcpyToSymbol etc.

CUDA/HIP let users access device variables in host code by shadow variables. In 
host compilation,
clang emits a shadow variable for each device variable, and calls 
__*RegisterVariable to
register it in init function. The address of the shadow variable and the device 
side mangled
name of the device variable is passed to __*RegisterVariable. Runtime looks up 
the symbol
by name in the device binary  to find the address of the device variable.

The problem with static device variables is that they have internal linkage, 
therefore their
name may be changed by the linker if there are multiple symbols with the same 
name. Also
they end up as local symbols in the elf file, whereas the runtime only looks up 
the global symbols.

Another reason for making the static device variables external linkage is that 
they may be
initialized externally by host code and their final value may be accessed by 
host code
after kernel execution, therefore they actually have external linkage. Giving 
them internal
linkage will cause incorrect optimizations on them.

To support accessing static device var in host code for -fno-gpu-rdc mode, 
change the intnernal
linkage to external linkage. The name does not need change since there is only 
one TU for
-fno-gpu-rdc mode. Also the externalization is done only if the device static 
var is referenced
by host code.

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

Added: 
clang/test/CodeGenCUDA/static-device-var-no-rdc.cu

Modified: 
clang/include/clang/AST/ASTContext.h
clang/lib/AST/ASTContext.cpp
clang/lib/Sema/SemaExpr.cpp
clang/test/CodeGenCUDA/constexpr-variables.cu

Removed: 




diff  --git a/clang/include/clang/AST/ASTContext.h 
b/clang/include/clang/AST/ASTContext.h
index 6c00fe86f282d..78207a4aad31b 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -43,6 +43,7 @@
 #include "llvm/ADT/APSInt.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/FoldingSet.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include "llvm/ADT/MapVector.h"
@@ -999,6 +1000,9 @@ class ASTContext : public RefCountedBase {
   // Implicitly-declared type 'struct _GUID'.
   mutable TagDecl *MSGuidTagDecl = nullptr;
 
+  /// Keep track of CUDA/HIP static device variables referenced by host code.
+  llvm::DenseSet CUDAStaticDeviceVarReferencedByHost;
+
   ASTContext(LangOptions &LOpts, SourceManager &SM, IdentifierTable &idents,
  SelectorTable &sels, Builtin::Context &builtins);
   ASTContext(const ASTContext &) = delete;
@@ -3030,6 +3034,9 @@ OPT_LIST(V)
   /// Return a new OMPTraitInfo object owned by this context.
   OMPTraitInfo &getNewOMPTraitInfo();
 
+  /// Whether a C++ static variable should be externalized.
+  bool shouldExternalizeStaticVar(const Decl *D) const;
+
 private:
   /// All OMPTraitInfo objects live in this collection, one per
   /// `pragma omp [begin] declare variant` directive.

diff  --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 43bbe41fb6112..04a4c5482db75 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -10325,12 +10325,17 @@ static GVALinkage adjustGVALinkageForAttributes(const 
ASTContext &Context,
   } else if (D->hasAttr()) {
 if (L == GVA_DiscardableODR)
   return GVA_StrongODR;
-  } else if (Context.getLangOpts().CUDA && Context.getLangOpts().CUDAIsDevice 
&&
- D->hasAttr()) {
+  } else if (Context.getLangOpts().CUDA && Context.getLangOpts().CUDAIsDevice) 
{
 // Device-side functions with __global__ attribute must always be
 // visible externally so they can be launched from host.
-if (L == GVA_DiscardableODR || L == GVA_Internal)
+if (D->hasAttr() &&
+(L == GVA_DiscardableODR || L == GVA_Internal))
   return GVA_StrongODR;
+// Single source offloading languages like CUDA/HIP need to be able to
+// access static device variables from host code of the same compilation
+// unit. This is done by externalizing the static variable.
+if (Context.shouldExternalizeStaticVar(D))
+  return GVA_StrongExternal;
   }
   return L;
 }
@@ -11185,3 +11190,11 @@ clang::operator<<(const DiagnosticBuilder &DB,
 return DB << Section.Decl;
   return DB << "a prior #pragma section";
 }
+
+bool ASTContext::shouldExternalizeStaticVar(const De

[PATCH] D80858: [CUDA][HIP] Support accessing static device variable in host code for -fno-gpu-rdc

2020-08-05 Thread Yaxun Liu via Phabricator via cfe-commits
This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG45f2a56856e2: [CUDA][HIP] Support accessing static device 
variable in host code for -fno-gpu… (authored by yaxunl).
Herald added a project: clang.

Changed prior to commit:
  https://reviews.llvm.org/D80858?vs=282952&id=283202#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80858

Files:
  clang/include/clang/AST/ASTContext.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/CodeGenCUDA/constexpr-variables.cu
  clang/test/CodeGenCUDA/static-device-var-no-rdc.cu

Index: clang/test/CodeGenCUDA/static-device-var-no-rdc.cu
===
--- /dev/null
+++ clang/test/CodeGenCUDA/static-device-var-no-rdc.cu
@@ -0,0 +1,94 @@
+// REQUIRES: x86-registered-target
+// REQUIRES: amdgpu-registered-target
+
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -fcuda-is-device \
+// RUN:   -emit-llvm -o - -x hip %s | FileCheck \
+// RUN:   -check-prefixes=DEV %s
+
+// RUN: %clang_cc1 -triple x86_64-gnu-linux \
+// RUN:   -emit-llvm -o - -x hip %s | FileCheck \
+// RUN:   -check-prefixes=HOST %s
+
+#include "Inputs/cuda.h"
+
+// Test function scope static device variable, which should not be externalized.
+// DEV-DAG: @_ZZ6kernelPiPPKiE1w = internal addrspace(4) constant i32 1
+
+// Check a static device variable referenced by host function is externalized.
+// DEV-DAG: @_ZL1x = addrspace(1) externally_initialized global i32 0
+// HOST-DAG: @_ZL1x = internal global i32 undef
+// HOST-DAG: @[[DEVNAMEX:[0-9]+]] = {{.*}}c"_ZL1x\00"
+
+static __device__ int x;
+
+// Check a static device variables referenced only by device functions and kernels
+// is not externalized.
+// DEV-DAG: @_ZL2x2 = internal addrspace(1) global i32 0
+static __device__ int x2;
+
+// Check a static device variable referenced by host device function is externalized.
+// DEV-DAG: @_ZL2x3 = addrspace(1) externally_initialized global i32 0
+static __device__ int x3;
+
+// Check a static device variable referenced in file scope is externalized.
+// DEV-DAG: @_ZL2x4 = addrspace(1) externally_initialized global i32 0
+static __device__ int x4;
+int& x4_ref = x4;
+
+// Check a static device variable in anonymous namespace.
+// DEV-DAG: @_ZN12_GLOBAL__N_12x5E = addrspace(1) externally_initialized global i32 0
+namespace {
+static __device__ int x5;
+}
+
+// Check a static constant variable referenced by host is externalized.
+// DEV-DAG: @_ZL1y = addrspace(4) externally_initialized global i32 0
+// HOST-DAG: @_ZL1y = internal global i32 undef
+// HOST-DAG: @[[DEVNAMEY:[0-9]+]] = {{.*}}c"_ZL1y\00"
+
+static __constant__ int y;
+
+// Test static host variable, which should not be externalized nor registered.
+// HOST-DAG: @_ZL1z = internal global i32 0
+// DEV-NOT: @_ZL1z
+static int z;
+
+// Test static device variable in inline function, which should not be
+// externalized nor registered.
+// DEV-DAG: @_ZZ6devfunPPKiE1p = linkonce_odr addrspace(4) constant i32 2, comdat
+
+inline __device__ void devfun(const int ** b) {
+  const static int p = 2;
+  b[0] = &p;
+  b[1] = &x2;
+}
+
+__global__ void kernel(int *a, const int **b) {
+  const static int w = 1;
+  a[0] = x;
+  a[1] = y;
+  a[2] = x2;
+  a[3] = x3;
+  a[4] = x4;
+  a[5] = x5;
+  b[0] = &w;
+  devfun(b);
+}
+
+__host__ __device__ void hdf(int *a) {
+  a[0] = x3;
+}
+
+int* getDeviceSymbol(int *x);
+
+void foo(int *a) {
+  getDeviceSymbol(&x);
+  getDeviceSymbol(&x5);
+  getDeviceSymbol(&y);
+  z = 123;
+}
+
+// HOST: __hipRegisterVar({{.*}}@_ZL1x {{.*}}@[[DEVNAMEX]]
+// HOST: __hipRegisterVar({{.*}}@_ZL1y {{.*}}@[[DEVNAMEY]]
+// HOST-NOT: __hipRegisterVar({{.*}}@_ZZ6kernelPiPPKiE1w
+// HOST-NOT: __hipRegisterVar({{.*}}@_ZZ6devfunPPKiE1p
Index: clang/test/CodeGenCUDA/constexpr-variables.cu
===
--- clang/test/CodeGenCUDA/constexpr-variables.cu
+++ clang/test/CodeGenCUDA/constexpr-variables.cu
@@ -19,7 +19,7 @@
   // CXX14: @_ZN1Q2k2E = {{.*}}externally_initialized constant i32 6
   // CXX17: @_ZN1Q2k2E = internal {{.*}}constant i32 6
   // CXX14: @_ZN1Q2k1E = available_externally {{.*}}constant i32 5
-  // CXX17: @_ZN1Q2k1E = linkonce_odr {{.*}}constant i32 5
+  // CXX17: @_ZN1Q2k1E = {{.*}} externally_initialized constant i32 5
   static constexpr int k1 = 5;
   static constexpr int k2 = 6;
 };
@@ -30,14 +30,14 @@
 
 template struct X {
   // CXX14: @_ZN1XIiE1aE = available_externally {{.*}}constant i32 123
-  // CXX17: @_ZN1XIiE1aE = linkonce_odr {{.*}}constant i32 123
+  // CXX17: @_ZN1XIiE1aE = {{.*}}externally_initialized constant i32 123
   static constexpr int a = 123;
 };
 __constant__ const int &use_X_a = X::a;
 
 template  struct A {
   // CXX14: @_ZN1AIiLi1ELi2EE1xE = available_externally {{.*}}consta

[PATCH] D85239: [DOCS] Add more detail to stack protector documentation

2020-08-05 Thread Peter Smith via Phabricator via cfe-commits
psmith marked an inline comment as done.
psmith added inline comments.



Comment at: clang/docs/ClangCommandLineReference.rst:2139
 
-Enable stack protectors for some functions vulnerable to stack smashing. This 
uses a loose heuristic which considers functions vulnerable if they contain a 
char (or 8bit integer) array or constant sized calls to alloca, which are of 
greater size than ssp-buffer-size (default: 8 bytes). All variable sized calls 
to alloca are considered vulnerable
+Enable stack protectors for some functions vulnerable to stack smashing. This 
uses a loose heuristic which considers functions vulnerable if they contain a 
char (or 8bit integer) array or constant sized calls to alloca, which are of 
greater size than ssp-buffer-size (default: 8 bytes). All variable sized calls 
to alloca are considered vulnerable. A function with a stack protector has a 
guard value added to the stack frame that is checked on function exit. The 
guard value must be positioned in the stack frame such that a buffer overflow 
from a vulnerable variable will overwrite to the guard value before overwriting 
the function's return address. The reference stack guard value is stored in a 
global variable.
 

probinson wrote:
> "overwrite to the guard variable" -> "overwrite the guard variable"
Thanks, now updated


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

https://reviews.llvm.org/D85239

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


[PATCH] D83717: [clang-tidy] Add check fo SEI CERT item ENV32-C

2020-08-05 Thread Whisperity via Phabricator via cfe-commits
whisperity added inline comments.



Comment at: clang-tools-extra/clang-tidy/cert/ExitHandlerCheck.cpp:64
+/// argument.
+void ExitHandlerCheck::registerMatchers(MatchFinder *Finder) {
+  const auto IsRegisterFunction =

aaron.ballman wrote:
> whisperity wrote:
> > aaron.ballman wrote:
> > > whisperity wrote:
> > > > What happens if this test is run on C++ code calling the same 
> > > > functions? I see the rule is only applicable to C, for some reason... 
> > > > Should it be disabled from registering if by accident the check is 
> > > > enabled on a C++ source file?
> > > The CERT C++ rules inherit many of the C rules, including this one. It's 
> > > listed towards the bottom of the set of inherited rules here: 
> > > https://wiki.sei.cmu.edu/confluence/pages/viewpage.action?pageId=88046336
> > Right, thanks for the heads-up. This should be somewhat more indicative on 
> > the Wiki on the page for the rule (especially because people won't 
> > immediately understand why a `-c` check reports on their cpp code, assuming 
> > they understand `-c` means C.)
> I would imagine people shouldn't be confused by a C check triggering in C++ 
> given that C++ incorporates much of C so there's a considerable amount of 
> overlap (for instance, this hasn't been an issue with `cert-env33-c` which 
> applies in both C and C++).
> 
> That said, what do you think should be indicated on the wiki (I assume you 
> mean the CERT wiki and not the clang-tidy documentation)? I'm happy to pass 
> the suggestion along to folks I still know at CERT.
> That said, what do you think should be indicated on the wiki (I assume you 
> mean the CERT wiki and not the clang-tidy documentation)?

On the page of a C rule "ABC-00", if it also applies for C++, it should be 
indicated. Just this fact: //"In addition, this rule is applicable for C++ 
programs [bla bla]."// where `[bla bla]` is something like "calling C standard 
library operations" or "dealing with C API" or "handling C data structures" or 
simply no extra "elaboration", depending on what the rule is targeting.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83717

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


[PATCH] D85295: [SyntaxTree] Implement the List construct.

2020-08-05 Thread Eduardo Caldas via Phabricator via cfe-commits
eduucaldas created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
eduucaldas requested review of this revision.

We defined a List construct to help with the implementation of list-like
grammar rules. This is a first implementation of this API.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85295

Files:
  clang/include/clang/Tooling/Syntax/Nodes.h
  clang/include/clang/Tooling/Syntax/Tree.h
  clang/lib/Tooling/Syntax/Nodes.cpp
  clang/lib/Tooling/Syntax/Tree.cpp

Index: clang/lib/Tooling/Syntax/Tree.cpp
===
--- clang/lib/Tooling/Syntax/Tree.cpp
+++ clang/lib/Tooling/Syntax/Tree.cpp
@@ -270,3 +270,104 @@
   }
   return nullptr;
 }
+
+std::vector>
+syntax::List::getElementsAsNodesAndDelimiters() {
+  if (!firstChild()) {
+assert(canBeEmpty());
+return {};
+  }
+  auto children = std::vector>();
+  syntax::Node *elementWithoutDelimiter = nullptr;
+  for (auto *C = firstChild(); C; C = C->nextSibling()) {
+switch (C->role()) {
+case syntax::NodeRole::List_element: {
+  if (elementWithoutDelimiter) {
+children.push_back({elementWithoutDelimiter, nullptr});
+  }
+  elementWithoutDelimiter = C;
+  break;
+}
+case syntax::NodeRole::List_delimiter: {
+  children.push_back(
+  {elementWithoutDelimiter, llvm::cast(C)});
+  elementWithoutDelimiter = nullptr;
+  break;
+}
+default:
+  llvm_unreachable("A list has only elements or delimiters.");
+}
+  }
+
+  switch (getTerminationKind()) {
+  case syntax::TerminationKind::Separated: {
+children.push_back({elementWithoutDelimiter, nullptr});
+break;
+  }
+  case syntax::TerminationKind::Terminated:
+  case syntax::TerminationKind::MaybeTerminated: {
+if (elementWithoutDelimiter) {
+  children.push_back({elementWithoutDelimiter, nullptr});
+}
+break;
+  }
+  }
+
+  return children;
+}
+
+// Almost the same implementation of `getElementsAsNodesAndDelimiters` but
+// ignoring delimiters
+std::vector syntax::List::getElementsAsNodes() {
+  if (!firstChild()) {
+assert(canBeEmpty());
+return {};
+  }
+  auto children = std::vector();
+  syntax::Node *elementWithoutDelimiter = nullptr;
+  for (auto *C = firstChild(); C; C = C->nextSibling()) {
+switch (C->role()) {
+case syntax::NodeRole::List_element: {
+  if (elementWithoutDelimiter) {
+children.push_back(elementWithoutDelimiter);
+  }
+  elementWithoutDelimiter = C;
+  break;
+}
+case syntax::NodeRole::List_delimiter: {
+  children.push_back(elementWithoutDelimiter);
+  elementWithoutDelimiter = nullptr;
+  break;
+}
+default:
+  llvm_unreachable("A list has only elements or delimiters.");
+}
+  }
+
+  switch (getTerminationKind()) {
+  case syntax::TerminationKind::Separated: {
+children.push_back(elementWithoutDelimiter);
+break;
+  }
+  case syntax::TerminationKind::Terminated:
+  case syntax::TerminationKind::MaybeTerminated: {
+if (elementWithoutDelimiter) {
+  children.push_back(elementWithoutDelimiter);
+}
+break;
+  }
+  }
+
+  return children;
+}
+
+// The methods below can't be implemented without additional information we
+// have. Should we give them default permissive values?
+
+clang::tok::TokenKind syntax::List::getDelimiterTokenKind() {
+  return clang::tok::TokenKind::unknown;
+}
+syntax::TerminationKind syntax::List::getTerminationKind() {
+  return syntax::TerminationKind::MaybeTerminated;
+}
+bool syntax::List::canBeEmpty() { return true; }
Index: clang/lib/Tooling/Syntax/Nodes.cpp
===
--- clang/lib/Tooling/Syntax/Nodes.cpp
+++ clang/lib/Tooling/Syntax/Nodes.cpp
@@ -144,6 +144,10 @@
 return OS << "ExternKeyword";
   case syntax::NodeRole::BodyStatement:
 return OS << "BodyStatement";
+  case syntax::NodeRole::List_element:
+return OS << "List_element";
+  case syntax::NodeRole::List_delimiter:
+return OS << "List_delimiter";
   case syntax::NodeRole::CaseStatement_value:
 return OS << "CaseStatement_value";
   case syntax::NodeRole::IfStatement_thenStatement:
Index: clang/include/clang/Tooling/Syntax/Tree.h
===
--- clang/include/clang/Tooling/Syntax/Tree.h
+++ clang/include/clang/Tooling/Syntax/Tree.h
@@ -191,6 +191,47 @@
   Node *FirstChild = nullptr;
 };
 
+enum TerminationKind {
+  Terminated,
+  MaybeTerminated,
+  Separated,
+};
+
+class List : public Tree {
+  template  struct ElementAndDelimiter {
+Element *element;
+Leaf *delimiter;
+  };
+
+  /// Returns the elements and corresponding delimiters. Missing elements
+  /// and delimiters are represented as null pointers.
+  ///
+  /// For example, in a separated list:
+  /// "a, b, c" <=> [("a", ","), ("b", ","), ("c", null)]
+  /// "a, , c" <=> [("a", ","), (null, ",")

[PATCH] D85295: [SyntaxTree] Implement the List construct.

2020-08-05 Thread Eduardo Caldas via Phabricator via cfe-commits
eduucaldas added a reviewer: gribozavr2.
eduucaldas added inline comments.



Comment at: clang/include/clang/Tooling/Syntax/Tree.h:223
+
+  // These can't be implemented with the information we have!
+

As this is a base List, we don't  have the necessary information tom implement 
the following methods. I chose to give them default values, check their 
definition


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85295

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


[PATCH] D85287: Extend -Wtautological-bitwise-compare "bitwise or with non-zero value" warnings

2020-08-05 Thread Stephan Bergmann via Phabricator via cfe-commits
sberg updated this revision to Diff 283204.

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

https://reviews.llvm.org/D85287

Files:
  clang/lib/Analysis/CFG.cpp
  clang/test/Sema/warn-bitwise-compare.c
  clang/test/SemaCXX/warn-bitwise-compare.cpp

Index: clang/test/SemaCXX/warn-bitwise-compare.cpp
===
--- clang/test/SemaCXX/warn-bitwise-compare.cpp
+++ clang/test/SemaCXX/warn-bitwise-compare.cpp
@@ -1,6 +1,12 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -Wtautological-bitwise-compare %s
 // RUN: %clang_cc1 -fsyntax-only -verify -Wall -Wno-unused %s
 
+int f() { return 3; }
+
+const int const1 = 1;
+constexpr int const2 = 2;
+const int const3 = f();
+
 void test(int x) {
   bool b1 = (8 & x) == 3;
   // expected-warning@-1 {{bitwise comparison always evaluates to false}}
@@ -10,4 +16,15 @@
   // expected-warning@-1 {{bitwise or with non-zero value always evaluates to true}}
   bool b4 = !!(x | 5);
   // expected-warning@-1 {{bitwise or with non-zero value always evaluates to true}}
+  bool b5 = (x | const1);
+  // expected-warning@-1 {{bitwise or with non-zero value always evaluates to true}}
+  bool b6 = (x | const2);
+  // expected-warning@-1 {{bitwise or with non-zero value always evaluates to true}}
+  bool b7 = (x | const3);
+
+  // No warnings at least for now:
+  bool b8 = (x & const1) == 8;
+  bool b9 = (x | const1) == 4;
+  bool b10 = (x & const2) == 8;
+  bool b11 = (x | const2) == 4;
 }
Index: clang/test/Sema/warn-bitwise-compare.c
===
--- clang/test/Sema/warn-bitwise-compare.c
+++ clang/test/Sema/warn-bitwise-compare.c
@@ -8,6 +8,8 @@
   ONE,
 };
 
+const int const1 = 1;
+
 void f(int x) {
   if ((8 & x) == 3) {}  // expected-warning {{bitwise comparison always evaluates to false}}
   if ((x & 8) == 4) {}  // expected-warning {{bitwise comparison always evaluates to false}}
@@ -34,6 +36,9 @@
 
   if ((x & mydefine) == 8) {}
   if ((x | mydefine) == 4) {}
+
+  if ((x & const1) == 8) {}
+  if ((x | const1) == 4) {}
 }
 
 void g(int x) {
@@ -45,4 +50,6 @@
   if (x | ONE) {}  // expected-warning {{bitwise or with non-zero value always evaluates to true}}
 
   if (x | ZERO) {}
+
+  if (x | const1) {}
 }
Index: clang/lib/Analysis/CFG.cpp
===
--- clang/lib/Analysis/CFG.cpp
+++ clang/lib/Analysis/CFG.cpp
@@ -93,29 +93,36 @@
   return isa(E);
 }
 
-/// Helper for tryNormalizeBinaryOperator. Attempts to extract an IntegerLiteral
-/// constant expression or EnumConstantDecl from the given Expr. If it fails,
+/// Helper for tryNormalizeBinaryOperator. Attempts to extract a suitable
+/// integer or enum constant from the given Expr. If it fails,
 /// returns nullptr.
-static const Expr *tryTransformToIntOrEnumConstant(const Expr *E) {
+static const Expr *tryTransformToIntOrEnumConstant(const Expr *E,
+   ASTContext &Ctx) {
   E = E->IgnoreParens();
   if (IsIntegerLiteralConstantExpr(E))
 return E;
-  if (auto *DR = dyn_cast(E->IgnoreParenImpCasts()))
-return isa(DR->getDecl()) ? DR : nullptr;
+  if (auto *DR = dyn_cast(E->IgnoreParenImpCasts())) {
+auto *D = DR->getDecl();
+if (isa(D))
+  return DR;
+if (auto *VD = dyn_cast(D))
+  if (VD->isUsableInConstantExpressions(Ctx))
+return DR;
+  }
   return nullptr;
 }
 
 /// Tries to interpret a binary operator into `Expr Op NumExpr` form, if
-/// NumExpr is an integer literal or an enum constant.
+/// NumExpr is a suitable integer or an enum constant.
 ///
 /// If this fails, at least one of the returned DeclRefExpr or Expr will be
 /// null.
 static std::tuple
-tryNormalizeBinaryOperator(const BinaryOperator *B) {
+tryNormalizeBinaryOperator(const BinaryOperator *B, ASTContext &Ctx) {
   BinaryOperatorKind Op = B->getOpcode();
 
   const Expr *MaybeDecl = B->getLHS();
-  const Expr *Constant = tryTransformToIntOrEnumConstant(B->getRHS());
+  const Expr *Constant = tryTransformToIntOrEnumConstant(B->getRHS(), Ctx);
   // Expr looked like `0 == Foo` instead of `Foo == 0`
   if (Constant == nullptr) {
 // Flip the operator
@@ -129,7 +136,7 @@
   Op = BO_GE;
 
 MaybeDecl = B->getRHS();
-Constant = tryTransformToIntOrEnumConstant(B->getLHS());
+Constant = tryTransformToIntOrEnumConstant(B->getLHS(), Ctx);
   }
 
   return std::make_tuple(MaybeDecl, Op, Constant);
@@ -140,7 +147,7 @@
 /// literals.
 ///
 /// It's an error to pass this arguments that are not either IntegerLiterals
-/// or DeclRefExprs (that have decls of type EnumConstantDecl)
+/// or DeclRefExprs
 static bool areExprTypesCompatible(const Expr *E1, const Expr *E2) {
   // User intent isn't clear if they're mixing int literals with enum
   // constants.
@@ -151,18 +158,21 @@
   if (!isa(E1))
 return true;
 
-  // IntegerLiterals are handled above and only EnumConstantDecls are expected
+ 

[PATCH] D83717: [clang-tidy] Add check fo SEI CERT item ENV32-C

2020-08-05 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang-tools-extra/clang-tidy/cert/ExitHandlerCheck.cpp:64
+/// argument.
+void ExitHandlerCheck::registerMatchers(MatchFinder *Finder) {
+  const auto IsRegisterFunction =

whisperity wrote:
> aaron.ballman wrote:
> > whisperity wrote:
> > > aaron.ballman wrote:
> > > > whisperity wrote:
> > > > > What happens if this test is run on C++ code calling the same 
> > > > > functions? I see the rule is only applicable to C, for some reason... 
> > > > > Should it be disabled from registering if by accident the check is 
> > > > > enabled on a C++ source file?
> > > > The CERT C++ rules inherit many of the C rules, including this one. 
> > > > It's listed towards the bottom of the set of inherited rules here: 
> > > > https://wiki.sei.cmu.edu/confluence/pages/viewpage.action?pageId=88046336
> > > Right, thanks for the heads-up. This should be somewhat more indicative 
> > > on the Wiki on the page for the rule (especially because people won't 
> > > immediately understand why a `-c` check reports on their cpp code, 
> > > assuming they understand `-c` means C.)
> > I would imagine people shouldn't be confused by a C check triggering in C++ 
> > given that C++ incorporates much of C so there's a considerable amount of 
> > overlap (for instance, this hasn't been an issue with `cert-env33-c` which 
> > applies in both C and C++).
> > 
> > That said, what do you think should be indicated on the wiki (I assume you 
> > mean the CERT wiki and not the clang-tidy documentation)? I'm happy to pass 
> > the suggestion along to folks I still know at CERT.
> > That said, what do you think should be indicated on the wiki (I assume you 
> > mean the CERT wiki and not the clang-tidy documentation)?
> 
> On the page of a C rule "ABC-00", if it also applies for C++, it should be 
> indicated. Just this fact: //"In addition, this rule is applicable for C++ 
> programs [bla bla]."// where `[bla bla]` is something like "calling C 
> standard library operations" or "dealing with C API" or "handling C data 
> structures" or simply no extra "elaboration", depending on what the rule is 
> targeting.
Thanks, I'll pass this suggestion along!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83717

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


[PATCH] D85295: [SyntaxTree] Implement the List construct.

2020-08-05 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added a comment.

I feel uneasy about adding this code without tests. Could we maybe port the 
function parameter list to use this infrastructure, and then add tests that 
exercise `getElementsAsNodesAndDelimiters`?




Comment at: clang/include/clang/Tooling/Syntax/Tree.h:194
 
+enum TerminationKind {
+  Terminated,

`enum class`?

Also, I think it should be renamed to `ListTerminationKind` or moved into 
`List` as a nested type.



Comment at: clang/include/clang/Tooling/Syntax/Tree.h:197
+  MaybeTerminated,
+  Separated,
+};

Add a "WithoutDelimiters" case as well?



Comment at: clang/include/clang/Tooling/Syntax/Tree.h:200
+
+class List : public Tree {
+  template  struct ElementAndDelimiter {

Could you add a doc comment?



Comment at: clang/include/clang/Tooling/Syntax/Tree.h:229
+  /// elements to empty or one-element lists.
+  clang::tok::TokenKind getDelimiterTokenKind();
+

Should we change this function to return optional to allow representing lists 
that don't have any delimiters?



Comment at: clang/include/clang/Tooling/Syntax/Tree.h:232
+  TerminationKind getTerminationKind();
+  bool canBeEmpty();
+};

Please add a doc comment that emphasizes that any list can be empty when the 
syntax tree represents code with syntax errors.



Comment at: clang/lib/Tooling/Syntax/Tree.cpp:277
+  if (!firstChild()) {
+assert(canBeEmpty());
+return {};

This assert is only correct for valid code. When a syntax tree represents 
invalid code, any list can be empty.



Comment at: clang/lib/Tooling/Syntax/Tree.cpp:293
+  children.push_back(
+  {elementWithoutDelimiter, llvm::cast(C)});
+  elementWithoutDelimiter = nullptr;





Comment at: clang/lib/Tooling/Syntax/Tree.cpp:298
+default:
+  llvm_unreachable("A list has only elements or delimiters.");
+}





Comment at: clang/lib/Tooling/Syntax/Tree.cpp:368
+clang::tok::TokenKind syntax::List::getDelimiterTokenKind() {
+  return clang::tok::TokenKind::unknown;
+}

I think the eventual implementation should switch on the syntax tree node kind 
and return the appropriate information. Right now, there are no subclasses of 
List, to these methods should be implemented as llvm_unreachable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85295

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


[clang] 00b89f6 - [clang][NFC] Remove spurious +x flag on DeclTemplate.cpp and DeclTemplate.h

2020-08-05 Thread Bruno Ricci via cfe-commits

Author: Bruno Ricci
Date: 2020-08-05T13:54:30+01:00
New Revision: 00b89f66f988e9ec6f366ed46a51ace39fac07c8

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

LOG: [clang][NFC] Remove spurious +x flag on DeclTemplate.cpp and DeclTemplate.h

Added: 


Modified: 
clang/include/clang/AST/DeclTemplate.h
clang/lib/AST/DeclTemplate.cpp

Removed: 




diff  --git a/clang/include/clang/AST/DeclTemplate.h 
b/clang/include/clang/AST/DeclTemplate.h
old mode 100755
new mode 100644

diff  --git a/clang/lib/AST/DeclTemplate.cpp b/clang/lib/AST/DeclTemplate.cpp
old mode 100755
new mode 100644



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


[clang] 6f2fa9d - [clang][NFC] Document NamedDecl::printName

2020-08-05 Thread Bruno Ricci via cfe-commits

Author: Bruno Ricci
Date: 2020-08-05T13:54:37+01:00
New Revision: 6f2fa9d312fcea2448706a8e410c7bc1b6436ea7

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

LOG: [clang][NFC] Document NamedDecl::printName

Added: 


Modified: 
clang/include/clang/AST/Decl.h

Removed: 




diff  --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index 4dd5e14d36e1..c2511514fe72 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -265,6 +265,8 @@ class NamedDecl : public Decl {
   // FIXME: Deprecated, move clients to getName().
   std::string getNameAsString() const { return Name.getAsString(); }
 
+  /// Pretty-print the unqualified name of this declaration. Can be overloaded
+  /// by derived classes to provide a more user-friendly name when appropriate.
   virtual void printName(raw_ostream &os) const;
 
   /// Get the actual, stored name of the declaration, which may be a special



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


[clang] bc29634 - [clang][NFC] Remove an old workaround for MSVC 2013

2020-08-05 Thread Bruno Ricci via cfe-commits

Author: Bruno Ricci
Date: 2020-08-05T13:54:37+01:00
New Revision: bc29634b93acf2e55c82dd906f0d9af196c66ff3

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

LOG: [clang][NFC] Remove an old workaround for MSVC 2013

Added: 


Modified: 
clang/include/clang/AST/DeclTemplate.h

Removed: 




diff  --git a/clang/include/clang/AST/DeclTemplate.h 
b/clang/include/clang/AST/DeclTemplate.h
index e9c4879b41e8..4feb1d45251d 100644
--- a/clang/include/clang/AST/DeclTemplate.h
+++ b/clang/include/clang/AST/DeclTemplate.h
@@ -204,10 +204,6 @@ class TemplateParameterList final
  bool OmitTemplateKW = false) const;
   void print(raw_ostream &Out, const ASTContext &Context,
  const PrintingPolicy &Policy, bool OmitTemplateKW = false) const;
-
-public:
-  // FIXME: workaround for MSVC 2013; remove when no longer needed
-  using FixedSizeStorageOwner = TrailingObjects::FixedSizeStorageOwner;
 };
 
 /// Stores a list of template parameters and the associated



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


[clang] 1970145 - [clang][nearly-NFC] Remove some superfluous uses of NamedDecl::getNameAsString

2020-08-05 Thread Bruno Ricci via cfe-commits

Author: Bruno Ricci
Date: 2020-08-05T13:54:37+01:00
New Revision: 19701458d4691ee7ec59e5aa7217a479b0fb10e7

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

LOG: [clang][nearly-NFC] Remove some superfluous uses of 
NamedDecl::getNameAsString

`OS << ND->getDeclName();` is equivalent to `OS << ND->getNameAsString();`
without the extra temporary string.

This is not quite a NFC since two uses of `getNameAsString` in a
diagnostic are replaced, which results in the named entity being
quoted with additional "'"s (ie: 'var' instead of var).

Added: 


Modified: 
clang-tools-extra/clang-include-fixer/find-all-symbols/FindAllSymbols.cpp
clang-tools-extra/clang-move/HelperDeclRefGraph.cpp
clang-tools-extra/clang-move/Move.cpp
clang/lib/AST/ASTDiagnostic.cpp
clang/lib/AST/Interp/Disasm.cpp
clang/lib/AST/TextNodeDumper.cpp
clang/lib/Frontend/FrontendAction.cpp
clang/lib/Index/FileIndexRecord.cpp
clang/lib/Sema/AnalysisBasedWarnings.cpp
clang/lib/Sema/SemaChecking.cpp
clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp

clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
clang/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
clang/lib/Tooling/Refactoring/ASTSelection.cpp
clang/test/Index/error-on-deserialized.c
clang/test/SemaCXX/warn-msvc-enum-bitfield.cpp

Removed: 




diff  --git 
a/clang-tools-extra/clang-include-fixer/find-all-symbols/FindAllSymbols.cpp 
b/clang-tools-extra/clang-include-fixer/find-all-symbols/FindAllSymbols.cpp
index 7d540d83037b..70d4d7cfdff3 100644
--- a/clang-tools-extra/clang-include-fixer/find-all-symbols/FindAllSymbols.cpp
+++ b/clang-tools-extra/clang-include-fixer/find-all-symbols/FindAllSymbols.cpp
@@ -99,7 +99,7 @@ CreateSymbolInfo(const NamedDecl *ND, const SourceManager &SM,
 
   SourceLocation Loc = SM.getExpansionLoc(ND->getLocation());
   if (!Loc.isValid()) {
-llvm::errs() << "Declaration " << ND->getNameAsString() << "("
+llvm::errs() << "Declaration " << ND->getDeclName() << "("
  << ND->getDeclKindName()
  << ") has invalid declaration location.";
 return llvm::None;

diff  --git a/clang-tools-extra/clang-move/HelperDeclRefGraph.cpp 
b/clang-tools-extra/clang-move/HelperDeclRefGraph.cpp
index 271bd3d6ef20..a9b773353fe6 100644
--- a/clang-tools-extra/clang-move/HelperDeclRefGraph.cpp
+++ b/clang-tools-extra/clang-move/HelperDeclRefGraph.cpp
@@ -116,7 +116,7 @@ void HelperDeclRGBuilder::run(
 const auto *DC = Result.Nodes.getNodeAs("dc");
 assert(DC);
 LLVM_DEBUG(llvm::dbgs() << "Find helper function usage: "
-<< FuncRef->getDecl()->getNameAsString() << " ("
+<< FuncRef->getDecl()->getDeclName() << " ("
 << FuncRef->getDecl() << ")\n");
 RG->addEdge(
 getOutmostClassOrFunDecl(DC->getCanonicalDecl()),
@@ -126,7 +126,7 @@ void HelperDeclRGBuilder::run(
 const auto *DC = Result.Nodes.getNodeAs("dc");
 assert(DC);
 LLVM_DEBUG(llvm::dbgs()
-   << "Find helper class usage: " << UsedClass->getNameAsString()
+   << "Find helper class usage: " << UsedClass->getDeclName()
<< " (" << UsedClass << ")\n");
 RG->addEdge(getOutmostClassOrFunDecl(DC->getCanonicalDecl()), UsedClass);
   }

diff  --git a/clang-tools-extra/clang-move/Move.cpp 
b/clang-tools-extra/clang-move/Move.cpp
index 3f09f68a8046..24f819ca4ca2 100644
--- a/clang-tools-extra/clang-move/Move.cpp
+++ b/clang-tools-extra/clang-move/Move.cpp
@@ -675,8 +675,8 @@ void ClangMoveTool::run(const 
ast_matchers::MatchFinder::MatchResult &Result) {
  Result.Nodes.getNodeAs("helper_decls")) {
 MovedDecls.push_back(ND);
 HelperDeclarations.push_back(ND);
-LLVM_DEBUG(llvm::dbgs() << "Add helper : " << ND->getNameAsString() << " ("
-<< ND << ")\n");
+LLVM_DEBUG(llvm::dbgs()
+   << "Add helper : " << ND->getDeclName() << " (" << ND << ")\n");
   } else if (const auto *UD = Result.Nodes.getNodeAs("using_decl")) 
{
 MovedDecls.push_back(UD);
   }
@@ -735,12 +735,12 @@ void ClangMoveTool::removeDeclsInOldFiles() {
 // We remove the helper declarations which are not used in the old.cc after
 // moving the given declarations.
 for (const auto *D : HelperDeclarations) {
-  LLVM_DEBUG(llvm::dbgs() << "Check helper is used: "
-  << D->getNameAsString() << " (" << D << ")\n");
+  LLVM_DEB

[clang] 94b4311 - [clang][NFCI] Get rid of ConstantMatrixTypeBitfields to avoid increasing the size of every type.

2020-08-05 Thread Bruno Ricci via cfe-commits

Author: Bruno Ricci
Date: 2020-08-05T13:54:37+01:00
New Revision: 94b43118e2203fed8ca0377ae762c08189aa6f3d

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

LOG: [clang][NFCI] Get rid of ConstantMatrixTypeBitfields to avoid increasing 
the size of every type.

sizeof(ConstantMatrixTypeBitfields) > 8 which increases the size of every type.
This was not detected because no corresponding static_assert for its size was
added.

To prevent this from occuring again replace the various static_asserts for
the size of each of the bit-field classes by a single static_assert for the
size of Type.

I have left ConstantMatrixType::MaxElementsPerDimension unchanged since
the limit is exercised by multiple tests.

Added: 


Modified: 
clang/include/clang/AST/Type.h
clang/lib/AST/Type.cpp

Removed: 




diff  --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h
index 7d943ebc78c0..df9c926ce902 100644
--- a/clang/include/clang/AST/Type.h
+++ b/clang/include/clang/AST/Type.h
@@ -1680,19 +1680,6 @@ class alignas(8) Type : public ExtQualsTypeCommonBase {
 uint32_t NumElements;
   };
 
-  class ConstantMatrixTypeBitfields {
-friend class ConstantMatrixType;
-
-unsigned : NumTypeBits;
-
-/// Number of rows and columns. Using 20 bits allows supporting very large
-/// matrixes, while keeping 24 bits to accommodate NumTypeBits.
-unsigned NumRows : 20;
-unsigned NumColumns : 20;
-
-static constexpr uint32_t MaxElementsPerDimension = (1 << 20) - 1;
-  };
-
   class AttributedTypeBitfields {
 friend class AttributedType;
 
@@ -1802,46 +1789,11 @@ class alignas(8) Type : public ExtQualsTypeCommonBase {
 TypeWithKeywordBitfields TypeWithKeywordBits;
 ElaboratedTypeBitfields ElaboratedTypeBits;
 VectorTypeBitfields VectorTypeBits;
-ConstantMatrixTypeBitfields ConstantMatrixTypeBits;
 SubstTemplateTypeParmPackTypeBitfields SubstTemplateTypeParmPackTypeBits;
 TemplateSpecializationTypeBitfields TemplateSpecializationTypeBits;
 DependentTemplateSpecializationTypeBitfields
   DependentTemplateSpecializationTypeBits;
 PackExpansionTypeBitfields PackExpansionTypeBits;
-
-static_assert(sizeof(TypeBitfields) <= 8,
-  "TypeBitfields is larger than 8 bytes!");
-static_assert(sizeof(ArrayTypeBitfields) <= 8,
-  "ArrayTypeBitfields is larger than 8 bytes!");
-static_assert(sizeof(AttributedTypeBitfields) <= 8,
-  "AttributedTypeBitfields is larger than 8 bytes!");
-static_assert(sizeof(AutoTypeBitfields) <= 8,
-  "AutoTypeBitfields is larger than 8 bytes!");
-static_assert(sizeof(BuiltinTypeBitfields) <= 8,
-  "BuiltinTypeBitfields is larger than 8 bytes!");
-static_assert(sizeof(FunctionTypeBitfields) <= 8,
-  "FunctionTypeBitfields is larger than 8 bytes!");
-static_assert(sizeof(ObjCObjectTypeBitfields) <= 8,
-  "ObjCObjectTypeBitfields is larger than 8 bytes!");
-static_assert(sizeof(ReferenceTypeBitfields) <= 8,
-  "ReferenceTypeBitfields is larger than 8 bytes!");
-static_assert(sizeof(TypeWithKeywordBitfields) <= 8,
-  "TypeWithKeywordBitfields is larger than 8 bytes!");
-static_assert(sizeof(ElaboratedTypeBitfields) <= 8,
-  "ElaboratedTypeBitfields is larger than 8 bytes!");
-static_assert(sizeof(VectorTypeBitfields) <= 8,
-  "VectorTypeBitfields is larger than 8 bytes!");
-static_assert(sizeof(SubstTemplateTypeParmPackTypeBitfields) <= 8,
-  "SubstTemplateTypeParmPackTypeBitfields is larger"
-  " than 8 bytes!");
-static_assert(sizeof(TemplateSpecializationTypeBitfields) <= 8,
-  "TemplateSpecializationTypeBitfields is larger"
-  " than 8 bytes!");
-static_assert(sizeof(DependentTemplateSpecializationTypeBitfields) <= 8,
-  "DependentTemplateSpecializationTypeBitfields is larger"
-  " than 8 bytes!");
-static_assert(sizeof(PackExpansionTypeBitfields) <= 8,
-  "PackExpansionTypeBitfields is larger than 8 bytes");
   };
 
 private:
@@ -1858,6 +1810,10 @@ class alignas(8) Type : public ExtQualsTypeCommonBase {
   Type(TypeClass tc, QualType canon, TypeDependence Dependence)
   : ExtQualsTypeCommonBase(this,
canon.isNull() ? QualType(this_(), 0) : canon) {
+static_assert(sizeof(*this) <= 8 + sizeof(ExtQualsTypeCommonBase),
+  "changing bitfields changed sizeof(Type)!");
+static_assert(alignof(decltype(*this)) % sizeof(void *) == 0,
+  "Insufficient alignment!");
 TypeBits.TC = tc;
 TypeBits.Depende

[clang] 98b4b45 - [clang][NFC] Add a test showcasing an unnamed template parameter in a diagnostic

2020-08-05 Thread Bruno Ricci via cfe-commits

Author: Bruno Ricci
Date: 2020-08-05T13:54:36+01:00
New Revision: 98b4b4570542a255e9a81e4a349183402a2d478d

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

LOG: [clang][NFC] Add a test showcasing an unnamed template parameter in a 
diagnostic

Added: 


Modified: 
clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp

Removed: 




diff  --git a/clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp 
b/clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
index 2a3f312ebd8e..e992c7c916f3 100644
--- a/clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
+++ b/clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
@@ -172,6 +172,10 @@ namespace nondeducible {
   template
   X(float) -> X; // ok
+
+  template  struct UnnamedTemplateParam {};
+  template   // expected-note 
{{non-deducible template parameter (anonymous)}}
+  UnnamedTemplateParam() -> UnnamedTemplateParam; // expected-error 
{{deduction guide template contains a template parameter that cannot be 
deduced}}
 }
 
 namespace default_args_from_ctor {



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


[clang] f7a039d - [clang][NFC] DeclPrinter: use NamedDecl::getDeclName instead of NamedDecl::printName to print the name of enumerations, namespaces and template parameters.

2020-08-05 Thread Bruno Ricci via cfe-commits

Author: Bruno Ricci
Date: 2020-08-05T13:54:38+01:00
New Revision: f7a039de7af7b83105f3e0345d65dceda1a0e0d4

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

LOG: [clang][NFC] DeclPrinter: use NamedDecl::getDeclName instead of 
NamedDecl::printName to print the name of enumerations, namespaces and template 
parameters.

NamedDecl::printName will print the pretty-printed name of the entity, which
is not what we want here (we should print "enum { e };" instead of "enum
(unnamed enum at input.cc:1:5) { e };").

For now only DecompositionDecl and MDGuidDecl have an overloaded printName so
this does not result in any functional change, but this change is needed since
I will be adding overloads to better handle unnamed entities in diagnostics.

Added: 


Modified: 
clang/lib/AST/DeclPrinter.cpp
clang/unittests/AST/DeclPrinterTest.cpp

Removed: 




diff  --git a/clang/lib/AST/DeclPrinter.cpp b/clang/lib/AST/DeclPrinter.cpp
index 2e48b2b46c4d..ca64f8f6cfbe 100644
--- a/clang/lib/AST/DeclPrinter.cpp
+++ b/clang/lib/AST/DeclPrinter.cpp
@@ -528,7 +528,8 @@ void DeclPrinter::VisitEnumDecl(EnumDecl *D) {
 
   prettyPrintAttributes(D);
 
-  Out << ' ' << *D;
+  if (D->getDeclName())
+Out << ' ' << D->getDeclName();
 
   if (D->isFixed())
 Out << " : " << D->getIntegerType().stream(Policy);
@@ -933,7 +934,12 @@ void DeclPrinter::VisitStaticAssertDecl(StaticAssertDecl 
*D) {
 void DeclPrinter::VisitNamespaceDecl(NamespaceDecl *D) {
   if (D->isInline())
 Out << "inline ";
-  Out << "namespace " << *D << " {\n";
+
+  Out << "namespace ";
+  if (D->getDeclName())
+Out << D->getDeclName() << ' ';
+  Out << "{\n";
+
   VisitDeclContext(D);
   Indent() << "}";
 }
@@ -1091,10 +1097,15 @@ void DeclPrinter::VisitTemplateDecl(const TemplateDecl 
*D) {
 
   if (const TemplateTemplateParmDecl *TTP =
 dyn_cast(D)) {
-Out << "class ";
+Out << "class";
+
 if (TTP->isParameterPack())
-  Out << "...";
-Out << D->getName();
+  Out << " ...";
+else if (TTP->getDeclName())
+  Out << ' ';
+
+if (TTP->getDeclName())
+  Out << TTP->getDeclName();
   } else if (auto *TD = D->getTemplatedDecl())
 Visit(TD);
   else if (const auto *Concept = dyn_cast(D)) {
@@ -1216,7 +1227,7 @@ void DeclPrinter::PrintObjCTypeParams(ObjCTypeParamList 
*Params) {
   break;
 }
 
-Out << Param->getDeclName().getAsString();
+Out << Param->getDeclName();
 
 if (Param->hasExplicitBound()) {
   Out << " : " << Param->getUnderlyingType().getAsString(Policy);
@@ -1695,10 +1706,11 @@ void DeclPrinter::VisitTemplateTypeParmDecl(const 
TemplateTypeParmDecl *TTP) {
 
   if (TTP->isParameterPack())
 Out << " ...";
-  else if (!TTP->getName().empty())
+  else if (TTP->getDeclName())
 Out << ' ';
 
-  Out << *TTP;
+  if (TTP->getDeclName())
+Out << TTP->getDeclName();
 
   if (TTP->hasDefaultArgument()) {
 Out << " = ";

diff  --git a/clang/unittests/AST/DeclPrinterTest.cpp 
b/clang/unittests/AST/DeclPrinterTest.cpp
index 939c8b52c12c..38e46a378b47 100644
--- a/clang/unittests/AST/DeclPrinterTest.cpp
+++ b/clang/unittests/AST/DeclPrinterTest.cpp
@@ -37,6 +37,7 @@ void PrintDecl(raw_ostream &Out, const ASTContext *Context, 
const Decl *D,
PrintingPolicyModifier PolicyModifier) {
   PrintingPolicy Policy = Context->getPrintingPolicy();
   Policy.TerseOutput = true;
+  Policy.Indentation = 0;
   if (PolicyModifier)
 PolicyModifier(Policy);
   D->print(Out, Policy, /*Indentation*/ 0, /*PrintInstantiation*/ false);
@@ -162,14 +163,21 @@ ::testing::AssertionResult PrintedDeclCXX11nonMSCMatches(
 }
 
 ::testing::AssertionResult
-PrintedDeclCXX1ZMatches(StringRef Code, const DeclarationMatcher &NodeMatch,
-StringRef ExpectedPrinted) {
-  std::vector Args(1, "-std=c++1z");
-  return PrintedDeclMatches(Code,
-Args,
-NodeMatch,
-ExpectedPrinted,
-"input.cc");
+PrintedDeclCXX17Matches(StringRef Code, const DeclarationMatcher &NodeMatch,
+StringRef ExpectedPrinted,
+PrintingPolicyModifier PolicyModifier = nullptr) {
+  std::vector Args(1, "-std=c++17");
+  return PrintedDeclMatches(Code, Args, NodeMatch, ExpectedPrinted, "input.cc",
+PolicyModifier);
+}
+
+::testing::AssertionResult
+PrintedDeclC11Matches(StringRef Code, const DeclarationMatcher &NodeMatch,
+  StringRef ExpectedPrinted,
+  PrintingPolicyModifier PolicyModifier = nullptr) {
+  std::vector Args(1, "-std=c11");
+  return PrintedDeclMatches(Code, Args, NodeMatch, ExpectedPrinted, "input.c",
+Polic

[PATCH] D79744: clang: Use byref for aggregate kernel arguments

2020-08-05 Thread Matt Arsenault via Phabricator via cfe-commits
arsenm added inline comments.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:1997
   case ABIArgInfo::Ignore:
+  case ABIArgInfo::IndirectAliased:
 return false;

arsenm wrote:
> rjmccall wrote:
> > arsenm wrote:
> > > rjmccall wrote:
> > > > In principle, this can be `inreg` just as much as Indirect can.
> > > The IR verifier currently will reject byref + inreg
> > Why?  `inreg` is essentially orthogonal.
> Mostly inherited from the other similar attribute handling. It can be lifted 
> if there's a use
Plus the name here is isArgInAlloca; this is not necessarily passed in an alloca


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

https://reviews.llvm.org/D79744

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


[clang] 4dcbb9c - [clang] Add -fno-delayed-template-parsing to the added unit tests in DeclPrinterTest.cpp

2020-08-05 Thread Bruno Ricci via cfe-commits

Author: Bruno Ricci
Date: 2020-08-05T14:13:05+01:00
New Revision: 4dcbb9cef71afa549afe8f6b4d335b1c996f8079

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

LOG: [clang] Add -fno-delayed-template-parsing to the added unit tests in 
DeclPrinterTest.cpp

Added: 


Modified: 
clang/unittests/AST/DeclPrinterTest.cpp

Removed: 




diff  --git a/clang/unittests/AST/DeclPrinterTest.cpp 
b/clang/unittests/AST/DeclPrinterTest.cpp
index 38e46a378b47..6b7ceac3cb02 100644
--- a/clang/unittests/AST/DeclPrinterTest.cpp
+++ b/clang/unittests/AST/DeclPrinterTest.cpp
@@ -153,8 +153,7 @@ ::testing::AssertionResult PrintedDeclCXX11nonMSCMatches(
   StringRef Code,
   const DeclarationMatcher &NodeMatch,
   StringRef ExpectedPrinted) {
-  std::vector Args(1, "-std=c++11");
-  Args.push_back("-fno-delayed-template-parsing");
+  std::vector Args{"-std=c++11", "-fno-delayed-template-parsing"};
   return PrintedDeclMatches(Code,
 Args,
 NodeMatch,
@@ -166,7 +165,7 @@ ::testing::AssertionResult
 PrintedDeclCXX17Matches(StringRef Code, const DeclarationMatcher &NodeMatch,
 StringRef ExpectedPrinted,
 PrintingPolicyModifier PolicyModifier = nullptr) {
-  std::vector Args(1, "-std=c++17");
+  std::vector Args{"-std=c++17", "-fno-delayed-template-parsing"};
   return PrintedDeclMatches(Code, Args, NodeMatch, ExpectedPrinted, "input.cc",
 PolicyModifier);
 }



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


[PATCH] D84736: [analyzer][RFC] Handle pointer difference of ElementRegion and SymbolicRegion

2020-08-05 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:1035-1038
+return evalBinOpNN(state, BO_Mul, Index,
+   makeArrayIndex(SingleElementSize.getQuantity()),
+   ArrayIndexTy)
+.castAs();

steakhal wrote:
> vsavchenko wrote:
> > I would prefer having a comment for this line with a very simple formula of 
> > the thing we are calculating.  I think it can increase the readability of 
> > the code because when there is a call accepting a whole bunch of arguments 
> > it is never obvious and it always takes time to parse through.
> Sure, I will add something like this:
> ```
> // T buf[n];   Element{buf,2} --> 2 * sizeof(T)
> ```
Yeah, something like this would be good.  I would prefer more explicit version 
of it simply because `Stmt;   Smth` can be a bit confusing for notation. 



Comment at: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:1041
+
+  // If both has the same memory region, and the index has a concrete 
value,
+  // we can evaluate equality operators.

steakhal wrote:
> vsavchenko wrote:
> > This comment is a bit deceiving IMO.  It returns a concrete value even when 
> > `HasSameMemRegions` is false, but from the comment it seems like we 
> > evaluate the operator ONLY when `HasSameMemRegions` is true.
> No, the comment is up to date.
> This lambda handles both equality and inequality operators, and this is why 
> its called EvaluateEqualityOperators.
> 
> EQResult will be true only if the two memory regions are the same and the 
> index is zero.
> The result of the **inequality**  is just the negated version of the result 
> of equality.
> If both have the same and the index has a concrete value, we can evaluate 
> equality operators.

In this setting, I assume that we talk about "if and only if", and in any case 
other than "//both have the same and the index has a concrete value//" we 
**can't** evaluate equality operators.
Also, I didn't say that it is necessarily incorrect, I said that it is 
deceiving because if it confused me, it can confuse other people as well.

I would prefer something more like this:

```
// For a situation, a OP b + index, where OP is equality operator, 
// we can infer the result for known `index` values.
// We are able to do this because ...
```



Comment at: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:1068
+}
+  } else if (LeftER && !RightER) {
+NonLoc LeftIndex = ByteOffsetOfElement(LeftER);

steakhal wrote:
> vsavchenko wrote:
> > I think it's better to add comments for each case describing what is the 
> > situation we handle here in a simplified form.
> > And for each return within the case - a short comment with the reason.
> I'm not sure if it's necessary.
> I have already described these 3 cases at the beginning.
> Quote:
> ```
> // Handle: \forall MemRegion X, \forall NonLoc n, m:
> //  - Element{X,n} OP Element{X,m}
> //  - Element{X,n} OP X
> //  -X OP Element{X,n}
> // We don't handle here nested ElementRegions like in the this expression:
> // Element{Element{x,3,int [10]},5,int}  ==  Element{x,35,int}
> ```
> If you still insist, I will defenately add more comments though.
In that comment, you don't say anything about `OP` and how we //handle// those.

Also I want to add that you probably want to add an arbitrary `MemRegion Y` as 
well because you handle not only cases when `MemRegion`s are same.


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

https://reviews.llvm.org/D84736

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


[PATCH] D85301: [clang-tidy] Fix crashes in bugprone-bad-signal-to-kill-thread check.

2020-08-05 Thread Aleksandr Platonov via Phabricator via cfe-commits
ArcsinX created this revision.
Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, 
dexonsmith, steven_wu, jkorous, hiraditya, xazax.hun.
Herald added a project: clang.
ArcsinX requested review of this revision.
Herald added a subscriber: ilya-biryukov.

This patch fixes crashes in the following cases:

- `SIGTERM` is not a literal
- `SIGTERM` is a literal but `Token::PtrData == nullptr` (happens in `clangd`, 
because `clang-tidy` in `clangd` has some limitations to ensure reasonable 
performance)
- `SIGTERM` undefined after definition


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85301

Files:
  clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-bad-signal-to-kill-thread-sigterm-not-a-literal.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-bad-signal-to-kill-thread-undef-sigterm.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-bad-signal-to-kill-thread-undef-sigterm.cpp
===
--- /dev/null
+++ 
clang-tools-extra/test/clang-tidy/checkers/bugprone-bad-signal-to-kill-thread-undef-sigterm.cpp
@@ -0,0 +1,11 @@
+// RUN: clang-tidy %s --checks="-*,bugprone-bad-signal-to-kill-thread"
+
+#define SIGTERM 15
+#undef SIGTERM // no-crash
+using pthread_t = int;
+int pthread_kill(pthread_t thread, int sig);
+
+int func() {
+  pthread_t thread;
+  return pthread_kill(thread, 0);
+}
Index: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-bad-signal-to-kill-thread-sigterm-not-a-literal.cpp
===
--- /dev/null
+++ 
clang-tools-extra/test/clang-tidy/checkers/bugprone-bad-signal-to-kill-thread-sigterm-not-a-literal.cpp
@@ -0,0 +1,10 @@
+// RUN: clang-tidy %s --checks="-*,bugprone-bad-signal-to-kill-thread"
+
+#define SIGTERM ((unsigned)15) // no-crash
+using pthread_t = int;
+int pthread_kill(pthread_t thread, int sig);
+
+int func() {
+  pthread_t thread;
+  return pthread_kill(thread, 0);
+}
Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -438,6 +438,21 @@
   EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre());
 }
 
+TEST(DiagnosticTest, ClangTidyBadSignalToKillThread) {
+  Annotations Main(R"cpp(
+#define SIGTERM 15
+using pthread_t = int;
+int pthread_kill(pthread_t thread, int sig);
+int func() {
+  pthread_t thread;
+  return pthread_kill(thread, 0);
+}
+  )cpp");
+  TestTU TU = TestTU::withCode(Main.code());
+  TU.ClangTidyChecks = "-*,bugprone-bad-signal-to-kill-thread";
+  EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre()); // no-crash
+}
+
 TEST(DiagnosticsTest, Preprocessor) {
   // This looks like a preamble, but there's an #else in the middle!
   // Check that:
Index: clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp
@@ -30,7 +30,8 @@
 
 void BadSignalToKillThreadCheck::check(const MatchFinder::MatchResult &Result) 
{
   const auto IsSigterm = [](const auto &KeyValue) -> bool {
-return KeyValue.first->getName() == "SIGTERM";
+return KeyValue.first->getName() == "SIGTERM" &&
+   KeyValue.first->hasMacroDefinition();
   };
   const auto TryExpandAsInteger =
   [](Preprocessor::macro_iterator It) -> Optional {
@@ -38,6 +39,8 @@
   return llvm::None;
 const MacroInfo *MI = PP->getMacroInfo(It->first);
 const Token &T = MI->tokens().back();
+if (!T.isLiteral() || !T.getLiteralData())
+  return llvm::None;
 StringRef ValueStr = StringRef(T.getLiteralData(), T.getLength());
 
 llvm::APInt IntValue;


Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-bad-signal-to-kill-thread-undef-sigterm.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-bad-signal-to-kill-thread-undef-sigterm.cpp
@@ -0,0 +1,11 @@
+// RUN: clang-tidy %s --checks="-*,bugprone-bad-signal-to-kill-thread"
+
+#define SIGTERM 15
+#undef SIGTERM // no-crash
+using pthread_t = int;
+int pthread_kill(pthread_t thread, int sig);
+
+int func() {
+  pthread_t thread;
+  return pthread_kill(thread, 0);
+}
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-bad-signal-to-kill-thread-sigterm-not-a-literal.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-bad-signal-to-kill-thread-sigterm-not-a-literal.cpp
@@ -0,0 +1,10 

[PATCH] D85305: [SyntaxTree] Remove dead code on dump functions

2020-08-05 Thread Eduardo Caldas via Phabricator via cfe-commits
eduucaldas created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
eduucaldas requested review of this revision.

- `Node::dumpTokens` was never used.
- `::dumpTokens(llvm::raw_ostream, ArrayRef, const

SourceManager &SM)` was used twice, once by `Node::dumpTokens`.
Additionally it always received as parameter a `syntax::Token *` pointing to 
one token only, instead of an `ArrayRef`

I removed the first and inlined the simplified version of the second in its 
only caller


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85305

Files:
  clang/include/clang/Tooling/Syntax/Tree.h
  clang/lib/Tooling/Syntax/Tree.cpp


Index: clang/lib/Tooling/Syntax/Tree.cpp
===
--- clang/lib/Tooling/Syntax/Tree.cpp
+++ clang/lib/Tooling/Syntax/Tree.cpp
@@ -135,24 +135,6 @@
 }
 
 namespace {
-static void dumpTokens(llvm::raw_ostream &OS, ArrayRef Tokens,
-   const SourceManager &SM) {
-  assert(!Tokens.empty());
-  bool First = true;
-  for (const auto &T : Tokens) {
-if (!First)
-  OS << " ";
-else
-  First = false;
-// Handle 'eof' separately, calling text() on it produces an empty string.
-if (T.kind() == tok::eof) {
-  OS << "";
-  continue;
-}
-OS << T.text(SM);
-  }
-}
-
 static void dumpTree(llvm::raw_ostream &OS, const syntax::Node *N,
  const syntax::Arena &A, std::vector IndentMask) {
   std::string Marks;
@@ -166,7 +148,13 @@
 OS << Marks << ": ";
 
   if (auto *L = llvm::dyn_cast(N)) {
-dumpTokens(OS, *L->token(), A.sourceManager());
+auto *Token = L->token();
+assert(Token);
+// Handle 'eof' separately, calling text() on it produces an empty string.
+if (Token->kind() == tok::eof)
+  OS << "";
+else
+  OS << Token->text(A.sourceManager());
 OS << "\n";
 return;
   }
@@ -174,7 +162,8 @@
   auto *T = llvm::cast(N);
   OS << T->kind() << "\n";
 
-  for (auto It = T->firstChild(); It != nullptr; It = It->nextSibling()) {
+  for (const auto *It = T->firstChild(); It != nullptr;
+   It = It->nextSibling()) {
 for (bool Filled : IndentMask) {
   if (Filled)
 OS << "| ";
@@ -201,19 +190,6 @@
   return std::move(OS.str());
 }
 
-std::string syntax::Node::dumpTokens(const Arena &A) const {
-  std::string Storage;
-  llvm::raw_string_ostream OS(Storage);
-  traverse(this, [&](const syntax::Node *N) {
-auto *L = llvm::dyn_cast(N);
-if (!L)
-  return;
-::dumpTokens(OS, *L->token(), A.sourceManager());
-OS << " ";
-  });
-  return OS.str();
-}
-
 void syntax::Node::assertInvariants() const {
 #ifndef NDEBUG
   if (isDetached())
Index: clang/include/clang/Tooling/Syntax/Tree.h
===
--- clang/include/clang/Tooling/Syntax/Tree.h
+++ clang/include/clang/Tooling/Syntax/Tree.h
@@ -107,8 +107,6 @@
 
   /// Dumps the structure of a subtree. For debugging and testing purposes.
   std::string dump(const Arena &A) const;
-  /// Dumps the tokens forming this subtree.
-  std::string dumpTokens(const Arena &A) const;
 
   /// Asserts invariants on this node of the tree and its immediate children.
   /// Will not recurse into the subtree. No-op if NDEBUG is set.


Index: clang/lib/Tooling/Syntax/Tree.cpp
===
--- clang/lib/Tooling/Syntax/Tree.cpp
+++ clang/lib/Tooling/Syntax/Tree.cpp
@@ -135,24 +135,6 @@
 }
 
 namespace {
-static void dumpTokens(llvm::raw_ostream &OS, ArrayRef Tokens,
-   const SourceManager &SM) {
-  assert(!Tokens.empty());
-  bool First = true;
-  for (const auto &T : Tokens) {
-if (!First)
-  OS << " ";
-else
-  First = false;
-// Handle 'eof' separately, calling text() on it produces an empty string.
-if (T.kind() == tok::eof) {
-  OS << "";
-  continue;
-}
-OS << T.text(SM);
-  }
-}
-
 static void dumpTree(llvm::raw_ostream &OS, const syntax::Node *N,
  const syntax::Arena &A, std::vector IndentMask) {
   std::string Marks;
@@ -166,7 +148,13 @@
 OS << Marks << ": ";
 
   if (auto *L = llvm::dyn_cast(N)) {
-dumpTokens(OS, *L->token(), A.sourceManager());
+auto *Token = L->token();
+assert(Token);
+// Handle 'eof' separately, calling text() on it produces an empty string.
+if (Token->kind() == tok::eof)
+  OS << "";
+else
+  OS << Token->text(A.sourceManager());
 OS << "\n";
 return;
   }
@@ -174,7 +162,8 @@
   auto *T = llvm::cast(N);
   OS << T->kind() << "\n";
 
-  for (auto It = T->firstChild(); It != nullptr; It = It->nextSibling()) {
+  for (const auto *It = T->firstChild(); It != nullptr;
+   It = It->nextSibling()) {
 for (bool Filled : IndentMask) {
   if (Filled)
 OS << "| ";
@@ -201,19 +190,6 @@
   return std::move(OS.str());
 }
 
-std::string syntax::Node::dumpTokens(

[PATCH] D85304: [clang-format] fix BreakBeforeBraces.MultiLine with for each macros

2020-08-05 Thread Vincent Thiberville via Phabricator via cfe-commits
vthib created this revision.
vthib added reviewers: MyDeveloperDay, mitchell-stellar.
vthib added projects: clang-format, clang, clang-tools-extra.
Herald added a subscriber: cfe-commits.
vthib requested review of this revision.

The MultiLine option in BreakBeforeBraces was only handling standard
control statement, leading to invalid indentation with for each macros:

Previous behavior:

  /* invalid: brace should be on the same line */
  Q_FOREACH(int a; list)
  {
  foo();
  }
  
  /* valid */
  Q_FOREACH(int longVariable;
list)
  {
  foo();
  }

To fix this, simply add the TT_ForEachMacro kind in the list of
recognized control statements for the multiline option.

This is a fix for https://bugs.llvm.org/show_bug.cgi?id=44632


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85304

Files:
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1663,6 +1663,20 @@
 "  foo();\n"
 "}",
 format("for(int i=0;i<10;++i){foo();}", Style));
+  EXPECT_EQ("foreach (int i,\n"
+" list)\n"
+"{\n"
+"  foo();\n"
+"}",
+format("foreach(int i, list){foo();}", Style));
+  Style.ColumnLimit =
+  40; // to concentrate at brace wrapping, not line wrap due to column 
limit
+  EXPECT_EQ("foreach (int i, list) {\n"
+"  foo();\n"
+"}",
+format("foreach(int i, list){foo();}", Style));
+  Style.ColumnLimit =
+  20; // to concentrate at brace wrapping, not line wrap due to column 
limit
   EXPECT_EQ("while (foo || bar ||\n"
 "   baz)\n"
 "{\n"
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -309,7 +309,8 @@
 // Try to merge a control statement block with left brace wrapped
 if (I[1]->First->is(tok::l_brace) &&
 (TheLine->First->isOneOf(tok::kw_if, tok::kw_while, tok::kw_for,
- tok::kw_switch, tok::kw_try, tok::kw_do) ||
+ tok::kw_switch, tok::kw_try, tok::kw_do,
+ TT_ForEachMacro) ||
  (TheLine->First->is(tok::r_brace) && TheLine->First->Next &&
   TheLine->First->Next->isOneOf(tok::kw_else, tok::kw_catch))) &&
 Style.BraceWrapping.AfterControlStatement ==


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1663,6 +1663,20 @@
 "  foo();\n"
 "}",
 format("for(int i=0;i<10;++i){foo();}", Style));
+  EXPECT_EQ("foreach (int i,\n"
+" list)\n"
+"{\n"
+"  foo();\n"
+"}",
+format("foreach(int i, list){foo();}", Style));
+  Style.ColumnLimit =
+  40; // to concentrate at brace wrapping, not line wrap due to column limit
+  EXPECT_EQ("foreach (int i, list) {\n"
+"  foo();\n"
+"}",
+format("foreach(int i, list){foo();}", Style));
+  Style.ColumnLimit =
+  20; // to concentrate at brace wrapping, not line wrap due to column limit
   EXPECT_EQ("while (foo || bar ||\n"
 "   baz)\n"
 "{\n"
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -309,7 +309,8 @@
 // Try to merge a control statement block with left brace wrapped
 if (I[1]->First->is(tok::l_brace) &&
 (TheLine->First->isOneOf(tok::kw_if, tok::kw_while, tok::kw_for,
- tok::kw_switch, tok::kw_try, tok::kw_do) ||
+ tok::kw_switch, tok::kw_try, tok::kw_do,
+ TT_ForEachMacro) ||
  (TheLine->First->is(tok::r_brace) && TheLine->First->Next &&
   TheLine->First->Next->isOneOf(tok::kw_else, tok::kw_catch))) &&
 Style.BraceWrapping.AfterControlStatement ==
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D84422: [OpenMP] Fix `present` for exit from `omp target data`

2020-08-05 Thread Joel E. Denny via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG002d61db2b77: [OpenMP] Fix `present` for exit from `omp 
target data` (authored by jdenny).

Changed prior to commit:
  https://reviews.llvm.org/D84422?vs=281330&id=283224#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84422

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.h
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/test/OpenMP/target_data_codegen.cpp
  openmp/libomptarget/src/omptarget.cpp
  openmp/libomptarget/test/mapping/present/target_data_at_exit.c

Index: openmp/libomptarget/test/mapping/present/target_data_at_exit.c
===
--- /dev/null
+++ openmp/libomptarget/test/mapping/present/target_data_at_exit.c
@@ -0,0 +1,37 @@
+// RUN: %libomptarget-compile-aarch64-unknown-linux-gnu -fopenmp-version=51
+// RUN: %libomptarget-run-aarch64-unknown-linux-gnu 2>&1 \
+// RUN: | %fcheck-aarch64-unknown-linux-gnu
+
+// RUN: %libomptarget-compile-powerpc64-ibm-linux-gnu -fopenmp-version=51
+// RUN: %libomptarget-run-powerpc64-ibm-linux-gnu 2>&1 \
+// RUN: | %fcheck-powerpc64-ibm-linux-gnu
+
+// RUN: %libomptarget-compile-powerpc64le-ibm-linux-gnu -fopenmp-version=51
+// RUN: %libomptarget-run-powerpc64le-ibm-linux-gnu 2>&1 \
+// RUN: | %fcheck-powerpc64le-ibm-linux-gnu
+
+// RUN: %libomptarget-compile-x86_64-pc-linux-gnu -fopenmp-version=51
+// RUN: %libomptarget-run-x86_64-pc-linux-gnu 2>&1 \
+// RUN: | %fcheck-x86_64-pc-linux-gnu
+
+#include 
+
+int main() {
+  int i;
+
+#pragma omp target enter data map(alloc:i)
+
+  // i isn't present at the end of the target data region, but the "present"
+  // modifier is only checked at the beginning of a region.
+#pragma omp target data map(present, alloc: i)
+  {
+#pragma omp target exit data map(delete:i)
+  }
+
+  // CHECK-NOT: Libomptarget
+  // CHECK: success
+  // CHECK-NOT: Libomptarget
+  fprintf(stderr, "success\n");
+
+  return 0;
+}
Index: openmp/libomptarget/src/omptarget.cpp
===
--- openmp/libomptarget/src/omptarget.cpp
+++ openmp/libomptarget/src/omptarget.cpp
@@ -506,8 +506,14 @@
   DP("Mapping does not exist (%s)\n",
  (HasPresentModifier ? "'present' map type modifier" : "ignored"));
   if (HasPresentModifier) {
-// FIXME: This should not be an error on exit from "omp target data",
-// but it should be an error upon entering an "omp target exit data".
+// This should be an error upon entering an "omp target exit data".  It
+// should not be an error upon exiting an "omp target data" or "omp
+// target".  For "omp target data", Clang thus doesn't include present
+// modifiers for end calls.  For "omp target", we have not found a valid
+// OpenMP program for which the error matters: it appears that, if a
+// program can guarantee that data is present at the beginning of an
+// "omp target" region so that there's no error there, that data is also
+// guaranteed to be present at the end.
 MESSAGE("device mapping required by 'present' map type modifier does "
 "not exist for host address " DPxMOD " (%ld bytes)",
 DPxPTR(HstPtrBegin), DataSize);
Index: clang/test/OpenMP/target_data_codegen.cpp
===
--- clang/test/OpenMP/target_data_codegen.cpp
+++ clang/test/OpenMP/target_data_codegen.cpp
@@ -256,10 +256,16 @@
 double gc[100];
 
 // PRESENT=0x1000 | TARGET_PARAM=0x20 | TO=0x1 = 0x1021
-// CK1A: [[MTYPE00:@.+]] = {{.+}}constant [1 x i64] [i64 [[#0x1021]]]
+// CK1A: [[MTYPE00Begin:@.+]] = {{.+}}constant [1 x i64] [i64 [[#0x1021]]]
+
+// TARGET_PARAM=0x20 | TO=0x1 = 0x21
+// CK1A: [[MTYPE00End:@.+]] = {{.+}}constant [1 x i64] [i64 [[#0x21]]]
 
 // PRESENT=0x1000 | CLOSE=0x400 | TARGET_PARAM=0x20 | ALWAYS=0x4 | TO=0x1 = 0x1425
-// CK1A: [[MTYPE01:@.+]] = {{.+}}constant [1 x i64] [i64 [[#0x1425]]]
+// CK1A: [[MTYPE01Begin:@.+]] = {{.+}}constant [1 x i64] [i64 [[#0x1425]]]
+
+// CLOSE=0x400 | TARGET_PARAM=0x20 | ALWAYS=0x4 | TO=0x1 = 0x425
+// CK1A: [[MTYPE01End:@.+]] = {{.+}}constant [1 x i64] [i64 [[#0x425]]]
 
 // CK1A-LABEL: _Z3fooi
 void foo(int arg) {
@@ -267,7 +273,7 @@
   float lb[arg];
 
   // Region 00
-  // CK1A-DAG: call void @__tgt_target_data_begin_mapper(i64 -1, i32 1, i8** [[GEPBP:%.+]], i8** [[GEPP:%.+]], i[[sz:32|64]]* [[GEPS:%.+]], {{.+}}getelementptr {{.+}}[1 x i{{.+}}]* [[MTYPE00]]{{.+}})
+  // CK1A-DAG: call void @__tgt_target_data_begin_mapper(i64 -1, i32 1, i8** [[GEPBP:%.+]], i8** [[GEPP:%.+]], i[[sz:32|64]]* [[GEPS:%.+]], {{.+}}getelementptr {{.+}}[1 x i{{.+}}]* [[MTYPE00Begin]]{{.+}})
   // CK1A-DAG: [[GEPBP]] = getelementptr inbounds {{.+}}[[BP:%[^,]+]]
   // CK1A-DAG: [[GEPP]] = getelementptr inbounds {{.+}

[clang] 03bb545 - [OpenMP][Docs] Mark `present` map type modifier as done

2020-08-05 Thread Joel E. Denny via cfe-commits

Author: Joel E. Denny
Date: 2020-08-05T10:03:31-04:00
New Revision: 03bb545b68c2edb9dc5bd092104bdb83a8e5e347

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

LOG: [OpenMP][Docs] Mark `present` map type modifier as done

Added: 


Modified: 
clang/docs/OpenMPSupport.rst

Removed: 




diff  --git a/clang/docs/OpenMPSupport.rst b/clang/docs/OpenMPSupport.rst
index 9bad27ea8817..e63d91586b95 100644
--- a/clang/docs/OpenMPSupport.rst
+++ b/clang/docs/OpenMPSupport.rst
@@ -270,7 +270,7 @@ want to help with the implementation.
 
+--+--+--+---+
 | loop extension   | Loop tiling transformation
   | :part:`worked on`| D76342  
  |
 
+--+--+--+---+
-| device extension | 'present' map type modifier   
   | :part:`mostly done`  | D83061, D83062, D84422  
  |
+| device extension | 'present' map type modifier   
   | :good:`done` | D83061, D83062, D84422  
  |
 
+--+--+--+---+
 | device extension | 'present' motion modifier 
   | :good:`done` | D84711, D84712  
  |
 
+--+--+--+---+



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


[clang] 002d61d - [OpenMP] Fix `present` for exit from `omp target data`

2020-08-05 Thread Joel E. Denny via cfe-commits

Author: Joel E. Denny
Date: 2020-08-05T10:03:31-04:00
New Revision: 002d61db2b7790dc884953bf9271878bf0af3a8e

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

LOG: [OpenMP] Fix `present` for exit from `omp target data`

Without this patch, the following example fails but shouldn't
according to OpenMP TR8:

```
 #pragma omp target enter data map(alloc:i)
 #pragma omp target data map(present, alloc: i)
 {
   #pragma omp target exit data map(delete:i)
 } // fails presence check here
```

OpenMP TR8 sec. 2.22.7.1 "map Clause", p. 321, L23-26 states:

> If the map clause appears on a target, target data, target enter
> data or target exit data construct with a present map-type-modifier
> then on entry to the region if the corresponding list item does not
> appear in the device data environment an error occurs and the
> program terminates.

There is no corresponding statement about the exit from a region.
Thus, the `present` modifier should:

1. Check for presence upon entry into any region, including a `target
   exit data` region.  This behavior is already implemented correctly.

2. Should not check for presence upon exit from any region, including
   a `target` or `target data` region.  Without this patch, this
   behavior is not implemented correctly, breaking the above example.

In the case of `target data`, this patch fixes the latter behavior by
removing the `present` modifier from the map types Clang generates for
the runtime call at the end of the region.

In the case of `target`, we have not found a valid OpenMP program for
which such a fix would matter.  It appears that, if a program can
guarantee that data is present at the beginning of a `target` region
so that there's no error there, that data is also guaranteed to be
present at the end.  This patch adds a comment to the runtime to
document this case.

Reviewed By: grokos, RaviNarayanaswamy, ABataev

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

Added: 
openmp/libomptarget/test/mapping/present/target_data_at_exit.c

Modified: 
clang/lib/CodeGen/CGOpenMPRuntime.cpp
clang/lib/CodeGen/CGOpenMPRuntime.h
clang/lib/CodeGen/CGStmtOpenMP.cpp
clang/test/OpenMP/target_data_codegen.cpp
openmp/libomptarget/src/omptarget.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGOpenMPRuntime.cpp 
b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
index 60c7081b135b..547a9307dce2 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -8826,6 +8826,30 @@ emitOffloadingArrays(CodeGenFunction &CGF,
 MapTypesArrayGbl->setUnnamedAddr(llvm::GlobalValue::UnnamedAddr::Global);
 Info.MapTypesArray = MapTypesArrayGbl;
 
+// If there's a present map type modifier, it must not be applied to the 
end
+// of a region, so generate a separate map type array in that case.
+if (Info.separateBeginEndCalls()) {
+  bool EndMapTypesDiffer = false;
+  for (uint64_t &Type : Mapping) {
+if (Type & MappableExprsHandler::OMP_MAP_PRESENT) {
+  Type &= ~MappableExprsHandler::OMP_MAP_PRESENT;
+  EndMapTypesDiffer = true;
+}
+  }
+  if (EndMapTypesDiffer) {
+MapTypesArrayInit =
+llvm::ConstantDataArray::get(CGF.Builder.getContext(), Mapping);
+MaptypesName = CGM.getOpenMPRuntime().getName({"offload_maptypes"});
+MapTypesArrayGbl = new llvm::GlobalVariable(
+CGM.getModule(), MapTypesArrayInit->getType(),
+/*isConstant=*/true, llvm::GlobalValue::PrivateLinkage,
+MapTypesArrayInit, MaptypesName);
+MapTypesArrayGbl->setUnnamedAddr(
+llvm::GlobalValue::UnnamedAddr::Global);
+Info.MapTypesArrayEnd = MapTypesArrayGbl;
+  }
+}
+
 for (unsigned I = 0; I < Info.NumberOfPtrs; ++I) {
   llvm::Value *BPVal = *CombinedInfo.BasePointers[I];
   llvm::Value *BP = CGF.Builder.CreateConstInBoundsGEP2_32(
@@ -8878,12 +8902,16 @@ emitOffloadingArrays(CodeGenFunction &CGF,
 }
 
 /// Emit the arguments to be passed to the runtime library based on the
-/// arrays of base pointers, pointers, sizes, map types, and mappers.
+/// arrays of base pointers, pointers, sizes, map types, and mappers.  If
+/// ForEndCall, emit map types to be passed for the end of the region instead 
of
+/// the beginning.
 static void emitOffloadingArraysArgument(
 CodeGenFunction &CGF, llvm::Value *&BasePointersArrayArg,
 llvm::Value *&PointersArrayArg, llvm::Value *&SizesArrayArg,
 llvm::Value *&MapTypesArrayArg, llvm::Value *&MappersArrayArg,
-CGOpenMPRuntime::TargetDataInfo &Info) {
+CGOpenMPRuntime::TargetDataInfo &Info, bool ForEndCall = false) {
+  assert((!ForEndCall || Info.separateBeginEndCalls()) &&
+ "expected region end call to runtime o

[clang] 26cf9c1 - [OpenMP][Docs] Add map clause reordering status as unclaimed

2020-08-05 Thread Joel E. Denny via cfe-commits

Author: Joel E. Denny
Date: 2020-08-05T10:03:31-04:00
New Revision: 26cf9c17044515cdde3e7baeea843001ba33be59

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

LOG: [OpenMP][Docs] Add map clause reordering status as unclaimed

Added: 


Modified: 
clang/docs/OpenMPSupport.rst

Removed: 




diff  --git a/clang/docs/OpenMPSupport.rst b/clang/docs/OpenMPSupport.rst
index af5e538b1435..9bad27ea8817 100644
--- a/clang/docs/OpenMPSupport.rst
+++ b/clang/docs/OpenMPSupport.rst
@@ -221,6 +221,8 @@ implementation.
 
+--+--+--+---+
 | device extension | pointer attachment
   | :none:`unclaimed`| 
  |
 
+--+--+--+---+
+| device extension | map clause reordering based on map types  
   | :none:`unclaimed`| 
  |
++--+--+--+---+
 | atomic extension | hints for the atomic construct
   | :good:`done` | D51233  
  |
 
+--+--+--+---+
 | base language| C11 support   
   | :good:`done` | 
  |
@@ -272,3 +274,5 @@ want to help with the implementation.
 
+--+--+--+---+
 | device extension | 'present' motion modifier 
   | :good:`done` | D84711, D84712  
  |
 
+--+--+--+---+
+| device extension | map clause reordering reordering based on 
'present' modifier | :none:`unclaimed`| 
  |
++--+--+--+---+



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


[PATCH] D85105: [doxygen] Fix bad doxygen results for BugReporterVisitors.h

2020-08-05 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added a comment.

Hey, thanks again for cleaning up the analyzer's docs 😄




Comment at: 
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:56
   /// last node should be unique.
-  /// Use {@code getEndPath} to customize the note associated with the report
+  /// Use `getEndPath` to customize the note associated with the report
   /// end instead.

Maybe it is better to create a reference to this function using the [[ 
https://www.doxygen.nl/manual/commands.html#cmdref | \ref ]] command?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85105

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


[PATCH] D85309: [WIP][clang][Driver] Support GNU ld on Solaris

2020-08-05 Thread Rainer Orth via Phabricator via cfe-commits
ro created this revision.
ro added a reviewer: MaskRay.
ro added a project: clang.
Herald added subscribers: fedor.sergeev, mgorny.
ro requested review of this revision.

This patch is posted just for reference on top of D84029 
 and D84412 .

It's a very first attempt to support GNU `ld` on Solaris, which is non-trivial 
given that
both linkers have some options not supported by the other.  Currently linker 
selection
is done at `cmake` time; it probably needs to be made runtime-selectable.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85309

Files:
  clang/CMakeLists.txt
  clang/include/clang/Config/config.h.cmake
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/Solaris.cpp
  clang/lib/Driver/ToolChains/Solaris.h

Index: clang/lib/Driver/ToolChains/Solaris.h
===
--- clang/lib/Driver/ToolChains/Solaris.h
+++ clang/lib/Driver/ToolChains/Solaris.h
@@ -65,10 +65,7 @@
   SanitizerMask getSupportedSanitizers() const override;
   unsigned GetDefaultDwarfVersion() const override { return 2; }
 
-  const char *getDefaultLinker() const override {
-// clang currently uses Solaris ld-only options.
-return "/usr/bin/ld";
-  }
+  const char *getDefaultLinker() const override;
 
 protected:
   Tool *buildAssembler() const override;
Index: clang/lib/Driver/ToolChains/Solaris.cpp
===
--- clang/lib/Driver/ToolChains/Solaris.cpp
+++ clang/lib/Driver/ToolChains/Solaris.cpp
@@ -53,7 +53,9 @@
   ArgStringList CmdArgs;
 
   // Demangle C++ names in errors
+#if !CLANG_ENABLE_GLD
   CmdArgs.push_back("-C");
+#endif
 
   if (!Args.hasArg(options::OPT_nostdlib, options::OPT_shared)) {
 CmdArgs.push_back("-e");
@@ -75,6 +77,36 @@
 Args.ClaimAllArgs(options::OPT_pthreads);
   }
 
+#if CLANG_ENABLE_GLD
+  // FIXME: Correct comment.
+  // Explicitly set the linker emulation for platforms that might not
+  // be the default emulation for the linker.
+   const toolchains::Solaris &ToolChain =
+  static_cast(getToolChain());
+  const llvm::Triple::ArchType Arch = ToolChain.getArch();
+
+  switch (Arch) {
+  case llvm::Triple::x86:
+CmdArgs.push_back("-m");
+CmdArgs.push_back("elf_i386_sol2");
+break;
+  case llvm::Triple::x86_64:
+CmdArgs.push_back("-m");
+CmdArgs.push_back("elf_x86_64_sol2");
+break;
+  case llvm::Triple::sparc:
+CmdArgs.push_back("-m");
+CmdArgs.push_back("elf32_sparc_sol2");
+break;
+  case llvm::Triple::sparcv9:
+CmdArgs.push_back("-m");
+CmdArgs.push_back("elf64_sparc_sol2");
+break;
+  default:
+break;
+  }
+#endif
+
   if (Output.isFilename()) {
 CmdArgs.push_back("-o");
 CmdArgs.push_back(Output.getFilename());
@@ -214,6 +246,17 @@
   return Res;
 }
 
+const char *Solaris::getDefaultLinker() const {
+#if CLANG_ENABLE_GLD
+  // FIXME: Hack around /usr/gnu/bin/ld being configure with --with-sysroot.
+  return "/vol/gcc/bin/gld-2.35";
+  //return "/usr/gnu/bin/ld";
+#else
+  // clang currently uses Solaris ld-only options.
+  return "/usr/bin/ld";
+#endif
+}
+
 Tool *Solaris::buildAssembler() const {
   return new tools::solaris::Assembler(*this);
 }
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -615,7 +615,7 @@
 StringRef Sanitizer) {
   // Solaris ld defaults to --export-dynamic behaviour but doesn't support
   // the option, so don't try to pass it.
-  if (TC.getTriple().getOS() == llvm::Triple::Solaris)
+  if (TC.getTriple().getOS() == llvm::Triple::Solaris && !CLANG_ENABLE_GLD)
 return true;
   // Myriad is static linking only.  Furthermore, some versions of its
   // linker have the bug where --export-dynamic overrides -static, so
@@ -634,7 +634,9 @@
   // While the Solaris 11.2 ld added --as-needed/--no-as-needed as aliases
   // for the native forms -z ignore/-z record, they are missing in Illumos,
   // so always use the native form.
-  if (TC.getTriple().isOSSolaris())
+  // GNU ld doesn't support -z ignore/-z record, so don't use them even on
+  // Solaris.
+  if (TC.getTriple().isOSSolaris() && !CLANG_ENABLE_GLD)
 return as_needed ? "-zignore" : "-zrecord";
   else
 return as_needed ? "--as-needed" : "--no-as-needed";
Index: clang/include/clang/Config/config.h.cmake
===
--- clang/include/clang/Config/config.h.cmake
+++ clang/include/clang/Config/config.h.cmake
@@ -11,6 +11,9 @@
 /* Default linker to use. */
 #define CLANG_DEFAULT_LINKER "${CLANG_DEFAULT_LINKER}"
 
+/* Default to GNU ld. */
+#cmakedefine01 CLANG_ENABLE_GLD
+
 /* Default C/ObjC standard to use. */
 #cmakedefine CLANG_DEFAULT_STD_C LangStandard::lang

[PATCH] D85310: [clangd] Disable define out-of-line code action on templates

2020-08-05 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision.
kadircet added a reviewer: hokein.
Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous.
Herald added a project: clang.
kadircet requested review of this revision.
Herald added subscribers: MaskRay, ilya-biryukov.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85310

Files:
  clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
  clang-tools-extra/clangd/unittests/TweakTests.cpp


Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -2008,6 +2008,13 @@
   EXPECT_UNAVAILABLE(R"cpp(
 template  struct Foo { void fo^o(){} };
 )cpp");
+
+  // Not available on function templates and specializations, as definition 
must
+  // be visible to all translation units.
+  EXPECT_UNAVAILABLE(R"cpp(
+template  void fo^o() {};
+template <> void fo^o() {};
+  )cpp");
 }
 
 TEST_F(DefineOutlineTest, FailsWithoutSource) {
@@ -2037,27 +2044,6 @@
   "void foo() ;",
   "void foo() { return; }",
   },
-  // Templated function.
-  {
-  "template  void fo^o(T, T x) { return; }",
-  "template  void foo(T, T x) ;",
-  "template  void foo(T, T x) { return; }",
-  },
-  {
-  "template  void fo^o() { return; }",
-  "template  void foo() ;",
-  "template  void foo() { return; }",
-  },
-  // Template specialization.
-  {
-  R"cpp(
-template  void foo();
-template <> void fo^o() { return; })cpp",
-  R"cpp(
-template  void foo();
-template <> void foo() ;)cpp",
-  "template <> void foo() { return; }",
-  },
   // Default args.
   {
   "void fo^o(int x, int y = 5, int = 2, int (*foo)(int) = nullptr) {}",
Index: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
@@ -387,6 +387,15 @@
 Source->isOutOfLine())
   return false;
 
+// Bail out if this is a function template or specialization, as their
+// definitions need to be visible in all including translation units.
+if (auto *PT = Source->getDescribedFunctionTemplate()) {
+  if (PT->getTemplatedDecl() == Source)
+return false;
+}
+if (auto *TSI = Source->getTemplateSpecializationInfo())
+  return false;
+
 // Bail out in templated classes, as it is hard to spell the class name, 
i.e
 // if the template parameter is unnamed.
 if (auto *MD = llvm::dyn_cast(Source)) {


Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -2008,6 +2008,13 @@
   EXPECT_UNAVAILABLE(R"cpp(
 template  struct Foo { void fo^o(){} };
 )cpp");
+
+  // Not available on function templates and specializations, as definition must
+  // be visible to all translation units.
+  EXPECT_UNAVAILABLE(R"cpp(
+template  void fo^o() {};
+template <> void fo^o() {};
+  )cpp");
 }
 
 TEST_F(DefineOutlineTest, FailsWithoutSource) {
@@ -2037,27 +2044,6 @@
   "void foo() ;",
   "void foo() { return; }",
   },
-  // Templated function.
-  {
-  "template  void fo^o(T, T x) { return; }",
-  "template  void foo(T, T x) ;",
-  "template  void foo(T, T x) { return; }",
-  },
-  {
-  "template  void fo^o() { return; }",
-  "template  void foo() ;",
-  "template  void foo() { return; }",
-  },
-  // Template specialization.
-  {
-  R"cpp(
-template  void foo();
-template <> void fo^o() { return; })cpp",
-  R"cpp(
-template  void foo();
-template <> void foo() ;)cpp",
-  "template <> void foo() { return; }",
-  },
   // Default args.
   {
   "void fo^o(int x, int y = 5, int = 2, int (*foo)(int) = nullptr) {}",
Index: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
@@ -387,6 +387,15 @@
 Source->isOutOfLine())
   return false;
 
+// Bail out if this is a function template or specialization, as their
+// definitions need to be visible in all including translation units.
+if (auto *PT = Source->getDescribedFunctionTemplate()) {
+  if (PT->getTemplatedDecl() == Source)
+return false;
+}
+if (auto *TSI = Source->getTempla

[PATCH] D84412: [clang][Driver] Don't hardcode --as-needed/--no-as-needed on Illumos

2020-08-05 Thread Rainer Orth via Phabricator via cfe-commits
ro updated this revision to Diff 283232.
ro added a comment.

Incorporate review comments: clarify comment and arg name.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84412

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp


Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -630,6 +630,16 @@
   return false;
 }
 
+static const char *getAsNeededOption(const ToolChain &TC, bool as_needed) {
+  // While the Solaris 11.2 ld added --as-needed/--no-as-needed as aliases
+  // for the native forms -z ignore/-z record, they are missing in Illumos,
+  // so always use the native form.
+  if (TC.getTriple().isOSSolaris())
+return as_needed ? "-zignore" : "-zrecord";
+  else
+return as_needed ? "--as-needed" : "--no-as-needed";
+}
+
 void tools::linkSanitizerRuntimeDeps(const ToolChain &TC,
  ArgStringList &CmdArgs) {
   // Fuchsia never needs these.  Any sanitizer runtimes with system
@@ -639,7 +649,7 @@
 
   // Force linking against the system libraries sanitizers depends on
   // (see PR15823 why this is necessary).
-  CmdArgs.push_back("--no-as-needed");
+  CmdArgs.push_back(getAsNeededOption(TC, false));
   // There's no libpthread or librt on RTEMS & Android.
   if (TC.getTriple().getOS() != llvm::Triple::RTEMS &&
   !TC.getTriple().isAndroid()) {
@@ -836,7 +846,7 @@
 }
 
 void tools::linkXRayRuntimeDeps(const ToolChain &TC, ArgStringList &CmdArgs) {
-  CmdArgs.push_back("--no-as-needed");
+  CmdArgs.push_back(getAsNeededOption(TC, false));
   CmdArgs.push_back("-lpthread");
   if (!TC.getTriple().isOSOpenBSD())
 CmdArgs.push_back("-lrt");
@@ -1261,7 +1271,7 @@
   bool AsNeeded = LGT == LibGccType::UnspecifiedLibGcc &&
   !TC.getTriple().isAndroid() && !TC.getTriple().isOSCygMing();
   if (AsNeeded)
-CmdArgs.push_back("--as-needed");
+CmdArgs.push_back(getAsNeededOption(TC, true));
 
   switch (UNW) {
   case ToolChain::UNW_None:
@@ -1289,7 +1299,7 @@
   }
 
   if (AsNeeded)
-CmdArgs.push_back("--no-as-needed");
+CmdArgs.push_back(getAsNeededOption(TC, false));
 }
 
 static void AddLibgcc(const ToolChain &TC, const Driver &D,


Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -630,6 +630,16 @@
   return false;
 }
 
+static const char *getAsNeededOption(const ToolChain &TC, bool as_needed) {
+  // While the Solaris 11.2 ld added --as-needed/--no-as-needed as aliases
+  // for the native forms -z ignore/-z record, they are missing in Illumos,
+  // so always use the native form.
+  if (TC.getTriple().isOSSolaris())
+return as_needed ? "-zignore" : "-zrecord";
+  else
+return as_needed ? "--as-needed" : "--no-as-needed";
+}
+
 void tools::linkSanitizerRuntimeDeps(const ToolChain &TC,
  ArgStringList &CmdArgs) {
   // Fuchsia never needs these.  Any sanitizer runtimes with system
@@ -639,7 +649,7 @@
 
   // Force linking against the system libraries sanitizers depends on
   // (see PR15823 why this is necessary).
-  CmdArgs.push_back("--no-as-needed");
+  CmdArgs.push_back(getAsNeededOption(TC, false));
   // There's no libpthread or librt on RTEMS & Android.
   if (TC.getTriple().getOS() != llvm::Triple::RTEMS &&
   !TC.getTriple().isAndroid()) {
@@ -836,7 +846,7 @@
 }
 
 void tools::linkXRayRuntimeDeps(const ToolChain &TC, ArgStringList &CmdArgs) {
-  CmdArgs.push_back("--no-as-needed");
+  CmdArgs.push_back(getAsNeededOption(TC, false));
   CmdArgs.push_back("-lpthread");
   if (!TC.getTriple().isOSOpenBSD())
 CmdArgs.push_back("-lrt");
@@ -1261,7 +1271,7 @@
   bool AsNeeded = LGT == LibGccType::UnspecifiedLibGcc &&
   !TC.getTriple().isAndroid() && !TC.getTriple().isOSCygMing();
   if (AsNeeded)
-CmdArgs.push_back("--as-needed");
+CmdArgs.push_back(getAsNeededOption(TC, true));
 
   switch (UNW) {
   case ToolChain::UNW_None:
@@ -1289,7 +1299,7 @@
   }
 
   if (AsNeeded)
-CmdArgs.push_back("--no-as-needed");
+CmdArgs.push_back(getAsNeededOption(TC, false));
 }
 
 static void AddLibgcc(const ToolChain &TC, const Driver &D,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D84412: [clang][Driver] Don't hardcode --as-needed/--no-as-needed on Illumos

2020-08-05 Thread Rainer Orth via Phabricator via cfe-commits
ro marked 3 inline comments as done.
ro added a comment.

In D84412#2193600 , @MaskRay wrote:

> #clang  does not have many people. You 
> might need to add people explicitly..

I always found finding reviewers sort of a black art: sometimes the people from 
the `CODE_OWNERS.TXT` files work, sometimes people that
recently reviewed changes to the same files are willing to do so again, while 
at others only repeated nagging works...




Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:633
 
+static const char *getAsNeededOption(const ToolChain &TC, bool ignore) {
+  // FIXME: Need to handle GNU ld on Solaris.

MaskRay wrote:
> `ignore` seems strange. How about `start`?
I'd used `ignore` based no the description of the option in the `ld` man page:
```
   -z ignore | record
   --as-needed | --no-as-needed

   Ignores, or records, shared object dependencies that are not refer-
   enced as part  of  the  link-edit.  These  options  are  positional
```
`start` doesn't tell me much either, so I've gone for `as_needed`.



Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:634
+static const char *getAsNeededOption(const ToolChain &TC, bool ignore) {
+  // FIXME: Need to handle GNU ld on Solaris.
+  if (TC.getTriple().isOSSolaris())

MaskRay wrote:
> Can you improve the comment to mention that this is to work around illumnos 
> ld?
I hope it's better now.  I've removed the GNU `ld` reference.
You can see my current hack to make `clang` work with `gld` on Solaris in 
D85309.



Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:636
+  if (TC.getTriple().isOSSolaris())
+return ignore ? "-zignore" : "-zrecord";
+  else

MaskRay wrote:
> I'll assume that `-zignore` has semantics similar to --as-needed.
Right: the two are identical semantically.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84412

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


[PATCH] D85311: [analyzer][tests] Understand when diagnostics change between builds

2020-08-05 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko created this revision.
vsavchenko added reviewers: NoQ, dcoughlin, xazax.hun, Szelethus.
Herald added subscribers: cfe-commits, steakhal, ASDenysPetrov, Charusso, 
dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, 
baloghadamsoftware.
Herald added a project: clang.
vsavchenko requested review of this revision.

Before the patch `SATest compare`, produced quite obscure results
when something about the diagnostic have changed (i.e. its description
or the name of the corresponding checker) because it was simply two
lists of warnings, ADDED and REMOVED.  It was up to the developer
to match those warnings, understand that they are essentially the
same, and figure out what caused the difference.

This patch introduces another category of results: MODIFIED.
It tries to match new warnings against the old ones and prints out
clues on what is different between two builds.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85311

Files:
  clang/utils/analyzer/CmpRuns.py

Index: clang/utils/analyzer/CmpRuns.py
===
--- clang/utils/analyzer/CmpRuns.py
+++ clang/utils/analyzer/CmpRuns.py
@@ -35,14 +35,16 @@
 from collections import defaultdict
 from copy import copy
 from enum import Enum
-from typing import (Any, cast, Dict, List, NamedTuple, Optional, Sequence,
-TextIO, TypeVar, Tuple, Union)
+from typing import (Any, DefaultDict, Dict, List, NamedTuple, Optional,
+Sequence, TextIO, TypeVar, Tuple, Union)
 
 
 Number = Union[int, float]
 Stats = Dict[str, Dict[str, Number]]
 Plist = Dict[str, Any]
 JSON = Dict[str, Any]
+# Diff in a form: field -> (before, after)
+JSONDiff = Dict[str, Tuple[str, str]]
 # Type for generics
 T = TypeVar('T')
 
@@ -136,6 +138,9 @@
 def get_description(self) -> str:
 return self._data['description']
 
+def get_location(self) -> str:
+return f"{self.get_file_name()}:{self.get_line()}:{self.get_column()}"
+
 def get_issue_identifier(self) -> str:
 id = self.get_file_name() + "+"
 
@@ -172,11 +177,32 @@
 return f"{file_prefix}{funcname_postfix}:{line}:{col}" \
 f", {self.get_category()}: {self.get_description()}"
 
+KEY_FIELDS = ["check_name", "category", "description"]
+
+def is_similar_to(self, other: "AnalysisDiagnostic") -> bool:
+# We consider two diagnostics similar only if at least one
+# of the key fields is the same in both diagnostics.
+return len(self.get_diffs(other)) != len(self.KEY_FIELDS)
+
+def get_diffs(self, other: "AnalysisDiagnostic") -> JSONDiff:
+return {field: (self._data[field], other._data[field])
+for field in self.KEY_FIELDS
+if self._data[field] != other._data[field]}
+
 # Note, the data format is not an API and may change from one analyzer
 # version to another.
 def get_raw_data(self) -> Plist:
 return self._data
 
+def __eq__(self, other: object) -> bool:
+return hash(self) == hash(other)
+
+def __ne__(self, other: object) -> bool:
+return hash(self) != hash(other)
+
+def __hash__(self) -> int:
+return hash(self.get_issue_identifier())
+
 
 class AnalysisRun:
 def __init__(self, info: SingleRunInfo):
@@ -283,12 +309,39 @@
 return d.get_issue_identifier()
 
 
-PresentInBoth = Tuple[AnalysisDiagnostic, AnalysisDiagnostic]
-PresentOnlyInOld = Tuple[AnalysisDiagnostic, None]
-PresentOnlyInNew = Tuple[None, AnalysisDiagnostic]
-ComparisonResult = List[Union[PresentInBoth,
-  PresentOnlyInOld,
-  PresentOnlyInNew]]
+AnalysisDiagnosticPair = Tuple[AnalysisDiagnostic, AnalysisDiagnostic]
+
+
+class ComparisonResult:
+def __init__(self):
+self.present_in_both: List[AnalysisDiagnostic] = []
+self.present_only_in_old: List[AnalysisDiagnostic] = []
+self.present_only_in_new: List[AnalysisDiagnostic] = []
+self.changed_between_new_and_old: List[AnalysisDiagnosticPair] = []
+
+def add_common(self, issue: AnalysisDiagnostic):
+self.present_in_both.append(issue)
+
+def add_removed(self, issue: AnalysisDiagnostic):
+self.present_only_in_old.append(issue)
+
+def add_added(self, issue: AnalysisDiagnostic):
+self.present_only_in_new.append(issue)
+
+def add_changed(self, old_issue: AnalysisDiagnostic,
+new_issue: AnalysisDiagnostic):
+self.changed_between_new_and_old.append((old_issue, new_issue))
+
+
+GroupedDiagnostics = DefaultDict[str, List[AnalysisDiagnostic]]
+
+
+def get_grouped_diagnostics(diagnostics: List[AnalysisDiagnostic]
+) -> GroupedDiagnostics:
+result: GroupedDiagnostics = defaultdict(list)
+for diagnostic in diagnostics:
+result[diagnostic.get_location()].append(diagnostic)
+return result
 
 
 def compare_results(results_ol

[PATCH] D84534: [AIX] Static init frontend recovery and backend support

2020-08-05 Thread Jason Liu via Phabricator via cfe-commits
jasonliu added inline comments.



Comment at: format:1
+//===-- PPCAsmPrinter.cpp - Print machine instrs to PowerPC assembly 
--===//
+//

Redundant file?



Comment at: llvm/include/llvm/CodeGen/AsmPrinter.h:455
   }
 
+  struct Structor {

The new block is not all `Overridable Hooks`, seems better belong in `Coarse 
grained IR lowering routines`.



Comment at: llvm/include/llvm/CodeGen/AsmPrinter.h:456
 
+  struct Structor {
+int Priority = 0;

This struct got moved to header, means it's not an implementation detail any 
more. 
We would need doxygen comment on it.



Comment at: llvm/include/llvm/CodeGen/AsmPrinter.h:460
+// In most cases, `Key` represents a `ComdatKey`. Except on AIX, a `Key` is
+// used to identify a csect where we should emit `.ref` when needed.
+GlobalValue *Key = nullptr;

I have a slight preference to leave it as ComdatKey, and change the name when 
we actually need to handle `.ref` because without seeing the actual 
implementation for `.ref` we don't know if this is the way we want to pursue. 



Comment at: llvm/include/llvm/CodeGen/AsmPrinter.h:466
+
+  bool preprocessStructorList(const DataLayout &DL, const Constant *List,
+  SmallVector &Structors);

A doxygen comment describe what this function does, and what its return value 
means, and mention `Structors` is an output argument.
By looking at what this function does, it seems `buildStructorList` is a better 
name.



Comment at: llvm/include/llvm/CodeGen/AsmPrinter.h:468
+  SmallVector &Structors);
+  /// Targets can override this to change how `llvm.global_ctors` and
+  /// `llvm.global_dtors` lists get handled.

Add a blank line above this.



Comment at: llvm/test/CodeGen/PowerPC/aix-static-init-key-object.ll:6
+
+@llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, 
void ()*, i8* } { i32 65535, void ()* @foo, i8* @v}]
+

Adding this test case would looks like we already decided how to handle .ref in 
clang and llvm. But in fact, we haven't. I would prefer not having this test.


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

https://reviews.llvm.org/D84534

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


[PATCH] D85312: [ADT] Move FixedPoint.h from Clang to LLVM.

2020-08-05 Thread Bevin Hansson via Phabricator via cfe-commits
ebevhan created this revision.
Herald added subscribers: llvm-commits, cfe-commits, dexonsmith, hiraditya, 
mgorny.
Herald added projects: clang, LLVM.
ebevhan requested review of this revision.

This patch moves FixedPointSemantics and APFixedPoint
from Clang to LLVM ADT.

This will make it easier to use the fixed-point
classes in LLVM for constructing an IR builder for
fixed-point and for reusing the APFixedPoint class
for constant evaluation purposes.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85312

Files:
  clang/include/clang/AST/APValue.h
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/Expr.h
  clang/include/clang/AST/OptionalDiagnostic.h
  clang/include/clang/Basic/FixedPoint.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ExprConstant.cpp
  clang/lib/AST/Type.cpp
  clang/lib/Basic/CMakeLists.txt
  clang/lib/Basic/FixedPoint.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/unittests/Basic/CMakeLists.txt
  clang/unittests/Basic/FixedPointTest.cpp
  llvm/include/llvm/ADT/APFixedPoint.h
  llvm/lib/Support/APFixedPoint.cpp
  llvm/lib/Support/CMakeLists.txt
  llvm/unittests/ADT/APFixedPointTest.cpp
  llvm/unittests/ADT/CMakeLists.txt

Index: llvm/unittests/ADT/CMakeLists.txt
===
--- llvm/unittests/ADT/CMakeLists.txt
+++ llvm/unittests/ADT/CMakeLists.txt
@@ -4,6 +4,7 @@
 
 add_llvm_unittest(ADTTests
   AnyTest.cpp
+  APFixedPointTest.cpp
   APFloatTest.cpp
   APIntTest.cpp
   APSIntTest.cpp
Index: llvm/unittests/ADT/APFixedPointTest.cpp
===
--- llvm/unittests/ADT/APFixedPointTest.cpp
+++ llvm/unittests/ADT/APFixedPointTest.cpp
@@ -1,4 +1,4 @@
-//===- unittests/Basic/FixedPointTest.cpp -- fixed point number tests -===//
+//===- unittests/ADT/FixedPointTest.cpp -- fixed point number tests -===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -6,12 +6,12 @@
 //
 //===--===//
 
-#include "clang/Basic/FixedPoint.h"
+#include "llvm/ADT/APFixedPoint.h"
 #include "llvm/ADT/APSInt.h"
 #include "gtest/gtest.h"
 
-using clang::APFixedPoint;
-using clang::FixedPointSemantics;
+using llvm::APFixedPoint;
+using llvm::FixedPointSemantics;
 using llvm::APInt;
 using llvm::APSInt;
 
Index: llvm/lib/Support/CMakeLists.txt
===
--- llvm/lib/Support/CMakeLists.txt
+++ llvm/lib/Support/CMakeLists.txt
@@ -56,6 +56,7 @@
   ABIBreak.cpp
   ARMTargetParser.cpp
   AMDGPUMetadata.cpp
+  APFixedPoint.cpp
   APFloat.cpp
   APInt.cpp
   APSInt.cpp
Index: llvm/lib/Support/APFixedPoint.cpp
===
--- llvm/lib/Support/APFixedPoint.cpp
+++ llvm/lib/Support/APFixedPoint.cpp
@@ -1,4 +1,4 @@
-//===- FixedPoint.cpp - Fixed point constant handling ---*- C++ -*-===//
+//===- APFixedPoint.cpp - Fixed point constant handling -*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -11,13 +11,13 @@
 //
 //===--===//
 
-#include "clang/Basic/FixedPoint.h"
+#include "llvm/ADT/APFixedPoint.h"
 
-namespace clang {
+namespace llvm {
 
 APFixedPoint APFixedPoint::convert(const FixedPointSemantics &DstSema,
bool *Overflow) const {
-  llvm::APSInt NewVal = Val;
+  APSInt NewVal = Val;
   unsigned DstWidth = DstSema.getWidth();
   unsigned DstScale = DstSema.getScale();
   bool Upscaling = DstScale > getScale();
@@ -31,10 +31,10 @@
 NewVal >>= (getScale() - DstScale);
   }
 
-  auto Mask = llvm::APInt::getBitsSetFrom(
+  auto Mask = APInt::getBitsSetFrom(
   NewVal.getBitWidth(),
   std::min(DstScale + DstSema.getIntegralBits(), NewVal.getBitWidth()));
-  llvm::APInt Masked(NewVal & Mask);
+  APInt Masked(NewVal & Mask);
 
   // Change in the bits above the sign
   if (!(Masked == Mask || Masked == 0)) {
@@ -61,8 +61,8 @@
 }
 
 int APFixedPoint::compare(const APFixedPoint &Other) const {
-  llvm::APSInt ThisVal = getValue();
-  llvm::APSInt OtherVal = Other.getValue();
+  APSInt ThisVal = getValue();
+  APSInt OtherVal = Other.getValue();
   bool ThisSigned = Val.isSigned();
   bool OtherSigned = OtherVal.isSigned();
   unsigned OtherScale = Other.getScale();
@@ -113,14 +113,14 @@
 
 APFixedPoint APFixedPoint::getMax(const FixedPointSemantics &Sema) {
   bool IsUnsigned = !Sema.isSigned();
-  auto Val = llvm::APSInt::getMaxValue(Sema.getWidth(), IsUnsigned);
+  auto Val = APSInt::getMaxValue(Sema.getWidth(), Is

[PATCH] D75181: [AArch64] Handle BTI/PAC in case of generated functions.

2020-08-05 Thread Kyrill Tkachov via Phabricator via cfe-commits
ktkachov added a comment.

In D75181#2193447 , @danielkiss wrote:

> Would it be better to add a new value to `"sign-return-address"` as `"none"`? 
> I don't see any other alternative option, I'm open to any other idea.

FWIW GCC has a "sign-return-address" function attribute with a default value of 
"none". It is considered deprecated, however, in favour of "branch-protection"


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

https://reviews.llvm.org/D75181

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


[PATCH] D85315: [AIX][Clang] Add C++ linker option to the driver toolchain

2020-08-05 Thread Shuhong Liu via Phabricator via cfe-commits
ShuhongL created this revision.
ShuhongL added reviewers: daltenty, hubert.reinterpretcast, stevewan.
ShuhongL added a project: LLVM.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
ShuhongL requested review of this revision.

Add C++ linker option to the AIX clang driver toolchain


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85315

Files:
  clang/lib/Driver/ToolChains/AIX.cpp
  clang/lib/Driver/ToolChains/AIX.h


Index: clang/lib/Driver/ToolChains/AIX.h
===
--- clang/lib/Driver/ToolChains/AIX.h
+++ clang/lib/Driver/ToolChains/AIX.h
@@ -67,6 +67,11 @@
   AddClangSystemIncludeArgs(const llvm::opt::ArgList &DriverArgs,
 llvm::opt::ArgStringList &CC1Args) const override;
 
+  void AddCXXStdlibLibArgs(const llvm::opt::ArgList &Args,
+   llvm::opt::ArgStringList &CmdArgs) const override;
+
+  CXXStdlibType GetDefaultCXXStdlibType() const override;
+
 protected:
   Tool *buildAssembler() const override;
   Tool *buildLinker() const override;
Index: clang/lib/Driver/ToolChains/AIX.cpp
===
--- clang/lib/Driver/ToolChains/AIX.cpp
+++ clang/lib/Driver/ToolChains/AIX.cpp
@@ -149,6 +149,9 @@
 CmdArgs.push_back("-lc");
   }
 
+  if (getToolChain().ShouldLinkCXXStdlib(Args))
+getToolChain().AddCXXStdlibLibArgs(Args, CmdArgs);
+
   const char *Exec = Args.MakeArgString(ToolChain.GetLinkerPath());
   C.addCommand(std::make_unique(JA, *this, 
ResponseFileSupport::None(),
  Exec, CmdArgs, Inputs));
@@ -197,6 +200,21 @@
   addSystemInclude(DriverArgs, CC1Args, UP.str());
 }
 
+void AIX::AddCXXStdlibLibArgs(const llvm::opt::ArgList &DriverArgs,
+  llvm::opt::ArgStringList &CC1Args) const {
+  switch (GetCXXStdlibType(DriverArgs)) {
+  case ToolChain::CST_Libcxx:
+CC1Args.push_back("-lc++");
+break;
+  case ToolChain::CST_Libstdcxx:
+llvm_unreachable("linking libstdc++ unimplemented.");
+  }
+}
+
+ToolChain::CXXStdlibType AIX::GetDefaultCXXStdlibType() const {
+  return ToolChain::CST_Libcxx;
+}
+
 auto AIX::buildAssembler() const -> Tool * { return new aix::Assembler(*this); 
}
 
 auto AIX::buildLinker() const -> Tool * { return new aix::Linker(*this); }


Index: clang/lib/Driver/ToolChains/AIX.h
===
--- clang/lib/Driver/ToolChains/AIX.h
+++ clang/lib/Driver/ToolChains/AIX.h
@@ -67,6 +67,11 @@
   AddClangSystemIncludeArgs(const llvm::opt::ArgList &DriverArgs,
 llvm::opt::ArgStringList &CC1Args) const override;
 
+  void AddCXXStdlibLibArgs(const llvm::opt::ArgList &Args,
+   llvm::opt::ArgStringList &CmdArgs) const override;
+
+  CXXStdlibType GetDefaultCXXStdlibType() const override;
+
 protected:
   Tool *buildAssembler() const override;
   Tool *buildLinker() const override;
Index: clang/lib/Driver/ToolChains/AIX.cpp
===
--- clang/lib/Driver/ToolChains/AIX.cpp
+++ clang/lib/Driver/ToolChains/AIX.cpp
@@ -149,6 +149,9 @@
 CmdArgs.push_back("-lc");
   }
 
+  if (getToolChain().ShouldLinkCXXStdlib(Args))
+getToolChain().AddCXXStdlibLibArgs(Args, CmdArgs);
+
   const char *Exec = Args.MakeArgString(ToolChain.GetLinkerPath());
   C.addCommand(std::make_unique(JA, *this, ResponseFileSupport::None(),
  Exec, CmdArgs, Inputs));
@@ -197,6 +200,21 @@
   addSystemInclude(DriverArgs, CC1Args, UP.str());
 }
 
+void AIX::AddCXXStdlibLibArgs(const llvm::opt::ArgList &DriverArgs,
+  llvm::opt::ArgStringList &CC1Args) const {
+  switch (GetCXXStdlibType(DriverArgs)) {
+  case ToolChain::CST_Libcxx:
+CC1Args.push_back("-lc++");
+break;
+  case ToolChain::CST_Libstdcxx:
+llvm_unreachable("linking libstdc++ unimplemented.");
+  }
+}
+
+ToolChain::CXXStdlibType AIX::GetDefaultCXXStdlibType() const {
+  return ToolChain::CST_Libcxx;
+}
+
 auto AIX::buildAssembler() const -> Tool * { return new aix::Assembler(*this); }
 
 auto AIX::buildLinker() const -> Tool * { return new aix::Linker(*this); }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D85253: [clangd] Show correct hover tooltip for non-preamble macro definition.

2020-08-05 Thread Ilya Golovenko via Phabricator via cfe-commits
ilya-golovenko added a comment.

I agree the fix is not correct. I will check how it currently works with clangd 
built from master branch.

I'm not quite sure there should be no tooltip for a `#define`. Such tooltip is 
not really useful, but at the same time clangd shows tooltips for function 
declarations/defnitions, namespaces, variables, etc. and not showing it for 
macro definition will be a little bit inconsistent from my point of view.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85253

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


[PATCH] D85310: [clangd] Disable define out-of-line code action on templates

2020-08-05 Thread Haojian Wu via Phabricator via cfe-commits
hokein added inline comments.



Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp:125
"Couldn't get range for function.");
   // Include template parameter list.
   if (auto *FTD = FD->getDescribedFunctionTemplate())

since we don't support template functions, I think this code can be removed.



Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp:393
+if (auto *PT = Source->getDescribedFunctionTemplate()) {
+  if (PT->getTemplatedDecl() == Source)
+return false;

is this inner `if` important? It seems that we could just return false.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85310

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


[PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-08-05 Thread Hans Wennborg via Phabricator via cfe-commits
hans added a comment.

In D79219#2153585 , @phosek wrote:

> In D79219#2152747 , @labath wrote:
>
>> I wouldn't mind separate (internal) cache variable, though I am somewhat 
>> surprised by this problem. A (non-cache) variable set in one of the parent 
>> scopes should still take precedence over a cache variable with the same 
>> name. And since config-ix.cmake is included from the top CMakeLists.txt, the 
>> value it defines should be available everywhere. Was this a problem for the 
>> regular build, or only for some of the more exotic build setups that don't 
>> start with llvm/CMakeLists.txt ?
>
> Never mind, it's working as expected. The problem is that we disable zlib 
> detection on Windows which I missed before. I'm not sure why that's the case, 
> I tested this on Windows and it seems to be working fine, but for now I've 
> kept the existing behavior, we can consider enabling zlib on Windows in a 
> follow up change.

:-( We rely on zlib on Windows, and we pass -DLLVM_ENABLE_ZLIB=FORCE_ON to 
ensure that the build breaks if it can't be used for some reason.

It seems your change both disabled use of zlib on Windows, and also removed the 
check which made that an error when using FORCE_ON, which means we didn't catch 
this until further down the line.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79219

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


[PATCH] D79219: [CMake] Simplify CMake handling for zlib

2020-08-05 Thread Hans Wennborg via Phabricator via cfe-commits
hans added a comment.

Reverted in 3ab01550b632dad46f9595d74855749557ffd25c 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79219

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


[PATCH] D85315: [AIX][Clang] Add C++ linker option to the driver toolchain

2020-08-05 Thread Shuhong Liu via Phabricator via cfe-commits
ShuhongL added a comment.

Working on adding lit test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85315

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


[PATCH] D85272: [clangd] Semantic highlighting for dependent template name in template argument

2020-08-05 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added inline comments.



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:305
+  break;
+default:;
+}

hokein wrote:
> nit: move the trailing `;` to a new line.
I have done this, but clang-format moves it back... do we really want to fight 
wit it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85272

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


[PATCH] D85316: [SyntaxTree] Proposition of new tree dump

2020-08-05 Thread Eduardo Caldas via Phabricator via cfe-commits
eduucaldas created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
eduucaldas requested review of this revision.

Some key choices to highlight:

- Surround Tokens with "''"
- Do not print `UnknownRole`, to reduce noise
- Surround Roles with "<", to clarify the difference in meaning


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85316

Files:
  clang/unittests/Tooling/Syntax/TreeTest.cpp


Index: clang/unittests/Tooling/Syntax/TreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/TreeTest.cpp
+++ clang/unittests/Tooling/Syntax/TreeTest.cpp
@@ -307,45 +307,45 @@
 }
 )cpp",
   R"txt(
-*: TranslationUnit
+TranslationUnit   <*>
 `-SimpleDeclaration
-  |-int
-  |-SimpleDeclarator
-  | |-main
+  |-'int'
+  |-SimpleDeclarator   
+  | |-'main'
   | `-ParametersAndQualifiers
-  |   |-(
-  |   `-)
+  |   |-'('   
+  |   `-')'   
   `-CompoundStatement
-|-{
-|-IfStatement
-| |-if
-| |-(
+|-'{'   
+|-IfStatement   
+| |-'if'   
+| |-'('
 | |-IntegerLiteralExpression
-| | `-1
-| |-)
-| `-CompoundStatement
-|   |-{
-|   `-}
-|-IfStatement
-| |-if
-| |-(
+| | `-'1'   
+| |-')'
+| `-CompoundStatement   
+|   |-'{'   
+|   `-'}'   
+|-IfStatement   
+| |-'if'   
+| |-'('
 | |-IntegerLiteralExpression
-| | `-1
-| |-)
-| |-CompoundStatement
-| | |-{
-| | `-}
-| |-else
-| `-IfStatement
-|   |-if
-|   |-(
+| | `-'1'   
+| |-')'
+| |-CompoundStatement   
+| | |-'{'   
+| | `-'}'   
+| |-'else'   
+| `-IfStatement   
+|   |-'if'   
+|   |-'('
 |   |-IntegerLiteralExpression
-|   | `-0
-|   |-)
-|   `-CompoundStatement
-| |-{
-| `-}
-`-}
+|   | `-'0'   
+|   |-')'
+|   `-CompoundStatement   
+| |-'{'   
+| `-'}'   
+`-'}'   
 )txt"));
 }
 


Index: clang/unittests/Tooling/Syntax/TreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/TreeTest.cpp
+++ clang/unittests/Tooling/Syntax/TreeTest.cpp
@@ -307,45 +307,45 @@
 }
 )cpp",
   R"txt(
-*: TranslationUnit
+TranslationUnit   <*>
 `-SimpleDeclaration
-  |-int
-  |-SimpleDeclarator
-  | |-main
+  |-'int'
+  |-SimpleDeclarator   
+  | |-'main'
   | `-ParametersAndQualifiers
-  |   |-(
-  |   `-)
+  |   |-'('   
+  |   `-')'   
   `-CompoundStatement
-|-{
-|-IfStatement
-| |-if
-| |-(
+|-'{'   
+|-IfStatement   
+| |-'if'   
+| |-'('
 | |-IntegerLiteralExpression
-| | `-1
-| |-)
-| `-CompoundStatement
-|   |-{
-|   `-}
-|-IfStatement
-| |-if
-| |-(
+| | `-'1'   
+| |-')'
+| `-CompoundStatement   
+|   |-'{'   
+|   `-'}'   
+|-IfStatement   
+| |-'if'   
+| |-'('
 | |-IntegerLiteralExpression
-| | `-1
-| |-)
-| |-CompoundStatement
-| | |-{
-| | `-}
-| |-else
-| `-IfStatement
-|   |-if
-|   |-(
+| | `-'1'   
+| |-')'
+| |-CompoundStatement   
+| | |-'{'   
+| | `-'}'   
+| |-'else'   
+| `-IfStatement   
+|   |-'if'   
+|   |-'('
 |   |-IntegerLiteralExpression
-|   | `-0
-|   |-)
-|   `-CompoundStatement
-| |-{
-| `-}
-`-}
+|   | `-'0'   
+|   |-')'
+|   `-CompoundStatement   
+| |-'{'   
+| `-'}'   
+`-'}'   
 )txt"));
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D85318: [clangd] Hide "swap if branch" tweak

2020-08-05 Thread Haojian Wu via Phabricator via cfe-commits
hokein created this revision.
hokein added a reviewer: kadircet.
Herald added subscribers: usaxena95, arphaman, jkorous.
Herald added a project: clang.
hokein requested review of this revision.
Herald added subscribers: MaskRay, ilya-biryukov.

This tweak is more like a demo, and doesn't provide much value in
practice.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85318

Files:
  clang-tools-extra/clangd/refactor/tweaks/SwapIfBranches.cpp


Index: clang-tools-extra/clangd/refactor/tweaks/SwapIfBranches.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/SwapIfBranches.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/SwapIfBranches.cpp
@@ -40,6 +40,7 @@
   Expected apply(const Selection &Inputs) override;
   std::string title() const override { return "Swap if branches"; }
   Intent intent() const override { return Refactor; }
+  bool hidden() const override { return true; }
 
 private:
   const IfStmt *If = nullptr;


Index: clang-tools-extra/clangd/refactor/tweaks/SwapIfBranches.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/SwapIfBranches.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/SwapIfBranches.cpp
@@ -40,6 +40,7 @@
   Expected apply(const Selection &Inputs) override;
   std::string title() const override { return "Swap if branches"; }
   Intent intent() const override { return Refactor; }
+  bool hidden() const override { return true; }
 
 private:
   const IfStmt *If = nullptr;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D85319: [analyzer] Get info from the LLVM IR for precision

2020-08-05 Thread Gabor Marton via Phabricator via cfe-commits
martong created this revision.
martong added reviewers: NoQ, Szelethus, balazske, vsavchenko.
Herald added subscribers: cfe-commits, ASDenysPetrov, steakhal, Charusso, 
gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, 
baloghadamsoftware, xazax.hun, whisperity, mgorny.
Herald added a project: clang.
martong requested review of this revision.

Access the IR from the components of the Clang Static Analyzer. There are many
important and useful analyses in the LLVM layer that we can use during the path
sensitive analysis. Most notably, the "readnone" and "readonly" function
attributes  which can be used to identify "pure" functions (those without side
effects). Here, I am using the pureness info from the IR to avoid invalidation
of any variables during conservative evaluation (when we evaluate a pure
function).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85319

Files:
  clang/include/clang/CodeGen/CodeGenMangling.h
  clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
  clang/include/clang/StaticAnalyzer/Core/IRContext.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
  clang/include/clang/StaticAnalyzer/Frontend/AnalysisConsumer.h
  clang/include/clang/StaticAnalyzer/Frontend/FrontendActions.h
  clang/lib/CodeGen/CMakeLists.txt
  clang/lib/CodeGen/CodeGenMangling.cpp
  clang/lib/StaticAnalyzer/Core/CMakeLists.txt
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
  clang/lib/StaticAnalyzer/Core/IRContext.cpp
  clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
  clang/lib/StaticAnalyzer/Frontend/CMakeLists.txt
  clang/lib/StaticAnalyzer/Frontend/FrontendActions.cpp
  clang/test/Analysis/analyzer-config.c
  clang/test/Analysis/ctu-different-triples.cpp
  clang/test/Analysis/ircontext.c
  clang/test/Analysis/ircontext.cpp
  clang/unittests/StaticAnalyzer/Reusables.h

Index: clang/unittests/StaticAnalyzer/Reusables.h
===
--- clang/unittests/StaticAnalyzer/Reusables.h
+++ clang/unittests/StaticAnalyzer/Reusables.h
@@ -10,8 +10,9 @@
 #define LLVM_CLANG_UNITTESTS_STATICANALYZER_REUSABLES_H
 
 #include "clang/ASTMatchers/ASTMatchFinder.h"
-#include "clang/Frontend/CompilerInstance.h"
 #include "clang/CrossTU/CrossTranslationUnit.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/StaticAnalyzer/Core/IRContext.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
 
 namespace clang {
@@ -41,11 +42,13 @@
 class ExprEngineConsumer : public ASTConsumer {
 protected:
   CompilerInstance &C;
+  CodeGenerator *CG = nullptr;
 
 private:
   // We need to construct all of these in order to construct ExprEngine.
   CheckerManager ChkMgr;
   cross_tu::CrossTranslationUnitContext CTU;
+  IRContext IRCtx;
   PathDiagnosticConsumers Consumers;
   AnalysisManager AMgr;
   SetOfConstDecls VisitedCallees;
@@ -58,12 +61,12 @@
   ExprEngineConsumer(CompilerInstance &C)
   : C(C),
 ChkMgr(C.getASTContext(), *C.getAnalyzerOpts(), C.getPreprocessor()),
-CTU(C), Consumers(),
+CTU(C), IRCtx(CG), Consumers(),
 AMgr(C.getASTContext(), C.getPreprocessor(), Consumers,
  CreateRegionStoreManager, CreateRangeConstraintManager, &ChkMgr,
  *C.getAnalyzerOpts()),
-VisitedCallees(), FS(),
-Eng(CTU, AMgr, &VisitedCallees, &FS, ExprEngine::Inline_Regular) {}
+VisitedCallees(), FS(), Eng(CTU, IRCtx, AMgr, &VisitedCallees, &FS,
+ExprEngine::Inline_Regular) {}
 };
 
 } // namespace ento
Index: clang/test/Analysis/ircontext.cpp
===
--- /dev/null
+++ clang/test/Analysis/ircontext.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -analyzer-checker=core,debug.ExprInspection \
+// RUN:   -analyzer-config eagerly-assume=false \
+// RUN:   -analyzer-config ipa=none \
+// RUN:   -analyzer-config generate-llvm-ir=true \
+// RUN:   -triple i686-unknown-linux \
+// RUN:   -verify
+
+void clang_analyzer_eval(int);
+
+int g = 0;
+int foo(int *x) { return *x; }
+
+void test() {
+  g = 3;
+  int l = 0;
+  foo(&l);
+  clang_analyzer_eval(g == 3); // expected-warning{{TRUE}}
+}
Index: clang/test/Analysis/ircontext.c
===
--- /dev/null
+++ clang/test/Analysis/ircontext.c
@@ -0,0 +1,18 @@
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-config ipa=none \
+// RUN:   -triple i686-unknown-linux \
+// RUN:   -verify
+
+// expected-no-diagnostics
+
+void clang_analyzer_eval(int);
+
+int g = 0;
+int foo(int *x) { return *x; }
+
+void test() {
+  g = 3;
+  int l = 0;
+  foo(&l);
+}
Index: clang/test/Analysis/ctu-different-triples.cpp
===
--- clang/test/Analysis/ctu-different-triples.cpp
+++ cl

[PATCH] D84782: [PGO] Include the mem ops into the function hash.

2020-08-05 Thread Hans Wennborg via Phabricator via cfe-commits
hans added a comment.

In D84782#2191429 , @yamauchi wrote:

> In D84782#2191243 , @MaskRay wrote:
>
>> @yamauchi Do we need cd890944ad344b1b8cac58332ab11c9eec6b61e9 
>>  and 
>> 3d6f53018f845e893ad34f64ff2851a2e5c3ba1d 
>>  in 
>> https://github.com/llvm/llvm-project/tree/release/11.x ?
>
> I think it'd be good to have them, if possible, though it's a latent, 
> non-recent bug.

They're hard to apply without 50da55a58534e9207d8d5e31c8b4b5cf0c624175 
. Do you 
think we should take that also? Or maybe this can wait since it's not a new bug.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84782

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


[PATCH] D85319: [analyzer] Get info from the LLVM IR for precision

2020-08-05 Thread Gabor Marton via Phabricator via cfe-commits
martong updated this revision to Diff 283254.
martong added a comment.

- Remove dummy test file


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85319

Files:
  clang/include/clang/CodeGen/CodeGenMangling.h
  clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
  clang/include/clang/StaticAnalyzer/Core/IRContext.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
  clang/include/clang/StaticAnalyzer/Frontend/AnalysisConsumer.h
  clang/include/clang/StaticAnalyzer/Frontend/FrontendActions.h
  clang/lib/CodeGen/CMakeLists.txt
  clang/lib/CodeGen/CodeGenMangling.cpp
  clang/lib/StaticAnalyzer/Core/CMakeLists.txt
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
  clang/lib/StaticAnalyzer/Core/IRContext.cpp
  clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
  clang/lib/StaticAnalyzer/Frontend/CMakeLists.txt
  clang/lib/StaticAnalyzer/Frontend/FrontendActions.cpp
  clang/test/Analysis/analyzer-config.c
  clang/test/Analysis/ctu-different-triples.cpp
  clang/test/Analysis/ircontext.cpp
  clang/unittests/StaticAnalyzer/Reusables.h

Index: clang/unittests/StaticAnalyzer/Reusables.h
===
--- clang/unittests/StaticAnalyzer/Reusables.h
+++ clang/unittests/StaticAnalyzer/Reusables.h
@@ -10,8 +10,9 @@
 #define LLVM_CLANG_UNITTESTS_STATICANALYZER_REUSABLES_H
 
 #include "clang/ASTMatchers/ASTMatchFinder.h"
-#include "clang/Frontend/CompilerInstance.h"
 #include "clang/CrossTU/CrossTranslationUnit.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/StaticAnalyzer/Core/IRContext.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
 
 namespace clang {
@@ -41,11 +42,13 @@
 class ExprEngineConsumer : public ASTConsumer {
 protected:
   CompilerInstance &C;
+  CodeGenerator *CG = nullptr;
 
 private:
   // We need to construct all of these in order to construct ExprEngine.
   CheckerManager ChkMgr;
   cross_tu::CrossTranslationUnitContext CTU;
+  IRContext IRCtx;
   PathDiagnosticConsumers Consumers;
   AnalysisManager AMgr;
   SetOfConstDecls VisitedCallees;
@@ -58,12 +61,12 @@
   ExprEngineConsumer(CompilerInstance &C)
   : C(C),
 ChkMgr(C.getASTContext(), *C.getAnalyzerOpts(), C.getPreprocessor()),
-CTU(C), Consumers(),
+CTU(C), IRCtx(CG), Consumers(),
 AMgr(C.getASTContext(), C.getPreprocessor(), Consumers,
  CreateRegionStoreManager, CreateRangeConstraintManager, &ChkMgr,
  *C.getAnalyzerOpts()),
-VisitedCallees(), FS(),
-Eng(CTU, AMgr, &VisitedCallees, &FS, ExprEngine::Inline_Regular) {}
+VisitedCallees(), FS(), Eng(CTU, IRCtx, AMgr, &VisitedCallees, &FS,
+ExprEngine::Inline_Regular) {}
 };
 
 } // namespace ento
Index: clang/test/Analysis/ircontext.cpp
===
--- /dev/null
+++ clang/test/Analysis/ircontext.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -analyzer-checker=core,debug.ExprInspection \
+// RUN:   -analyzer-config eagerly-assume=false \
+// RUN:   -analyzer-config ipa=none \
+// RUN:   -analyzer-config generate-llvm-ir=true \
+// RUN:   -triple i686-unknown-linux \
+// RUN:   -verify
+
+void clang_analyzer_eval(int);
+
+int g = 0;
+int foo(int *x) { return *x; }
+
+void test() {
+  g = 3;
+  int l = 0;
+  foo(&l);
+  clang_analyzer_eval(g == 3); // expected-warning{{TRUE}}
+}
Index: clang/test/Analysis/ctu-different-triples.cpp
===
--- clang/test/Analysis/ctu-different-triples.cpp
+++ clang/test/Analysis/ctu-different-triples.cpp
@@ -12,6 +12,8 @@
 
 // We expect an error in this file, but without a location.
 // expected-error-re@./ctu-different-triples.cpp:*{{imported AST from {{.*}} had been generated for a different target, current: powerpc64-montavista-linux-gnu, imported: x86_64-pc-linux-gnu}}
+// FIXME Why do we get the error twice?
+// expected-error-re@./ctu-different-triples.cpp:*{{imported AST from {{.*}} had been generated for a different target, current: powerpc64-montavista-linux-gnu, imported: x86_64-pc-linux-gnu}}
 
 int f(int);
 
Index: clang/test/Analysis/analyzer-config.c
===
--- clang/test/Analysis/analyzer-config.c
+++ clang/test/Analysis/analyzer-config.c
@@ -80,6 +80,7 @@
 // CHECK-NEXT: experimental-enable-naive-ctu-analysis = false
 // CHECK-NEXT: exploration_strategy = unexplored_first_queue
 // CHECK-NEXT: faux-bodies = true
+// CHECK-NEXT: generate-llvm-ir = false
 // CHECK-NEXT: graph-trim-interval = 1000
 // CHECK-NEXT: inline-lambdas = true
 // CHECK-NEXT: ipa = dynamic-bifurcate
Index: clang/lib/StaticAnalyzer/Frontend/FrontendActions.cpp

[clang-tools-extra] 0117328 - [clangd] Fix a crash in DefineInline

2020-08-05 Thread Kadir Cetinkaya via cfe-commits

Author: Kadir Cetinkaya
Date: 2020-08-05T17:38:17+02:00
New Revision: 011732852c2c1ca1015fac1bed831308dc521583

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

LOG: [clangd] Fix a crash in DefineInline

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

Added: 


Modified: 
clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
clang-tools-extra/clangd/unittests/TweakTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp 
b/clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
index e28a2c46c374..698d2a406811 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
@@ -339,7 +339,7 @@ renameParameters(const FunctionDecl *Dest, const 
FunctionDecl *Source) {
 // specialization.
 const FunctionDecl *findTarget(const FunctionDecl *FD) {
   auto CanonDecl = FD->getCanonicalDecl();
-  if (!FD->isFunctionTemplateSpecialization())
+  if (!FD->isFunctionTemplateSpecialization() || CanonDecl == FD)
 return CanonDecl;
   // For specializations CanonicalDecl is the TemplatedDecl, which is not the
   // target we want to inline into. Instead we traverse previous decls to find

diff  --git a/clang-tools-extra/clangd/unittests/TweakTests.cpp 
b/clang-tools-extra/clangd/unittests/TweakTests.cpp
index 319d9e088c2d..791965160055 100644
--- a/clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ b/clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -1093,6 +1093,11 @@ TEST_F(DefineInlineTest, TemplateSpec) {
 template<> void f^oo() {
   bar();
 })cpp");
+  EXPECT_UNAVAILABLE(R"cpp(
+namespace bar {
+  template  void f^oo() {}
+  template void foo();
+})cpp");
 }
 
 TEST_F(DefineInlineTest, CheckForCanonDecl) {



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


[PATCH] D85291: [clangd] Fix a crash in DefineInline

2020-08-05 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG011732852c2c: [clangd] Fix a crash in DefineInline (authored 
by kadircet).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85291

Files:
  clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
  clang-tools-extra/clangd/unittests/TweakTests.cpp


Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -1093,6 +1093,11 @@
 template<> void f^oo() {
   bar();
 })cpp");
+  EXPECT_UNAVAILABLE(R"cpp(
+namespace bar {
+  template  void f^oo() {}
+  template void foo();
+})cpp");
 }
 
 TEST_F(DefineInlineTest, CheckForCanonDecl) {
Index: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
@@ -339,7 +339,7 @@
 // specialization.
 const FunctionDecl *findTarget(const FunctionDecl *FD) {
   auto CanonDecl = FD->getCanonicalDecl();
-  if (!FD->isFunctionTemplateSpecialization())
+  if (!FD->isFunctionTemplateSpecialization() || CanonDecl == FD)
 return CanonDecl;
   // For specializations CanonicalDecl is the TemplatedDecl, which is not the
   // target we want to inline into. Instead we traverse previous decls to find


Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -1093,6 +1093,11 @@
 template<> void f^oo() {
   bar();
 })cpp");
+  EXPECT_UNAVAILABLE(R"cpp(
+namespace bar {
+  template  void f^oo() {}
+  template void foo();
+})cpp");
 }
 
 TEST_F(DefineInlineTest, CheckForCanonDecl) {
Index: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
@@ -339,7 +339,7 @@
 // specialization.
 const FunctionDecl *findTarget(const FunctionDecl *FD) {
   auto CanonDecl = FD->getCanonicalDecl();
-  if (!FD->isFunctionTemplateSpecialization())
+  if (!FD->isFunctionTemplateSpecialization() || CanonDecl == FD)
 return CanonDecl;
   // For specializations CanonicalDecl is the TemplatedDecl, which is not the
   // target we want to inline into. Instead we traverse previous decls to find
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D75181: [AArch64] Handle BTI/PAC in case of generated functions.

2020-08-05 Thread Daniel Kiss via Phabricator via cfe-commits
danielkiss added a comment.

> FWIW GCC has a "sign-return-address" function attribute with a default value 
> of "none". It is considered deprecated, however, in favour of 
> "branch-protection"

This is just the internal representation, the function attribute in C/C++ 
source is the "branch-protection".

Old version of the patch used attribute value to represent the 
"ignore"/disabled state https://reviews.llvm.org/D75181?id=247201 so I have no 
idea what would be the right solution.


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

https://reviews.llvm.org/D75181

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


[PATCH] D80791: [AArch64] Generate .note.gnu.property based on module flags.

2020-08-05 Thread Szabolcs Nagy via Phabricator via cfe-commits
nsz added a comment.

the gcc behaviour is not exactly ideal, but it's better if llvm is compatible 
with it or fix gcc if something is broken there.

the assumption is that the intended branch protection is implied via cmdline 
flags for the tu and function attributes are only used in source code for some 
hack. a common reason for such hack is to disable bti somewhere but still keep 
the bti elf marking. (if the intention was to mark the code non-bti compatible 
then just dont compile it with bti, using a non-portable  bti specfic function 
attribute would not work well for such use anyway)

now this may not work well with lto when functions can come from different 
places and the function attributes encode how those were compiled. so hopefully 
there is something that preserves the flags of the translation units and 
explicitly specified function attributes can be treated separately (such that 
they dont affect elf markings).


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

https://reviews.llvm.org/D80791

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


[PATCH] D85321: [OPENMP]Do not capture base pointer by reference if it is used as a base for array-like reduction.

2020-08-05 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev created this revision.
ABataev added a reviewer: jdoerfert.
Herald added subscribers: guansong, yaxunl.
Herald added a project: clang.
ABataev requested review of this revision.
Herald added a subscriber: sstefan1.

If the declaration is used in the reduction clause, it is captured by
reference by default. But if the declaration is a pointer and it is a
base for array-like reduction, this declaration can be captured by
value, since the pointee is reduced but not the original declaration.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85321

Files:
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/distribute_parallel_for_reduction_task_codegen.cpp
  clang/test/OpenMP/for_reduction_task_codegen.cpp
  clang/test/OpenMP/parallel_for_reduction_task_codegen.cpp
  clang/test/OpenMP/parallel_master_reduction_task_codegen.cpp
  clang/test/OpenMP/parallel_reduction_task_codegen.cpp
  clang/test/OpenMP/parallel_sections_reduction_task_codegen.cpp
  clang/test/OpenMP/sections_reduction_task_codegen.cpp
  clang/test/OpenMP/target_parallel_for_reduction_task_codegen.cpp
  clang/test/OpenMP/target_parallel_reduction_task_codegen.cpp
  
clang/test/OpenMP/target_teams_distribute_parallel_for_reduction_task_codegen.cpp
  clang/test/OpenMP/teams_distribute_parallel_for_reduction_task_codegen.cpp

Index: clang/test/OpenMP/teams_distribute_parallel_for_reduction_task_codegen.cpp
===
--- clang/test/OpenMP/teams_distribute_parallel_for_reduction_task_codegen.cpp
+++ clang/test/OpenMP/teams_distribute_parallel_for_reduction_task_codegen.cpp
@@ -20,9 +20,9 @@
   }
 }
 
-// CHECK: call void (%struct.ident_t*, i32, void (i32*, i32*, ...)*, ...) @__kmpc_fork_call(%struct.ident_t* @{{.+}}, i32 4, void (i32*, i32*, ...)* bitcast (void (i32*, i32*, i64, i64, i32*, i8***)* [[OUTLINED:@.+]] to void (i32*, i32*, ...)*), i64 %{{.+}}, i64 %{{.+}}, i32* %{{.+}}, i8*** %{{.+}})
+// CHECK: call void (%struct.ident_t*, i32, void (i32*, i32*, ...)*, ...) @__kmpc_fork_call(%struct.ident_t* @{{.+}}, i32 4, void (i32*, i32*, ...)* bitcast (void (i32*, i32*, i64, i64, i32*, i8**)* [[OUTLINED:@.+]] to void (i32*, i32*, ...)*), i64 %{{.+}}, i64 %{{.+}}, i32* %{{.+}}, i8** %{{.+}})
 
-// CHECK: define internal void [[OUTLINED]](i32* noalias %{{.+}}, i32* noalias %{{.+}}, i64 %{{.+}}, i64 %{{.+}}, i32* {{.+}}, i8*** {{.+}})
+// CHECK: define internal void [[OUTLINED]](i32* noalias %{{.+}}, i32* noalias %{{.+}}, i64 %{{.+}}, i64 %{{.+}}, i32* {{.+}}, i8** {{.+}})
 // CHECK: alloca i32,
 // CHECK: [[ARGC_FP_ADDR:%.+]] = alloca i32,
 // CHECK: [[TR:%.+]] = alloca [2 x [[TASKRED_TY:%struct.kmp_taskred_input_t.*]]],
@@ -124,7 +124,6 @@
 // CHECK_DAG: [[TG]] = load i8*, i8** [[TG_ADDR]],
 // CHECK-DAG: [[ARGV_REF]] = load i8*, i8** [[ARGV_ADDR:%.+]],
 // CHECK-DAG: [[ARGV_ADDR]] = load i8**, i8*** [[ARGV_ADDR_REF:%.+]],
-// CHECK-DAG: [[ARGV_ADDR_REF:%.+]] = load i8***, i8 [[ARGV:%.+]],
-// CHECK-DAG: [[ARGV]] = getelementptr inbounds [[CAPS_TY]], [[CAPS_TY]]* [[CAP]], i32 0, i32 2
+// CHECK-DAG: [[ARGV_ADDR_REF]] = getelementptr inbounds [[CAPS_TY]], [[CAPS_TY]]* [[CAP]], i32 0, i32 2
 
 #endif
Index: clang/test/OpenMP/target_teams_distribute_parallel_for_reduction_task_codegen.cpp
===
--- clang/test/OpenMP/target_teams_distribute_parallel_for_reduction_task_codegen.cpp
+++ clang/test/OpenMP/target_teams_distribute_parallel_for_reduction_task_codegen.cpp
@@ -19,9 +19,9 @@
   }
 }
 
-// CHECK: call void (%struct.ident_t*, i32, void (i32*, i32*, ...)*, ...) @__kmpc_fork_call(%struct.ident_t* @{{.+}}, i32 4, void (i32*, i32*, ...)* bitcast (void (i32*, i32*, i64, i64, i32*, i8***)* [[OUTLINED:@.+]] to void (i32*, i32*, ...)*), i64 %{{.+}}, i64 %{{.+}}, i32* %{{.+}}, i8*** %{{.+}})
+// CHECK: call void (%struct.ident_t*, i32, void (i32*, i32*, ...)*, ...) @__kmpc_fork_call(%struct.ident_t* @{{.+}}, i32 4, void (i32*, i32*, ...)* bitcast (void (i32*, i32*, i64, i64, i32*, i8**)* [[OUTLINED:@.+]] to void (i32*, i32*, ...)*), i64 %{{.+}}, i64 %{{.+}}, i32* %{{.+}}, i8** %{{.+}})
 
-// CHECK: define internal void [[OUTLINED]](i32* noalias %{{.+}}, i32* noalias %{{.+}}, i64 %{{.+}}, i64 %{{.+}}, i32* {{.+}}, i8*** {{.+}})
+// CHECK: define internal void [[OUTLINED]](i32* noalias %{{.+}}, i32* noalias %{{.+}}, i64 %{{.+}}, i64 %{{.+}}, i32* {{.+}}, i8** {{.+}})
 // CHECK: alloca i32,
 // CHECK: [[ARGC_FP_ADDR:%.+]] = alloca i32,
 // CHECK: [[TR:%.+]] = alloca [2 x [[TASKRED_TY:%struct.kmp_taskred_input_t.*]]],
@@ -123,7 +123,6 @@
 // CHECK_DAG: [[TG]] = load i8*, i8** [[TG_ADDR]],
 // CHECK-DAG: [[ARGV_REF]] = load i8*, i8** [[ARGV_ADDR:%.+]],
 // CHECK-DAG: [[ARGV_ADDR]] = load i8**, i8*** [[ARGV_ADDR_REF:%.+]],
-// CHECK-DAG: [[ARGV_ADDR_REF:%.+]] = load i8***, i8 [[ARGV:%.+]],
-// CHECK-DAG: [[ARGV]] = getelementptr inbounds [[CAPS_TY]], [[CAPS_TY]]* [[CAP]], i32 0, i32 2
+// CHECK-DAG: [[ARGV_ADDR_REF]

[PATCH] D80791: [AArch64] Generate .note.gnu.property based on module flags.

2020-08-05 Thread Momchil Velikov via Phabricator via cfe-commits
chill added inline comments.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:5550
+auto &VMContext = CGM.getLLVMContext();
+M->addModuleFlag(llvm::Module::Override, "sign-return-address",
+ Scope == LangOptions::SignReturnAddressScopeKind::All

Wouldn't that cause the sanitiser functions to be also compiled with PAC/BTI?  
(re: D75181)


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

https://reviews.llvm.org/D80791

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


[PATCH] D85319: [analyzer] Get info from the LLVM IR for precision

2020-08-05 Thread Whisperity via Phabricator via cfe-commits
whisperity added subscribers: dcoughlin, rjmccall, rsmith.
whisperity added a comment.

What will happen with the ability to analyse a translation unit whose target 
was not part of `LLVM_TARGETS_TO_BUILD` of the current `clang` binary? Will it 
break, because the binary lacks the information on how to generate for the 
given target?




Comment at: clang/include/clang/StaticAnalyzer/Core/IRContext.h:39-41
+  // Get the LLVM code for a function. We use the complete versions of the
+  // constructors and desctructors in this overload. Use the other overload to
+  // get the base object ctor/dtor.

Aren't documentation comments in LLVM `///`?



Comment at: clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp:127
 PP(CI.getPreprocessor()), OutDir(outdir), Opts(std::move(opts)),
-Plugins(plugins), Injector(injector), CTU(CI) {
+Plugins(plugins), Injector(injector), CTU(CI), IRCtx(CG) {
 DigestAnalyzerOptions();

Where's this `CG` defined?



Comment at: clang/lib/StaticAnalyzer/Frontend/FrontendActions.cpp:53-57
+LLVMCtx = std::make_shared();
+auto CGConsumer = BuildCodeGen(CI, *LLVMCtx, *CodeGenDiags);
+AConsumer->setCodeGen(CGConsumer.get());
+ASTConsumers.push_back(std::move(CGConsumer));
+  }

Isn't there a lifetime issue here? `LLVMCtx` is given as a raw reference to the 
`BuildCodeGen` and the shared pointer leaves scope at the end of the branch.



Comment at: clang/test/Analysis/ircontext.cpp:17-18
+  int l = 0;
+  foo(&l);
+  clang_analyzer_eval(g == 3); // expected-warning{{TRUE}}
+}

What are we testing here? Without the ability to read the pureness of `foo` 
from the IR, the knowledge about `g`'s value would be scrapped at the call? 
`foo` is defined in the current TU.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85319

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


[PATCH] D85305: [SyntaxTree] Remove dead code on dump functions

2020-08-05 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added a comment.

> Node::dumpTokens was never used.

I think it is because it was meant to be a debugger-only helper. I think we 
should keep it. However, simplifying `static void dumpTokens` to only take one 
token instead of `ArrayRef` makes sense!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85305

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


[PATCH] D85310: [clangd] Disable define out-of-line code action on templates

2020-08-05 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 283266.
kadircet marked an inline comment as done.
kadircet added a comment.

- Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85310

Files:
  clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
  clang-tools-extra/clangd/unittests/TweakTests.cpp


Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -2008,6 +2008,13 @@
   EXPECT_UNAVAILABLE(R"cpp(
 template  struct Foo { void fo^o(){} };
 )cpp");
+
+  // Not available on function templates and specializations, as definition 
must
+  // be visible to all translation units.
+  EXPECT_UNAVAILABLE(R"cpp(
+template  void fo^o() {};
+template <> void fo^o() {};
+  )cpp");
 }
 
 TEST_F(DefineOutlineTest, FailsWithoutSource) {
@@ -2037,27 +2044,6 @@
   "void foo() ;",
   "void foo() { return; }",
   },
-  // Templated function.
-  {
-  "template  void fo^o(T, T x) { return; }",
-  "template  void foo(T, T x) ;",
-  "template  void foo(T, T x) { return; }",
-  },
-  {
-  "template  void fo^o() { return; }",
-  "template  void foo() ;",
-  "template  void foo() { return; }",
-  },
-  // Template specialization.
-  {
-  R"cpp(
-template  void foo();
-template <> void fo^o() { return; })cpp",
-  R"cpp(
-template  void foo();
-template <> void foo() ;)cpp",
-  "template <> void foo() { return; }",
-  },
   // Default args.
   {
   "void fo^o(int x, int y = 5, int = 2, int (*foo)(int) = nullptr) {}",
Index: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
@@ -122,9 +122,8 @@
   if (!OrigFuncRange)
 return llvm::createStringError(llvm::inconvertibleErrorCode(),
"Couldn't get range for function.");
-  // Include template parameter list.
-  if (auto *FTD = FD->getDescribedFunctionTemplate())
-OrigFuncRange->setBegin(FTD->getBeginLoc());
+  assert(!FD->getDescribedFunctionTemplate() &&
+ "Define out-of-line doesn't apply to function templates.");
 
   // Get new begin and end positions for the qualified function definition.
   unsigned FuncBegin = SM.getFileOffset(OrigFuncRange->getBegin());
@@ -387,6 +386,13 @@
 Source->isOutOfLine())
   return false;
 
+// Bail out if this is a function template or specialization, as their
+// definitions need to be visible in all including translation units.
+if (auto *PT = Source->getDescribedFunctionTemplate())
+  return false;
+if (auto *TSI = Source->getTemplateSpecializationInfo())
+  return false;
+
 // Bail out in templated classes, as it is hard to spell the class name, 
i.e
 // if the template parameter is unnamed.
 if (auto *MD = llvm::dyn_cast(Source)) {


Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -2008,6 +2008,13 @@
   EXPECT_UNAVAILABLE(R"cpp(
 template  struct Foo { void fo^o(){} };
 )cpp");
+
+  // Not available on function templates and specializations, as definition must
+  // be visible to all translation units.
+  EXPECT_UNAVAILABLE(R"cpp(
+template  void fo^o() {};
+template <> void fo^o() {};
+  )cpp");
 }
 
 TEST_F(DefineOutlineTest, FailsWithoutSource) {
@@ -2037,27 +2044,6 @@
   "void foo() ;",
   "void foo() { return; }",
   },
-  // Templated function.
-  {
-  "template  void fo^o(T, T x) { return; }",
-  "template  void foo(T, T x) ;",
-  "template  void foo(T, T x) { return; }",
-  },
-  {
-  "template  void fo^o() { return; }",
-  "template  void foo() ;",
-  "template  void foo() { return; }",
-  },
-  // Template specialization.
-  {
-  R"cpp(
-template  void foo();
-template <> void fo^o() { return; })cpp",
-  R"cpp(
-template  void foo();
-template <> void foo() ;)cpp",
-  "template <> void foo() { return; }",
-  },
   // Default args.
   {
   "void fo^o(int x, int y = 5, int = 2, int (*foo)(int) = nullptr) {}",
Index: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/De

[PATCH] D85310: [clangd] Disable define out-of-line code action on templates

2020-08-05 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet marked 2 inline comments as done.
kadircet added inline comments.



Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp:393
+if (auto *PT = Source->getDescribedFunctionTemplate()) {
+  if (PT->getTemplatedDecl() == Source)
+return false;

hokein wrote:
> is this inner `if` important? It seems that we could just return false.
yeah I was considering redeclarations, but they should be handled in the 
previous if anyways.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85310

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


[PATCH] D85316: [SyntaxTree] Proposition of new tree dump

2020-08-05 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added a comment.

I'd suggest to drop the `<>` quotes (because the AST dump does not add quotes 
unless it is printing a multi-word thing, and because `<>` don't exactly scream 
"role" helping to read the output).

I'd also suggest to replace multiple spaces before `<>` with a single space.

Otherwise, looks great!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85316

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


[PATCH] D85319: [analyzer] Get info from the LLVM IR for precision

2020-08-05 Thread Gabor Marton via Phabricator via cfe-commits
martong marked 3 inline comments as done.
martong added inline comments.



Comment at: clang/include/clang/StaticAnalyzer/Core/IRContext.h:39-41
+  // Get the LLVM code for a function. We use the complete versions of the
+  // constructors and desctructors in this overload. Use the other overload to
+  // get the base object ctor/dtor.

whisperity wrote:
> Aren't documentation comments in LLVM `///`?
Yep, I forgot about that.



Comment at: clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp:127
 PP(CI.getPreprocessor()), OutDir(outdir), Opts(std::move(opts)),
-Plugins(plugins), Injector(injector), CTU(CI) {
+Plugins(plugins), Injector(injector), CTU(CI), IRCtx(CG) {
 DigestAnalyzerOptions();

whisperity wrote:
> Where's this `CG` defined?
In `AnalysisConsumer.h`. And set in `FrontendActions.cpp` 
`AConsumer->setCodeGen(CGConsumer.get());`



Comment at: clang/lib/StaticAnalyzer/Frontend/FrontendActions.cpp:53-57
+LLVMCtx = std::make_shared();
+auto CGConsumer = BuildCodeGen(CI, *LLVMCtx, *CodeGenDiags);
+AConsumer->setCodeGen(CGConsumer.get());
+ASTConsumers.push_back(std::move(CGConsumer));
+  }

whisperity wrote:
> Isn't there a lifetime issue here? `LLVMCtx` is given as a raw reference to 
> the `BuildCodeGen` and the shared pointer leaves scope at the end of the 
> branch.
Nope, `LLVMCtx` is a member of `AnalysisAction`.



Comment at: clang/test/Analysis/ircontext.cpp:17-18
+  int l = 0;
+  foo(&l);
+  clang_analyzer_eval(g == 3); // expected-warning{{TRUE}}
+}

whisperity wrote:
> What are we testing here? Without the ability to read the pureness of `foo` 
> from the IR, the knowledge about `g`'s value would be scrapped at the call? 
> `foo` is defined in the current TU.
We test here that with no-inlining (i.e. always doing conservative evaluation) 
we do not invalidate the state after a call to a pure function.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85319

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


[PATCH] D84780: Setting the /bigobj option globally for Windows debug build. https://bugs.llvm.org/show_bug.cgi?id=46733

2020-08-05 Thread Michael Kruse via Phabricator via cfe-commits
Meinersbur added a comment.

Hasn't this already been done in 80bd6ae13ea23d453a1f45d6ccdbfc7d0c877bb0 
?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84780

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


[PATCH] D85319: [analyzer] Get info from the LLVM IR for precision

2020-08-05 Thread Gabor Marton via Phabricator via cfe-commits
martong marked 3 inline comments as done.
martong added a comment.

In D85319#2196648 , @whisperity wrote:

> What will happen with the ability to analyse a translation unit whose target 
> was not part of `LLVM_TARGETS_TO_BUILD` of the current `clang` binary? Will 
> it break, because the binary lacks the information on how to generate for the 
> given target?

We get the target info from the `CompilerInstance` that is used for the command 
line of the analysis. So if clang cannot handle that then we'll probably get a 
diagnostics anyway by the `AnalysisASTConsumer`s diag engine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85319

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


[PATCH] D84636: [RFC] Make the default LibCall implementations from compiler-rt builtins library more customizable

2020-08-05 Thread Anatoly Trosinenko via Phabricator via cfe-commits
atrosinenko added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84636

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


[PATCH] D85253: [clangd] Show correct hover tooltip for non-preamble macro definition.

2020-08-05 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment.

> I'm not quite sure there should be no tooltip for a #define. Such tooltip is 
> not really useful, but at the same time clangd shows tooltips for function 
> declarations/defnitions, namespaces, variables, etc. and not showing it for 
> macro definition will be a little bit inconsistent from my point of view.

We actually had a couple users saying those are redundant too. I agree with 
them mostly, i suppose these are only useful because we talk about the 
associated namespace/class/accesspecifer. E.g. you can easily figure out what's 
the class owning a field, and whether it is private/public without worrying 
traversing upwards.
Who knows, maybe we should only be displaying that information on the 
declaration/definition of symbols rather than all the stuff that's already 
probably next to user's cursor (I say probably because you can have 
declarations coming from macro invocations).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85253

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


Re: [PATCH] D84782: [PGO] Include the mem ops into the function hash.

2020-08-05 Thread Hiroshi Yamauchi via cfe-commits
On Wed, Aug 5, 2020 at 8:30 AM Hans Wennborg via Phabricator <
revi...@reviews.llvm.org> wrote:

> hans added a comment.
>
> In D84782#2191429 , @yamauchi
> wrote:
>
> > In D84782#2191243 , @MaskRay
> wrote:
> >
> >> @yamauchi Do we need cd890944ad344b1b8cac58332ab11c9eec6b61e9 <
> https://reviews.llvm.org/rGcd890944ad344b1b8cac58332ab11c9eec6b61e9> and
> 3d6f53018f845e893ad34f64ff2851a2e5c3ba1d <
> https://reviews.llvm.org/rG3d6f53018f845e893ad34f64ff2851a2e5c3ba1d> in
> https://github.com/llvm/llvm-project/tree/release/11.x ?
> >
> > I think it'd be good to have them, if possible, though it's a latent,
> non-recent bug.
>
> They're hard to apply without 50da55a58534e9207d8d5e31c8b4b5cf0c624175 <
> https://reviews.llvm.org/rG50da55a58534e9207d8d5e31c8b4b5cf0c624175>. Do
> you think we should take that also? Or maybe this can wait since it's not a
> new bug.
>

50da55a58534e9207d8d5e31c8b4b5cf0c624175 isn't a bug fix but it's not
changing the default behavior. So, it is probably safe to take it. That
said, I'd say this can wait unless there's some immediate issue, since it's
not a new bug.


>
>
> Repository:
>   rG LLVM Github Monorepo
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D84782/new/
>
> https://reviews.llvm.org/D84782
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D85318: [clangd] Hide "swap if branch" tweak

2020-08-05 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment.

you might want this to be cherry-picked into 11 release


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85318

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


[PATCH] D85324: [z/OS] Add z/OS Target and define macros

2020-08-05 Thread Abhina Sreeskantharajan via Phabricator via cfe-commits
abhina.sreeskantharajan created this revision.
abhina.sreeskantharajan added reviewers: uweigand, Kai, hubert.reinterpretcast, 
yusra.syeda, SeanP.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
abhina.sreeskantharajan requested review of this revision.

This patch adds the z/OS target and defines macros which is a stepping stone 
towards enabling a native build on z/OS.

Dependent on https://reviews.llvm.org/D82081 being merged first.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85324

Files:
  clang/lib/Basic/Targets.cpp
  clang/lib/Basic/Targets/OSTargets.h
  clang/test/Preprocessor/init.c

Index: clang/test/Preprocessor/init.c
===
--- clang/test/Preprocessor/init.c
+++ clang/test/Preprocessor/init.c
@@ -5630,6 +5630,45 @@
 // S390X:#define __s390__ 1
 // S390X:#define __s390x__ 1
 //
+// RUN: %clang_cc1 -E -dM -ffreestanding -triple=s390x-none-zos -fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix S390X-ZOS %s
+// RUN: %clang_cc1 -x c -E -dM -ffreestanding -triple=s390x-none-zos -fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix S390X-ZOS -check-prefix S390X-ZOS-C %s
+// RUN: %clang_cc1 -E -dM -std=gnu99 -ffreestanding -triple=s390x-none-zos -fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix S390X-ZOS %s
+// RUN: %clang_cc1 -E -dM -std=gnu11 -ffreestanding -triple=s390x-none-zos -fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix S390X-ZOS %s
+// RUN: %clang_cc1 -x c++ -E -dM -ffreestanding -triple=s390x-none-zos -fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix S390X-ZOS -check-prefix S390X-ZOS-CXX %s
+// RUN: %clang_cc1 -x c -std=c99 -E -dM -ffreestanding -triple=s390x-none-zos -fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix S390X-ZOS -check-prefix S390X-ZOS-C99 %s
+// RUN: %clang_cc1 -x c++ -std=c++11 -E -dM -ffreestanding -triple=s390x-none-zos -fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix S390X-ZOS -check-prefix S390X-ZOS-CXX %s
+// RUN: %clang_cc1 -x c++ -std=c++14 -E -dM -ffreestanding -triple=s390x-none-zos -fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix S390X-ZOS -check-prefix S390X-ZOS-CXX %s
+// RUN: %clang_cc1 -x c++ -std=gnu++11 -E -dM -ffreestanding -triple=s390x-none-zos -fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix S390X-ZOS -check-prefix S390X-ZOS-CXX -check-prefix S390X-ZOS-GXX %s
+// RUN: %clang_cc1 -x c++ -std=gnu++14 -E -dM -ffreestanding -triple=s390x-none-zos -fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix S390X-ZOS -check-prefix S390X-ZOS-CXX -check-prefix S390X-ZOS-GXX %s
+//
+// S390X-ZOS-GXX:#define _EXT 1
+// S390X-ZOS-C99:#define _ISOC99_SOURCE 1
+// S390X-ZOS:#define _LONG_LONG 1
+// S390X-ZOS-GXX:#define _MI_BUILTIN 1
+// S390X-ZOS:#define _OPEN_DEFAULT 1
+// S390X-ZOS:#define _UNIX03_WITHDRAWN 1
+// S390X-ZOS-CXX:#define _XOPEN_SOURCE 600
+// S390X-ZOS:#define __370__ 1
+// S390X-ZOS:#define __64BIT__ 1
+// S390X-ZOS:#define __BFP__ 1
+// S390X-ZOS:#define __BOOL__ 1
+// S390X-ZOS-CXX:#define __DLL__ 1
+// S390X-ZOS-C:#define __IBMC_GENERIC 1
+// S390X-ZOS-C:#define __IBMC_NORETURN 1
+// S390X-ZOS-C:#define __IBM_UTF_LITERAL 1
+// S390X-ZOS-GXX:#define __IBMCPP_UTF_LITERAL__ 1
+// S390X-ZOS-GXX:#define __IBMC_NORETURN 1
+// S390X-ZOS-GXX:#define __IBM_CHAR16_T__ 1
+// S390X-ZOS-GXX:#define __IBM_CHAR32_T__ 1
+// S390X-ZOS:#define __LONGNAME__ 1
+// S390X-ZOS:#define __MVS__ 1
+// S390X-ZOS:#define __THW_370__ 1
+// S390X-ZOS:#define __THW_BIG_ENDIAN__ 1
+// S390X-ZOS:#define __TOS_390__ 1
+// S390X-ZOS:#define __TOS_MVS__ 1
+// S390X-ZOS:#define __XPLINK__ 1
+// S390X-ZOS-CXX:#define __wchar_t 1
+//
 // RUN: %clang_cc1 -E -dM -ffreestanding -fgnuc-version=4.2.1 -triple=sparc-none-none < /dev/null | FileCheck -match-full-lines -check-prefix SPARC -check-prefix SPARC-DEFAULT %s
 // RUN: %clang_cc1 -E -dM -ffreestanding -fgnuc-version=4.2.1 -triple=sparc-rtems-elf < /dev/null | FileCheck -match-full-lines -check-prefix SPARC -check-prefix SPARC-DEFAULT %s
 // RUN: %clang_cc1 -E -dM -ffreestanding -fgnuc-version=4.2.1 -triple=sparc-none-netbsd < /dev/null | FileCheck -match-full-lines -check-prefix SPARC -check-prefix SPARC-NETOPENBSD %s
Index: clang/lib/Basic/Targets/OSTargets.h
===
--- clang/lib/Basic/Targets/OSTargets.h
+++ clang/lib/Basic/Targets/OSTargets.h
@@ -721,6 +721,71 @@
   bool hasInt128Type() const override { return false; }
 };
 
+// z/OS target
+template 
+class LLVM_LIBRARY_VISIBILITY ZOSTargetInfo : public OSTargetInfo {
+protected:
+  void getOSDefines(const LangOptions &Opts, const llvm::Triple &Triple,
+MacroBuilder &Builder) const override {
+Builder.defineMacro("_LONG_LONG");
+Builder.defineMacro("_OPEN_DEFAULT");
+

[PATCH] D85324: [z/OS] Add z/OS Target and define macros

2020-08-05 Thread Anirudh Prasad via Phabricator via cfe-commits
anirudhp added inline comments.



Comment at: clang/lib/Basic/Targets/OSTargets.h:745
+if (this->PointerWidth == 64) {
+  Builder.defineMacro("__64BIT__");
+}

What about the `__LP64__` and the `_LP64` macros?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85324

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


[PATCH] D85276: [PGO][CUDA][HIP] Skip generating profile on the device stub and wrong-side functions.

2020-08-05 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment.

LGTM for CUDA.




Comment at: clang/lib/CodeGen/CodeGenPGO.cpp:839-840
 
+  // Skip host-only functions in the CUDA device compilation and device-only
+  // functions in the host compilation.
+  if (CGM.getLangOpts().CUDA &&

We will still have around some functions that may never be used on the host 
side (HD functions referenced from device code only).  I'm not sure if that's a 
problem for profiling, though. I wonder if we can somehow tie 
`skipRegionMappingForDecl` to whether we've actually codegen'ed the function. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85276

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


[PATCH] D85324: [z/OS] Add z/OS Target and define macros

2020-08-05 Thread Abhina Sreeskantharajan via Phabricator via cfe-commits
abhina.sreeskantharajan added inline comments.



Comment at: clang/lib/Basic/Targets/OSTargets.h:745
+if (this->PointerWidth == 64) {
+  Builder.defineMacro("__64BIT__");
+}

anirudhp wrote:
> What about the `__LP64__` and the `_LP64` macros?
These two are already added in the common code, so it would be redundant to add 
them again.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85324

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


[PATCH] D85324: [z/OS] Add z/OS Target and define macros

2020-08-05 Thread Anirudh Prasad via Phabricator via cfe-commits
anirudhp added inline comments.



Comment at: clang/lib/Basic/Targets/OSTargets.h:745
+if (this->PointerWidth == 64) {
+  Builder.defineMacro("__64BIT__");
+}

abhina.sreeskantharajan wrote:
> anirudhp wrote:
> > What about the `__LP64__` and the `_LP64` macros?
> These two are already added in the common code, so it would be redundant to 
> add them again.
Alright Thanks! 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85324

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


Re: [PATCH] D84782: [PGO] Include the mem ops into the function hash.

2020-08-05 Thread Hans Wennborg via cfe-commits
On Wed, Aug 5, 2020 at 6:26 PM Hiroshi Yamauchi via llvm-commits
 wrote:
>
>
>
> On Wed, Aug 5, 2020 at 8:30 AM Hans Wennborg via Phabricator 
>  wrote:
>>
>> hans added a comment.
>>
>> In D84782#2191429 , @yamauchi wrote:
>>
>> > In D84782#2191243 , @MaskRay 
>> > wrote:
>> >
>> >> @yamauchi Do we need cd890944ad344b1b8cac58332ab11c9eec6b61e9 
>> >>  and 
>> >> 3d6f53018f845e893ad34f64ff2851a2e5c3ba1d 
>> >>  in 
>> >> https://github.com/llvm/llvm-project/tree/release/11.x ?
>> >
>> > I think it'd be good to have them, if possible, though it's a latent, 
>> > non-recent bug.
>>
>> They're hard to apply without 50da55a58534e9207d8d5e31c8b4b5cf0c624175 
>> . Do 
>> you think we should take that also? Or maybe this can wait since it's not a 
>> new bug.
>
>
> 50da55a58534e9207d8d5e31c8b4b5cf0c624175 isn't a bug fix but it's not 
> changing the default behavior. So, it is probably safe to take it. That said, 
> I'd say this can wait unless there's some immediate issue, since it's not a 
> new bug.

Okay, let's not worry about it for 11.x then.

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


[PATCH] D85324: [z/OS] Add z/OS Target and define macros

2020-08-05 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments.



Comment at: clang/lib/Basic/Targets/OSTargets.h:745
+if (this->PointerWidth == 64) {
+  Builder.defineMacro("__64BIT__");
+}

anirudhp wrote:
> abhina.sreeskantharajan wrote:
> > anirudhp wrote:
> > > What about the `__LP64__` and the `_LP64` macros?
> > These two are already added in the common code, so it would be redundant to 
> > add them again.
> Alright Thanks! 
Omit braces

https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85324

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


[PATCH] D82582: [SVE] Remove calls to VectorType::getNumElements from clang

2020-08-05 Thread Christopher Tetreault via Phabricator via cfe-commits
ctetreau planned changes to this revision.
ctetreau added a comment.

I plan to investigate the changes proposed by @c-rhodes. I'm a bit swamped 
right now so it may take a bit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82582

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


  1   2   3   >