[PATCH] D98388: [RISCV][Clang] Add RVV vle/vse intrinsic functions.

2021-03-10 Thread Roger Ferrer Ibanez via Phabricator via cfe-commits
rogfer01 added a comment.

Overall LGTM. Thanks @khchen!




Comment at: clang/include/clang/Basic/riscv_vector.td:175
+  // builtin to C/C++. It is parameter of the unmasked version without VL
+  // operand.
+  list PermuteOperands = [];

Not sure if we want to clarify that when this list is empty, the permutation is 
assumed to be equivalent to `[0, 1, 2, 3, ...]`.



Comment at: clang/include/clang/Basic/riscv_vector.td:224
+IntrinsicTypes = {ResultType, Ops[1]->getType()};
+Ops[0] = Builder.CreateBitCast(Ops[0],
+llvm::PointerType::getUnqual(ResultType)); }],

I think you may want to indent this, and then lines 228 and 248 like you 
indented line 252.



Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:689
+  skew = 1;
+for (unsigned i = 0; i < PermuteOperands.size(); ++i) {
+  if (i != PermuteOperands[i])

These are only suggestions of sanity checks we could do here:
- I understand `PermuteOperand.size()` should be `<=` than `CTypeOrder.size()`. 
- Also `PermuteOperands[i] + skew` should be `<` than `CTypeOrder.size()`. 
right?
- We could check the result is indeed a permutation (e.g. sorting a copy of 
`CTypeOrder` is equivalent to the iota above). This one might be expensive 
although the sequences are short, not sure.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98388

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


[PATCH] D96404: [Android] Default to --rtlib=compiler-rt

2021-03-10 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added a comment.

Hilariously enough, this breaks building compiler-rt itself inside LLVM's 
runtime builds setup for us. The runtimes build setup builds clang and then 
uses the just-built clang to build compiler-rt. That build fails to link since 
my just-built clang doesn't have compiler-rt available, because it's currently 
trying to build compiler-rt itself. That's a bug in the compiler-rt build 
system, and I sent out 
https://lists.llvm.org/pipermail/llvm-dev/2021-March/149137.html to ask what we 
should do about it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96404

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


[PATCH] D97854: [RFC][nsan] A Floating-point numerical sanitizer.

2021-03-10 Thread Clement Courbet via Phabricator via cfe-commits
courbet added a comment.

In D97854#2617097 , @scanon wrote:

> Is there a mechanism to instruct the sanitizer to ignore a specific 
> expression or function? From a cursory reading, I am mildly concerned about a 
> deluge of false positives from primitives that compute exact (or approximate) 
> residuals; these are acting to eliminate or precisely control floating-point 
> errors, but tend to show up as "unstable" in a naive analysis that isn't 
> aware of them.

Yes: like all sanitizers, what happens behind the scenes is that the frontend 
(`clang`) sets an annotation on each function in the program. It can be 
disabled for a specific function with the no_sanitize 
 attribute.

If `nsan` is disabled for a specific function, any return value will be 
re-extended again to shadow precision, and the computations will resume from 
here. This is equivalent to  assuming that the function, its parameters, and 
any memory reads were correct.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97854

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


[PATCH] D46443: [libc++] Add missing cstdalign header

2021-03-10 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments.



Comment at: libcxx/include/cstdalign:24
+#include <__config>
+#include 
+

sbc100 wrote:
> hubert.reinterpretcast wrote:
> > curdeius wrote:
> > > hubert.reinterpretcast wrote:
> > > > curdeius wrote:
> > > > > curdeius wrote:
> > > > > > hubert.reinterpretcast wrote:
> > > > > > > sbc100 wrote:
> > > > > > > > hubert.reinterpretcast wrote:
> > > > > > > > > This seems to be assuming that the underlying C library's 
> > > > > > > > > `stdalign.h` is C++ friendly. A C11 `stdalign.h` //does// 
> > > > > > > > > define `alignof` and `alignas` as macros.
> > > > > > > > Should I just remove this `#include` then?
> > > > > > > The idea would be to //add// a `stdalign.h` alongside this header 
> > > > > > > that doesn't `#include_next` the underlying C library's 
> > > > > > > `stdalign.h`.
> > > > > > I'm not sure if that should be the solution. At least gcc's 
> > > > > > libstdc++ assumes that `stdalign.h` is C++-compatbile (cf. 
> > > > > > https://github.com/gcc-mirror/gcc/blob/16e2427f50c208dfe07d07f18009969502c25dc8/libstdc%2B%2B-v3/include/c_global/cstdalign).
> > > > > > 
> > > > > > Clang provides a compatible header: 
> > > > > > https://github.com/llvm/llvm-project/commit/8acb4044d83ecc9df81b1c9f327d5bd4325e1756.
> > > > > > Gcc too of course: 
> > > > > > https://github.com/gcc-mirror/gcc/blob/16e2427f50c208dfe07d07f18009969502c25dc8/gcc/ginclude/stdalign.h.
> > > > > > 
> > > > > > MSVC's STL on the other hand, doesn't include `` 
> > > > > > (https://github.com/microsoft/STL/blob/main/stl/inc/cstdalign).
> > > > > > 
> > > > > > @hubert.reinterpretcast, are you aware of an environment which has 
> > > > > > non-friendly `stdalign.h`?
> > > > > FYI, musl is also C++ friendly: 
> > > > > https://git.musl-libc.org/cgit/musl/tree/include/stdalign.h.
> > > > >>! Quote:
> > > > > @hubert.reinterpretcast, are you aware of an environment which has 
> > > > > non-friendly stdalign.h?
> > > > 
> > > > The one GCC provides disagrees with the interpretation I gave of which 
> > > > macros should be present. The one that Clang provides //does// match my 
> > > > interpretation. It seems the GCC one is non-friendly (albeit a 
> > > > different form of non-friendly than the one I opened with).
> > > Oh, you mean that `__alignas_is_defined` and `__alignof_is_defined` won't 
> > > be defined in this case, right?
> > > In this case, I guess we won't avoid having `stdalign.h` as you had 
> > > suggested.
> > > And indeed the test fails with gcc:
> > > ```
> > > bin/llvm-lit -vv ../../libcxx/test/std/language.support/cstdalign/ 
> > > --param=std=c++17 --param=cxx_under_test=`which g++`
> > > ...
> > > libcxx/test/std/language.support/cstdalign/cstdalign.pass.cpp:21:2: 
> > > error: #error __alignas_is_defined not defined
> > >21 | #error __alignas_is_defined not defined
> > >   |  ^
> > > ```
> > > 
> > > That's unfortunately a configuration which is not tested in the CI.
> > Yes, it looks like adding a `stdalign.h` for libc++ is needed to reliably 
> > get `__alignas_is_defined` and `__alignof_is_defined`.
> Alternatively, could we just defined them if they are not already and skip 
> including `stdalign.h` here?
`stdalign.h` is a header defined in C++17 like `stdbool.h` is one. libc++ has a 
`stdbool.h`, so I think having a `stdalign.h` makes sense (since we know that 
`stdalign.h` in some environments does not do what we want).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D46443

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


[PATCH] D98379: [RISCV] Add additional checking to tablgen RISCVVEmitter requested in D95016.

2021-03-10 Thread Craig Topper 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 rG9773cad51939: [RISCV] Add additional checking to tablgen 
RISCVVEmitter requested in D95016. (authored by craig.topper).

Changed prior to commit:
  https://reviews.llvm.org/D98379?vs=329807=329831#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98379

Files:
  clang/utils/TableGen/RISCVVEmitter.cpp


Index: clang/utils/TableGen/RISCVVEmitter.cpp
===
--- clang/utils/TableGen/RISCVVEmitter.cpp
+++ clang/utils/TableGen/RISCVVEmitter.cpp
@@ -19,6 +19,7 @@
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringSet.h"
 #include "llvm/ADT/Twine.h"
+#include "llvm/TableGen/Error.h"
 #include "llvm/TableGen/Record.h"
 #include 
 
@@ -536,7 +537,7 @@
 ScalarType = ScalarTypeKind::Float;
 break;
   default:
-llvm_unreachable("Unhandled type code!");
+PrintFatalError("Unhandled type code!");
   }
   assert(ElementBitwidth != 0 && "Bad element bitwidth!");
 }
@@ -587,7 +588,7 @@
 Scale = 0;
 break;
   default:
-llvm_unreachable("Illegal primitive type transformers!");
+PrintFatalError("Illegal primitive type transformers!");
   }
   Transformer = Transformer.drop_back();
 
@@ -595,9 +596,15 @@
   for (char I : Transformer) {
 switch (I) {
 case 'P':
+  if (IsConstant)
+PrintFatalError("'P' transformer cannot be used after 'C'");
+  if (IsPointer)
+PrintFatalError("'P' transformer cannot be used twice");
   IsPointer = true;
   break;
 case 'C':
+  if (IsConstant)
+PrintFatalError("'C' transformer cannot be used twice");
   IsConstant = true;
   break;
 case 'K':
@@ -614,11 +621,11 @@
   break;
 case 'S':
   LMUL = LMULType(0);
-  // Update ElementBitwidth need ot update Scale too.
+  // Update ElementBitwidth need to update Scale too.
   Scale = LMUL.getScale(ElementBitwidth);
   break;
 default:
-  llvm_unreachable("Illegal non-primitive type transformer!");
+  PrintFatalError("Illegal non-primitive type transformer!");
 }
   }
 }


Index: clang/utils/TableGen/RISCVVEmitter.cpp
===
--- clang/utils/TableGen/RISCVVEmitter.cpp
+++ clang/utils/TableGen/RISCVVEmitter.cpp
@@ -19,6 +19,7 @@
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringSet.h"
 #include "llvm/ADT/Twine.h"
+#include "llvm/TableGen/Error.h"
 #include "llvm/TableGen/Record.h"
 #include 
 
@@ -536,7 +537,7 @@
 ScalarType = ScalarTypeKind::Float;
 break;
   default:
-llvm_unreachable("Unhandled type code!");
+PrintFatalError("Unhandled type code!");
   }
   assert(ElementBitwidth != 0 && "Bad element bitwidth!");
 }
@@ -587,7 +588,7 @@
 Scale = 0;
 break;
   default:
-llvm_unreachable("Illegal primitive type transformers!");
+PrintFatalError("Illegal primitive type transformers!");
   }
   Transformer = Transformer.drop_back();
 
@@ -595,9 +596,15 @@
   for (char I : Transformer) {
 switch (I) {
 case 'P':
+  if (IsConstant)
+PrintFatalError("'P' transformer cannot be used after 'C'");
+  if (IsPointer)
+PrintFatalError("'P' transformer cannot be used twice");
   IsPointer = true;
   break;
 case 'C':
+  if (IsConstant)
+PrintFatalError("'C' transformer cannot be used twice");
   IsConstant = true;
   break;
 case 'K':
@@ -614,11 +621,11 @@
   break;
 case 'S':
   LMUL = LMULType(0);
-  // Update ElementBitwidth need ot update Scale too.
+  // Update ElementBitwidth need to update Scale too.
   Scale = LMUL.getScale(ElementBitwidth);
   break;
 default:
-  llvm_unreachable("Illegal non-primitive type transformer!");
+  PrintFatalError("Illegal non-primitive type transformer!");
 }
   }
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] 9773cad - [RISCV] Add additional checking to tablgen RISCVVEmitter requested in D95016.

2021-03-10 Thread Craig Topper via cfe-commits

Author: Craig Topper
Date: 2021-03-10T19:46:25-08:00
New Revision: 9773cad51939d3980d841876d77064eb7a11002f

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

LOG: [RISCV] Add additional checking to tablgen RISCVVEmitter requested in 
D95016.

This errors, but doesn't give source location. We'd need to pass
the Record through several layers to get to the location.

Reviewed By: jrtc27

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

Added: 


Modified: 
clang/utils/TableGen/RISCVVEmitter.cpp

Removed: 




diff  --git a/clang/utils/TableGen/RISCVVEmitter.cpp 
b/clang/utils/TableGen/RISCVVEmitter.cpp
index 3a84590c3a65..49574e45d5bd 100644
--- a/clang/utils/TableGen/RISCVVEmitter.cpp
+++ b/clang/utils/TableGen/RISCVVEmitter.cpp
@@ -19,6 +19,7 @@
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringSet.h"
 #include "llvm/ADT/Twine.h"
+#include "llvm/TableGen/Error.h"
 #include "llvm/TableGen/Record.h"
 #include 
 
@@ -536,7 +537,7 @@ void RVVType::applyBasicType() {
 ScalarType = ScalarTypeKind::Float;
 break;
   default:
-llvm_unreachable("Unhandled type code!");
+PrintFatalError("Unhandled type code!");
   }
   assert(ElementBitwidth != 0 && "Bad element bitwidth!");
 }
@@ -587,7 +588,7 @@ void RVVType::applyModifier(StringRef Transformer) {
 Scale = 0;
 break;
   default:
-llvm_unreachable("Illegal primitive type transformers!");
+PrintFatalError("Illegal primitive type transformers!");
   }
   Transformer = Transformer.drop_back();
 
@@ -595,9 +596,15 @@ void RVVType::applyModifier(StringRef Transformer) {
   for (char I : Transformer) {
 switch (I) {
 case 'P':
+  if (IsConstant)
+PrintFatalError("'P' transformer cannot be used after 'C'");
+  if (IsPointer)
+PrintFatalError("'P' transformer cannot be used twice");
   IsPointer = true;
   break;
 case 'C':
+  if (IsConstant)
+PrintFatalError("'C' transformer cannot be used twice");
   IsConstant = true;
   break;
 case 'K':
@@ -614,11 +621,11 @@ void RVVType::applyModifier(StringRef Transformer) {
   break;
 case 'S':
   LMUL = LMULType(0);
-  // Update ElementBitwidth need ot update Scale too.
+  // Update ElementBitwidth need to update Scale too.
   Scale = LMUL.getScale(ElementBitwidth);
   break;
 default:
-  llvm_unreachable("Illegal non-primitive type transformer!");
+  PrintFatalError("Illegal non-primitive type transformer!");
 }
   }
 }



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


[PATCH] D96090: [analyzer] Replace StoreManager::CastRetrievedVal with SValBuilder::evalCast

2021-03-10 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment.

> Actually many cases don't need to know an exact original type.

In all cases we're supposed to have an original type, whether we need it or 
not. Simply because we're simulating a typed language. If we don't have it it's 
a bug.

> Some cases can extract the type from `SVal`

Which is generally impossible because integral casts aren't modeled correctly. 
Which, again, is a bug. I don't know whether we'll have a need to pass the type 
separately when all other parts of the static analyzer become type-correct; I 
suspect that we won't need it. But one way or another, my point is, one or more 
of the following is true: "we have the ability to provide the type in all 
cases", "we don't need to provide the type in any of the cases". None of these 
ideal situations leave room for an API that receives the type //optionally//. 
For this reason I suggest to either always have the caller supply the original 
type (even if means extra work on the caller side) or never have it supply the 
original type.


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

https://reviews.llvm.org/D96090

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


[PATCH] D98379: [RISCV] Add additional checking to tablgen RISCVVEmitter requested in D95016.

2021-03-10 Thread Craig Topper via Phabricator via cfe-commits
craig.topper updated this revision to Diff 329807.
craig.topper added a comment.

Turn some llvm_unreachables into fatal errors


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98379

Files:
  clang/utils/TableGen/RISCVVEmitter.cpp


Index: clang/utils/TableGen/RISCVVEmitter.cpp
===
--- clang/utils/TableGen/RISCVVEmitter.cpp
+++ clang/utils/TableGen/RISCVVEmitter.cpp
@@ -19,6 +19,7 @@
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringSet.h"
 #include "llvm/ADT/Twine.h"
+#include "llvm/TableGen/Error.h"
 #include "llvm/TableGen/Record.h"
 
 using namespace llvm;
@@ -535,7 +536,7 @@
 ScalarType = ScalarTypeKind::Float;
 break;
   default:
-llvm_unreachable("Unhandled type code!");
+PrintFatalError("Unhandled type code!");
   }
   assert(ElementBitwidth != 0 && "Bad element bitwidth!");
 }
@@ -586,7 +587,7 @@
 Scale = 0;
 break;
   default:
-llvm_unreachable("Illegal primitive type transformers!");
+PrintFatalError("Illegal primitive type transformers!");
   }
   Transformer = Transformer.drop_back();
 
@@ -594,9 +595,15 @@
   for (char I : Transformer) {
 switch (I) {
 case 'P':
+  if (IsConstant)
+PrintFatalError("'P' transformer cannot be used after 'C'");
+  if (IsPointer)
+PrintFatalError("'P' transformer cannot be used twice");
   IsPointer = true;
   break;
 case 'C':
+  if (IsConstant)
+PrintFatalError("'C' transformer cannot be used twice");
   IsConstant = true;
   break;
 case 'K':
@@ -613,11 +620,11 @@
   break;
 case 'S':
   LMUL = LMULType(0);
-  // Update ElementBitwidth need ot update Scale too.
+  // Update ElementBitwidth need to update Scale too.
   Scale = LMUL.getScale(ElementBitwidth);
   break;
 default:
-  llvm_unreachable("Illegal non-primitive type transformer!");
+  PrintFatalError("Illegal non-primitive type transformer!");
 }
   }
 }


Index: clang/utils/TableGen/RISCVVEmitter.cpp
===
--- clang/utils/TableGen/RISCVVEmitter.cpp
+++ clang/utils/TableGen/RISCVVEmitter.cpp
@@ -19,6 +19,7 @@
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringSet.h"
 #include "llvm/ADT/Twine.h"
+#include "llvm/TableGen/Error.h"
 #include "llvm/TableGen/Record.h"
 
 using namespace llvm;
@@ -535,7 +536,7 @@
 ScalarType = ScalarTypeKind::Float;
 break;
   default:
-llvm_unreachable("Unhandled type code!");
+PrintFatalError("Unhandled type code!");
   }
   assert(ElementBitwidth != 0 && "Bad element bitwidth!");
 }
@@ -586,7 +587,7 @@
 Scale = 0;
 break;
   default:
-llvm_unreachable("Illegal primitive type transformers!");
+PrintFatalError("Illegal primitive type transformers!");
   }
   Transformer = Transformer.drop_back();
 
@@ -594,9 +595,15 @@
   for (char I : Transformer) {
 switch (I) {
 case 'P':
+  if (IsConstant)
+PrintFatalError("'P' transformer cannot be used after 'C'");
+  if (IsPointer)
+PrintFatalError("'P' transformer cannot be used twice");
   IsPointer = true;
   break;
 case 'C':
+  if (IsConstant)
+PrintFatalError("'C' transformer cannot be used twice");
   IsConstant = true;
   break;
 case 'K':
@@ -613,11 +620,11 @@
   break;
 case 'S':
   LMUL = LMULType(0);
-  // Update ElementBitwidth need ot update Scale too.
+  // Update ElementBitwidth need to update Scale too.
   Scale = LMUL.getScale(ElementBitwidth);
   break;
 default:
-  llvm_unreachable("Illegal non-primitive type transformer!");
+  PrintFatalError("Illegal non-primitive type transformer!");
 }
   }
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D98364: [clangd] Use ref counted strings throughout for File Contents

2021-03-10 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment.

Avoiding copies seems nice, but this makes some interfaces more awkward. Do you 
have any measurements that some of these copies matter?

(The dirty FS changes avoided coping all the dirty buffers at once, but it 
seems like these changes will mostly save ~1 file copy per operation)




Comment at: clang-tools-extra/clangd/ClangdServer.h:190
   /// diagnostics etc built from it. If empty, a version number is generated.
+  void addDocument(PathRef File, std::shared_ptr Contents,
+   llvm::StringRef Version = "null",

what's the purpose of changing this interface?

The only callers that matter are in ClangdLSPServer, and this signature 
requires it to copy anyway. So why not copy inside ClangdServer?

(StringRef is more convenient to for tests and C++ embedders, and just a bit 
less churn)



Comment at: clang-tools-extra/clangd/Preamble.cpp:231
 llvm::Expected
-scanPreamble(llvm::StringRef Contents, const tooling::CompileCommand ) {
+scanPreamble(std::shared_ptr Contents,
+ const tooling::CompileCommand ) {

I don't think this change makes sense, we don't need ownership here



Comment at: clang-tools-extra/clangd/Preamble.cpp:324
   // without those.
-  auto ContentsBuffer =
-  llvm::MemoryBuffer::getMemBuffer(Inputs.Contents, FileName);
+  auto ContentsBuffer = getSharedStringBuffer(Inputs.Contents, FileName);
   auto Bounds = ComputePreambleBounds(*CI.getLangOpts(), *ContentsBuffer, 0);

again, no need for ownership



Comment at: clang-tools-extra/clangd/Preamble.cpp:376
   const CompilerInvocation ) {
-  auto ContentsBuffer =
-  llvm::MemoryBuffer::getMemBuffer(Inputs.Contents, FileName);
+  auto ContentsBuffer = getSharedStringBuffer(Inputs.Contents, FileName);
   auto Bounds = ComputePreambleBounds(*CI.getLangOpts(), *ContentsBuffer, 0);

and here


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98364

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


[PATCH] D98379: [RISCV] Add additional checking to tablgen RISCVVEmitter requested in D95016.

2021-03-10 Thread Jessica Clarke via Phabricator via cfe-commits
jrtc27 accepted this revision.
jrtc27 added a comment.
This revision is now accepted and ready to land.

Any TableGen backend error that's a real message and not an assertion failure 
with a backtrace is a win in my books :)




Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:627
 default:
   llvm_unreachable("Illegal non-primitive type transformer!");
 }

Hm, if this is the actual parser then these should also be PrintFatalError 
(another instance earlier in the function, and one in applyBasicType)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98379

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


[PATCH] D98377: [clangd] Show padding following a field on field hover.

2021-03-10 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 329805.
sammccall added a comment.

Stop reporting offset for fields in a union, too


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98377

Files:
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/Hover.h
  clang-tools-extra/clangd/unittests/HoverTests.cpp

Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -68,8 +68,9 @@
   // Field
   {R"cpp(
   namespace ns1 { namespace ns2 {
-struct Foo {
+class Foo {
   char [[b^ar]];
+  double y[2];
 };
   }}
   )cpp",
@@ -82,6 +83,41 @@
  HI.Type = "char";
  HI.Offset = 0;
  HI.Size = 1;
+ HI.Padding = 7;
+ HI.AccessSpecifier = "private";
+   }},
+  // Union field
+  {R"cpp(
+union Foo {
+  char [[b^ar]];
+  double y[2];
+};
+  )cpp",
+   [](HoverInfo ) {
+ HI.NamespaceScope = "";
+ HI.LocalScope = "Foo::";
+ HI.Name = "bar";
+ HI.Kind = index::SymbolKind::Field;
+ HI.Definition = "char bar";
+ HI.Type = "char";
+ HI.Size = 1;
+ HI.Padding = 15;
+ HI.AccessSpecifier = "public";
+   }},
+  // Bitfield
+  {R"cpp(
+struct Foo {
+  int [[^x]] : 1;
+  int y : 1;
+};
+  )cpp",
+   [](HoverInfo ) {
+ HI.NamespaceScope = "";
+ HI.LocalScope = "Foo::";
+ HI.Name = "x";
+ HI.Kind = index::SymbolKind::Field;
+ HI.Definition = "int x : 1";
+ HI.Type = "int";
  HI.AccessSpecifier = "public";
}},
   // Local to class method.
@@ -2544,13 +2580,14 @@
 HI.Definition = "def";
 HI.Size = 4;
 HI.Offset = 12;
+HI.Padding = 4;
   },
   R"(field foo
 
 Type: type
 Value = value
 Offset: 12 bytes
-Size: 4 bytes
+Size: 4 bytes (+4 padding)
 
 // In test::Bar
 def)",
Index: clang-tools-extra/clangd/Hover.h
===
--- clang-tools-extra/clangd/Hover.h
+++ clang-tools-extra/clangd/Hover.h
@@ -77,6 +77,8 @@
   llvm::Optional Size;
   /// Contains the offset of fields within the enclosing class.
   llvm::Optional Offset;
+  /// Contains the padding following a field within the enclosing class.
+  llvm::Optional Padding;
   // Set when symbol is inside function call. Contains information extracted
   // from the callee definition about the argument this is passed as.
   llvm::Optional CalleeArgInfo;
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -28,6 +28,7 @@
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/OperationKinds.h"
 #include "clang/AST/PrettyPrinter.h"
+#include "clang/AST/RecordLayout.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/Type.h"
 #include "clang/Basic/SourceLocation.h"
@@ -769,10 +770,29 @@
 const auto *Record = FD->getParent();
 if (Record)
   Record = Record->getDefinition();
-if (Record && !Record->isInvalidDecl() && !Record->isDependentType()) {
-  HI.Offset = Ctx.getFieldOffset(FD) / 8;
-  if (auto Size = Ctx.getTypeSizeInCharsIfKnown(FD->getType()))
-HI.Size = Size->getQuantity();
+if (Record && !Record->isInvalidDecl() && !Record->isDependentType() &&
+!FD->isBitField()) {
+  const ASTRecordLayout  = Ctx.getASTRecordLayout(Record);
+  HI.Offset = Layout.getFieldOffset(FD->getFieldIndex()) / 8;
+  if (auto Size = Ctx.getTypeSizeInCharsIfKnown(FD->getType())) {
+HI.Size = FD->isZeroSize(Ctx) ? 0 : Size->getQuantity();
+unsigned EndOfField = *HI.Offset + *HI.Size;
+
+// Calculate padding following the field.
+if (!Record->isUnion() &&
+FD->getFieldIndex() + 1 < Layout.getFieldCount()) {
+  // Measure padding up to the next class field.
+  unsigned NextOffset =
+  Layout.getFieldOffset(FD->getFieldIndex() + 1) / 8;
+  if (NextOffset >= EndOfField) // next field could be a bitfield!
+HI.Padding = NextOffset - EndOfField;
+} else {
+  // Measure padding up to the end of the object.
+  HI.Padding = Layout.getSize().getQuantity() - EndOfField;
+}
+  }
+  if (Record->isUnion())
+HI.Offset.reset();
 }
 return;
   }
@@ -1012,9 +1032,12 @@
 Output.addParagraph().appendText(
 llvm::formatv("Offset: {0} byte{1}", *Offset, *Offset == 1 ? "" : "s")
  

[PATCH] D98379: [RISCV] Add additional checking to tablgen RISCVVEmitter requested in D95016.

2021-03-10 Thread Craig Topper via Phabricator via cfe-commits
craig.topper created this revision.
craig.topper added reviewers: jrtc27, khchen.
Herald added subscribers: vkmr, frasercrmck, evandro, luismarques, apazos, 
sameer.abuasal, s.egerton, Jim, benna, psnobl, jocewei, PkmX, the_o, 
brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, shiva0217, 
kito-cheng, niosHD, sabuasal, simoncook, johnrusso, rbar, asb.
craig.topper requested review of this revision.
Herald added a subscriber: MaskRay.
Herald added a project: clang.

This errors, but doesn't give source location. We'd need to pass
the Record through several layers to get to the location.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D98379

Files:
  clang/utils/TableGen/RISCVVEmitter.cpp


Index: clang/utils/TableGen/RISCVVEmitter.cpp
===
--- clang/utils/TableGen/RISCVVEmitter.cpp
+++ clang/utils/TableGen/RISCVVEmitter.cpp
@@ -19,6 +19,7 @@
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringSet.h"
 #include "llvm/ADT/Twine.h"
+#include "llvm/TableGen/Error.h"
 #include "llvm/TableGen/Record.h"
 
 using namespace llvm;
@@ -594,9 +595,15 @@
   for (char I : Transformer) {
 switch (I) {
 case 'P':
+  if (IsConstant)
+PrintFatalError("'P' transformer cannot be used after 'C'");
+  if (IsPointer)
+PrintFatalError("'P' transformer cannot be used twice");
   IsPointer = true;
   break;
 case 'C':
+  if (IsConstant)
+PrintFatalError("'C' transformer cannot be used twice");
   IsConstant = true;
   break;
 case 'K':
@@ -613,7 +620,7 @@
   break;
 case 'S':
   LMUL = LMULType(0);
-  // Update ElementBitwidth need ot update Scale too.
+  // Update ElementBitwidth need to update Scale too.
   Scale = LMUL.getScale(ElementBitwidth);
   break;
 default:


Index: clang/utils/TableGen/RISCVVEmitter.cpp
===
--- clang/utils/TableGen/RISCVVEmitter.cpp
+++ clang/utils/TableGen/RISCVVEmitter.cpp
@@ -19,6 +19,7 @@
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringSet.h"
 #include "llvm/ADT/Twine.h"
+#include "llvm/TableGen/Error.h"
 #include "llvm/TableGen/Record.h"
 
 using namespace llvm;
@@ -594,9 +595,15 @@
   for (char I : Transformer) {
 switch (I) {
 case 'P':
+  if (IsConstant)
+PrintFatalError("'P' transformer cannot be used after 'C'");
+  if (IsPointer)
+PrintFatalError("'P' transformer cannot be used twice");
   IsPointer = true;
   break;
 case 'C':
+  if (IsConstant)
+PrintFatalError("'C' transformer cannot be used twice");
   IsConstant = true;
   break;
 case 'K':
@@ -613,7 +620,7 @@
   break;
 case 'S':
   LMUL = LMULType(0);
-  // Update ElementBitwidth need ot update Scale too.
+  // Update ElementBitwidth need to update Scale too.
   Scale = LMUL.getScale(ElementBitwidth);
   break;
 default:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D98377: [clangd] Show padding following a field on field hover.

2021-03-10 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments.



Comment at: clang-tools-extra/clangd/unittests/HoverTests.cpp:105
+ HI.Size = 1;
+ HI.Padding = 15;
+ HI.AccessSpecifier = "public";

njames93 wrote:
> Does it make any sense to include the padding for union fields?
I think so - it tells you how much room you have to grow/are wasting in 
particular branches of the union. True, it's easy enough to infer by looking at 
the union size and the field size. But on the other hand, the *purpose* of a 
union is to get a compact memory layout, so memory layout information is more 
likely to be useful.

Maybe we should omit Offset though...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98377

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


[PATCH] D98377: [clangd] Show padding following a field on field hover.

2021-03-10 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments.



Comment at: clang-tools-extra/clangd/unittests/HoverTests.cpp:105
+ HI.Size = 1;
+ HI.Padding = 15;
+ HI.AccessSpecifier = "public";

Does it make any sense to include the padding for union fields?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98377

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


[PATCH] D98264: [AArch64] Implement __rndr, __rndrrs intrinsics

2021-03-10 Thread Stelios Ioannou via Phabricator via cfe-commits
stelios-arm updated this revision to Diff 329799.
stelios-arm marked 2 inline comments as done.
stelios-arm added a comment.

Addressed the comments made by @SjoerdMeijer and @dmgreen.


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

https://reviews.llvm.org/D98264

Files:
  clang/include/clang/Basic/BuiltinsAArch64.def
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/Basic/Targets/AArch64.h
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Headers/arm_acle.h
  clang/test/CodeGen/arm_acle.c
  clang/test/CodeGen/builtins-arm64.c
  llvm/include/llvm/IR/IntrinsicsAArch64.td
  llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.h
  llvm/lib/Target/AArch64/AArch64InstrFormats.td
  llvm/lib/Target/AArch64/AArch64InstrInfo.td
  llvm/test/CodeGen/AArch64/rand.ll
  llvm/test/CodeGen/AArch64/stp-opt-with-renaming.mir

Index: llvm/test/CodeGen/AArch64/stp-opt-with-renaming.mir
===
--- llvm/test/CodeGen/AArch64/stp-opt-with-renaming.mir
+++ llvm/test/CodeGen/AArch64/stp-opt-with-renaming.mir
@@ -100,11 +100,11 @@
   bb.0:
 liveins: $x0
 
-renamable $x8 = MRS 58880
+renamable $x8 = MRS 58880, implicit-def $nzcv
 renamable $x8 = MOVZXi 15309, 0
 renamable $x8 = MOVKXi renamable $x8, 26239, 16
 STRXui renamable $x8, renamable $x0, 0, implicit killed $x8 :: (store 8)
-renamable $x8 = MRS 55840
+renamable $x8 = MRS 55840, implicit-def $nzcv
 STRXui killed renamable  $x8, renamable killed $x0, 1, implicit killed $x8 :: (store 8)
 RET undef $lr
 
@@ -134,9 +134,9 @@
   bb.0:
 liveins: $x0, $x1
 
-renamable $x8 = MRS 58880
+renamable $x8 = MRS 58880, implicit-def $nzcv
 STRXui renamable $x8, renamable $x0, 0, implicit killed $x8 :: (store 4)
-renamable $x8 = MRS 55840
+renamable $x8 = MRS 55840, implicit-def $nzcv
 STRXui killed renamable  $x8, renamable killed $x0, 1, implicit killed $x8 :: (store 4)
 RET undef $lr
 
@@ -166,9 +166,9 @@
   bb.0:
 liveins: $x0, $x1
 
-renamable $x8 = MRS 58880
+renamable $x8 = MRS 58880, implicit-def $nzcv
 STRWui renamable $w8, renamable $x0, 0, implicit killed $x8 :: (store 4)
-renamable $x8 = MRS 55840
+renamable $x8 = MRS 55840, implicit-def $nzcv
 STRWui killed renamable $w8, renamable killed $x0, 1, implicit killed $x8 :: (store 4)
 RET undef $lr
 
@@ -275,10 +275,10 @@
   bb.0:
 liveins: $x0, $x1
 
-renamable $x8 = MRS 58880
+renamable $x8 = MRS 58880, implicit-def $nzcv
 renamable $w8 = ORRWrs $wzr, killed renamable $w8, 0, implicit-def $x8
 STRWui renamable $w8, renamable $x0, 0, implicit killed $x8 :: (store 4)
-renamable $x8 = MRS 55840
+renamable $x8 = MRS 55840, implicit-def $nzcv
 STRWui killed renamable $w8, renamable killed $x0, 1, implicit killed $x8 :: (store 4)
 RET undef $lr
 
Index: llvm/test/CodeGen/AArch64/rand.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/rand.ll
@@ -0,0 +1,38 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=aarch64 -mcpu=neoverse-v1 -mattr=+rand %s -o - | FileCheck %s
+
+define  i32 @rndr(i64* %__addr) {
+; CHECK-LABEL: rndr:
+; CHECK:   // %bb.0:
+; CHECK-NEXT:mrs x8, RNDR
+; CHECK-NEXT:cset w9, eq
+; CHECK-NEXT:str x8, [x0]
+; CHECK-NEXT:and w0, w9, #0x1
+; CHECK-NEXT:ret
+  %1 = tail call { i64, i1 } @llvm.aarch64.rndr()
+  %2 = extractvalue { i64, i1 } %1, 0
+  %3 = extractvalue { i64, i1 } %1, 1
+  store i64 %2, i64* %__addr, align 8
+  %4 = zext i1 %3 to i32
+  ret i32 %4
+}
+
+
+define  i32 @rndrrs(i64*  %__addr) {
+; CHECK-LABEL: rndrrs:
+; CHECK:   // %bb.0:
+; CHECK-NEXT:mrs x8, RNDRRS
+; CHECK-NEXT:cset w9, eq
+; CHECK-NEXT:str x8, [x0]
+; CHECK-NEXT:and w0, w9, #0x1
+; CHECK-NEXT:ret
+  %1 = tail call { i64, i1 } @llvm.aarch64.rndrrs()
+  %2 = extractvalue { i64, i1 } %1, 0
+  %3 = extractvalue { i64, i1 } %1, 1
+  store i64 %2, i64* %__addr, align 8
+  %4 = zext i1 %3 to i32
+  ret i32 %4
+}
+
+declare { i64, i1 } @llvm.aarch64.rndr()
+declare { i64, i1 } @llvm.aarch64.rndrrs()
Index: llvm/lib/Target/AArch64/AArch64InstrInfo.td
===
--- llvm/lib/Target/AArch64/AArch64InstrInfo.td
+++ llvm/lib/Target/AArch64/AArch64InstrInfo.td
@@ -607,7 +607,10 @@
 def AArch64stnp : SDNode<"AArch64ISD::STNP", SDT_AArch64stnp, [SDNPHasChain, SDNPMayStore, SDNPMemOperand]>;
 
 def AArch64tbl : SDNode<"AArch64ISD::TBL", SDT_AArch64TBL>;
-
+// MRS from CodeGen.
+def AArch64mrs : SDNode<"AArch64ISD::MRS",
+SDTypeProfile<1, 1, [SDTCisVT<0, i64>, SDTCisVT<1, i32>]>,
+[SDNPHasChain, SDNPOutGlue]>;
 //===--===//
 
 

[PATCH] D98375: [clang][Driver] Expose -fexperimental-relative-c++-abi-vtables flag

2021-03-10 Thread Leonard Chan 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 rG70af0bf6fe46: [clang][Driver] Expose 
-fexperimental-relative-c++-abi-vtables flag (authored by leonardchan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98375

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/relative-vtables-flag.cpp


Index: clang/test/Driver/relative-vtables-flag.cpp
===
--- /dev/null
+++ clang/test/Driver/relative-vtables-flag.cpp
@@ -0,0 +1,7 @@
+// RUN: %clangxx --target=aarch64-unknown-fuchsia 
-fexperimental-relative-c++-abi-vtables -c %s -### 2>&1 | FileCheck %s 
--check-prefix=RV
+// RUN: %clangxx --target=aarch64-unknown-fuchsia 
-fno-experimental-relative-c++-abi-vtables -c %s -### 2>&1 | FileCheck %s 
--check-prefix=NO-RV
+// RUN: %clangxx --target=aarch64-unknown-fuchsia -c %s -### 2>&1 | FileCheck 
%s --check-prefix=NO-RV
+// RUN: %clangxx --target=aarch64-unknown-linux-gnu -c %s -### 2>&1 | 
FileCheck %s --check-prefix=NO-RV
+
+// RV: "-fexperimental-relative-c++-abi-vtables"
+// NO-RV-NOT: "-fexperimental-relative-c++-abi-vtables"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4869,6 +4869,9 @@
   Args.AddLastArg(CmdArgs, options::OPT_ffine_grained_bitfield_accesses,
   options::OPT_fno_fine_grained_bitfield_accesses);
 
+  Args.AddLastArg(CmdArgs, options::OPT_fexperimental_relative_cxx_abi_vtables,
+  options::OPT_fno_experimental_relative_cxx_abi_vtables);
+
   // Handle segmented stacks.
   if (Args.hasArg(options::OPT_fsplit_stack))
 CmdArgs.push_back("-split-stacks");


Index: clang/test/Driver/relative-vtables-flag.cpp
===
--- /dev/null
+++ clang/test/Driver/relative-vtables-flag.cpp
@@ -0,0 +1,7 @@
+// RUN: %clangxx --target=aarch64-unknown-fuchsia -fexperimental-relative-c++-abi-vtables -c %s -### 2>&1 | FileCheck %s --check-prefix=RV
+// RUN: %clangxx --target=aarch64-unknown-fuchsia -fno-experimental-relative-c++-abi-vtables -c %s -### 2>&1 | FileCheck %s --check-prefix=NO-RV
+// RUN: %clangxx --target=aarch64-unknown-fuchsia -c %s -### 2>&1 | FileCheck %s --check-prefix=NO-RV
+// RUN: %clangxx --target=aarch64-unknown-linux-gnu -c %s -### 2>&1 | FileCheck %s --check-prefix=NO-RV
+
+// RV: "-fexperimental-relative-c++-abi-vtables"
+// NO-RV-NOT: "-fexperimental-relative-c++-abi-vtables"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4869,6 +4869,9 @@
   Args.AddLastArg(CmdArgs, options::OPT_ffine_grained_bitfield_accesses,
   options::OPT_fno_fine_grained_bitfield_accesses);
 
+  Args.AddLastArg(CmdArgs, options::OPT_fexperimental_relative_cxx_abi_vtables,
+  options::OPT_fno_experimental_relative_cxx_abi_vtables);
+
   // Handle segmented stacks.
   if (Args.hasArg(options::OPT_fsplit_stack))
 CmdArgs.push_back("-split-stacks");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] 70af0bf - [clang][Driver] Expose -fexperimental-relative-c++-abi-vtables flag

2021-03-10 Thread Leonard Chan via cfe-commits

Author: Leonard Chan
Date: 2021-03-10T16:28:40-08:00
New Revision: 70af0bf6fe462cc16a2e56657d20eeed126d41d0

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

LOG: [clang][Driver] Expose -fexperimental-relative-c++-abi-vtables flag

Initially, this flag was meant to only be used through cc1 and not directly
through the clang driver. However, we accidentally ended up using this flag
as a driver flag already for selecting multilibs within the fuchsia toolchain.
We're currently in an awkward state where it's only accepted as a driver flag
when targeting Fuchsia, and all other instances it can only be added via
-Xclang. Since we're ready to use this in Fuchsia, we can just expose this to
the driver for simplicity.

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

Added: 
clang/test/Driver/relative-vtables-flag.cpp

Modified: 
clang/lib/Driver/ToolChains/Clang.cpp

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index 7e7ad9437a1c..246bdf42a66a 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -4869,6 +4869,9 @@ void Clang::ConstructJob(Compilation , const JobAction 
,
   Args.AddLastArg(CmdArgs, options::OPT_ffine_grained_bitfield_accesses,
   options::OPT_fno_fine_grained_bitfield_accesses);
 
+  Args.AddLastArg(CmdArgs, options::OPT_fexperimental_relative_cxx_abi_vtables,
+  options::OPT_fno_experimental_relative_cxx_abi_vtables);
+
   // Handle segmented stacks.
   if (Args.hasArg(options::OPT_fsplit_stack))
 CmdArgs.push_back("-split-stacks");

diff  --git a/clang/test/Driver/relative-vtables-flag.cpp 
b/clang/test/Driver/relative-vtables-flag.cpp
new file mode 100644
index ..1253809e1d42
--- /dev/null
+++ b/clang/test/Driver/relative-vtables-flag.cpp
@@ -0,0 +1,7 @@
+// RUN: %clangxx --target=aarch64-unknown-fuchsia 
-fexperimental-relative-c++-abi-vtables -c %s -### 2>&1 | FileCheck %s 
--check-prefix=RV
+// RUN: %clangxx --target=aarch64-unknown-fuchsia 
-fno-experimental-relative-c++-abi-vtables -c %s -### 2>&1 | FileCheck %s 
--check-prefix=NO-RV
+// RUN: %clangxx --target=aarch64-unknown-fuchsia -c %s -### 2>&1 | FileCheck 
%s --check-prefix=NO-RV
+// RUN: %clangxx --target=aarch64-unknown-linux-gnu -c %s -### 2>&1 | 
FileCheck %s --check-prefix=NO-RV
+
+// RV: "-fexperimental-relative-c++-abi-vtables"
+// NO-RV-NOT: "-fexperimental-relative-c++-abi-vtables"



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


[PATCH] D46443: [libc++] Add missing cstdalign header

2021-03-10 Thread Sam Clegg via Phabricator via cfe-commits
sbc100 added inline comments.



Comment at: libcxx/include/cstdalign:24
+#include <__config>
+#include 
+

hubert.reinterpretcast wrote:
> curdeius wrote:
> > hubert.reinterpretcast wrote:
> > > curdeius wrote:
> > > > curdeius wrote:
> > > > > hubert.reinterpretcast wrote:
> > > > > > sbc100 wrote:
> > > > > > > hubert.reinterpretcast wrote:
> > > > > > > > This seems to be assuming that the underlying C library's 
> > > > > > > > `stdalign.h` is C++ friendly. A C11 `stdalign.h` //does// 
> > > > > > > > define `alignof` and `alignas` as macros.
> > > > > > > Should I just remove this `#include` then?
> > > > > > The idea would be to //add// a `stdalign.h` alongside this header 
> > > > > > that doesn't `#include_next` the underlying C library's 
> > > > > > `stdalign.h`.
> > > > > I'm not sure if that should be the solution. At least gcc's libstdc++ 
> > > > > assumes that `stdalign.h` is C++-compatbile (cf. 
> > > > > https://github.com/gcc-mirror/gcc/blob/16e2427f50c208dfe07d07f18009969502c25dc8/libstdc%2B%2B-v3/include/c_global/cstdalign).
> > > > > 
> > > > > Clang provides a compatible header: 
> > > > > https://github.com/llvm/llvm-project/commit/8acb4044d83ecc9df81b1c9f327d5bd4325e1756.
> > > > > Gcc too of course: 
> > > > > https://github.com/gcc-mirror/gcc/blob/16e2427f50c208dfe07d07f18009969502c25dc8/gcc/ginclude/stdalign.h.
> > > > > 
> > > > > MSVC's STL on the other hand, doesn't include `` 
> > > > > (https://github.com/microsoft/STL/blob/main/stl/inc/cstdalign).
> > > > > 
> > > > > @hubert.reinterpretcast, are you aware of an environment which has 
> > > > > non-friendly `stdalign.h`?
> > > > FYI, musl is also C++ friendly: 
> > > > https://git.musl-libc.org/cgit/musl/tree/include/stdalign.h.
> > > >>! Quote:
> > > > @hubert.reinterpretcast, are you aware of an environment which has 
> > > > non-friendly stdalign.h?
> > > 
> > > The one GCC provides disagrees with the interpretation I gave of which 
> > > macros should be present. The one that Clang provides //does// match my 
> > > interpretation. It seems the GCC one is non-friendly (albeit a different 
> > > form of non-friendly than the one I opened with).
> > Oh, you mean that `__alignas_is_defined` and `__alignof_is_defined` won't 
> > be defined in this case, right?
> > In this case, I guess we won't avoid having `stdalign.h` as you had 
> > suggested.
> > And indeed the test fails with gcc:
> > ```
> > bin/llvm-lit -vv ../../libcxx/test/std/language.support/cstdalign/ 
> > --param=std=c++17 --param=cxx_under_test=`which g++`
> > ...
> > libcxx/test/std/language.support/cstdalign/cstdalign.pass.cpp:21:2: error: 
> > #error __alignas_is_defined not defined
> >21 | #error __alignas_is_defined not defined
> >   |  ^
> > ```
> > 
> > That's unfortunately a configuration which is not tested in the CI.
> Yes, it looks like adding a `stdalign.h` for libc++ is needed to reliably get 
> `__alignas_is_defined` and `__alignof_is_defined`.
Alternatively, could we just defined them if they are not already and skip 
including `stdalign.h` here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D46443

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


[PATCH] D98264: [AArch64] Implement __rndr, __rndrrs intrinsics

2021-03-10 Thread Stelios Ioannou via Phabricator via cfe-commits
stelios-arm marked 4 inline comments as done.
stelios-arm added inline comments.



Comment at: clang/lib/Basic/Targets/AArch64.cpp:363
 
+  if (HasRandGen)
+Builder.defineMacro("__ARM_FEATURE_RNG", "1");

SjoerdMeijer wrote:
> Where/when is `HasRandGen` set?
Oh, I forgot to set it, I am going to address this in the new revision. 



Comment at: clang/test/Preprocessor/init-aarch64.c:28
 // AARCH64-NEXT: #define __ARM_FEATURE_NUMERIC_MAXMIN 1
+// AARCH64-NEXT: #define  __ARM_FEATURE_RNG 1
 // AARCH64-NEXT: #define __ARM_FEATURE_UNALIGNED 1

SjoerdMeijer wrote:
> Why are we expecting this here? Are we not only expecting this for v8.5?
> 
> We also need a negative test and CHECK-NOT of this where we don't expect this.
Correct this shouldn't be here. I am going to address this in a new revision. 



Comment at: llvm/lib/Target/AArch64/AArch64InstrFormats.td:1495
   let DecoderNamespace = "Fallback";
+  let Defs = [NZCV];
 }

SjoerdMeijer wrote:
> SjoerdMeijer wrote:
> > dmgreen wrote:
> > > SjoerdMeijer wrote:
> > > > dmgreen wrote:
> > > > > SjoerdMeijer wrote:
> > > > > > Do all MRS instructions do this?
> > > > > No, but some do and it's not obvious which ones do and don't. I think 
> > > > > it should be reasonable to always def NZCV here, even if they are 
> > > > > just dead. It should be very rare that it would be beneficial for 
> > > > > NZCV to be live across a MRS instruction.
> > > > True, but since creating another definition is cheap, what would the 
> > > > cons be of:
> > > > 
> > > >   class MRS_NZCV : MRSI {
> > > > ..
> > > > let Defs = [NZCV];
> > > >   }
> > > > 
> > > > The way I look at it is that the description would be more accurate?
> > > I believe that would have an over-lapping definition with the existing 
> > > MRS instruction?
> > > 
> > > It would need to be a pseudo I think, which would eventually be turned 
> > > into a MSR proper late enough on the pipeline for it to be valid (or the 
> > > other way around, the no-nzcv version gets turned into a the nzcv version 
> > > to keep the verifier happy).
> > > 
> > > It could also be a optional def, but those are only used in the arm 
> > > backend and I would not recommend using them anywhere else.  I would 
> > > probably suggest just setting MRS as a NZCV setting instruction, unless 
> > > we find a reason to need to implement it differently.
> > > I believe that would have an over-lapping definition with the existing 
> > > MRS instruction?
> > 
> > Ah yeah, that might be true.
> > 
> > > It would need to be a pseudo I think
> > 
> > How much work is adding a pseudo for this case? My first reaction would be 
> > just trying to model this correctly, then we potentially don't have to 
> > worry again later about this. 
> I just wanted to add that I don't have too strong opinions on this, but what 
> I suggested seemed more the correct, even though the consequence of setting 
> NCZV for all MRS is minimal. So I will leave this one up to you @dmgreen and 
> @stelios-arm .
I will talk with @dmgreen and if it is decided that it is needed to change, I 
will address it in a future revision. Please note that the next revision of 
patch, will not address this comment (temporarily). 



Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.td:1274
+def : Pat<(AArch64mrs imm:$id),
+  (MRS imm:$id)>;
+

dmgreen wrote:
> SjoerdMeijer wrote:
> > Nit: can be on the same line.
> I always prefer Pat's to have input and output lines separate, for what it's 
> worth. It makes them more easily readable.
The styling for Pat's is not consistent across the file, some are in one line 
and others have the input and output lines separate. I also prefer the latter 
for readability. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98264

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


[PATCH] D98377: [clangd] Show padding following a field on field hover.

2021-03-10 Thread Sam McCall via Phabricator via cfe-commits
sammccall created this revision.
sammccall added a reviewer: kbobyrev.
Herald added subscribers: usaxena95, kadircet, arphaman.
sammccall requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang.

This displays as: `Size: 4 bytes (+4 padding)`

Also stop showing (byte) offset/size for bitfields. They're not
meaningful and using them to calculate padding is dangerous!


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D98377

Files:
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/Hover.h
  clang-tools-extra/clangd/unittests/HoverTests.cpp

Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -68,8 +68,9 @@
   // Field
   {R"cpp(
   namespace ns1 { namespace ns2 {
-struct Foo {
+class Foo {
   char [[b^ar]];
+  double y[2];
 };
   }}
   )cpp",
@@ -82,6 +83,42 @@
  HI.Type = "char";
  HI.Offset = 0;
  HI.Size = 1;
+ HI.Padding = 7;
+ HI.AccessSpecifier = "private";
+   }},
+  // Union field
+  {R"cpp(
+union Foo {
+  char [[b^ar]];
+  double y[2];
+};
+  )cpp",
+   [](HoverInfo ) {
+ HI.NamespaceScope = "";
+ HI.LocalScope = "Foo::";
+ HI.Name = "bar";
+ HI.Kind = index::SymbolKind::Field;
+ HI.Definition = "char bar";
+ HI.Type = "char";
+ HI.Offset = 0;
+ HI.Size = 1;
+ HI.Padding = 15;
+ HI.AccessSpecifier = "public";
+   }},
+  // Bitfield
+  {R"cpp(
+struct Foo {
+  int [[^x]] : 1;
+  int y : 1;
+};
+  )cpp",
+   [](HoverInfo ) {
+ HI.NamespaceScope = "";
+ HI.LocalScope = "Foo::";
+ HI.Name = "x";
+ HI.Kind = index::SymbolKind::Field;
+ HI.Definition = "int x : 1";
+ HI.Type = "int";
  HI.AccessSpecifier = "public";
}},
   // Local to class method.
@@ -2544,13 +2581,14 @@
 HI.Definition = "def";
 HI.Size = 4;
 HI.Offset = 12;
+HI.Padding = 4;
   },
   R"(field foo
 
 Type: type
 Value = value
 Offset: 12 bytes
-Size: 4 bytes
+Size: 4 bytes (+4 padding)
 
 // In test::Bar
 def)",
Index: clang-tools-extra/clangd/Hover.h
===
--- clang-tools-extra/clangd/Hover.h
+++ clang-tools-extra/clangd/Hover.h
@@ -77,6 +77,8 @@
   llvm::Optional Size;
   /// Contains the offset of fields within the enclosing class.
   llvm::Optional Offset;
+  /// Contains the padding following a field within the enclosing class.
+  llvm::Optional Padding;
   // Set when symbol is inside function call. Contains information extracted
   // from the callee definition about the argument this is passed as.
   llvm::Optional CalleeArgInfo;
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -28,6 +28,7 @@
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/OperationKinds.h"
 #include "clang/AST/PrettyPrinter.h"
+#include "clang/AST/RecordLayout.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/Type.h"
 #include "clang/Basic/SourceLocation.h"
@@ -769,10 +770,27 @@
 const auto *Record = FD->getParent();
 if (Record)
   Record = Record->getDefinition();
-if (Record && !Record->isInvalidDecl() && !Record->isDependentType()) {
-  HI.Offset = Ctx.getFieldOffset(FD) / 8;
-  if (auto Size = Ctx.getTypeSizeInCharsIfKnown(FD->getType()))
-HI.Size = Size->getQuantity();
+if (Record && !Record->isInvalidDecl() && !Record->isDependentType() &&
+!FD->isBitField()) {
+  const ASTRecordLayout  = Ctx.getASTRecordLayout(Record);
+  HI.Offset = Layout.getFieldOffset(FD->getFieldIndex()) / 8;
+  if (auto Size = Ctx.getTypeSizeInCharsIfKnown(FD->getType())) {
+HI.Size = FD->isZeroSize(Ctx) ? 0 : Size->getQuantity();
+unsigned EndOfField = *HI.Offset + *HI.Size;
+
+// Calculate padding following the field.
+if (!Record->isUnion() &&
+FD->getFieldIndex() + 1 < Layout.getFieldCount()) {
+  // Measure padding up to the next class field.
+  unsigned NextOffset =
+  Layout.getFieldOffset(FD->getFieldIndex() + 1) / 8;
+  if (NextOffset >= EndOfField) // next field could be a bitfield!
+HI.Padding = NextOffset - EndOfField;
+} else {
+  // Measure padding up to the end of the object.
+  HI.Padding = 

[PATCH] D98375: [clang][Driver] Expose -fexperimental-relative-c++-abi-vtables flag

2021-03-10 Thread Petr Hosek via Phabricator via cfe-commits
phosek accepted this revision.
phosek added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98375

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


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

2021-03-10 Thread Joachim Protze via Phabricator via cfe-commits
protze.joachim added a comment.

Can you add the inoutset case to the ompt code as well? omp-tools.h already 
defines `ompt_dependence_type_inoutset`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97085

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


[PATCH] D98375: [clang][Driver] Expose -fexperimental-relative-c++-abi-vtables flag

2021-03-10 Thread Leonard Chan via Phabricator via cfe-commits
leonardchan created this revision.
leonardchan added reviewers: phosek, mcgrathr.
leonardchan added a project: clang.
leonardchan requested review of this revision.

Initially, this flag was meant to only be used through cc1 and not directly 
through the clang driver. However, we accidentally ended up using this flag as 
a driver flag already for selecting multilibs within the fuchsia toolchain. 
We're currently in an awkward state where it's only accepted as a driver flag 
when targeting Fuchsia, and all other instances it can only be added via 
`-Xclang`. Since we're ready to use this in Fuchsia, we can just expose this to 
the driver for simplicity.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D98375

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/relative-vtables-flag.cpp


Index: clang/test/Driver/relative-vtables-flag.cpp
===
--- /dev/null
+++ clang/test/Driver/relative-vtables-flag.cpp
@@ -0,0 +1,7 @@
+// RUN: %clangxx --target=aarch64-unknown-fuchsia 
-fexperimental-relative-c++-abi-vtables -c %s -### 2>&1 | FileCheck %s 
--check-prefix=RV
+// RUN: %clangxx --target=aarch64-unknown-fuchsia 
-fno-experimental-relative-c++-abi-vtables -c %s -### 2>&1 | FileCheck %s 
--check-prefix=NO-RV
+// RUN: %clangxx --target=aarch64-unknown-fuchsia -c %s -### 2>&1 | FileCheck 
%s --check-prefix=NO-RV
+// RUN: %clangxx --target=aarch64-unknown-linux-gnu -c %s -### 2>&1 | 
FileCheck %s --check-prefix=NO-RV
+
+// RV: "-fexperimental-relative-c++-abi-vtables"
+// NO-RV-NOT: "-fexperimental-relative-c++-abi-vtables"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4869,6 +4869,9 @@
   Args.AddLastArg(CmdArgs, options::OPT_ffine_grained_bitfield_accesses,
   options::OPT_fno_fine_grained_bitfield_accesses);
 
+  Args.AddLastArg(CmdArgs, options::OPT_fexperimental_relative_cxx_abi_vtables,
+  options::OPT_fno_experimental_relative_cxx_abi_vtables);
+
   // Handle segmented stacks.
   if (Args.hasArg(options::OPT_fsplit_stack))
 CmdArgs.push_back("-split-stacks");


Index: clang/test/Driver/relative-vtables-flag.cpp
===
--- /dev/null
+++ clang/test/Driver/relative-vtables-flag.cpp
@@ -0,0 +1,7 @@
+// RUN: %clangxx --target=aarch64-unknown-fuchsia -fexperimental-relative-c++-abi-vtables -c %s -### 2>&1 | FileCheck %s --check-prefix=RV
+// RUN: %clangxx --target=aarch64-unknown-fuchsia -fno-experimental-relative-c++-abi-vtables -c %s -### 2>&1 | FileCheck %s --check-prefix=NO-RV
+// RUN: %clangxx --target=aarch64-unknown-fuchsia -c %s -### 2>&1 | FileCheck %s --check-prefix=NO-RV
+// RUN: %clangxx --target=aarch64-unknown-linux-gnu -c %s -### 2>&1 | FileCheck %s --check-prefix=NO-RV
+
+// RV: "-fexperimental-relative-c++-abi-vtables"
+// NO-RV-NOT: "-fexperimental-relative-c++-abi-vtables"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4869,6 +4869,9 @@
   Args.AddLastArg(CmdArgs, options::OPT_ffine_grained_bitfield_accesses,
   options::OPT_fno_fine_grained_bitfield_accesses);
 
+  Args.AddLastArg(CmdArgs, options::OPT_fexperimental_relative_cxx_abi_vtables,
+  options::OPT_fno_experimental_relative_cxx_abi_vtables);
+
   // Handle segmented stacks.
   if (Args.hasArg(options::OPT_fsplit_stack))
 CmdArgs.push_back("-split-stacks");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] 774b707 - Revert "Workaround a -Wmisleading-indentation warning"

2021-03-10 Thread Stephen Kelly via cfe-commits

Author: Stephen Kelly
Date: 2021-03-10T23:35:41Z
New Revision: 774b707564e1eed8fec5b5874d0aa82628de0651

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

LOG: Revert "Workaround a -Wmisleading-indentation warning"

This reverts commit 5c22e2bec008760cc7078d8d14382ef4762c5d54.

Added: 


Modified: 
clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py

Removed: 




diff  --git a/clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py 
b/clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py
index aafae2c15b29..dee7a416e69a 100755
--- a/clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py
+++ b/clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py
@@ -171,7 +171,7 @@ def GenerateDynNodeVisitor(self, CladeNames):
 if (const auto *N = Node.get<{0}>())
   return GetLocations(const_cast<{0} *>(N));""".format(CladeName)
 
-self.implementationContent += '\nreturn {}; }'
+self.implementationContent += 'return {}; }'
 
 def GenerateEpilogue(self):
 



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


[clang] 14050dd - Revert "[AST] Add generator for source location introspection"

2021-03-10 Thread Stephen Kelly via cfe-commits

Author: Stephen Kelly
Date: 2021-03-10T23:36:06Z
New Revision: 14050ddc4080beb7a9143340be37ec05890cd537

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

LOG: Revert "[AST] Add generator for source location introspection"

This reverts commit d627a27d264b47eda3f15f086ff419dfe053ebf7.

This fails to link on Windows somehow.

Added: 


Modified: 
clang/lib/Tooling/CMakeLists.txt
clang/unittests/CMakeLists.txt

Removed: 
clang/include/clang/Tooling/NodeIntrospection.h
clang/lib/Tooling/DumpTool/APIData.h
clang/lib/Tooling/DumpTool/ASTSrcLocProcessor.cpp
clang/lib/Tooling/DumpTool/ASTSrcLocProcessor.h
clang/lib/Tooling/DumpTool/CMakeLists.txt
clang/lib/Tooling/DumpTool/ClangSrcLocDump.cpp
clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py
clang/lib/Tooling/NodeIntrospection.cpp
clang/unittests/Introspection/CMakeLists.txt
clang/unittests/Introspection/IntrospectionTest.cpp



diff  --git a/clang/include/clang/Tooling/NodeIntrospection.h 
b/clang/include/clang/Tooling/NodeIntrospection.h
deleted file mode 100644
index abaa58b674a1..
--- a/clang/include/clang/Tooling/NodeIntrospection.h
+++ /dev/null
@@ -1,85 +0,0 @@
-//===- NodeIntrospection.h *- C++ 
-*---===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-//
-//  This file contains the implementation of the NodeIntrospection.
-//
-//===--===//
-
-#ifndef LLVM_CLANG_TOOLING_NODEINTROSPECTION_H
-#define LLVM_CLANG_TOOLING_NODEINTROSPECTION_H
-
-#include "clang/AST/ASTTypeTraits.h"
-#include "clang/AST/DeclarationName.h"
-
-#include 
-#include 
-
-namespace clang {
-
-class Stmt;
-
-namespace tooling {
-
-class LocationCall {
-public:
-  enum LocationCallFlags { NoFlags, ReturnsPointer, IsCast };
-  LocationCall(std::shared_ptr on, std::string name,
-   LocationCallFlags flags = NoFlags)
-  : m_on(on), m_name(name), m_flags(flags) {}
-  LocationCall(std::shared_ptr on, std::string name,
-   std::vector const ,
-   LocationCallFlags flags = NoFlags)
-  : m_on(on), m_name(name), m_flags(flags) {}
-
-  LocationCall *on() const { return m_on.get(); }
-  StringRef name() const { return m_name; }
-  std::vector const () const { return m_args; }
-  bool returnsPointer() const { return m_flags & ReturnsPointer; }
-  bool isCast() const { return m_flags & IsCast; }
-
-private:
-  std::shared_ptr m_on;
-  std::string m_name;
-  std::vector m_args;
-  LocationCallFlags m_flags;
-};
-
-class LocationCallFormatterCpp {
-public:
-  static std::string format(LocationCall *Call);
-};
-
-namespace internal {
-struct RangeLessThan {
-  bool operator()(
-  std::pair> const ,
-  std::pair> const ) const;
-};
-} // namespace internal
-
-template >>
-using UniqueMultiMap = std::set, Comp>;
-
-using SourceLocationMap =
-UniqueMultiMap>;
-using SourceRangeMap =
-UniqueMultiMap,
-   internal::RangeLessThan>;
-
-struct NodeLocationAccessors {
-  SourceLocationMap LocationAccessors;
-  SourceRangeMap RangeAccessors;
-};
-
-namespace NodeIntrospection {
-NodeLocationAccessors GetLocations(clang::Stmt const *Object);
-NodeLocationAccessors GetLocations(clang::DynTypedNode const );
-} // namespace NodeIntrospection
-} // namespace tooling
-} // namespace clang
-#endif

diff  --git a/clang/lib/Tooling/CMakeLists.txt 
b/clang/lib/Tooling/CMakeLists.txt
index 9600bf7fa16d..7a58af59dad1 100644
--- a/clang/lib/Tooling/CMakeLists.txt
+++ b/clang/lib/Tooling/CMakeLists.txt
@@ -8,78 +8,10 @@ add_subdirectory(Core)
 add_subdirectory(Inclusions)
 add_subdirectory(Refactoring)
 add_subdirectory(ASTDiff)
-add_subdirectory(DumpTool)
 add_subdirectory(Syntax)
 add_subdirectory(DependencyScanning)
 add_subdirectory(Transformer)
 
-find_package(Python3 COMPONENTS Interpreter)
-
-# The generation of ASTNodeAPI.json takes a long time in a
-# Debug build due to parsing AST.h. Disable the processing
-# but setting CLANG_TOOLING_BUILD_AST_INTROSPECTION as an
-# internal hidden setting to override.
-# When the processing is disabled, a trivial/empty JSON
-# file is generated by clang-ast-dump and generate_cxx_src_locs.py
-# generates the same API, but with a trivial implementation.
-
-option(CLANG_TOOLING_BUILD_AST_INTROSPECTION "Enable AST introspection" TRUE)
-set(skip_expensive_processing 
$,$>>)
-
-add_custom_command(
-COMMENT Generate ASTNodeAPI.json
-OUTPUT ${CMAKE_BINARY_DIR}/ASTNodeAPI.json
-   

[PATCH] D97068: Run non-filechecked commands in update_cc_test_checks.py

2021-03-10 Thread Giorgis Georgakoudis via Phabricator via cfe-commits
ggeorgakoudis added a comment.

In D97068#2617751 , @jroelofs wrote:

> In D97068#2617642 , @ggeorgakoudis 
> wrote:
>
>> Add triple to avoid spurious failures in tests
>
> They're not really spurious, right... Doesn't this mean that 
> `utils/update_cc_test_checks.py` is now broken for people who develop on a 
> mac?

It's spurious in the sense that it's not this commit that breaks mac tests. 
Testing on mac is already broken. By investigating the issue, you are right 
that the USER_LABEL_PREFIX `_` is skipping generating the checks because 
function names mismatch in IR output. I suppose the issue should be fixed by 
identifying a mac triple and removing the `_` prefix?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97068

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


[PATCH] D97107: Replace func name with regex in update_cc_test_checks

2021-03-10 Thread Giorgis Georgakoudis via Phabricator via cfe-commits
ggeorgakoudis added a comment.

In D97107#2617897 , @thakis wrote:

> This breaks check-llvm on linux and mac:
> http://45.33.8.238/linux/41398/step_12.txt
> http://45.33.8.238/macm1/5291/step_10.txt
> http://lab.llvm.org:8011/#/builders/109
> (and on my local build too)
>
> Please take a look, and please revert for now if it takes a while to fix.

Reverted, I'm going to test better, fix and re-land.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97107

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


[clang] 5c22e2b - Workaround a -Wmisleading-indentation warning

2021-03-10 Thread Stephen Kelly via cfe-commits

Author: Stephen Kelly
Date: 2021-03-10T23:12:31Z
New Revision: 5c22e2bec008760cc7078d8d14382ef4762c5d54

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

LOG: Workaround a -Wmisleading-indentation warning

Because the generated code is not formatted, it can cause warnings.

Added: 


Modified: 
clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py

Removed: 




diff  --git a/clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py 
b/clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py
index dee7a416e69a..aafae2c15b29 100755
--- a/clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py
+++ b/clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py
@@ -171,7 +171,7 @@ def GenerateDynNodeVisitor(self, CladeNames):
 if (const auto *N = Node.get<{0}>())
   return GetLocations(const_cast<{0} *>(N));""".format(CladeName)
 
-self.implementationContent += 'return {}; }'
+self.implementationContent += '\nreturn {}; }'
 
 def GenerateEpilogue(self):
 



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


[clang] ecf6897 - Revert "Replace func name with regex in update_cc_test_checks"

2021-03-10 Thread Giorgis Georgakoudis via cfe-commits

Author: Giorgis Georgakoudis
Date: 2021-03-10T15:05:35-08:00
New Revision: ecf68972fd020cee80c5503bbe0f2028a184c8f3

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

LOG: Revert "Replace func name with regex in update_cc_test_checks"

This reverts commit bf58d6a1f92244c797a280d318a56d7d3fc4a704.

Breaks tests, fix

Added: 


Modified: 
llvm/utils/UpdateTestChecks/common.py

Removed: 
clang/test/utils/update_cc_test_checks/Inputs/generated-funcs-regex.c

clang/test/utils/update_cc_test_checks/Inputs/generated-funcs-regex.c.expected
clang/test/utils/update_cc_test_checks/generated-funcs-regex.test



diff  --git 
a/clang/test/utils/update_cc_test_checks/Inputs/generated-funcs-regex.c 
b/clang/test/utils/update_cc_test_checks/Inputs/generated-funcs-regex.c
deleted file mode 100644
index 3d51d48568ef..
--- a/clang/test/utils/update_cc_test_checks/Inputs/generated-funcs-regex.c
+++ /dev/null
@@ -1,12 +0,0 @@
-// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fopenmp %s -emit-llvm -o 
- | FileCheck %s
-
-void __test_offloading_42_abcdef_bar_l123();
-void use(int);
-
-void foo(int a)
-{
-#pragma omp target
-use(a);
-
-__test_offloading_42_abcdef_bar_l123();
-}

diff  --git 
a/clang/test/utils/update_cc_test_checks/Inputs/generated-funcs-regex.c.expected
 
b/clang/test/utils/update_cc_test_checks/Inputs/generated-funcs-regex.c.expected
deleted file mode 100644
index 49e041b93621..
--- 
a/clang/test/utils/update_cc_test_checks/Inputs/generated-funcs-regex.c.expected
+++ /dev/null
@@ -1,36 +0,0 @@
-// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py 
UTC_ARGS: --include-generated-funcs --replace-function-regex 
"__([a-z]+)_offloading_[a-z0-9]+_[a-z0-9]+_(.*)_l[0-9]+"
-// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fopenmp %s -emit-llvm -o 
- | FileCheck %s
-
-void __test_offloading_42_abcdef_bar_l123();
-void use(int);
-
-void foo(int a)
-{
-#pragma omp target
-use(a);
-
-__test_offloading_42_abcdef_bar_l123();
-}
-// CHECK-LABEL: @foo(
-// CHECK-NEXT:  entry:
-// CHECK-NEXT:[[A_ADDR:%.*]] = alloca i32, align 4
-// CHECK-NEXT:[[A_CASTED:%.*]] = alloca i64, align 8
-// CHECK-NEXT:store i32 [[A:%.*]], i32* [[A_ADDR]], align 4
-// CHECK-NEXT:[[TMP0:%.*]] = load i32, i32* [[A_ADDR]], align 4
-// CHECK-NEXT:[[CONV:%.*]] = bitcast i64* [[A_CASTED]] to i32*
-// CHECK-NEXT:store i32 [[TMP0]], i32* [[CONV]], align 4
-// CHECK-NEXT:[[TMP1:%.*]] = load i64, i64* [[A_CASTED]], align 8
-// CHECK-NEXT:call void 
@{{__omp_offloading_[a-z0-9]+_[a-z0-9]+_foo_l[0-9]+}}(i64 [[TMP1]]) 
[[ATTR3:#.*]]
-// CHECK-NEXT:call void (...) 
@{{__test_offloading_[a-z0-9]+_[a-z0-9]+_bar_l[0-9]+}}()
-// CHECK-NEXT:ret void
-//
-//
-// CHECK-LABEL: @{{__omp_offloading_[a-z0-9]+_[a-z0-9]+_foo_l[0-9]+}}(
-// CHECK-NEXT:  entry:
-// CHECK-NEXT:[[A_ADDR:%.*]] = alloca i64, align 8
-// CHECK-NEXT:store i64 [[A:%.*]], i64* [[A_ADDR]], align 8
-// CHECK-NEXT:[[CONV:%.*]] = bitcast i64* [[A_ADDR]] to i32*
-// CHECK-NEXT:[[TMP0:%.*]] = load i32, i32* [[CONV]], align 8
-// CHECK-NEXT:call void @use(i32 [[TMP0]])
-// CHECK-NEXT:ret void
-//

diff  --git a/clang/test/utils/update_cc_test_checks/generated-funcs-regex.test 
b/clang/test/utils/update_cc_test_checks/generated-funcs-regex.test
deleted file mode 100644
index 4051d0a5181f..
--- a/clang/test/utils/update_cc_test_checks/generated-funcs-regex.test
+++ /dev/null
@@ -1,9 +0,0 @@
-## Test that CHECK lines are generated for clang-generated functions replaced
-## by regex
-
-## RUN: cp %S/Inputs/generated-funcs-regex.c %t-generated-funcs-regex.c && 
%update_cc_test_checks --include-generated-funcs --replace-function-regex 
"__([a-z]+)_offloading_[a-z0-9]+_[a-z0-9]+_(.*)_l[0-9]+" -- 
%t-generated-funcs-regex.c
-# RUN: 
diff  -u %S/Inputs/generated-funcs-regex.c.expected %t-generated-funcs-regex.c
-
-## Check that re-running update_cc_test_checks doesn't change the output
-# RUN: %update_cc_test_checks %t-generated-funcs-regex.c
-# RUN: 
diff  -u %S/Inputs/generated-funcs-regex.c.expected %t-generated-funcs-regex.c

diff  --git a/llvm/utils/UpdateTestChecks/common.py 
b/llvm/utils/UpdateTestChecks/common.py
index 2118b003fb18..0082c4ecd181 100644
--- a/llvm/utils/UpdateTestChecks/common.py
+++ b/llvm/utils/UpdateTestChecks/common.py
@@ -30,8 +30,6 @@ def parse_commandline_args(parser):
help='Activate CHECK line generation from this point 
forward')
   parser.add_argument('--disable', action='store_false', dest='enabled',
   help='Deactivate CHECK line generation from this point 
forward')
-  parser.add_argument('--replace-function-regex', nargs='+', default=[],
- 

[clang] a89ac0d - Update __is_unsigned builtin to match the Standard.

2021-03-10 Thread via cfe-commits

Author: zoecarver
Date: 2021-03-10T15:00:26-08:00
New Revision: a89ac0dd185d72607d4ee1356467fffc48711c9a

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

LOG: Update __is_unsigned builtin to match the Standard.

Updates __is_unsigned to have the same behavior as the standard
specifies. This is in line with 511dbd8, which applied the same change
to __is_signed.

Refs D67897.

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

Added: 


Modified: 
clang/docs/LanguageExtensions.rst
clang/lib/Sema/SemaExprCXX.cpp
clang/test/SemaCXX/type-traits.cpp

Removed: 




diff  --git a/clang/docs/LanguageExtensions.rst 
b/clang/docs/LanguageExtensions.rst
index 6c4cca0fe5e8..a906dc79e03b 100644
--- a/clang/docs/LanguageExtensions.rst
+++ b/clang/docs/LanguageExtensions.rst
@@ -1194,7 +1194,9 @@ The following type trait primitives are supported by 
Clang. Those traits marked
 * ``__is_sealed`` (Microsoft):
   Synonym for ``__is_final``.
 * ``__is_signed`` (C++, Embarcadero):
-  Returns false for enumeration types, and returns true for floating-point 
types. Note, before Clang 10, returned true for enumeration types if the 
underlying type was signed, and returned false for floating-point types.
+  Returns false for enumeration types, and returns true for floating-point
+  types. Note, before Clang 10, returned true for enumeration types if the
+  underlying type was signed, and returned false for floating-point types.
 * ``__is_standard_layout`` (C++, GNU, Microsoft, Embarcadero)
 * ``__is_trivial`` (C++, GNU, Microsoft, Embarcadero)
 * ``__is_trivially_assignable`` (C++, GNU, Microsoft)
@@ -1202,10 +1204,9 @@ The following type trait primitives are supported by 
Clang. Those traits marked
 * ``__is_trivially_copyable`` (C++, GNU, Microsoft)
 * ``__is_trivially_destructible`` (C++, MSVC 2013)
 * ``__is_union`` (C++, GNU, Microsoft, Embarcadero)
-* ``__is_unsigned`` (C++, Embarcadero)
-  Note that this currently returns true for enumeration types if the underlying
-  type is unsigned, in violation of the requirements for ``std::is_unsigned``.
-  This behavior is likely to change in a future version of Clang.
+* ``__is_unsigned`` (C++, Embarcadero):
+  Returns false for enumeration types. Note, before Clang 13, returned true for
+  enumeration types if the underlying type was unsigned.
 * ``__is_void`` (C++, Embarcadero)
 * ``__is_volatile`` (C++, Embarcadero)
 * ``__reference_binds_to_temporary(T, U)`` (Clang):  Determines whether a

diff  --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index 6a79d621f5de..9dbe347e1c90 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -4856,9 +4856,11 @@ static bool EvaluateUnaryTypeTrait(Sema , TypeTrait 
UTT,
   case UTT_IsSigned:
 // Enum types should always return false.
 // Floating points should always return true.
-return !T->isEnumeralType() && (T->isFloatingType() || 
T->isSignedIntegerType());
+return T->isFloatingType() ||
+   (T->isSignedIntegerType() && !T->isEnumeralType());
   case UTT_IsUnsigned:
-return T->isUnsignedIntegerType();
+// Enum types should always return false.
+return T->isUnsignedIntegerType() && !T->isEnumeralType();
 
 // Type trait expressions which query classes regarding their construction,
 // destruction, and copying. Rather than being based directly on the

diff  --git a/clang/test/SemaCXX/type-traits.cpp 
b/clang/test/SemaCXX/type-traits.cpp
index 5adb1f99c27f..d576e4388d6f 100644
--- a/clang/test/SemaCXX/type-traits.cpp
+++ b/clang/test/SemaCXX/type-traits.cpp
@@ -13,6 +13,7 @@ typedef NonPOD NonPODArMB[10][2];
 // PODs
 enum Enum { EV };
 enum SignedEnum : signed int { };
+enum UnsignedEnum : unsigned int { };
 struct POD { Enum e; int i; float f; NonPOD* p; };
 struct Empty {};
 struct IncompleteStruct;
@@ -1440,6 +1441,7 @@ void is_signed()
   int t25[F(__is_signed(IntArNB))];
   int t26[F(__is_signed(Union))];
   int t27[F(__is_signed(UnionAr))];
+  int t28[F(__is_signed(UnsignedEnum))];
 }
 
 void is_unsigned()
@@ -1450,7 +1452,6 @@ void is_unsigned()
   int t04[T(__is_unsigned(unsigned int))];
   int t05[T(__is_unsigned(unsigned long))];
   int t06[T(__is_unsigned(unsigned long long))];
-  int t07[T(__is_unsigned(Enum))];
 
   int t10[F(__is_unsigned(void))];
   int t11[F(__is_unsigned(cvoid))];
@@ -1468,6 +1469,9 @@ void is_unsigned()
   int t24[F(__is_unsigned(Derives))];
   int t25[F(__is_unsigned(ClassType))];
   int t26[F(__is_unsigned(IntArNB))];
+  int t27[F(__is_unsigned(Enum))];
+  int t28[F(__is_unsigned(UnsignedEnum))];
+  int t29[F(__is_unsigned(SignedEnum))];
 }
 
 typedef Int& IntRef;



___
cfe-commits mailing list

[PATCH] D98104: Update __is_unsigned builtin to match the Standard.

2021-03-10 Thread Zoe Carver 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 rGa89ac0dd185d: Update __is_unsigned builtin to match the 
Standard. (authored by zoecarver).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98104

Files:
  clang/docs/LanguageExtensions.rst
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaCXX/type-traits.cpp


Index: clang/test/SemaCXX/type-traits.cpp
===
--- clang/test/SemaCXX/type-traits.cpp
+++ clang/test/SemaCXX/type-traits.cpp
@@ -13,6 +13,7 @@
 // PODs
 enum Enum { EV };
 enum SignedEnum : signed int { };
+enum UnsignedEnum : unsigned int { };
 struct POD { Enum e; int i; float f; NonPOD* p; };
 struct Empty {};
 struct IncompleteStruct;
@@ -1440,6 +1441,7 @@
   int t25[F(__is_signed(IntArNB))];
   int t26[F(__is_signed(Union))];
   int t27[F(__is_signed(UnionAr))];
+  int t28[F(__is_signed(UnsignedEnum))];
 }
 
 void is_unsigned()
@@ -1450,7 +1452,6 @@
   int t04[T(__is_unsigned(unsigned int))];
   int t05[T(__is_unsigned(unsigned long))];
   int t06[T(__is_unsigned(unsigned long long))];
-  int t07[T(__is_unsigned(Enum))];
 
   int t10[F(__is_unsigned(void))];
   int t11[F(__is_unsigned(cvoid))];
@@ -1468,6 +1469,9 @@
   int t24[F(__is_unsigned(Derives))];
   int t25[F(__is_unsigned(ClassType))];
   int t26[F(__is_unsigned(IntArNB))];
+  int t27[F(__is_unsigned(Enum))];
+  int t28[F(__is_unsigned(UnsignedEnum))];
+  int t29[F(__is_unsigned(SignedEnum))];
 }
 
 typedef Int& IntRef;
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -4856,9 +4856,11 @@
   case UTT_IsSigned:
 // Enum types should always return false.
 // Floating points should always return true.
-return !T->isEnumeralType() && (T->isFloatingType() || 
T->isSignedIntegerType());
+return T->isFloatingType() ||
+   (T->isSignedIntegerType() && !T->isEnumeralType());
   case UTT_IsUnsigned:
-return T->isUnsignedIntegerType();
+// Enum types should always return false.
+return T->isUnsignedIntegerType() && !T->isEnumeralType();
 
 // Type trait expressions which query classes regarding their construction,
 // destruction, and copying. Rather than being based directly on the
Index: clang/docs/LanguageExtensions.rst
===
--- clang/docs/LanguageExtensions.rst
+++ clang/docs/LanguageExtensions.rst
@@ -1194,7 +1194,9 @@
 * ``__is_sealed`` (Microsoft):
   Synonym for ``__is_final``.
 * ``__is_signed`` (C++, Embarcadero):
-  Returns false for enumeration types, and returns true for floating-point 
types. Note, before Clang 10, returned true for enumeration types if the 
underlying type was signed, and returned false for floating-point types.
+  Returns false for enumeration types, and returns true for floating-point
+  types. Note, before Clang 10, returned true for enumeration types if the
+  underlying type was signed, and returned false for floating-point types.
 * ``__is_standard_layout`` (C++, GNU, Microsoft, Embarcadero)
 * ``__is_trivial`` (C++, GNU, Microsoft, Embarcadero)
 * ``__is_trivially_assignable`` (C++, GNU, Microsoft)
@@ -1202,10 +1204,9 @@
 * ``__is_trivially_copyable`` (C++, GNU, Microsoft)
 * ``__is_trivially_destructible`` (C++, MSVC 2013)
 * ``__is_union`` (C++, GNU, Microsoft, Embarcadero)
-* ``__is_unsigned`` (C++, Embarcadero)
-  Note that this currently returns true for enumeration types if the underlying
-  type is unsigned, in violation of the requirements for ``std::is_unsigned``.
-  This behavior is likely to change in a future version of Clang.
+* ``__is_unsigned`` (C++, Embarcadero):
+  Returns false for enumeration types. Note, before Clang 13, returned true for
+  enumeration types if the underlying type was unsigned.
 * ``__is_void`` (C++, Embarcadero)
 * ``__is_volatile`` (C++, Embarcadero)
 * ``__reference_binds_to_temporary(T, U)`` (Clang):  Determines whether a


Index: clang/test/SemaCXX/type-traits.cpp
===
--- clang/test/SemaCXX/type-traits.cpp
+++ clang/test/SemaCXX/type-traits.cpp
@@ -13,6 +13,7 @@
 // PODs
 enum Enum { EV };
 enum SignedEnum : signed int { };
+enum UnsignedEnum : unsigned int { };
 struct POD { Enum e; int i; float f; NonPOD* p; };
 struct Empty {};
 struct IncompleteStruct;
@@ -1440,6 +1441,7 @@
   int t25[F(__is_signed(IntArNB))];
   int t26[F(__is_signed(Union))];
   int t27[F(__is_signed(UnionAr))];
+  int t28[F(__is_signed(UnsignedEnum))];
 }
 
 void is_unsigned()
@@ -1450,7 +1452,6 @@
   int t04[T(__is_unsigned(unsigned int))];
   int t05[T(__is_unsigned(unsigned long))];
   int t06[T(__is_unsigned(unsigned long long))];
-  int 

[PATCH] D46443: [libc++] Add missing cstdalign header

2021-03-10 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments.



Comment at: libcxx/include/cstdalign:24
+#include <__config>
+#include 
+

curdeius wrote:
> hubert.reinterpretcast wrote:
> > curdeius wrote:
> > > curdeius wrote:
> > > > hubert.reinterpretcast wrote:
> > > > > sbc100 wrote:
> > > > > > hubert.reinterpretcast wrote:
> > > > > > > This seems to be assuming that the underlying C library's 
> > > > > > > `stdalign.h` is C++ friendly. A C11 `stdalign.h` //does// define 
> > > > > > > `alignof` and `alignas` as macros.
> > > > > > Should I just remove this `#include` then?
> > > > > The idea would be to //add// a `stdalign.h` alongside this header 
> > > > > that doesn't `#include_next` the underlying C library's `stdalign.h`.
> > > > I'm not sure if that should be the solution. At least gcc's libstdc++ 
> > > > assumes that `stdalign.h` is C++-compatbile (cf. 
> > > > https://github.com/gcc-mirror/gcc/blob/16e2427f50c208dfe07d07f18009969502c25dc8/libstdc%2B%2B-v3/include/c_global/cstdalign).
> > > > 
> > > > Clang provides a compatible header: 
> > > > https://github.com/llvm/llvm-project/commit/8acb4044d83ecc9df81b1c9f327d5bd4325e1756.
> > > > Gcc too of course: 
> > > > https://github.com/gcc-mirror/gcc/blob/16e2427f50c208dfe07d07f18009969502c25dc8/gcc/ginclude/stdalign.h.
> > > > 
> > > > MSVC's STL on the other hand, doesn't include `` 
> > > > (https://github.com/microsoft/STL/blob/main/stl/inc/cstdalign).
> > > > 
> > > > @hubert.reinterpretcast, are you aware of an environment which has 
> > > > non-friendly `stdalign.h`?
> > > FYI, musl is also C++ friendly: 
> > > https://git.musl-libc.org/cgit/musl/tree/include/stdalign.h.
> > >>! Quote:
> > > @hubert.reinterpretcast, are you aware of an environment which has 
> > > non-friendly stdalign.h?
> > 
> > The one GCC provides disagrees with the interpretation I gave of which 
> > macros should be present. The one that Clang provides //does// match my 
> > interpretation. It seems the GCC one is non-friendly (albeit a different 
> > form of non-friendly than the one I opened with).
> Oh, you mean that `__alignas_is_defined` and `__alignof_is_defined` won't be 
> defined in this case, right?
> In this case, I guess we won't avoid having `stdalign.h` as you had suggested.
> And indeed the test fails with gcc:
> ```
> bin/llvm-lit -vv ../../libcxx/test/std/language.support/cstdalign/ 
> --param=std=c++17 --param=cxx_under_test=`which g++`
> ...
> libcxx/test/std/language.support/cstdalign/cstdalign.pass.cpp:21:2: error: 
> #error __alignas_is_defined not defined
>21 | #error __alignas_is_defined not defined
>   |  ^
> ```
> 
> That's unfortunately a configuration which is not tested in the CI.
Yes, it looks like adding a `stdalign.h` for libc++ is needed to reliably get 
`__alignas_is_defined` and `__alignof_is_defined`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D46443

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


[PATCH] D98242: [clangd] Store system relative includes as verbatim

2021-03-10 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment.

I measured this as a mild (3%) increase in preamble indexing time when working 
on clangd itself, more details in D98329 

This regression could be more significant with a long include search path, so 
I'd like to avoid adding this without caching. If we need this urgently then 
D98329  is enough, if not then D98371 
 is more comprehensive. Up to you how to 
proceed here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98242

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


[PATCH] D93164: [AST] Add generator for source location introspection

2021-03-10 Thread Stephen Kelly via Phabricator via cfe-commits
steveire added a comment.

@thakis Presumably you'll have to update the GN build now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93164

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


[PATCH] D98329: [clangd] Add cache for FID to Header mappings

2021-03-10 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment.

Some performance measurements...
(workload is PreambleCallback for clangd/XRefs.cpp in sync mode with a fixed 
baseline of 99b01cf28db9db1a3ec0e25367bd325b7aca6d43 
, opt 
build without asserts)

baseline: 1.21s
baseline+D98242 : 1.25s
baseline+D98242 +D98329 
: 1.21s
baseline+D98371 : 1.02s

So this patch does indeed help, and cancels out the extra work done in D98242 
 with this workload at least.
Nevertheless I think the speedups from D98371  
clearly justify the more invasive approach to caching, so we should do 
something like that instead. (We don't need both as it caches at a slightly 
higher level).




Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:792
+  return HeaderToInclude[FID] =
+ getIncludeHeader(S, SM.getFileID(SM.getIncludeLoc(FID)));
 // Conservatively refuse to insert #includes to files without guards.

this is correct but subtle: if we get to here we know the symbol does not have 
any special handling so it's safe to cache with the file Id alone


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98329

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


[PATCH] D93164: [AST] Add generator for source location introspection

2021-03-10 Thread Stephen Kelly 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 rGd627a27d264b: [AST] Add generator for source location 
introspection (authored by stephenkelly).

Changed prior to commit:
  https://reviews.llvm.org/D93164?vs=329759=329780#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93164

Files:
  clang/include/clang/Tooling/NodeIntrospection.h
  clang/lib/Tooling/CMakeLists.txt
  clang/lib/Tooling/DumpTool/APIData.h
  clang/lib/Tooling/DumpTool/ASTSrcLocProcessor.cpp
  clang/lib/Tooling/DumpTool/ASTSrcLocProcessor.h
  clang/lib/Tooling/DumpTool/CMakeLists.txt
  clang/lib/Tooling/DumpTool/ClangSrcLocDump.cpp
  clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py
  clang/lib/Tooling/NodeIntrospection.cpp
  clang/unittests/CMakeLists.txt
  clang/unittests/Introspection/CMakeLists.txt
  clang/unittests/Introspection/IntrospectionTest.cpp

Index: clang/unittests/Introspection/IntrospectionTest.cpp
===
--- /dev/null
+++ clang/unittests/Introspection/IntrospectionTest.cpp
@@ -0,0 +1,100 @@
+//===- unittest/Introspection/IntrospectionTest.cpp --*- C++ -*---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// Tests for AST location API introspection.
+//
+//===--===//
+
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Tooling/NodeIntrospection.h"
+#include "clang/Tooling/Tooling.h"
+#include "gmock/gmock-matchers.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+using namespace clang;
+using namespace clang::ast_matchers;
+using namespace clang::tooling;
+
+using ::testing::UnorderedElementsAre;
+using ::testing::Pair;
+
+#if SKIP_INTROSPECTION_GENERATION
+
+TEST(Introspection, NonFatalAPI) {
+  auto AST = buildASTFromCode("void foo() {} void bar() { foo(); }", "foo.cpp",
+  std::make_shared());
+  auto  = AST->getASTContext();
+  auto  = *Ctx.getTranslationUnitDecl();
+
+  auto BoundNodes = ast_matchers::match(
+  decl(hasDescendant(
+  callExpr(callee(functionDecl(hasName("foo".bind("fooCall"))),
+  TU, Ctx);
+
+  EXPECT_EQ(BoundNodes.size(), 1u);
+
+  auto *FooCall = BoundNodes[0].getNodeAs("fooCall");
+
+  auto result = NodeIntrospection::GetLocations(FooCall);
+
+  EXPECT_EQ(result.LocationAccessors.size(), 0);
+  EXPECT_EQ(result.RangeAccessors.size(), 0);
+}
+
+#else
+
+TEST(Introspection, SourceLocations) {
+  auto AST = buildASTFromCode("void foo() {} void bar() { foo(); }", "foo.cpp",
+  std::make_shared());
+  auto  = AST->getASTContext();
+  auto  = *Ctx.getTranslationUnitDecl();
+
+  auto BoundNodes = ast_matchers::match(
+  decl(hasDescendant(
+  callExpr(callee(functionDecl(hasName("foo".bind("fooCall"))),
+  TU, Ctx);
+
+  EXPECT_EQ(BoundNodes.size(), 1u);
+
+  auto *FooCall = BoundNodes[0].getNodeAs("fooCall");
+
+  auto result = NodeIntrospection::GetLocations(FooCall);
+
+  std::map ExpectedLocations;
+  llvm::transform(result.LocationAccessors,
+  std::inserter(ExpectedLocations, ExpectedLocations.end()),
+  [](const auto ) {
+return std::make_pair(
+LocationCallFormatterCpp::format(Accessor.second.get()),
+Accessor.first);
+  });
+
+  EXPECT_THAT(ExpectedLocations,
+  UnorderedElementsAre(
+  Pair("getBeginLoc()", FooCall->getBeginLoc()),
+  Pair("getEndLoc()", FooCall->getEndLoc()),
+  Pair("getExprLoc()", FooCall->getExprLoc()),
+  Pair("getRParenLoc()", FooCall->getRParenLoc(;
+
+  std::map ExpectedRanges;
+  llvm::transform(result.RangeAccessors,
+  std::inserter(ExpectedRanges, ExpectedRanges.end()),
+  [](const auto ) {
+return std::make_pair(
+LocationCallFormatterCpp::format(Accessor.second.get()),
+Accessor.first);
+  });
+
+  EXPECT_THAT(ExpectedRanges,
+  UnorderedElementsAre(
+  Pair("getSourceRange()", FooCall->getSourceRange(;
+}
+#endif
Index: clang/unittests/Introspection/CMakeLists.txt
===
--- /dev/null
+++ clang/unittests/Introspection/CMakeLists.txt
@@ -0,0 +1,26 @@
+set(LLVM_LINK_COMPONENTS
+  ${LLVM_TARGETS_TO_BUILD}
+ 

[clang] d627a27 - [AST] Add generator for source location introspection

2021-03-10 Thread Stephen Kelly via cfe-commits

Author: Stephen Kelly
Date: 2021-03-10T22:38:39Z
New Revision: d627a27d264b47eda3f15f086ff419dfe053ebf7

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

LOG: [AST] Add generator for source location introspection

Generate a json file containing descriptions of AST classes and their
public accessors which return SourceLocation or SourceRange.

Use the JSON file to generate a C++ API and implementation for accessing
the source locations and method names for accessing them for a given AST
node.

This new API can be used to implement 'srcloc' output in clang-query:

  http://ce.steveire.com/z/m_kTIo

In this first version of this feature, only the accessors for Stmt
classes are generated, not Decls, TypeLocs etc.  Those can be added
after this change is reviewed, as this change is mostly about
infrastructure of these code generators.

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

Added: 
clang/include/clang/Tooling/NodeIntrospection.h
clang/lib/Tooling/DumpTool/APIData.h
clang/lib/Tooling/DumpTool/ASTSrcLocProcessor.cpp
clang/lib/Tooling/DumpTool/ASTSrcLocProcessor.h
clang/lib/Tooling/DumpTool/CMakeLists.txt
clang/lib/Tooling/DumpTool/ClangSrcLocDump.cpp
clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py
clang/lib/Tooling/NodeIntrospection.cpp
clang/unittests/Introspection/CMakeLists.txt
clang/unittests/Introspection/IntrospectionTest.cpp

Modified: 
clang/lib/Tooling/CMakeLists.txt
clang/unittests/CMakeLists.txt

Removed: 




diff  --git a/clang/include/clang/Tooling/NodeIntrospection.h 
b/clang/include/clang/Tooling/NodeIntrospection.h
new file mode 100644
index ..abaa58b674a1
--- /dev/null
+++ b/clang/include/clang/Tooling/NodeIntrospection.h
@@ -0,0 +1,85 @@
+//===- NodeIntrospection.h *- C++ 
-*---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+//  This file contains the implementation of the NodeIntrospection.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLING_NODEINTROSPECTION_H
+#define LLVM_CLANG_TOOLING_NODEINTROSPECTION_H
+
+#include "clang/AST/ASTTypeTraits.h"
+#include "clang/AST/DeclarationName.h"
+
+#include 
+#include 
+
+namespace clang {
+
+class Stmt;
+
+namespace tooling {
+
+class LocationCall {
+public:
+  enum LocationCallFlags { NoFlags, ReturnsPointer, IsCast };
+  LocationCall(std::shared_ptr on, std::string name,
+   LocationCallFlags flags = NoFlags)
+  : m_on(on), m_name(name), m_flags(flags) {}
+  LocationCall(std::shared_ptr on, std::string name,
+   std::vector const ,
+   LocationCallFlags flags = NoFlags)
+  : m_on(on), m_name(name), m_flags(flags) {}
+
+  LocationCall *on() const { return m_on.get(); }
+  StringRef name() const { return m_name; }
+  std::vector const () const { return m_args; }
+  bool returnsPointer() const { return m_flags & ReturnsPointer; }
+  bool isCast() const { return m_flags & IsCast; }
+
+private:
+  std::shared_ptr m_on;
+  std::string m_name;
+  std::vector m_args;
+  LocationCallFlags m_flags;
+};
+
+class LocationCallFormatterCpp {
+public:
+  static std::string format(LocationCall *Call);
+};
+
+namespace internal {
+struct RangeLessThan {
+  bool operator()(
+  std::pair> const ,
+  std::pair> const ) const;
+};
+} // namespace internal
+
+template >>
+using UniqueMultiMap = std::set, Comp>;
+
+using SourceLocationMap =
+UniqueMultiMap>;
+using SourceRangeMap =
+UniqueMultiMap,
+   internal::RangeLessThan>;
+
+struct NodeLocationAccessors {
+  SourceLocationMap LocationAccessors;
+  SourceRangeMap RangeAccessors;
+};
+
+namespace NodeIntrospection {
+NodeLocationAccessors GetLocations(clang::Stmt const *Object);
+NodeLocationAccessors GetLocations(clang::DynTypedNode const );
+} // namespace NodeIntrospection
+} // namespace tooling
+} // namespace clang
+#endif

diff  --git a/clang/lib/Tooling/CMakeLists.txt 
b/clang/lib/Tooling/CMakeLists.txt
index 7a58af59dad1..9600bf7fa16d 100644
--- a/clang/lib/Tooling/CMakeLists.txt
+++ b/clang/lib/Tooling/CMakeLists.txt
@@ -8,10 +8,78 @@ add_subdirectory(Core)
 add_subdirectory(Inclusions)
 add_subdirectory(Refactoring)
 add_subdirectory(ASTDiff)
+add_subdirectory(DumpTool)
 add_subdirectory(Syntax)
 add_subdirectory(DependencyScanning)
 add_subdirectory(Transformer)
 
+find_package(Python3 COMPONENTS Interpreter)
+
+# The generation of ASTNodeAPI.json takes a long time in a
+# Debug 

[PATCH] D93164: [AST] Add generator for source location introspection

2021-03-10 Thread Nathan James via Phabricator via cfe-commits
njames93 accepted this revision.
njames93 added a comment.
This revision is now accepted and ready to land.

nit: A few reformat hints


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93164

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


[PATCH] D93164: [AST] Add generator for source location introspection

2021-03-10 Thread Stephen Kelly via Phabricator via cfe-commits
steveire marked 9 inline comments as done.
steveire added inline comments.



Comment at: clang/lib/Tooling/CMakeLists.txt:99
+  NodeIntrospection.cpp
+  NodeIntrospection.inc
   Tooling.cpp

njames93 wrote:
> This shouldn't appear in the source list.
We need to tell CMake the dependency.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93164

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


[PATCH] D98371: [clangd] Group filename calculations in SymbolCollector, and cache mroe.

2021-03-10 Thread Sam McCall via Phabricator via cfe-commits
sammccall created this revision.
sammccall added a reviewer: kadircet.
Herald added subscribers: usaxena95, arphaman.
sammccall requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang.

Also give CanonicalIncludes a less powerful interface (canonicalizes
symbols vs headers separately) so we can cache its results better.

Prior to this:

- path->uri conversions were not consistently cached, this is particularly 
cheap when we start from a FileEntry* (which we often can)
- only a small fraction of header-to-include calculation was cached

This is a significant speedup at least for dynamic indexing of preambles.
On my machine, opening XRefs.cpp:

  PreambleCallback 1.208 -> 1.019 (-15.7%)
  BuildPreamble5.538 -> 5.214 (-5.8%)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D98371

Files:
  clang-tools-extra/clangd/index/CanonicalIncludes.cpp
  clang-tools-extra/clangd/index/CanonicalIncludes.h
  clang-tools-extra/clangd/index/FileIndex.cpp
  clang-tools-extra/clangd/index/SymbolCollector.cpp
  clang-tools-extra/clangd/index/SymbolCollector.h
  clang-tools-extra/clangd/unittests/CanonicalIncludesTests.cpp
  clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp

Index: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
===
--- clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -1463,9 +1463,6 @@
   }
   )cpp",
   /*Main=*/"");
-  for (const auto  : Symbols)
-llvm::errs() << S.Scope << S.Name << " in " << S.IncludeHeaders.size()
- << "\n";
   EXPECT_THAT(
   Symbols,
   UnorderedElementsAre(
Index: clang-tools-extra/clangd/unittests/CanonicalIncludesTests.cpp
===
--- clang-tools-extra/clangd/unittests/CanonicalIncludesTests.cpp
+++ clang-tools-extra/clangd/unittests/CanonicalIncludesTests.cpp
@@ -20,11 +20,8 @@
   Language.C11 = true;
   CI.addSystemHeadersMapping(Language);
   // Usual standard library symbols are mapped correctly.
-  EXPECT_EQ("", CI.mapHeader("path/stdio.h", "printf"));
-  // Suffix mapping isn't available for C, instead of mapping to ` we
-  // just leave the header as-is.
-  EXPECT_EQ("include/stdio.h",
-CI.mapHeader("include/stdio.h", "unknown_symbol"));
+  EXPECT_EQ("", CI.mapSymbol("printf"));
+  EXPECT_EQ("", CI.mapSymbol("unknown_symbol"));
 }
 
 TEST(CanonicalIncludesTest, CXXStandardLibrary) {
@@ -34,17 +31,16 @@
   CI.addSystemHeadersMapping(Language);
 
   // Usual standard library symbols are mapped correctly.
-  EXPECT_EQ("", CI.mapHeader("path/vector.h", "std::vector"));
-  EXPECT_EQ("", CI.mapHeader("path/stdio.h", "std::printf"));
+  EXPECT_EQ("", CI.mapSymbol("std::vector"));
+  EXPECT_EQ("", CI.mapSymbol("std::printf"));
   // std::move is ambiguous, currently always mapped to 
-  EXPECT_EQ("",
-CI.mapHeader("libstdc++/bits/stl_algo.h", "std::move"));
+  EXPECT_EQ("", CI.mapSymbol("std::move"));
   // Unknown std symbols aren't mapped.
-  EXPECT_EQ("foo/bar.h", CI.mapHeader("foo/bar.h", "std::notathing"));
+  EXPECT_EQ("", CI.mapSymbol("std::notathing"));
   // iosfwd declares some symbols it doesn't own.
-  EXPECT_EQ("", CI.mapHeader("iosfwd", "std::ostream"));
+  EXPECT_EQ("", CI.mapSymbol("std::ostream"));
   // And (for now) we assume it owns the others.
-  EXPECT_EQ("", CI.mapHeader("iosfwd", "std::notwathing"));
+  EXPECT_EQ("", CI.mapHeader("iosfwd"));
 }
 
 TEST(CanonicalIncludesTest, PathMapping) {
@@ -52,20 +48,8 @@
   CanonicalIncludes CI;
   CI.addMapping("foo/bar", "");
 
-  EXPECT_EQ("", CI.mapHeader("foo/bar", "some::symbol"));
-  EXPECT_EQ("bar/bar", CI.mapHeader("bar/bar", "some::symbol"));
-}
-
-TEST(CanonicalIncludesTest, SymbolMapping) {
-  // As used for standard library.
-  CanonicalIncludes CI;
-  LangOptions Language;
-  Language.CPlusPlus = true;
-  // Ensures 'std::vector' is mapped to ''.
-  CI.addSystemHeadersMapping(Language);
-
-  EXPECT_EQ("", CI.mapHeader("foo/bar", "std::vector"));
-  EXPECT_EQ("foo/bar", CI.mapHeader("foo/bar", "other::symbol"));
+  EXPECT_EQ("", CI.mapHeader("foo/bar"));
+  EXPECT_EQ("", CI.mapHeader("bar/bar"));
 }
 
 TEST(CanonicalIncludesTest, Precedence) {
@@ -76,15 +60,9 @@
   CI.addSystemHeadersMapping(Language);
 
   // We added a mapping from some/path to .
-  ASSERT_EQ("", CI.mapHeader("some/path", ""));
+  ASSERT_EQ("", CI.mapHeader("some/path"));
   // We should have a path from 'bits/stl_vector.h' to ''.
-  ASSERT_EQ("", CI.mapHeader("bits/stl_vector.h", ""));
-  // We should also have a symbol mapping from 'std::map' to ''.
-  ASSERT_EQ("", CI.mapHeader("some/header.h", "std::map"));
-
-  // And the symbol mapping should take precedence over paths mapping.
-  EXPECT_EQ("", CI.mapHeader("bits/stl_vector.h", "std::map"));
-  EXPECT_EQ("", 

[PATCH] D97107: Replace func name with regex in update_cc_test_checks

2021-03-10 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment.

This breaks check-llvm on linux and mac:
http://45.33.8.238/linux/41398/step_12.txt
http://45.33.8.238/macm1/5291/step_10.txt
http://lab.llvm.org:8011/#/builders/109
(and on my local build too)

Please take a look, and please revert for now if it takes a while to fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97107

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


[PATCH] D97955: [clang-tidy] Refactor loop-convert to bring most of the checking into matchers

2021-03-10 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 329765.
njames93 marked an inline comment as done.
njames93 added a comment.

Update.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97955

Files:
  clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
  clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.h
  clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-basic.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-basic.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-basic.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-basic.cpp
@@ -474,6 +474,14 @@
   // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
   // CHECK-FIXES: for (int It : V)
   // CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", It);
+
+  for (dependent::const_iterator It{V.begin()}, E = V.end();
+   It != E; ++It) {
+printf("Fibonacci number is %d\n", *It);
+  }
+  // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
+  // CHEfCK-FIXES: for (int It : V)
+  // CHECfK-FIXES-NEXT: printf("Fibonacci number is %d\n", It);
 }
 
 // Tests to ensure that an implicit 'this' is picked up as the container.
Index: clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.h
===
--- clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.h
+++ clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.h
@@ -68,9 +68,6 @@
 const UsageResult ,
 RangeDescriptor );
 
-  bool isConvertible(ASTContext *Context, const ast_matchers::BoundNodes ,
- const ForStmt *Loop, LoopFixerKind FixerKind);
-
   StringRef getReverseFunction() const;
   StringRef getReverseHeader() const;
 
Index: clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
@@ -61,9 +61,8 @@
 static const char LoopNameReverseIterator[] = "forLoopReverseIterator";
 static const char LoopNamePseudoArray[] = "forLoopPseudoArray";
 static const char ConditionBoundName[] = "conditionBound";
+static const char ContainerMemberExpr[] = "containerMemberExpr";
 static const char InitVarName[] = "initVar";
-static const char BeginCallName[] = "beginCall";
-static const char EndCallName[] = "endCall";
 static const char EndVarName[] = "endVar";
 static const char DerefByValueResultName[] = "derefByValueResult";
 static const char DerefByRefResultName[] = "derefByRefResult";
@@ -119,6 +118,44 @@
   .bind(LoopNameArray);
 }
 
+namespace {
+AST_MATCHER(VarDecl, isCStyleInit) {
+  return Node.getInitStyle() == VarDecl::CInit;
+}
+
+AST_MATCHER(VarDecl, samePtrTypeAsBeginResult) {
+  QualType VarType = Node.getType().getCanonicalType();
+  if (!VarType->isPointerType())
+return true;
+  return Builder->removeBindings(
+  [&](const ast_matchers::internal::BoundNodesMap ) {
+const auto *BeginMember =
+Nodes.getNodeAs(ContainerMemberExpr);
+assert(BeginMember);
+QualType RetType = cast(BeginMember->getMemberDecl())
+   ->getReturnType()
+   .getCanonicalType();
+if (RetType->isPointerType())
+  return !Finder->getASTContext().hasSameUnqualifiedType(
+  VarType->getPointeeType(), RetType->getPointeeType());
+return false;
+  });
+}
+
+AST_MATCHER(MemberExpr, matchesBoundBeginContainer) {
+  return Builder->removeBindings(
+  [&](const ast_matchers::internal::BoundNodesMap ) {
+const auto *BeginMember =
+Nodes.getNodeAs(ContainerMemberExpr);
+assert(BeginMember);
+if (BeginMember->isArrow() != Node.isArrow())
+  return true;
+return !areSameExpr(>getASTContext(), BeginMember->getBase(),
+Node.getBase());
+  });
+}
+} // namespace
+
 /// The matcher used for iterator-based for loops.
 ///
 /// This matcher is more flexible than array-based loops. It will match
@@ -127,20 +164,34 @@
 ///
 /// \code
 ///   for (containerType::iterator it = container.begin(),
-///e = createIterator(); it != e; ++it) { ... }
+///e = container.end(); it != e; ++it) { ... }
 ///   for (containerType::iterator it = container.begin();
-///it != anotherContainer.end(); ++it) { ... }
+///it != container.end(); ++it) { ... }
 /// \endcode
 /// The following string identifiers are bound to the parts of the AST:
 ///   InitVarName: 'it' (as a VarDecl)
 ///   LoopName: The entire for loop (as a ForStmt)
+///   ContainerMemberExpr: 'container.begin' (as 

[PATCH] D97955: [clang-tidy] Refactor loop-convert to bring most of the checking into matchers

2021-03-10 Thread Nathan James via Phabricator via cfe-commits
njames93 marked 3 inline comments as done.
njames93 added inline comments.



Comment at: clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp:859
   const auto *LoopVar = Nodes.getNodeAs(InitVarName);
-  const auto *EndVar = Nodes.getNodeAs(EndVarName);
-
-  // If the loop calls end()/size() after each iteration, lower our confidence
-  // level.
-  if (FixerKind != LFK_Array && !EndVar)
-ConfidenceLevel.lowerTo(Confidence::CL_Reasonable);
+  const VarDecl *EndVar = Nodes.getNodeAs(EndVarName);
 

steveire wrote:
> Why change `auto` to `VarDecl` (repeated) here?
Changed it when I removed the initializer, forget to undo it when I added the 
initializer back.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97955

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


[PATCH] D95016: [Clang][RISCV] Add custom TableGen backend for riscv-vector intrinsics.

2021-03-10 Thread Jessica Clarke via Phabricator via cfe-commits
jrtc27 added inline comments.



Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:58-62
+  bool IsPointer = false;
+  // IsConstant indices are "int", but have the constant expression.
+  bool IsImmediate = false;
+  // const qualifier.
+  bool IsConstant = false;

craig.topper wrote:
> craig.topper wrote:
> > jrtc27 wrote:
> > > This isn't expressive enough for the grammar you defined. `PCPCec` is 
> > > supposed to give `const i8 * const i8 *`, whereas this will interpret it 
> > > as `const i8 *`. Given such types are presumably not needed you need to 
> > > tighten the rules of your grammar.
> > @jrtc, are you asking for RVVType::applyModifier to verify that that C 
> > doesn't appear twice for example?
> Oops I meant to write @jrtc27 above. Are you asking for 
> RVVType::applyModifier to verify that that C doesn't appear twice for example?
That P and C don't appear twice, and that C appears in the "right" order wrt P 
(i.e. it's always PC never CP).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95016

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


[PATCH] D95016: [Clang][RISCV] Add custom TableGen backend for riscv-vector intrinsics.

2021-03-10 Thread Craig Topper via Phabricator via cfe-commits
craig.topper accepted this revision.
craig.topper added a comment.
This revision is now accepted and ready to land.

LGTM. I think we can address any remaining issues post-commit. I'd like to see 
us start adding the intrinsics that use this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95016

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


[PATCH] D98104: Update __is_unsigned builtin to match the Standard.

2021-03-10 Thread Zoe Carver via Phabricator via cfe-commits
zoecarver updated this revision to Diff 329761.
zoecarver added a comment.

- Fix review comments: add colon, reflow line, and fix typo.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98104

Files:
  clang/docs/LanguageExtensions.rst
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaCXX/type-traits.cpp


Index: clang/test/SemaCXX/type-traits.cpp
===
--- clang/test/SemaCXX/type-traits.cpp
+++ clang/test/SemaCXX/type-traits.cpp
@@ -13,6 +13,7 @@
 // PODs
 enum Enum { EV };
 enum SignedEnum : signed int { };
+enum UnsignedEnum : unsigned int { };
 struct POD { Enum e; int i; float f; NonPOD* p; };
 struct Empty {};
 struct IncompleteStruct;
@@ -1440,6 +1441,7 @@
   int t25[F(__is_signed(IntArNB))];
   int t26[F(__is_signed(Union))];
   int t27[F(__is_signed(UnionAr))];
+  int t28[F(__is_signed(UnsignedEnum))];
 }
 
 void is_unsigned()
@@ -1450,7 +1452,6 @@
   int t04[T(__is_unsigned(unsigned int))];
   int t05[T(__is_unsigned(unsigned long))];
   int t06[T(__is_unsigned(unsigned long long))];
-  int t07[T(__is_unsigned(Enum))];
 
   int t10[F(__is_unsigned(void))];
   int t11[F(__is_unsigned(cvoid))];
@@ -1468,6 +1469,9 @@
   int t24[F(__is_unsigned(Derives))];
   int t25[F(__is_unsigned(ClassType))];
   int t26[F(__is_unsigned(IntArNB))];
+  int t27[F(__is_unsigned(Enum))];
+  int t28[F(__is_unsigned(UnsignedEnum))];
+  int t29[F(__is_unsigned(SignedEnum))];
 }
 
 typedef Int& IntRef;
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -4831,9 +4831,11 @@
   case UTT_IsSigned:
 // Enum types should always return false.
 // Floating points should always return true.
-return !T->isEnumeralType() && (T->isFloatingType() || 
T->isSignedIntegerType());
+return T->isFloatingType() ||
+   (T->isSignedIntegerType() && !T->isEnumeralType());
   case UTT_IsUnsigned:
-return T->isUnsignedIntegerType();
+// Enum types should always return false.
+return T->isUnsignedIntegerType() && !T->isEnumeralType();
 
 // Type trait expressions which query classes regarding their construction,
 // destruction, and copying. Rather than being based directly on the
Index: clang/docs/LanguageExtensions.rst
===
--- clang/docs/LanguageExtensions.rst
+++ clang/docs/LanguageExtensions.rst
@@ -1194,7 +1194,9 @@
 * ``__is_sealed`` (Microsoft):
   Synonym for ``__is_final``.
 * ``__is_signed`` (C++, Embarcadero):
-  Returns false for enumeration types, and returns true for floating-point 
types. Note, before Clang 10, returned true for enumeration types if the 
underlying type was signed, and returned false for floating-point types.
+  Returns false for enumeration types, and returns true for floating-point
+  types. Note, before Clang 10, returned true for enumeration types if the
+  underlying type was signed, and returned false for floating-point types.
 * ``__is_standard_layout`` (C++, GNU, Microsoft, Embarcadero)
 * ``__is_trivial`` (C++, GNU, Microsoft, Embarcadero)
 * ``__is_trivially_assignable`` (C++, GNU, Microsoft)
@@ -1202,10 +1204,9 @@
 * ``__is_trivially_copyable`` (C++, GNU, Microsoft)
 * ``__is_trivially_destructible`` (C++, MSVC 2013)
 * ``__is_union`` (C++, GNU, Microsoft, Embarcadero)
-* ``__is_unsigned`` (C++, Embarcadero)
-  Note that this currently returns true for enumeration types if the underlying
-  type is unsigned, in violation of the requirements for ``std::is_unsigned``.
-  This behavior is likely to change in a future version of Clang.
+* ``__is_unsigned`` (C++, Embarcadero):
+  Returns false for enumeration types. Note, before Clang 13, returned true for
+  enumeration types if the underlying type was unsigned.
 * ``__is_void`` (C++, Embarcadero)
 * ``__is_volatile`` (C++, Embarcadero)
 * ``__reference_binds_to_temporary(T, U)`` (Clang):  Determines whether a


Index: clang/test/SemaCXX/type-traits.cpp
===
--- clang/test/SemaCXX/type-traits.cpp
+++ clang/test/SemaCXX/type-traits.cpp
@@ -13,6 +13,7 @@
 // PODs
 enum Enum { EV };
 enum SignedEnum : signed int { };
+enum UnsignedEnum : unsigned int { };
 struct POD { Enum e; int i; float f; NonPOD* p; };
 struct Empty {};
 struct IncompleteStruct;
@@ -1440,6 +1441,7 @@
   int t25[F(__is_signed(IntArNB))];
   int t26[F(__is_signed(Union))];
   int t27[F(__is_signed(UnionAr))];
+  int t28[F(__is_signed(UnsignedEnum))];
 }
 
 void is_unsigned()
@@ -1450,7 +1452,6 @@
   int t04[T(__is_unsigned(unsigned int))];
   int t05[T(__is_unsigned(unsigned long))];
   int t06[T(__is_unsigned(unsigned long long))];
-  int t07[T(__is_unsigned(Enum))];
 
   int t10[F(__is_unsigned(void))];
   int t11[F(__is_unsigned(cvoid))];
@@ -1468,6 

[PATCH] D98104: Update __is_unsigned builtin to match the Standard.

2021-03-10 Thread Zoe Carver via Phabricator via cfe-commits
zoecarver added a comment.

Sorry for the delay in updating. All review comments have been addressed. 
Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98104

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


[PATCH] D97857: [Matrix] Add support for matrix-by-scalar division.

2021-03-10 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments.



Comment at: clang/docs/MatrixTypes.rst:127
+is only supported for expression ``M1 / S1``, where ``M1`` is of matrix type
+and ``S1`` is of a real type.
 

You don't need to be so formal in this paragraph, because you're about to get 
into a more formal treatment.  I'd just say something like:

> Given two matrixes, the ``+`` and ``-`` operators perform element-wise 
> addition and subtraction, while the ``*`` operator performs matrix 
> multiplication.  ``+``, ``-``, ``*``, and ``/`` can also be used with a 
> matrix and a scalar value, applying the operation to each element of the 
> matrix.
>
> Earlier versions of this extension did not support division by a scalar.  You 
> can test for the availability of this feature with 
> ``__has_extension(matrix_types_scalar_division)``.



Comment at: clang/docs/MatrixTypes.rst:131
 at least one of ``M1`` or ``M2`` is of matrix type and, for `*`, the other is 
of
 a real type:
 

You need to rework this paragraph, something like:

> For the expression ``M1 BIN_OP M2`` where
> - ``BIN_OP`` is one of ``+`` or ``-``, one of ``M1`` and ``M2`` is of matrix 
> type, and the other is of matrix type or real type; or
> - ``BIN_OP`` is ``*``, one of ``M1`` and ``M2`` is of matrix type, and the 
> other is of a real type; or
> - ``BIN_OP`` is ``/``, ``M1`` is of matrix type, and ``M2`` is of a real type:



Comment at: clang/include/clang/Basic/LangOptions.def:399
+LANGOPT(MatrixTypesScalarDiv, 1, 1, "Enable or disable scalar division support 
"
+"for matrix types")
 

Why is this a language option?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97857

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


[PATCH] D93164: [AST] Add generator for source location introspection

2021-03-10 Thread Stephen Kelly via Phabricator via cfe-commits
steveire updated this revision to Diff 329759.
steveire added a comment.

Update


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93164

Files:
  clang/include/clang/Tooling/NodeIntrospection.h
  clang/lib/Tooling/CMakeLists.txt
  clang/lib/Tooling/DumpTool/APIData.h
  clang/lib/Tooling/DumpTool/ASTSrcLocProcessor.cpp
  clang/lib/Tooling/DumpTool/ASTSrcLocProcessor.h
  clang/lib/Tooling/DumpTool/CMakeLists.txt
  clang/lib/Tooling/DumpTool/ClangSrcLocDump.cpp
  clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py
  clang/lib/Tooling/NodeIntrospection.cpp
  clang/unittests/CMakeLists.txt
  clang/unittests/Introspection/CMakeLists.txt
  clang/unittests/Introspection/IntrospectionTest.cpp

Index: clang/unittests/Introspection/IntrospectionTest.cpp
===
--- /dev/null
+++ clang/unittests/Introspection/IntrospectionTest.cpp
@@ -0,0 +1,100 @@
+//===- unittest/Introspection/IntrospectionTest.cpp --*- C++ -*---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// Tests for AST location API introspection.
+//
+//===--===//
+
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Tooling/NodeIntrospection.h"
+#include "clang/Tooling/Tooling.h"
+#include "gmock/gmock-matchers.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+using namespace clang;
+using namespace clang::ast_matchers;
+using namespace clang::tooling;
+
+using ::testing::UnorderedElementsAre;
+using ::testing::Pair;
+
+#if SKIP_INTROSPECTION_GENERATION
+
+TEST(Introspection, NonFatalAPI) {
+  auto AST = buildASTFromCode("void foo() {} void bar() { foo(); }", "foo.cpp",
+  std::make_shared());
+  auto  = AST->getASTContext();
+  auto  = *Ctx.getTranslationUnitDecl();
+
+  auto BoundNodes = ast_matchers::match(
+  decl(hasDescendant(
+  callExpr(callee(functionDecl(hasName("foo".bind("fooCall"))),
+  TU, Ctx);
+
+  EXPECT_EQ(BoundNodes.size(), 1u);
+
+  auto *FooCall = BoundNodes[0].getNodeAs("fooCall");
+
+  auto result = NodeIntrospection::GetLocations(FooCall);
+
+  EXPECT_EQ(result.LocationAccessors.size(), 0);
+  EXPECT_EQ(result.RangeAccessors.size(), 0);
+}
+
+#else
+
+TEST(Introspection, SourceLocations) {
+  auto AST = buildASTFromCode("void foo() {} void bar() { foo(); }", "foo.cpp",
+  std::make_shared());
+  auto  = AST->getASTContext();
+  auto  = *Ctx.getTranslationUnitDecl();
+
+  auto BoundNodes = ast_matchers::match(
+  decl(hasDescendant(
+  callExpr(callee(functionDecl(hasName("foo".bind("fooCall"))),
+  TU, Ctx);
+
+  EXPECT_EQ(BoundNodes.size(), 1u);
+
+  auto *FooCall = BoundNodes[0].getNodeAs("fooCall");
+
+  auto result = NodeIntrospection::GetLocations(FooCall);
+
+  std::map ExpectedLocations;
+  llvm::transform(result.LocationAccessors,
+  std::inserter(ExpectedLocations, ExpectedLocations.end()),
+  [](const auto ) {
+return std::make_pair(
+LocationCallFormatterCpp::format(Accessor.second.get()),
+Accessor.first);
+  });
+
+  EXPECT_THAT(ExpectedLocations,
+  UnorderedElementsAre(
+  Pair("getBeginLoc()", FooCall->getBeginLoc()),
+  Pair("getEndLoc()", FooCall->getEndLoc()),
+  Pair("getExprLoc()", FooCall->getExprLoc()),
+  Pair("getRParenLoc()", FooCall->getRParenLoc(;
+
+  std::map ExpectedRanges;
+  llvm::transform(result.RangeAccessors,
+  std::inserter(ExpectedRanges, ExpectedRanges.end()),
+  [](const auto ) {
+return std::make_pair(
+LocationCallFormatterCpp::format(Accessor.second.get()),
+Accessor.first);
+  });
+
+  EXPECT_THAT(ExpectedRanges,
+  UnorderedElementsAre(
+  Pair("getSourceRange()", FooCall->getSourceRange(;
+}
+#endif
Index: clang/unittests/Introspection/CMakeLists.txt
===
--- /dev/null
+++ clang/unittests/Introspection/CMakeLists.txt
@@ -0,0 +1,26 @@
+set(LLVM_LINK_COMPONENTS
+  ${LLVM_TARGETS_TO_BUILD}
+  FrontendOpenMP
+  Support
+  )
+
+add_clang_unittest(IntrospectionTests
+  IntrospectionTest.cpp
+  )
+
+clang_target_link_libraries(IntrospectionTests
+  PRIVATE
+  clangAST
+  clangASTMatchers
+  clangTooling
+  clangBasic
+  

[PATCH] D98214: [clang-format] Fix aligning with linebreaks

2021-03-10 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks updated this revision to Diff 329755.
HazardyKnusperkeks marked 2 inline comments as done.
HazardyKnusperkeks added a comment.

- Addressed comments
- Fixed handling of continued string literals when aligning


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98214

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -14297,6 +14297,102 @@
Alignment);
 }
 
+TEST_F(FormatTest, AlignWithLineBreaks) {
+  auto Style = getLLVMStyleWithColumns(120);
+
+  EXPECT_EQ(Style.AlignConsecutiveAssignments, FormatStyle::ACS_None);
+  EXPECT_EQ(Style.AlignConsecutiveDeclarations, FormatStyle::ACS_None);
+  verifyFormat("void foo() {\n"
+   "  int myVar = 5;\n"
+   "  double x = 3.14;\n"
+   "  auto str = \"Hello \"\n"
+   " \"World\";\n"
+   "  auto s = \"Hello \"\n"
+   "   \"Again\";\n"
+   "}",
+   Style);
+
+  // clang-format off
+  verifyFormat("void foo() {\n"
+   "  const int capacityBefore = Entries.capacity();\n"
+   "  const auto newEntry = Entries.emplaceHint(std::piecewise_construct, std::forward_as_tuple(uniqueId),\n"
+   "std::forward_as_tuple(id, uniqueId, name, threadCreation));\n"
+   "  const X newEntry2 = Entries.emplaceHint(std::piecewise_construct, std::forward_as_tuple(uniqueId),\n"
+   "  std::forward_as_tuple(id, uniqueId, name, threadCreation));\n"
+   "}",
+   Style);
+  // clang-format on
+
+  Style.AlignConsecutiveAssignments = FormatStyle::ACS_Consecutive;
+  verifyFormat("void foo() {\n"
+   "  int myVar = 5;\n"
+   "  double x  = 3.14;\n"
+   "  auto str  = \"Hello \"\n"
+   "  \"World\";\n"
+   "  auto s= \"Hello \"\n"
+   "  \"Again\";\n"
+   "}",
+   Style);
+
+  // clang-format off
+  verifyFormat("void foo() {\n"
+   "  const int capacityBefore = Entries.capacity();\n"
+   "  const auto newEntry  = Entries.emplaceHint(std::piecewise_construct, std::forward_as_tuple(uniqueId),\n"
+   " std::forward_as_tuple(id, uniqueId, name, threadCreation));\n"
+   "  const X newEntry2= Entries.emplaceHint(std::piecewise_construct, std::forward_as_tuple(uniqueId),\n"
+   " std::forward_as_tuple(id, uniqueId, name, threadCreation));\n"
+   "}",
+   Style);
+  // clang-format on
+
+  Style.AlignConsecutiveAssignments = FormatStyle::ACS_None;
+  Style.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
+  verifyFormat("void foo() {\n"
+   "  intmyVar = 5;\n"
+   "  double x = 3.14;\n"
+   "  auto   str = \"Hello \"\n"
+   "   \"World\";\n"
+   "  auto   s = \"Hello \"\n"
+   " \"Again\";\n"
+   "}",
+   Style);
+
+  // clang-format off
+  verifyFormat("void foo() {\n"
+   "  const int  capacityBefore = Entries.capacity();\n"
+   "  const auto newEntry = Entries.emplaceHint(std::piecewise_construct, std::forward_as_tuple(uniqueId),\n"
+   "std::forward_as_tuple(id, uniqueId, name, threadCreation));\n"
+   "  const XnewEntry2 = Entries.emplaceHint(std::piecewise_construct, std::forward_as_tuple(uniqueId),\n"
+   " std::forward_as_tuple(id, uniqueId, name, threadCreation));\n"
+   "}",
+   Style);
+  // clang-format on
+
+  Style.AlignConsecutiveAssignments = FormatStyle::ACS_Consecutive;
+  Style.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
+
+  verifyFormat("void foo() {\n"
+   "  intmyVar = 5;\n"
+   "  double x = 3.14;\n"
+   "  auto   str   = \"Hello \"\n"
+   " \"World\";\n"
+   "  auto   s = \"Hello \"\n"
+   " \"Again\";\n"
+   "}",
+   Style);
+
+  // clang-format off
+  verifyFormat("void foo() {\n"
+   "  const int  capacityBefore = Entries.capacity();\n"
+   "  const auto newEntry   = Entries.emplaceHint(std::piecewise_construct, std::forward_as_tuple(uniqueId),\n"
+  

[PATCH] D96090: [analyzer] Replace StoreManager::CastRetrievedVal with SValBuilder::evalCast

2021-03-10 Thread Denys Petrov via Phabricator via cfe-commits
ASDenysPetrov added a comment.

In D96090#261 , @steakhal wrote:

> You could still pass a default constructed QualType at each callsite.

We can try. At least it will be like this is a hack for the particular cases 
then, that will emphesys to pay attention to. Well, I'll present this vertion 
tomorrow.


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

https://reviews.llvm.org/D96090

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


[PATCH] D97107: Replace func name with regex in update_cc_test_checks

2021-03-10 Thread Giorgis Georgakoudis 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 rGbf58d6a1f922: Replace func name with regex in 
update_cc_test_checks (authored by ggeorgakoudis).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97107

Files:
  clang/test/utils/update_cc_test_checks/Inputs/generated-funcs-regex.c
  clang/test/utils/update_cc_test_checks/Inputs/generated-funcs-regex.c.expected
  clang/test/utils/update_cc_test_checks/generated-funcs-regex.test
  llvm/utils/UpdateTestChecks/common.py

Index: llvm/utils/UpdateTestChecks/common.py
===
--- llvm/utils/UpdateTestChecks/common.py
+++ llvm/utils/UpdateTestChecks/common.py
@@ -30,6 +30,8 @@
help='Activate CHECK line generation from this point forward')
   parser.add_argument('--disable', action='store_false', dest='enabled',
   help='Deactivate CHECK line generation from this point forward')
+  parser.add_argument('--replace-function-regex', nargs='+', default=[],
+  help='List of regular expressions to replace matching function names')
   args = parser.parse_args()
   global _verbose
   _verbose = args.verbose
@@ -264,6 +266,8 @@
 self._record_args = flags.function_signature
 self._check_attributes = flags.check_attributes
 self._scrubber_args = scrubber_args
+# Strip double-quotes if input was read by UTC_ARGS
+self._replace_function_regex = list(map(lambda x: x.strip('"'), flags.replace_function_regex))
 self._func_dict = {}
 self._func_order = {}
 for tuple in run_list:
@@ -331,6 +335,30 @@
   self._func_dict[prefix][func] = None
   continue
 
+# Replace function names matching the regex.
+for regex in self._replace_function_regex:
+  # Pattern that matches capture groups in the regex in leftmost order.
+  group_regex = re.compile('\(.*?\)')
+  # Replace function name with regex.
+  match = re.match(regex, func)
+  if match:
+func_repl = regex
+# Replace any capture groups with their matched strings.
+for g in match.groups():
+  func_repl = group_regex.sub(g, func_repl, count=1)
+func = '{{' + func_repl + '}}'
+
+  # Replace all calls to regex matching functions.
+  matches = re.finditer(regex, scrubbed_body)
+  for match in matches:
+func_repl = regex
+# Replace any capture groups with their matched strings.
+for g in match.groups():
+func_repl = group_regex.sub(g, func_repl, count=1)
+# Substitute function call names that match the regex with the same
+# capture groups set.
+scrubbed_body = re.sub(func_repl, '{{' + func_repl + '}}', scrubbed_body)
+
 self._func_dict[prefix][func] = function_body(
 scrubbed_body, scrubbed_extra, args_and_sig, attrs)
 self._func_order[prefix].append(func)
@@ -633,6 +661,8 @@
   continue  # Don't add default values
 autogenerated_note_args += action.option_strings[0] + ' '
 if action.const is None:  # action takes a parameter
+  if action.nargs == '+':
+value = ' '.join(map(lambda v: '"' + v.strip('"') + '"', value))
   autogenerated_note_args += '%s ' % value
   if autogenerated_note_args:
 autogenerated_note_args = ' %s %s' % (UTC_ARGS_KEY, autogenerated_note_args[:-1])
Index: clang/test/utils/update_cc_test_checks/generated-funcs-regex.test
===
--- /dev/null
+++ clang/test/utils/update_cc_test_checks/generated-funcs-regex.test
@@ -0,0 +1,9 @@
+## Test that CHECK lines are generated for clang-generated functions replaced
+## by regex
+
+## RUN: cp %S/Inputs/generated-funcs-regex.c %t-generated-funcs-regex.c && %update_cc_test_checks --include-generated-funcs --replace-function-regex "__([a-z]+)_offloading_[a-z0-9]+_[a-z0-9]+_(.*)_l[0-9]+" -- %t-generated-funcs-regex.c
+# RUN: diff -u %S/Inputs/generated-funcs-regex.c.expected %t-generated-funcs-regex.c
+
+## Check that re-running update_cc_test_checks doesn't change the output
+# RUN: %update_cc_test_checks %t-generated-funcs-regex.c
+# RUN: diff -u %S/Inputs/generated-funcs-regex.c.expected %t-generated-funcs-regex.c
Index: clang/test/utils/update_cc_test_checks/Inputs/generated-funcs-regex.c.expected
===
--- /dev/null
+++ clang/test/utils/update_cc_test_checks/Inputs/generated-funcs-regex.c.expected
@@ -0,0 +1,36 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --include-generated-funcs --replace-function-regex "__([a-z]+)_offloading_[a-z0-9]+_[a-z0-9]+_(.*)_l[0-9]+"
+// RUN: 

[clang] bf58d6a - Replace func name with regex in update_cc_test_checks

2021-03-10 Thread Giorgis Georgakoudis via cfe-commits

Author: Giorgis Georgakoudis
Date: 2021-03-10T12:57:35-08:00
New Revision: bf58d6a1f92244c797a280d318a56d7d3fc4a704

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

LOG: Replace func name with regex in update_cc_test_checks

The patch adds an argument to update_cc_test_checks for replacing a function 
name matching a regex. This functionality is needed to match generated function 
signatures that include file hashes. Example:

The function signature for the following function:

`__omp_offloading_50_b84c41e__Z9ftemplateIiET_i_l30_worker`

with `--replace-function-regex "__omp_offloading_[0-9]+_[a-z0-9]+_(.*)"` will 
become:

`CHECK-LABEL: 
@{{__omp_offloading_[0-9]+_[a-z0-9]+__Z9ftemplateIiET_i_l30_worker}}(`

Reviewed By: jdoerfert

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

Added: 
clang/test/utils/update_cc_test_checks/Inputs/generated-funcs-regex.c

clang/test/utils/update_cc_test_checks/Inputs/generated-funcs-regex.c.expected
clang/test/utils/update_cc_test_checks/generated-funcs-regex.test

Modified: 
llvm/utils/UpdateTestChecks/common.py

Removed: 




diff  --git 
a/clang/test/utils/update_cc_test_checks/Inputs/generated-funcs-regex.c 
b/clang/test/utils/update_cc_test_checks/Inputs/generated-funcs-regex.c
new file mode 100644
index ..3d51d48568ef
--- /dev/null
+++ b/clang/test/utils/update_cc_test_checks/Inputs/generated-funcs-regex.c
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fopenmp %s -emit-llvm -o 
- | FileCheck %s
+
+void __test_offloading_42_abcdef_bar_l123();
+void use(int);
+
+void foo(int a)
+{
+#pragma omp target
+use(a);
+
+__test_offloading_42_abcdef_bar_l123();
+}

diff  --git 
a/clang/test/utils/update_cc_test_checks/Inputs/generated-funcs-regex.c.expected
 
b/clang/test/utils/update_cc_test_checks/Inputs/generated-funcs-regex.c.expected
new file mode 100644
index ..49e041b93621
--- /dev/null
+++ 
b/clang/test/utils/update_cc_test_checks/Inputs/generated-funcs-regex.c.expected
@@ -0,0 +1,36 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py 
UTC_ARGS: --include-generated-funcs --replace-function-regex 
"__([a-z]+)_offloading_[a-z0-9]+_[a-z0-9]+_(.*)_l[0-9]+"
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fopenmp %s -emit-llvm -o 
- | FileCheck %s
+
+void __test_offloading_42_abcdef_bar_l123();
+void use(int);
+
+void foo(int a)
+{
+#pragma omp target
+use(a);
+
+__test_offloading_42_abcdef_bar_l123();
+}
+// CHECK-LABEL: @foo(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[A_ADDR:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[A_CASTED:%.*]] = alloca i64, align 8
+// CHECK-NEXT:store i32 [[A:%.*]], i32* [[A_ADDR]], align 4
+// CHECK-NEXT:[[TMP0:%.*]] = load i32, i32* [[A_ADDR]], align 4
+// CHECK-NEXT:[[CONV:%.*]] = bitcast i64* [[A_CASTED]] to i32*
+// CHECK-NEXT:store i32 [[TMP0]], i32* [[CONV]], align 4
+// CHECK-NEXT:[[TMP1:%.*]] = load i64, i64* [[A_CASTED]], align 8
+// CHECK-NEXT:call void 
@{{__omp_offloading_[a-z0-9]+_[a-z0-9]+_foo_l[0-9]+}}(i64 [[TMP1]]) 
[[ATTR3:#.*]]
+// CHECK-NEXT:call void (...) 
@{{__test_offloading_[a-z0-9]+_[a-z0-9]+_bar_l[0-9]+}}()
+// CHECK-NEXT:ret void
+//
+//
+// CHECK-LABEL: @{{__omp_offloading_[a-z0-9]+_[a-z0-9]+_foo_l[0-9]+}}(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[A_ADDR:%.*]] = alloca i64, align 8
+// CHECK-NEXT:store i64 [[A:%.*]], i64* [[A_ADDR]], align 8
+// CHECK-NEXT:[[CONV:%.*]] = bitcast i64* [[A_ADDR]] to i32*
+// CHECK-NEXT:[[TMP0:%.*]] = load i32, i32* [[CONV]], align 8
+// CHECK-NEXT:call void @use(i32 [[TMP0]])
+// CHECK-NEXT:ret void
+//

diff  --git a/clang/test/utils/update_cc_test_checks/generated-funcs-regex.test 
b/clang/test/utils/update_cc_test_checks/generated-funcs-regex.test
new file mode 100644
index ..4051d0a5181f
--- /dev/null
+++ b/clang/test/utils/update_cc_test_checks/generated-funcs-regex.test
@@ -0,0 +1,9 @@
+## Test that CHECK lines are generated for clang-generated functions replaced
+## by regex
+
+## RUN: cp %S/Inputs/generated-funcs-regex.c %t-generated-funcs-regex.c && 
%update_cc_test_checks --include-generated-funcs --replace-function-regex 
"__([a-z]+)_offloading_[a-z0-9]+_[a-z0-9]+_(.*)_l[0-9]+" -- 
%t-generated-funcs-regex.c
+# RUN: 
diff  -u %S/Inputs/generated-funcs-regex.c.expected %t-generated-funcs-regex.c
+
+## Check that re-running update_cc_test_checks doesn't change the output
+# RUN: %update_cc_test_checks %t-generated-funcs-regex.c
+# RUN: 
diff  -u %S/Inputs/generated-funcs-regex.c.expected %t-generated-funcs-regex.c

diff  --git a/llvm/utils/UpdateTestChecks/common.py 
b/llvm/utils/UpdateTestChecks/common.py
index 0082c4ecd181..2118b003fb18 100644

[PATCH] D97068: Run non-filechecked commands in update_cc_test_checks.py

2021-03-10 Thread Jon Roelofs via Phabricator via cfe-commits
jroelofs added a comment.

In D97068#2617642 , @ggeorgakoudis 
wrote:

> Add triple to avoid spurious failures in tests

They're not really spurious, right... Doesn't this mean that 
`utils/update_cc_test_checks.py` is now broken for people who develop on a mac?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97068

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


[PATCH] D97107: Replace func name with regex in update_cc_test_checks

2021-03-10 Thread Giorgis Georgakoudis via Phabricator via cfe-commits
ggeorgakoudis updated this revision to Diff 329747.
ggeorgakoudis added a comment.

Add triple in test to avoid spurious failures


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97107

Files:
  clang/test/utils/update_cc_test_checks/Inputs/generated-funcs-regex.c
  clang/test/utils/update_cc_test_checks/Inputs/generated-funcs-regex.c.expected
  clang/test/utils/update_cc_test_checks/generated-funcs-regex.test
  llvm/utils/UpdateTestChecks/common.py

Index: llvm/utils/UpdateTestChecks/common.py
===
--- llvm/utils/UpdateTestChecks/common.py
+++ llvm/utils/UpdateTestChecks/common.py
@@ -30,6 +30,8 @@
help='Activate CHECK line generation from this point forward')
   parser.add_argument('--disable', action='store_false', dest='enabled',
   help='Deactivate CHECK line generation from this point forward')
+  parser.add_argument('--replace-function-regex', nargs='+', default=[],
+  help='List of regular expressions to replace matching function names')
   args = parser.parse_args()
   global _verbose
   _verbose = args.verbose
@@ -264,6 +266,8 @@
 self._record_args = flags.function_signature
 self._check_attributes = flags.check_attributes
 self._scrubber_args = scrubber_args
+# Strip double-quotes if input was read by UTC_ARGS
+self._replace_function_regex = list(map(lambda x: x.strip('"'), flags.replace_function_regex))
 self._func_dict = {}
 self._func_order = {}
 for tuple in run_list:
@@ -331,6 +335,30 @@
   self._func_dict[prefix][func] = None
   continue
 
+# Replace function names matching the regex.
+for regex in self._replace_function_regex:
+  # Pattern that matches capture groups in the regex in leftmost order.
+  group_regex = re.compile('\(.*?\)')
+  # Replace function name with regex.
+  match = re.match(regex, func)
+  if match:
+func_repl = regex
+# Replace any capture groups with their matched strings.
+for g in match.groups():
+  func_repl = group_regex.sub(g, func_repl, count=1)
+func = '{{' + func_repl + '}}'
+
+  # Replace all calls to regex matching functions.
+  matches = re.finditer(regex, scrubbed_body)
+  for match in matches:
+func_repl = regex
+# Replace any capture groups with their matched strings.
+for g in match.groups():
+func_repl = group_regex.sub(g, func_repl, count=1)
+# Substitute function call names that match the regex with the same
+# capture groups set.
+scrubbed_body = re.sub(func_repl, '{{' + func_repl + '}}', scrubbed_body)
+
 self._func_dict[prefix][func] = function_body(
 scrubbed_body, scrubbed_extra, args_and_sig, attrs)
 self._func_order[prefix].append(func)
@@ -633,6 +661,8 @@
   continue  # Don't add default values
 autogenerated_note_args += action.option_strings[0] + ' '
 if action.const is None:  # action takes a parameter
+  if action.nargs == '+':
+value = ' '.join(map(lambda v: '"' + v.strip('"') + '"', value))
   autogenerated_note_args += '%s ' % value
   if autogenerated_note_args:
 autogenerated_note_args = ' %s %s' % (UTC_ARGS_KEY, autogenerated_note_args[:-1])
Index: clang/test/utils/update_cc_test_checks/generated-funcs-regex.test
===
--- /dev/null
+++ clang/test/utils/update_cc_test_checks/generated-funcs-regex.test
@@ -0,0 +1,9 @@
+## Test that CHECK lines are generated for clang-generated functions replaced
+## by regex
+
+## RUN: cp %S/Inputs/generated-funcs-regex.c %t-generated-funcs-regex.c && %update_cc_test_checks --include-generated-funcs --replace-function-regex "__([a-z]+)_offloading_[a-z0-9]+_[a-z0-9]+_(.*)_l[0-9]+" -- %t-generated-funcs-regex.c
+# RUN: diff -u %S/Inputs/generated-funcs-regex.c.expected %t-generated-funcs-regex.c
+
+## Check that re-running update_cc_test_checks doesn't change the output
+# RUN: %update_cc_test_checks %t-generated-funcs-regex.c
+# RUN: diff -u %S/Inputs/generated-funcs-regex.c.expected %t-generated-funcs-regex.c
Index: clang/test/utils/update_cc_test_checks/Inputs/generated-funcs-regex.c.expected
===
--- /dev/null
+++ clang/test/utils/update_cc_test_checks/Inputs/generated-funcs-regex.c.expected
@@ -0,0 +1,36 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --include-generated-funcs --replace-function-regex "__([a-z]+)_offloading_[a-z0-9]+_[a-z0-9]+_(.*)_l[0-9]+"
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fopenmp %s -emit-llvm -o - | FileCheck %s
+
+void 

[PATCH] D97955: [clang-tidy] Refactor loop-convert to bring most of the checking into matchers

2021-03-10 Thread Stephen Kelly via Phabricator via cfe-commits
steveire added a comment.

Good progress! This check also implements its own `StmtAncestorASTVisitor` 
which should probably be removed in favor of the `ParentMapContext` eventually 
(not in this patch).




Comment at: clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp:118
+
+AST_MATCHER(ParmVarDecl, hasDefaultArg) {
+  return Node.getDefaultArg() != nullptr;

There's already a (deprecated)  `hasDefaultArgument` in `ASTMatchers.h` with 
the same implementation. 

It is deprecated in favor of `hasInitializer`.



Comment at: clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp:152
+  struct Predicate {
+bool operator()(const ast_matchers::internal::BoundNodesMap ) const {
+  const auto *BeginMember =

Why not use lambdas?



Comment at: clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp:236
 
-  DeclarationMatcher EndDeclMatcher =
-  varDecl(hasInitializer(anything())).bind(EndVarName);
-
-  StatementMatcher EndCallMatcher = cxxMemberCallExpr(
-  argumentCountIs(0), callee(cxxMethodDecl(EndNameMatcher)));
+  StatementMatcher EndCallMatcher =
+  cxxMemberCallExpr(argumentCountIs(0),

It always makes sense to use `auto` for matchers. Here, you hide an unnecessary 
implicit conversion from `BindableMatcher` to `Matcher`. 

Below you use `auto` for a matcher. Why be inconsistent?



Comment at: clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp:859
   const auto *LoopVar = Nodes.getNodeAs(InitVarName);
-  const auto *EndVar = Nodes.getNodeAs(EndVarName);
-
-  // If the loop calls end()/size() after each iteration, lower our confidence
-  // level.
-  if (FixerKind != LFK_Array && !EndVar)
-ConfidenceLevel.lowerTo(Confidence::CL_Reasonable);
+  const VarDecl *EndVar = Nodes.getNodeAs(EndVarName);
 

Why change `auto` to `VarDecl` (repeated) here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97955

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


[PATCH] D97687: [SEH] Fix capture of this in lambda functions

2021-03-10 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.

lgtm


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97687

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


[PATCH] D93164: [AST] Add generator for source location introspection

2021-03-10 Thread Stephen Kelly via Phabricator via cfe-commits
steveire updated this revision to Diff 329737.
steveire added a comment.

Update


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93164

Files:
  clang/include/clang/Tooling/NodeIntrospection.h
  clang/lib/Tooling/CMakeLists.txt
  clang/lib/Tooling/DumpTool/APIData.h
  clang/lib/Tooling/DumpTool/ASTSrcLocProcessor.cpp
  clang/lib/Tooling/DumpTool/ASTSrcLocProcessor.h
  clang/lib/Tooling/DumpTool/CMakeLists.txt
  clang/lib/Tooling/DumpTool/ClangSrcLocDump.cpp
  clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py
  clang/lib/Tooling/NodeIntrospection.cpp
  clang/unittests/CMakeLists.txt
  clang/unittests/Introspection/CMakeLists.txt
  clang/unittests/Introspection/IntrospectionTest.cpp

Index: clang/unittests/Introspection/IntrospectionTest.cpp
===
--- /dev/null
+++ clang/unittests/Introspection/IntrospectionTest.cpp
@@ -0,0 +1,100 @@
+//===- unittest/Introspection/IntrospectionTest.cpp --*- C++ -*---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// Tests for AST location API introspection.
+//
+//===--===//
+
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Tooling/NodeIntrospection.h"
+#include "clang/Tooling/Tooling.h"
+#include "gmock/gmock-matchers.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+using namespace clang;
+using namespace clang::ast_matchers;
+using namespace clang::tooling;
+
+using ::testing::UnorderedElementsAre;
+using ::testing::Pair;
+
+#if SKIP_INTROSPECTION_GENERATION
+
+TEST(Introspection, NonFatalAPI) {
+  auto AST = buildASTFromCode("void foo() {} void bar() { foo(); }", "foo.cpp",
+  std::make_shared());
+  auto  = AST->getASTContext();
+  auto  = *Ctx.getTranslationUnitDecl();
+
+  auto BoundNodes = ast_matchers::match(
+  decl(hasDescendant(
+  callExpr(callee(functionDecl(hasName("foo".bind("fooCall"))),
+  TU, Ctx);
+
+  EXPECT_EQ(BoundNodes.size(), 1u);
+
+  auto *FooCall = BoundNodes[0].getNodeAs("fooCall");
+
+  auto result = NodeIntrospection::GetLocations(FooCall);
+
+  EXPECT_EQ(result.LocationAccessors.size(), 0);
+  EXPECT_EQ(result.RangeAccessors.size(), 0);
+}
+
+#else
+
+TEST(Introspection, SourceLocations) {
+  auto AST = buildASTFromCode("void foo() {} void bar() { foo(); }", "foo.cpp",
+  std::make_shared());
+  auto  = AST->getASTContext();
+  auto  = *Ctx.getTranslationUnitDecl();
+
+  auto BoundNodes = ast_matchers::match(
+  decl(hasDescendant(
+  callExpr(callee(functionDecl(hasName("foo".bind("fooCall"))),
+  TU, Ctx);
+
+  EXPECT_EQ(BoundNodes.size(), 1u);
+
+  auto *FooCall = BoundNodes[0].getNodeAs("fooCall");
+
+  auto result = NodeIntrospection::GetLocations(FooCall);
+
+  std::map ExpectedLocations;
+  llvm::transform(result.LocationAccessors,
+  std::inserter(ExpectedLocations, ExpectedLocations.end()),
+  [](const auto ) {
+return std::make_pair(
+LocationCallFormatterCpp::format(Accessor.second.get()),
+Accessor.first);
+  });
+
+  EXPECT_THAT(ExpectedLocations,
+  UnorderedElementsAre(
+  Pair("getBeginLoc()", FooCall->getBeginLoc()),
+  Pair("getEndLoc()", FooCall->getEndLoc()),
+  Pair("getExprLoc()", FooCall->getExprLoc()),
+  Pair("getRParenLoc()", FooCall->getRParenLoc(;
+
+  std::map ExpectedRanges;
+  llvm::transform(result.RangeAccessors,
+  std::inserter(ExpectedRanges, ExpectedRanges.end()),
+  [](const auto ) {
+return std::make_pair(
+LocationCallFormatterCpp::format(Accessor.second.get()),
+Accessor.first);
+  });
+
+  EXPECT_THAT(ExpectedRanges,
+  UnorderedElementsAre(
+  Pair("getSourceRange()", FooCall->getSourceRange(;
+}
+#endif
Index: clang/unittests/Introspection/CMakeLists.txt
===
--- /dev/null
+++ clang/unittests/Introspection/CMakeLists.txt
@@ -0,0 +1,26 @@
+set(LLVM_LINK_COMPONENTS
+  ${LLVM_TARGETS_TO_BUILD}
+  FrontendOpenMP
+  Support
+  )
+
+add_clang_unittest(IntrospectionTests
+  IntrospectionTest.cpp
+  )
+
+clang_target_link_libraries(IntrospectionTests
+  PRIVATE
+  clangAST
+  clangASTMatchers
+  clangTooling
+  clangBasic
+  

[PATCH] D97080: [flang][driver] Add -fintrinsic-modules-path option

2021-03-10 Thread Arnamoy B via Phabricator via cfe-commits
arnamoy10 added a comment.

Thank you so much for this experiment @awarzynski .  I will modify the code 
accordingly.


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

https://reviews.llvm.org/D97080

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


[PATCH] D97068: Run non-filechecked commands in update_cc_test_checks.py

2021-03-10 Thread Giorgis Georgakoudis 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 rGa2abe2259c2d: Run non-filechecked commands in 
update_cc_test_checks.py (authored by ggeorgakoudis).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97068

Files:
  clang/test/utils/update_cc_test_checks/Inputs/exec-all-runlines.c
  clang/test/utils/update_cc_test_checks/Inputs/exec-all-runlines.c.expected
  clang/test/utils/update_cc_test_checks/exec-all-runlines.test
  llvm/utils/update_cc_test_checks.py

Index: llvm/utils/update_cc_test_checks.py
===
--- llvm/utils/update_cc_test_checks.py
+++ llvm/utils/update_cc_test_checks.py
@@ -203,6 +203,14 @@
   'are discouraged in Clang testsuite.', file=sys.stderr)
 sys.exit(1)
 
+def exec_run_line(exe):
+  popen = subprocess.Popen(exe, stdout=subprocess.PIPE, stderr=subprocess.PIPE, universal_newlines=True)
+  stdout, stderr = popen.communicate()
+  if popen.returncode != 0:
+sys.stderr.write('Failed to run ' + ' '.join(exe) + '\n')
+sys.stderr.write(stderr)
+sys.stderr.write(stdout)
+sys.exit(3)
 
 def main():
   initial_args, parser = config()
@@ -221,25 +229,31 @@
   if m:
 triple_in_cmd = m.groups()[0]
 
-  # Apply %clang substitution rule, replace %s by `filename`, and append args.clang_args
-  clang_args = shlex.split(commands[0])
-  if clang_args[0] not in SUBST:
-print('WARNING: Skipping non-clang RUN line: ' + l, file=sys.stderr)
+  # Parse executable args.
+  exec_args = shlex.split(commands[0])
+  # Execute non-clang runline.
+  if exec_args[0] not in SUBST:
+print('NOTE: Executing non-clang RUN line: ' + l, file=sys.stderr)
+# Replace %s by `filename`.
+exec_args = [i.replace('%s', ti.path) if '%s' in i else i for i in exec_args]
+exec_run_line(exec_args)
 continue
+  # This is a clang runline, apply %clang substitution rule, replace %s by `filename`,
+  # and append args.clang_args
+  clang_args = exec_args
   clang_args[0:1] = SUBST[clang_args[0]]
-  clang_args = [ti.path if i == '%s' else i for i in clang_args] + ti.args.clang_args
-
-  # Permit piping the output through opt
-  if not (len(commands) == 2 or
-  (len(commands) == 3 and commands[1].startswith('opt'))):
-print('WARNING: Skipping non-clang RUN line: ' + l, file=sys.stderr)
+  clang_args = [i.replace('%s', ti.path) if '%s' in i else i for i in clang_args] + ti.args.clang_args
 
   # Extract -check-prefix in FileCheck args
   filecheck_cmd = commands[-1]
   common.verify_filecheck_prefixes(filecheck_cmd)
   if not filecheck_cmd.startswith('FileCheck '):
-print('WARNING: Skipping non-FileChecked RUN line: ' + l, file=sys.stderr)
+print('NOTE: Executing non-FileChecked clang RUN line: ' + l, file=sys.stderr)
+# Execute non-filechecked clang runline.
+exe = [ti.args.clang] + clang_args
+exec_run_line(exe)
 continue
+
   check_prefixes = [item for m in common.CHECK_PREFIX_RE.finditer(filecheck_cmd)
for item in m.group(1).split(',')]
   if not check_prefixes:
Index: clang/test/utils/update_cc_test_checks/exec-all-runlines.test
===
--- /dev/null
+++ clang/test/utils/update_cc_test_checks/exec-all-runlines.test
@@ -0,0 +1,8 @@
+## Test that non-clang/non-filechecked runlines execute
+
+# RUN: cp %S/Inputs/exec-all-runlines.c %t-generated.c && %update_cc_test_checks %t-generated.c
+# RUN: diff -u %S/Inputs/exec-all-runlines.c.expected %t-generated.c
+
+## Check that re-running update_cc_test_checks doesn't change the output
+# RUN: %update_cc_test_checks %t-generated.c
+# RUN: diff -u %S/Inputs/exec-all-runlines.c.expected %t-generated.c
Index: clang/test/utils/update_cc_test_checks/Inputs/exec-all-runlines.c.expected
===
--- /dev/null
+++ clang/test/utils/update_cc_test_checks/Inputs/exec-all-runlines.c.expected
@@ -0,0 +1,17 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// Check that the non-clang/non-filechecked runlines execute
+// RUN: cp %s %s.copy.c
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fopenmp %s.copy.c -emit-llvm-bc -o %t-host.bc
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fopenmp -fopenmp-host-ir-file-path %t-host.bc %s.copy.c -emit-llvm -o - | FileCheck %s
+
+void use(int);
+
+// CHECK-LABEL: @test(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[A_ADDR:%.*]] = alloca i32, align 4
+// CHECK-NEXT:store i32 [[A:%.*]], i32* [[A_ADDR]], align 4
+// CHECK-NEXT:ret void
+//
+void test(int a)
+{
+}
Index: 

[clang] a2abe22 - Run non-filechecked commands in update_cc_test_checks.py

2021-03-10 Thread Giorgis Georgakoudis via cfe-commits

Author: Giorgis Georgakoudis
Date: 2021-03-10T12:25:35-08:00
New Revision: a2abe2259c2d5b8c494f3513b840adf1572b21bc

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

LOG: Run non-filechecked commands in update_cc_test_checks.py

Some tests in clang require running non-filechecked commands to generate the 
actual filecheck input. For example, tests for openmp offloading require 
generating the host bc without any checking, before running the clang command 
to actually generate the filechecked IR of the target device. This patch 
enables `update_cc_test_checks.py` to run non-filechecked run lines in-place.

Reviewed By: jdoerfert

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

Added: 
clang/test/utils/update_cc_test_checks/Inputs/exec-all-runlines.c
clang/test/utils/update_cc_test_checks/Inputs/exec-all-runlines.c.expected
clang/test/utils/update_cc_test_checks/exec-all-runlines.test

Modified: 
llvm/utils/update_cc_test_checks.py

Removed: 




diff  --git a/clang/test/utils/update_cc_test_checks/Inputs/exec-all-runlines.c 
b/clang/test/utils/update_cc_test_checks/Inputs/exec-all-runlines.c
new file mode 100644
index ..1626eb540841
--- /dev/null
+++ b/clang/test/utils/update_cc_test_checks/Inputs/exec-all-runlines.c
@@ -0,0 +1,10 @@
+// Check that the non-clang/non-filechecked runlines execute
+// RUN: cp %s %s.copy.c
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fopenmp %s.copy.c 
-emit-llvm-bc -o %t-host.bc
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fopenmp 
-fopenmp-host-ir-file-path %t-host.bc %s.copy.c -emit-llvm -o - | FileCheck %s
+
+void use(int);
+
+void test(int a)
+{
+}

diff  --git 
a/clang/test/utils/update_cc_test_checks/Inputs/exec-all-runlines.c.expected 
b/clang/test/utils/update_cc_test_checks/Inputs/exec-all-runlines.c.expected
new file mode 100644
index ..5edf11e668e4
--- /dev/null
+++ b/clang/test/utils/update_cc_test_checks/Inputs/exec-all-runlines.c.expected
@@ -0,0 +1,17 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// Check that the non-clang/non-filechecked runlines execute
+// RUN: cp %s %s.copy.c
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fopenmp %s.copy.c 
-emit-llvm-bc -o %t-host.bc
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fopenmp 
-fopenmp-host-ir-file-path %t-host.bc %s.copy.c -emit-llvm -o - | FileCheck %s
+
+void use(int);
+
+// CHECK-LABEL: @test(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[A_ADDR:%.*]] = alloca i32, align 4
+// CHECK-NEXT:store i32 [[A:%.*]], i32* [[A_ADDR]], align 4
+// CHECK-NEXT:ret void
+//
+void test(int a)
+{
+}

diff  --git a/clang/test/utils/update_cc_test_checks/exec-all-runlines.test 
b/clang/test/utils/update_cc_test_checks/exec-all-runlines.test
new file mode 100644
index ..caf39934266c
--- /dev/null
+++ b/clang/test/utils/update_cc_test_checks/exec-all-runlines.test
@@ -0,0 +1,8 @@
+## Test that non-clang/non-filechecked runlines execute
+
+# RUN: cp %S/Inputs/exec-all-runlines.c %t-generated.c && 
%update_cc_test_checks %t-generated.c
+# RUN: 
diff  -u %S/Inputs/exec-all-runlines.c.expected %t-generated.c
+
+## Check that re-running update_cc_test_checks doesn't change the output
+# RUN: %update_cc_test_checks %t-generated.c
+# RUN: 
diff  -u %S/Inputs/exec-all-runlines.c.expected %t-generated.c

diff  --git a/llvm/utils/update_cc_test_checks.py 
b/llvm/utils/update_cc_test_checks.py
index e5ca91502cf9..d084bc6d0795 100755
--- a/llvm/utils/update_cc_test_checks.py
+++ b/llvm/utils/update_cc_test_checks.py
@@ -203,6 +203,14 @@ def get_function_body(builder, args, filename, clang_args, 
extra_commands,
   'are discouraged in Clang testsuite.', file=sys.stderr)
 sys.exit(1)
 
+def exec_run_line(exe):
+  popen = subprocess.Popen(exe, stdout=subprocess.PIPE, 
stderr=subprocess.PIPE, universal_newlines=True)
+  stdout, stderr = popen.communicate()
+  if popen.returncode != 0:
+sys.stderr.write('Failed to run ' + ' '.join(exe) + '\n')
+sys.stderr.write(stderr)
+sys.stderr.write(stdout)
+sys.exit(3)
 
 def main():
   initial_args, parser = config()
@@ -221,25 +229,31 @@ def main():
   if m:
 triple_in_cmd = m.groups()[0]
 
-  # Apply %clang substitution rule, replace %s by `filename`, and append 
args.clang_args
-  clang_args = shlex.split(commands[0])
-  if clang_args[0] not in SUBST:
-print('WARNING: Skipping non-clang RUN line: ' + l, file=sys.stderr)
+  # Parse executable args.
+  exec_args = shlex.split(commands[0])
+  # Execute non-clang runline.
+  if exec_args[0] not in SUBST:
+print('NOTE: Executing non-clang RUN line: ' + l, file=sys.stderr)
+# Replace %s by `filename`.
+

[PATCH] D93164: [AST] Add generator for source location introspection

2021-03-10 Thread Stephen Kelly via Phabricator via cfe-commits
steveire updated this revision to Diff 329735.
steveire added a comment.

Update


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93164

Files:
  clang/include/clang/Tooling/NodeIntrospection.h
  clang/lib/Tooling/CMakeLists.txt
  clang/lib/Tooling/DumpTool/APIData.h
  clang/lib/Tooling/DumpTool/ASTSrcLocProcessor.cpp
  clang/lib/Tooling/DumpTool/ASTSrcLocProcessor.h
  clang/lib/Tooling/DumpTool/CMakeLists.txt
  clang/lib/Tooling/DumpTool/ClangSrcLocDump.cpp
  clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py
  clang/lib/Tooling/NodeIntrospection.cpp
  clang/unittests/CMakeLists.txt
  clang/unittests/Introspection/CMakeLists.txt
  clang/unittests/Introspection/IntrospectionTest.cpp

Index: clang/unittests/Introspection/IntrospectionTest.cpp
===
--- /dev/null
+++ clang/unittests/Introspection/IntrospectionTest.cpp
@@ -0,0 +1,99 @@
+//===- unittest/Introspection/IntrospectionTest.cpp --*- C++ -*---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// Tests for AST location API introspection.
+//
+//===--===//
+
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Tooling/NodeIntrospection.h"
+#include "clang/Tooling/Tooling.h"
+#include "gmock/gmock-matchers.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+using namespace clang;
+using namespace clang::ast_matchers;
+using namespace clang::tooling;
+
+using ::testing::UnorderedElementsAreArray;
+
+#if SKIP_INTROSPECTION_GENERATION
+
+TEST(Introspection, NonFatalAPI) {
+  auto AST = buildASTFromCode("void foo() {} void bar() { foo(); }", "foo.cpp",
+  std::make_shared());
+  auto  = AST->getASTContext();
+  auto  = *Ctx.getTranslationUnitDecl();
+
+  auto BoundNodes = ast_matchers::match(
+  decl(hasDescendant(
+  callExpr(callee(functionDecl(hasName("foo".bind("fooCall"))),
+  TU, Ctx);
+
+  EXPECT_EQ(BoundNodes.size(), 1u);
+
+  auto *FooCall = BoundNodes[0].getNodeAs("fooCall");
+
+  auto result = NodeIntrospection::GetLocations(FooCall);
+
+  EXPECT_EQ(result.LocationAccessors.size(), 0);
+  EXPECT_EQ(result.RangeAccessors.size(), 0);
+}
+
+#else
+
+TEST(Introspection, SourceLocations) {
+  auto AST = buildASTFromCode("void foo() {} void bar() { foo(); }", "foo.cpp",
+  std::make_shared());
+  auto  = AST->getASTContext();
+  auto  = *Ctx.getTranslationUnitDecl();
+
+  auto BoundNodes = ast_matchers::match(
+  decl(hasDescendant(
+  callExpr(callee(functionDecl(hasName("foo".bind("fooCall"))),
+  TU, Ctx);
+
+  EXPECT_EQ(BoundNodes.size(), 1u);
+
+  auto *FooCall = BoundNodes[0].getNodeAs("fooCall");
+
+  auto result = NodeIntrospection::GetLocations(FooCall);
+
+  std::map ExpectedLocations;
+  llvm::transform(result.LocationAccessors,
+  std::inserter(ExpectedLocations, ExpectedLocations.end()),
+  [](const auto ) {
+return std::make_pair(
+LocationCallFormatterCpp::format(Accessor.second.get()),
+Accessor.first);
+  });
+
+  EXPECT_THAT(ExpectedLocations,
+  UnorderedElementsAreArray(std::map{
+  {"getBeginLoc()", FooCall->getBeginLoc()},
+  {"getEndLoc()", FooCall->getEndLoc()},
+  {"getExprLoc()", FooCall->getExprLoc()},
+  {"getRParenLoc()", FooCall->getRParenLoc()}}));
+
+  std::map ExpectedRanges;
+  llvm::transform(result.RangeAccessors,
+  std::inserter(ExpectedRanges, ExpectedRanges.end()),
+  [](const auto ) {
+return std::make_pair(
+LocationCallFormatterCpp::format(Accessor.second.get()),
+Accessor.first);
+  });
+
+  EXPECT_THAT(ExpectedRanges,
+  UnorderedElementsAreArray(std::map{
+  {"getSourceRange()", FooCall->getSourceRange()}}));
+}
+#endif
Index: clang/unittests/Introspection/CMakeLists.txt
===
--- /dev/null
+++ clang/unittests/Introspection/CMakeLists.txt
@@ -0,0 +1,26 @@
+set(LLVM_LINK_COMPONENTS
+  ${LLVM_TARGETS_TO_BUILD}
+  FrontendOpenMP
+  Support
+  )
+
+add_clang_unittest(IntrospectionTests
+  IntrospectionTest.cpp
+  )
+
+clang_target_link_libraries(IntrospectionTests
+  PRIVATE
+  clangAST
+  clangASTMatchers
+  clangTooling
+  clangBasic
+  clangSerialization
+  

[PATCH] D97068: Run non-filechecked commands in update_cc_test_checks.py

2021-03-10 Thread Giorgis Georgakoudis via Phabricator via cfe-commits
ggeorgakoudis updated this revision to Diff 329734.
ggeorgakoudis added a comment.

Add triple to avoid spurious failures in tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97068

Files:
  clang/test/utils/update_cc_test_checks/Inputs/exec-all-runlines.c
  clang/test/utils/update_cc_test_checks/Inputs/exec-all-runlines.c.expected
  clang/test/utils/update_cc_test_checks/exec-all-runlines.test
  llvm/utils/update_cc_test_checks.py

Index: llvm/utils/update_cc_test_checks.py
===
--- llvm/utils/update_cc_test_checks.py
+++ llvm/utils/update_cc_test_checks.py
@@ -203,6 +203,14 @@
   'are discouraged in Clang testsuite.', file=sys.stderr)
 sys.exit(1)
 
+def exec_run_line(exe):
+  popen = subprocess.Popen(exe, stdout=subprocess.PIPE, stderr=subprocess.PIPE, universal_newlines=True)
+  stdout, stderr = popen.communicate()
+  if popen.returncode != 0:
+sys.stderr.write('Failed to run ' + ' '.join(exe) + '\n')
+sys.stderr.write(stderr)
+sys.stderr.write(stdout)
+sys.exit(3)
 
 def main():
   initial_args, parser = config()
@@ -221,25 +229,31 @@
   if m:
 triple_in_cmd = m.groups()[0]
 
-  # Apply %clang substitution rule, replace %s by `filename`, and append args.clang_args
-  clang_args = shlex.split(commands[0])
-  if clang_args[0] not in SUBST:
-print('WARNING: Skipping non-clang RUN line: ' + l, file=sys.stderr)
+  # Parse executable args.
+  exec_args = shlex.split(commands[0])
+  # Execute non-clang runline.
+  if exec_args[0] not in SUBST:
+print('NOTE: Executing non-clang RUN line: ' + l, file=sys.stderr)
+# Replace %s by `filename`.
+exec_args = [i.replace('%s', ti.path) if '%s' in i else i for i in exec_args]
+exec_run_line(exec_args)
 continue
+  # This is a clang runline, apply %clang substitution rule, replace %s by `filename`,
+  # and append args.clang_args
+  clang_args = exec_args
   clang_args[0:1] = SUBST[clang_args[0]]
-  clang_args = [ti.path if i == '%s' else i for i in clang_args] + ti.args.clang_args
-
-  # Permit piping the output through opt
-  if not (len(commands) == 2 or
-  (len(commands) == 3 and commands[1].startswith('opt'))):
-print('WARNING: Skipping non-clang RUN line: ' + l, file=sys.stderr)
+  clang_args = [i.replace('%s', ti.path) if '%s' in i else i for i in clang_args] + ti.args.clang_args
 
   # Extract -check-prefix in FileCheck args
   filecheck_cmd = commands[-1]
   common.verify_filecheck_prefixes(filecheck_cmd)
   if not filecheck_cmd.startswith('FileCheck '):
-print('WARNING: Skipping non-FileChecked RUN line: ' + l, file=sys.stderr)
+print('NOTE: Executing non-FileChecked clang RUN line: ' + l, file=sys.stderr)
+# Execute non-filechecked clang runline.
+exe = [ti.args.clang] + clang_args
+exec_run_line(exe)
 continue
+
   check_prefixes = [item for m in common.CHECK_PREFIX_RE.finditer(filecheck_cmd)
for item in m.group(1).split(',')]
   if not check_prefixes:
Index: clang/test/utils/update_cc_test_checks/exec-all-runlines.test
===
--- /dev/null
+++ clang/test/utils/update_cc_test_checks/exec-all-runlines.test
@@ -0,0 +1,8 @@
+## Test that non-clang/non-filechecked runlines execute
+
+# RUN: cp %S/Inputs/exec-all-runlines.c %t-generated.c && %update_cc_test_checks %t-generated.c
+# RUN: diff -u %S/Inputs/exec-all-runlines.c.expected %t-generated.c
+
+## Check that re-running update_cc_test_checks doesn't change the output
+# RUN: %update_cc_test_checks %t-generated.c
+# RUN: diff -u %S/Inputs/exec-all-runlines.c.expected %t-generated.c
Index: clang/test/utils/update_cc_test_checks/Inputs/exec-all-runlines.c.expected
===
--- /dev/null
+++ clang/test/utils/update_cc_test_checks/Inputs/exec-all-runlines.c.expected
@@ -0,0 +1,17 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// Check that the non-clang/non-filechecked runlines execute
+// RUN: cp %s %s.copy.c
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fopenmp %s.copy.c -emit-llvm-bc -o %t-host.bc
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fopenmp -fopenmp-host-ir-file-path %t-host.bc %s.copy.c -emit-llvm -o - | FileCheck %s
+
+void use(int);
+
+// CHECK-LABEL: @test(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[A_ADDR:%.*]] = alloca i32, align 4
+// CHECK-NEXT:store i32 [[A:%.*]], i32* [[A_ADDR]], align 4
+// CHECK-NEXT:ret void
+//
+void test(int a)
+{
+}
Index: clang/test/utils/update_cc_test_checks/Inputs/exec-all-runlines.c
===
--- /dev/null
+++ 

[PATCH] D95458: [PowerPC] Exploit xxsplti32dx (constant materialization) for scalars

2021-03-10 Thread Amy Kwan via Phabricator via cfe-commits
amyk added inline comments.



Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:16134
 if (Subtarget.hasPrefixInstrs()) {
-  // With prefixed instructions, we can materialize anything that can be
-  // represented with a 32-bit immediate, not just positive zero.
-  APFloat APFloatOfImm = Imm;
-  return convertToNonDenormSingle(APFloatOfImm);
+  // we can materialize all imms via XXSPLTI32dDX and XXSPLTIDP
+  return true;

Minor nit on comment.


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

https://reviews.llvm.org/D95458

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


[PATCH] D98237: [clang-format] Option for empty lines after an access modifier.

2021-03-10 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment.

Please also add an entry in the `clang/doc/ReleaseNotes.rst`.




Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:1284
   (!PreviousLine->InPPDirective || !RootToken.HasUnescapedNewline))
-Newlines = std::min(1u, Newlines);
+Newlines = Style.EmptyLinesAfterAccessModifier + 1u;
 

Max_S wrote:
> HazardyKnusperkeks wrote:
> > I don't know, I'm just asking:
> > Shouldn't this be `Newlines = std::min(Newlines, 
> > Style.EmptyLinesAfterAccessModifier + 1u);`?
> This is also possible but then the logic would be how many lines should be 
> kept at maximum after an access specifier.
> 
> The name would then be `Style.KeepMaximumLinesAfterAccessModifier`.
> 
> Currently the logic above:
> ```
>   if (Newlines == 0 && !RootToken.IsFirst)
> Newlines = 1;
> ```
> forces Newlines to be always 1 or bigger. Therefore the old logic would 
> always add one new line and I decided to implement the setting in the same 
> way.
With your explanation everything is fine here.



Comment at: clang/unittests/Format/FormatTest.cpp:9205
+   "};";
+  EXPECT_EQ(test1NL, format(test0NL));
+  verifyFormat(test1NL);

Max_S wrote:
> MyDeveloperDay wrote:
> > why can't you just verifyFormat them all?
> Yes. I will change this in the next update.
He `verifyFormat`s them with the right style, doesn't he?

With handling of empty lines I think it is useful to add the `EXPECT_EQ`.



Comment at: clang/unittests/Format/FormatTest.cpp:9212
+  StyleWithLine.EmptyLinesAfterAccessModifier = 1u;
+  EXPECT_EQ(test2NL, format(test0NL, StyleWithLine));
+  EXPECT_EQ(test2NL, format(test1NL, StyleWithLine));

Max_S wrote:
> MyDeveloperDay wrote:
> > yeah I'm not a fan of this like this... sorry... just write the test out in 
> > long form, when it goes wrong I don't have to be a compiler to understand 
> > what is going wrong I can just see it.
> I can change this, but the current output of the tests is (I forced the 
> error):
> ```
> //llvm-project/clang/unittests/Format/FormatTest.cpp:72: Failure
>   Expected: Expected.str()
>   Which is: "class Foo {\nprivate:\n\n  int i;\n};"
> To be equal to: format(Expected, Style)
>   Which is: "class Foo {\nprivate:\n  int i;\n};"
> With diff:
> @@ -1,5 @@
>  class Foo {
>  private:
> -
>int i;
>  };
> ```
> 
> Which is actually human readable in this case. Shall I still change it?
I'm a fan of it. :)
Especially with the demonstration that the string is still expanded.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98237

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


[PATCH] D98061: [InstrProfiling] Generate runtime hook for ELF platforms

2021-03-10 Thread Vedant Kumar via Phabricator via cfe-commits
vsk accepted this revision.
vsk added a comment.
This revision is now accepted and ready to land.

Thanks for the detailed explanation of the -fprofile-list workflow; given the 
difference constraints, this patch lgtm. Please document the divergent behavior 
re: no .profraw file when #counters == 0 for non-MachO in the clang docs.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98061

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


[PATCH] D97080: [flang][driver] Add -fintrinsic-modules-path option

2021-03-10 Thread Andrzej Warzynski via Phabricator via cfe-commits
awarzynski added a comment.

In D97080#2613628 , @awarzynski wrote:

> **Question**: What are the semantics for this flag in `gfortran`? Is the path 
> specified with `-fintrinsics-module-path` _prepended_ or _appended_ to the 
> default search path?

I believe that it is _prepended_. Take this dummy module:

  module ieee_arithmetic
  type::ieee_round_type
  integer(1),private::mode=0_2
  end type
  end

Compile it with e.g. `flang-new`:

  flang-new -fc1 -fsyntax-only dummy-module.f90

Next, take this Fortran file:

  program test
use ieee_arithmetic
implicit none
real(kind=4) :: x1
if (.not. ieee_support_nan(x1)) STOP 20
  end program test

And try compiling it with `gfortran`:

  mv ieee_arithmetic.mod tools/flang/
  gfortran -fintrinsic-modules-path tools/flang/ test.f90
  test.f90:2:6:
  
 use ieee_arithmetic
1
  Fatal Error: File ‘ieee_arithmetic.mod’ opened at (1) is not a GNU Fortran 
module file
  compilation terminated.

So `gfortran` did look in `tools/flang` first, i.e. the path specified with 
`-fintrinsic-modules-path`.


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

https://reviews.llvm.org/D97080

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


[PATCH] D98135: [OpenMP][InstrProfiling] Fix a missing instr profiling counter

2021-03-10 Thread Vedant Kumar via Phabricator via cfe-commits
vsk added a comment.

In D98135#2615492 , @MaskRay wrote:

> LG, but I hope an OpenMP expert can take a look. Perhaps @vsk can comment on 
> where the tests should be improved (currently we hardly have any 
> `@llvm.instrprof.increment` IR test for clang -fprofile-instr-generate).

This doesn't necessarily indicate a gap in test coverage. There are plenty of 
tests under clang/test/Profile that cover IR generation for profiling 
instrumentation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98135

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


[PATCH] D96850: [clang] Add -ffinite-loops & -fno-finite-loops options.

2021-03-10 Thread Florian Hahn via Phabricator via cfe-commits
fhahn abandoned this revision.
fhahn added a comment.

those have been picked onto 12.x


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96850

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


[PATCH] D93938: [clang-format] Fixed AfterEnum handling

2021-03-10 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment.

In D93938#2616044 , @atirit wrote:

> In D93938#2614825 , 
> @HazardyKnusperkeks wrote:
>
>> If the bugs are (very) similar, I could live with one fix for both. 
>> Otherwise you should fix the other bug first, if its blocking this change.
>
> The only thing linking the bugs is `AfterEnum`. Other than that I'd say 
> they're separate. If the bug should be fixed first (instead of merging 
> incorrect but passing tests) then I think it should be a separate diff.
>
> My concern with that course of action is that whoever reviews that diff will 
> all but certainly ask for unit tests, and if I wrote unit tests for 
> `AfterEnum` and `AllowShortCaseLabelsOnASingleLine` as I have here, they'll 
> still be incorrect because this bug is not merged. Would you foresee this 
> being an issue?
>
> I'll start working on a fix regardless.

Than I would say it should be one change. Omitting a (needed) test just so it 
passes is not good, and failing tests are worse. Just fix both at the same time.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93938

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


[PATCH] D98364: [clangd] Use ref counted strings throughout for File Contents

2021-03-10 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments.



Comment at: clang-tools-extra/clangd/ClangdServer.h:195-198
   void addDocument(PathRef File, StringRef Contents,
llvm::StringRef Version = "null",
WantDiagnostics WD = WantDiagnostics::Auto,
bool ForceRebuild = false);

This overload is now only used for tests, should it be removed and update all 
the tests that use it to the new version.



Comment at: clang-tools-extra/clangd/DraftStore.h:25-28
+std::unique_ptr
+getSharedStringBuffer(std::shared_ptr Contents,
+  StringRef BufferName = "");
+

This is here for convenience, but should probably go into another header, just 
unsure if any of them fit the purpose. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98364

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


[PATCH] D98364: [clangd] Use ref counted strings throughout for File Contents

2021-03-10 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision.
njames93 added reviewers: sammccall, kadircet.
Herald added subscribers: usaxena95, arphaman, javed.absar.
njames93 requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D98364

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/Compiler.h
  clang-tools-extra/clangd/DraftStore.cpp
  clang-tools-extra/clangd/DraftStore.h
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/Preamble.cpp
  clang-tools-extra/clangd/TUScheduler.cpp
  clang-tools-extra/clangd/TUScheduler.h
  clang-tools-extra/clangd/tool/Check.cpp
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
  clang-tools-extra/clangd/unittests/DraftStoreTests.cpp
  clang-tools-extra/clangd/unittests/FileIndexTests.cpp
  clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
  clang-tools-extra/clangd/unittests/TestTU.cpp

Index: clang-tools-extra/clangd/unittests/TestTU.cpp
===
--- clang-tools-extra/clangd/unittests/TestTU.cpp
+++ clang-tools-extra/clangd/unittests/TestTU.cpp
@@ -54,7 +54,7 @@
   Argv.push_back(FullFilename);
   Inputs.CompileCommand.Filename = FullFilename;
   Inputs.CompileCommand.Directory = testRoot();
-  Inputs.Contents = Code;
+  Inputs.Contents = std::make_shared(Code);
   if (OverlayRealFileSystemForModules)
 FS.OverlayRealFileSystemForModules = true;
   Inputs.TFS = 
Index: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
===
--- clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
+++ clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
@@ -93,7 +93,7 @@
 ParseInputs Inputs;
 Inputs.CompileCommand = *CDB.getCompileCommand(File);
 Inputs.TFS = 
-Inputs.Contents = std::move(Contents);
+Inputs.Contents = std::make_shared(std::move(Contents));
 Inputs.Opts = ParseOptions();
 return Inputs;
   }
@@ -528,7 +528,7 @@
 EXPECT_EQ(File, boundPath());
 
 ASSERT_TRUE((bool)AST);
-EXPECT_EQ(AST->Inputs.Contents, Inputs.Contents);
+EXPECT_EQ(*AST->Inputs.Contents, *Inputs.Contents);
 EXPECT_EQ(AST->Inputs.Version, Inputs.Version);
 EXPECT_EQ(AST->AST.version(), Inputs.Version);
 
@@ -548,7 +548,7 @@
 EXPECT_EQ(File, boundPath());
 
 ASSERT_TRUE((bool)Preamble);
-EXPECT_EQ(Preamble->Contents, Inputs.Contents);
+EXPECT_EQ(*Preamble->Contents, *Inputs.Contents);
 
 std::lock_guard Lock(Mut);
 ++TotalPreambleReads;
Index: clang-tools-extra/clangd/unittests/FileIndexTests.cpp
===
--- clang-tools-extra/clangd/unittests/FileIndexTests.cpp
+++ clang-tools-extra/clangd/unittests/FileIndexTests.cpp
@@ -295,12 +295,12 @@
   )cpp";
   PI.TFS = 
 
-  PI.Contents = R"cpp(
+  PI.Contents = std::make_shared(R"cpp(
 #include "foo.h"
 namespace ns_in_source {
   int func_in_source();
 }
-  )cpp";
+  )cpp");
 
   // Rebuild the file.
   IgnoreDiagnostics IgnoreDiags;
Index: clang-tools-extra/clangd/unittests/DraftStoreTests.cpp
===
--- clang-tools-extra/clangd/unittests/DraftStoreTests.cpp
+++ clang-tools-extra/clangd/unittests/DraftStoreTests.cpp
@@ -22,20 +22,21 @@
   DraftStore DS;
   Path File = "foo.cpp";
 
-  EXPECT_EQ("25", DS.addDraft(File, "25", ""));
+  EXPECT_EQ("25", DS.addDraft(File, "25", std::make_shared("")));
   EXPECT_EQ("25", DS.getDraft(File)->Version);
   EXPECT_EQ("", *DS.getDraft(File)->Contents);
 
-  EXPECT_EQ("26", DS.addDraft(File, "", "x"));
+  EXPECT_EQ("26", DS.addDraft(File, "", std::make_shared("x")));
   EXPECT_EQ("26", DS.getDraft(File)->Version);
   EXPECT_EQ("x", *DS.getDraft(File)->Contents);
 
-  EXPECT_EQ("27", DS.addDraft(File, "", "x")) << "no-op change";
+  EXPECT_EQ("27", DS.addDraft(File, "", std::make_shared("x")))
+  << "no-op change";
   EXPECT_EQ("27", DS.getDraft(File)->Version);
   EXPECT_EQ("x", *DS.getDraft(File)->Contents);
 
   // We allow versions to go backwards.
-  EXPECT_EQ("7", DS.addDraft(File, "7", "y"));
+  EXPECT_EQ("7", DS.addDraft(File, "7", std::make_shared("y")));
   EXPECT_EQ("7", DS.getDraft(File)->Version);
   EXPECT_EQ("y", *DS.getDraft(File)->Contents);
 }
Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -153,7 +153,8 @@
 
   MockFS 

[PATCH] D97183: [analyzer] Add NoteTag for smart-ptr get()

2021-03-10 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment.

> I can see that it adds many visitors to the `BugReport` passed to it.

Yes and some of these visitors will call `trackExpressionValue()` again in 
their `Visit...()` functions which corresponds to adding visitors in the middle 
of visitation which is arguably the most interesting part.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97183

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


[PATCH] D97857: [Matrix] Add support for matrix-by-scalar division.

2021-03-10 Thread Everton Constantino via Phabricator via cfe-commits
everton.constantino added inline comments.



Comment at: clang/test/CodeGen/matrix-type-operators.c:303
 }
 
+// CHECK-LABEL: @divide_double_matrix_scalar_float(

fhahn wrote:
> everton.constantino wrote:
> > Shouldn't a test for half floating point be added here as well? 
> Thanks for taking a look! I don't think we have any tests for half floating 
> points at the moment. The tests do not cover each possible combination of 
> types I think, because they re-use the existing code-gen for the stuff that's 
> element type specific (except deciding whether to use floating point or 
> integer ops).
> 
> I think it would probably be good to have some FP16 tests though, but that 
> should be a separate change IMO.
Thanks for the reply. Ok, that makes sense, I was under the false impression 
FP16 was already covered.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97857

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


[PATCH] D98363: [Sema] Fold VLA types in compound literals to constant arrays.

2021-03-10 Thread Eli Friedman via Phabricator via cfe-commits
efriedma created this revision.
efriedma added reviewers: rsmith, erik.pilkington, aaron.ballman.
efriedma requested review of this revision.
Herald added a project: clang.

Similar to variables with an initializer, this is never valid in standard C, so 
we can safely constant-fold as an extension.  I ran into this construct in a 
couple proprietary codebases.

While I'm here, drive-by fix for 090dd647 
: we 
should only fold variables with VLA types, not arbitrary variably modified 
types.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D98363

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/vla.c

Index: clang/test/Sema/vla.c
===
--- clang/test/Sema/vla.c
+++ clang/test/Sema/vla.c
@@ -129,4 +129,9 @@
 
   // expected-warning@+1{{variable length array folded to constant array as an extension}}
   char a8[2][ksize] = {{1,2,3,4},{4,3,2,1}};
+
+  // expected-warning@+1{{variable length array folded to constant array as an extension}}
+  char (*a9)[] = (char[2][ksize]) {{1,2,3,4},{4,3,2,1}};
+
+  char (*a10)[ksize] = 0;
 }
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -6833,9 +6833,12 @@
 diag::err_array_incomplete_or_sizeless_type,
 SourceRange(LParenLoc, LiteralExpr->getSourceRange().getEnd(
   return ExprError();
-if (literalType->isVariableArrayType())
-  return ExprError(Diag(LParenLoc, diag::err_variable_object_no_init)
-<< SourceRange(LParenLoc, LiteralExpr->getSourceRange().getEnd()));
+if (literalType->isVariableArrayType()) {
+  if (!tryToFixVariablyModifiedVarType(TInfo, literalType, LParenLoc,
+   diag::err_variable_object_no_init)) {
+return ExprError();
+  }
+}
   } else if (!literalType->isDependentType() &&
  RequireCompleteType(LParenLoc, literalType,
diag::err_typecheck_decl_incomplete_type,
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -6038,26 +6038,26 @@
 
 /// Attempt to fold a variable-sized type to a constant-sized type, returning
 /// true if we were successful.
-static bool tryToFixVariablyModifiedVarType(Sema , TypeSourceInfo *,
-QualType , SourceLocation Loc,
-unsigned FailedFoldDiagID) {
+bool Sema::tryToFixVariablyModifiedVarType(TypeSourceInfo *,
+   QualType , SourceLocation Loc,
+   unsigned FailedFoldDiagID) {
   bool SizeIsNegative;
   llvm::APSInt Oversized;
   TypeSourceInfo *FixedTInfo = TryToFixInvalidVariablyModifiedTypeSourceInfo(
-  TInfo, S.Context, SizeIsNegative, Oversized);
+  TInfo, Context, SizeIsNegative, Oversized);
   if (FixedTInfo) {
-S.Diag(Loc, diag::ext_vla_folded_to_constant);
+Diag(Loc, diag::ext_vla_folded_to_constant);
 TInfo = FixedTInfo;
 T = FixedTInfo->getType();
 return true;
   }
 
   if (SizeIsNegative)
-S.Diag(Loc, diag::err_typecheck_negative_array_size);
+Diag(Loc, diag::err_typecheck_negative_array_size);
   else if (Oversized.getBoolValue())
-S.Diag(Loc, diag::err_array_too_large) << Oversized.toString(10);
+Diag(Loc, diag::err_array_too_large) << Oversized.toString(10);
   else if (FailedFoldDiagID)
-S.Diag(Loc, FailedFoldDiagID);
+Diag(Loc, FailedFoldDiagID);
   return false;
 }
 
@@ -6894,10 +6894,10 @@
 }
   }
 
-  // If this variable has a variable-modified type and an initializer, try to
+  // If this variable has a VLA type and an initializer, try to
   // fold to a constant-sized type. This is otherwise invalid.
-  if (D.hasInitializer() && R->isVariablyModifiedType())
-tryToFixVariablyModifiedVarType(*this, TInfo, R, D.getIdentifierLoc(),
+  if (D.hasInitializer() && R->isVariableArrayType())
+tryToFixVariablyModifiedVarType(TInfo, R, D.getIdentifierLoc(),
 /*DiagID=*/0);
 
   bool IsMemberSpecialization = false;
@@ -16734,7 +16734,7 @@
   // than a variably modified type.
   if (!InvalidDecl && T->isVariablyModifiedType()) {
 if (!tryToFixVariablyModifiedVarType(
-*this, TInfo, T, Loc, diag::err_typecheck_field_variable_size))
+TInfo, T, Loc, diag::err_typecheck_field_variable_size))
   InvalidDecl = true;
   }
 
@@ -16962,7 +16962,7 @@
   // than a variably modified type.
   else if (T->isVariablyModifiedType()) {
 if (!tryToFixVariablyModifiedVarType(
-*this, TInfo, T, Loc, diag::err_typecheck_ivar_variable_size))
+   

[PATCH] D97857: [Matrix] Add support for matrix-by-scalar division.

2021-03-10 Thread Gerolf Hoflehner via Phabricator via cfe-commits
Gerolf added a comment.

Nice! This is a straightforward extension with minor code clean-up.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97857

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


[PATCH] D98135: [OpenMP][InstrProfiling] Fix a missing instr profiling counter

2021-03-10 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added a comment.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98135

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


[PATCH] D98135: [OpenMP][InstrProfiling] Fix a missing instr profiling counter

2021-03-10 Thread Xun Li via Phabricator via cfe-commits
lxfind added a comment.

In D98135#2617448 , @ABataev wrote:

> In D98135#2617446 , @lxfind wrote:
>
>> In D98135#2617432 , @ABataev wrote:
>>
>>> There is a problem. We actually do not emit `S` here directly, instead, we 
>>> use `CodeGen` lambdas, which may not be equal to `S`, in some cases `S` is 
>>> `nullptr` here. It may result in not quite accurate results.
>>
>> Thanks for the note! 
>> I don't really know anything about OMP though, not sure how to handle it. 
>> Would you mind taking a look at this issue? Feel free to send a different 
>> patch!
>
> Could you create a PR for the problem so we could track it?

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98135

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


[PATCH] D98358: [OpenMP] Restore backwards compatibility for libomptarget

2021-03-10 Thread Chi Chun Chen via Phabricator via cfe-commits
cchen accepted this revision.
cchen added a comment.

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98358

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


[PATCH] D98291: [compiler-rt] Fix stale incremental builds when using `LLVM_BUILD_EXTERNAL_COMPILER_RT=ON`.

2021-03-10 Thread Chris Bieneman via Phabricator via cfe-commits
beanz added a comment.

This should not impact the Apple Clang releases since those are always clean 
builds.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98291

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


[PATCH] D98135: [OpenMP][InstrProfiling] Fix a missing instr profiling counter

2021-03-10 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added a comment.

In D98135#2617446 , @lxfind wrote:

> In D98135#2617432 , @ABataev wrote:
>
>> There is a problem. We actually do not emit `S` here directly, instead, we 
>> use `CodeGen` lambdas, which may not be equal to `S`, in some cases `S` is 
>> `nullptr` here. It may result in not quite accurate results.
>
> Thanks for the note! 
> I don't really know anything about OMP though, not sure how to handle it. 
> Would you mind taking a look at this issue? Feel free to send a different 
> patch!

Could you create a PR for the problem so we could track it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98135

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


[PATCH] D98135: [OpenMP][InstrProfiling] Fix a missing instr profiling counter

2021-03-10 Thread Xun Li via Phabricator via cfe-commits
lxfind added a comment.

In D98135#2617432 , @ABataev wrote:

> There is a problem. We actually do not emit `S` here directly, instead, we 
> use `CodeGen` lambdas, which may not be equal to `S`, in some cases `S` is 
> `nullptr` here. It may result in not quite accurate results.

Thanks for the note! 
I don't really know anything about OMP though, not sure how to handle it. 
Would you mind taking a look at this issue? Feel free to send a different patch!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98135

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


[PATCH] D97183: [analyzer] Add NoteTag for smart-ptr get()

2021-03-10 Thread Deep Majumder via Phabricator via cfe-commits
RedDocMD added a comment.

@NoQ, looking through the source code of `trackExpressionValue` I can see that 
it adds many visitors to the `BugReport` passed to it. That I believe is the 
recursive attachment of visitors you described above.
So, as far as I understood, I have to make changes in this function to mark an 
`ExplodedNode` as interesting when it changes tracking mode. This change is 
marked by when recursively a new visitor is attached (or at least in some of 
those places, the exact places will have to be figured out).
Then this can be queried from the checker to obtain the information that is 
needed.
Am I thinking on the right track?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97183

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


[PATCH] D98135: [OpenMP][InstrProfiling] Fix a missing instr profiling counter

2021-03-10 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added a comment.

There is a problem. We actually do not emit `S` here directly, instead, we use 
`CodeGen` lambdas, which may not be equal to `S`, in some cases `S` is 
`nullptr` here. It may result in not quite accurate results.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98135

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


[PATCH] D94622: [clang-tidy] add concurrency-async-no-new-threads

2021-03-10 Thread Vasily Kulikov via Phabricator via cfe-commits
segoon added a comment.

It seems that the related file structure (IOW, "separated coroutine code and 
non-coroutine code" rule) is indeed specific to Yandex.Taxi. I asked multiple 
developers in different companies and no one could confirm they align with the 
rule. The checks in the current form seem to have lower value outside of 
Yandex.Taxi than I thought. So, I'll close PRs with the checks in the current 
form (unless someone tell me what part of the check(s) could be used in another 
way). Thanks for all who reviewed the patches and made suggestions :)

FWIW, we'll still use the checks in Yandex.Taxi. A clang static analyzer check 
or some other graph analysis / manual attributes marking approach would fit 
better for the LLVM upstream.


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

https://reviews.llvm.org/D94622

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


[PATCH] D94355: [Passes] Add relative lookup table converter pass

2021-03-10 Thread Leonard Chan via Phabricator via cfe-commits
leonardchan added a comment.

One thing that just occurred to me: do we also perhaps want to hide this behind 
a flag? Right now it's being added to various default optimization pipelines, 
so some users might be surprised if they suddenly see their lookup tables 
change (either compiler or user generated). Do we want to make this something 
more opt-in, or is the layout itself not necessarily a concern to most users?




Comment at: clang/test/CodeGen/switch-to-lookup-table.c:2
+// Check switch to lookup optimization in fPIC and fno-PIC mode
+// RUN: %clang_cc1 %s -triple=aarch64-unknown-fuchsia -O2 -fno-PIC -S 
-emit-llvm -o - | FileCheck %s --check-prefix=CHECK --check-prefix=FNOPIC
+// RUN: %clang_cc1 %s -triple=aarch64-unknown-fuchsia -O2 -fPIC -S -emit-llvm 
-o - | FileCheck %s --check-prefix=CHECK --check-prefix=FPIC

gulfem wrote:
> hans wrote:
> > gulfem wrote:
> > > hans wrote:
> > > > Clang codegen tests are not normally used to test LLVM optimizations. I 
> > > > think the tests for this should all live in LLVM, not Clang.
> > > > 
> > > > (Also aarch64 is not guaranteed to be included as a target in the LLVM 
> > > > build, so this test would not necessarily run.)
> > > I'm not able to use -fPIC and -fno-PIC options in the `opt` tool.
> > > I am setting the `PIC Level` flag to enable -fPIC in `opt.
> > > I thought that testing -fPIC and -fno-PIC in the same file seems easier 
> > > and more readable  in CodeGen tests.
> > > Please let me know if you have a good suggestion how to do that with 
> > > `opt`.
> > > 
> > > I changed the target to `x86_64-linux` in this test.
> > Buildbots may not have x86_64 as a registered target, so this test will 
> > break some buildbots.
> > 
> > I think the opt flags -relocation-model=pic and -relocation-model=static 
> > will do the right thing (see for example 
> > llvm/test/Transforms/SimplifyCFG/ARM/switch-to-lookup-table.ll
> I added `x86-registered-target` to ensure that it only runs on buildbots that 
> have `x86_64` as a registered target
+1 on this. Unless this functionally changes something in the clang codebase, 
this test shouldn't be here. As hans pointed out, setting the 
`-relocation-model` should be enough.



Comment at: llvm/include/llvm/CodeGen/BasicTTIImpl.h:384
 
+  bool shouldBuildRelLookupTables() {
+const TargetMachine  = getTLI()->getTargetMachine();

Sorry, I think you might have explained this offline, but what was the reason 
for needing this in TTI? I would've though this information could be found in 
the `Module` (PIC/no PIC, 64-bit or not, code model). If it turns out all of 
this is available in `Module`, then we could greatly simplify some logic here 
by just checking this at the start of the pass run.

If TTI is needed, then perhaps it may be better to just inline all these checks 
in `convertToRelativeLookupTables` since this is the only place this is called. 
I think we would only want to keep this here as a virtual method if we plan to 
have multiple TTI-impls overriding this.



Comment at: llvm/lib/Transforms/IPO/PassManagerBuilder.cpp:880
 
+  MPM.add(createRelLookupTableConverterPass());
+

Should this be added at the end of the pipeline? It could be possible that 
other passes insert lookup tables but they may be untouched by this if this 
pass runs before them.



Comment at: llvm/lib/Transforms/Utils/RelLookupTableConverter.cpp:30-31
+
+  /// If lookup table has more than one user,
+  /// do not generate a relative lookup table.
+  if (!GlobalVar.hasOneUse())

Nit: `//`



Comment at: llvm/lib/Transforms/Utils/RelLookupTableConverter.cpp:40
+
+  if (!dyn_cast(GEP->use_begin()->getUser()))
+return false;

`isa`



Comment at: llvm/lib/Transforms/Utils/RelLookupTableConverter.cpp:53-70
+  for (Use  : Array->operands()) {
+if (GlobalVariable *GlobalVarOp = dyn_cast(Operand)) {
+  /// If any of the pointer values in the lookup table is a global value
+  /// but not dso_local, do not generate a relative lookup table.
+  if (!(GlobalVarOp->isDSOLocal() || GlobalVarOp->isImplicitDSOLocal()))
+return false;
+} else {

I think I mentioned this in a previous round of comments, but what you probably 
want here is just a way to determine if the operand is either a global or some 
constant offset from global. Right now it looks this this will ignore other 
constant expressions like bitcasts or ptrtoints which we also want to catch. I 
think it might be better to use `IsConstantOffsetFromGlobal` which already 
handles these cases.



Comment at: llvm/lib/Transforms/Utils/RelLookupTableConverter.cpp:79
+  ConstantArray *LookupTableArr =
+  dyn_cast(LookupTable.getInitializer());
+  unsigned NumElts = LookupTableArr->getType()->getNumElements();

`cast`



[PATCH] D96280: [clang][cli] Round-trip the whole CompilerInvocation

2021-03-10 Thread Alan Phipps via Phabricator via cfe-commits
alanphipps added a comment.

In D96280#2611651 , @alanphipps wrote:

> We also encounter a build failure on the Mac related to above but in a 
> different file:
> clang/lib/CodeGen/CGStmtOpenMP.cpp:1916:3: error: too few template arguments 
> for class template 'SmallVector'
> SmallVector EffectiveArgs;
> ^
> clang/include/clang/Basic/LLVM.h:35:42: note: template is declared here
> template class SmallVector;

Disregard -- my issue is related to another commit and is addressed by 
https://reviews.llvm.org/D98265

In D96280#2611651 , @alanphipps wrote:

> We also encounter a build failure on the Mac related to above but in a 
> different file:
> clang/lib/CodeGen/CGStmtOpenMP.cpp:1916:3: error: too few template arguments 
> for class template 'SmallVector'
> SmallVector EffectiveArgs;
> ^
> clang/include/clang/Basic/LLVM.h:35:42: note: template is declared here
> template class SmallVector;




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96280

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


[PATCH] D94973: [clang][OpenMP] Use OpenMPIRBuilder for workshare loops.

2021-03-10 Thread Alan Phipps via Phabricator via cfe-commits
alanphipps added a comment.

In D94973#2617304 , @Meinersbur wrote:

> Is is that same problem that D98265  
> addresses?

Yes, that appears to be the same.  Thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94973

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


[PATCH] D98358: [OpenMP] Restore backwards compatibility for libomptarget

2021-03-10 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 updated this revision to Diff 329709.
jhuber6 added a comment.

Updating tests and adding the new function to the exports.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98358

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/test/OpenMP/target_teams_distribute_parallel_for_codegen.cpp
  clang/test/OpenMP/target_teams_distribute_parallel_for_if_codegen.cpp
  clang/test/OpenMP/target_teams_distribute_parallel_for_order_codegen.cpp
  clang/test/OpenMP/target_teams_distribute_parallel_for_simd_codegen.cpp
  clang/test/OpenMP/target_teams_distribute_parallel_for_simd_if_codegen.cpp
  clang/test/OpenMP/teams_distribute_codegen.cpp
  clang/test/OpenMP/teams_distribute_parallel_for_codegen.cpp
  clang/test/OpenMP/teams_distribute_parallel_for_simd_codegen.cpp
  clang/test/OpenMP/teams_distribute_simd_codegen.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
  llvm/test/Transforms/OpenMP/add_attributes.ll
  openmp/libomptarget/include/omptarget.h
  openmp/libomptarget/src/exports
  openmp/libomptarget/src/interface.cpp
  openmp/libomptarget/src/omptarget.cpp
  openmp/libomptarget/test/offloading/host_as_target.c

Index: openmp/libomptarget/test/offloading/host_as_target.c
===
--- openmp/libomptarget/test/offloading/host_as_target.c
+++ openmp/libomptarget/test/offloading/host_as_target.c
@@ -55,8 +55,8 @@
   printf("omp_is_initial_device() = %d\n", omp_is_initial_device());
   CHECK_DATA();
 
-  // Check that __kmpc_push_target_tripcount doesn't fail.  I'm not sure how to
-  // check that it actually pushes to the initial device.
+  // Check that __kmpc_push_target_tripcount_mapper doesn't fail. I'm not sure
+  // how to check that it actually pushes to the initial device.
   #pragma omp target teams device(DevInit) num_teams(1)
   #pragma omp distribute
   for (int i = 0; i < 2; ++i)
@@ -112,8 +112,8 @@
   printf("omp_is_initial_device() = %d\n", omp_is_initial_device());
   CHECK_DATA();
 
-  // Check that __kmpc_push_target_tripcount doesn't fail.  I'm not sure how to
-  // check that it actually pushes to the initial device.
+  // Check that __kmpc_push_target_tripcount_mapper doesn't fail. I'm not sure
+  // how to check that it actually pushes to the initial device.
   #pragma omp target teams num_teams(1)
   #pragma omp distribute
   for (int i = 0; i < 2; ++i)
Index: openmp/libomptarget/src/omptarget.cpp
===
--- openmp/libomptarget/src/omptarget.cpp
+++ openmp/libomptarget/src/omptarget.cpp
@@ -1023,8 +1023,8 @@
 
 /// Get loop trip count
 /// FIXME: This function will not work right if calling
-/// __kmpc_push_target_tripcount in one thread but doing offloading in another
-/// thread, which might occur when we call task yield.
+/// __kmpc_push_target_tripcount_mapper in one thread but doing offloading in
+/// another thread, which might occur when we call task yield.
 uint64_t getLoopTripCount(int64_t DeviceId) {
   DeviceTy  = PM->Devices[DeviceId];
   uint64_t LoopTripCount = 0;
Index: openmp/libomptarget/src/interface.cpp
===
--- openmp/libomptarget/src/interface.cpp
+++ openmp/libomptarget/src/interface.cpp
@@ -437,8 +437,13 @@
   MapComponentInfoTy(base, begin, size, type, name));
 }
 
-EXTERN void __kmpc_push_target_tripcount(ident_t *loc, int64_t device_id,
+EXTERN void __kmpc_push_target_tripcount(int64_t device_id,
  uint64_t loop_tripcount) {
+  __kmpc_push_target_tripcount_mapper(nullptr, device_id, loop_tripcount);
+}
+
+EXTERN void __kmpc_push_target_tripcount_mapper(ident_t *loc, int64_t device_id,
+uint64_t loop_tripcount) {
   TIMESCOPE_WITH_IDENT(loc);
   if (checkDeviceAndCtors(device_id, loc) != OFFLOAD_SUCCESS) {
 DP("Not offloading to device %" PRId64 "\n", device_id);
Index: openmp/libomptarget/src/exports
===
--- openmp/libomptarget/src/exports
+++ openmp/libomptarget/src/exports
@@ -25,6 +25,8 @@
 __tgt_target_teams_nowait_mapper;
 __tgt_mapper_num_components;
 __tgt_push_mapper_component;
+__kmpc_push_target_tripcount;
+__kmpc_push_target_tripcount_mapper;
 omp_get_num_devices;
 omp_get_initial_device;
 omp_target_alloc;
@@ -34,7 +36,6 @@
 omp_target_memcpy_rect;
 omp_target_associate_ptr;
 omp_target_disassociate_ptr;
-__kmpc_push_target_tripcount;
   local:
 *;
 };
Index: openmp/libomptarget/include/omptarget.h
===
--- openmp/libomptarget/include/omptarget.h
+++ openmp/libomptarget/include/omptarget.h
@@ -312,8 +312,10 @@
 int32_t thread_limit, int32_t depNum, void *depList, int32_t noAliasDepNum,
 void 

[PATCH] D97872: [clang] Don't assert in EmitAggregateCopy on trivial_abi types

2021-03-10 Thread Arthur Eubanks via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGc8227f06b335: [clang] Dont assert in EmitAggregateCopy 
on trivial_abi types (authored by aeubanks).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97872

Files:
  clang/lib/CodeGen/CGExprAgg.cpp
  clang/test/CodeGenCXX/trivial_abi.cpp


Index: clang/test/CodeGenCXX/trivial_abi.cpp
===
--- clang/test/CodeGenCXX/trivial_abi.cpp
+++ clang/test/CodeGenCXX/trivial_abi.cpp
@@ -262,3 +262,21 @@
 void testExceptionLarge() {
   calleeExceptionLarge(Large(), Large());
 }
+
+// PR42961
+
+// CHECK: define{{.*}} @"_ZN3$_08__invokeEv"()
+// CHECK: %[[RETVAL:.*]] = alloca %[[STRUCT_SMALL]], align 8
+// CHECK: %[[COERCE:.*]] = alloca %[[STRUCT_SMALL]], align 8
+// CHECK: %[[CALL:.*]] = call{{.*}} @"_ZNK3$_0clEv"
+// CHECK: %[[COERCEDIVE:.*]] = getelementptr{{.*}} %[[COERCE]]
+// CHECK: %[[COERCEVALIP:.*]] = inttoptr{{.*}} %[[CALL]]
+// CHECK: %[[RETVALP:.*]] = bitcast %[[STRUCT_SMALL]]* %[[RETVAL]]
+// CHECK: %[[COERCEP:.*]] = bitcast %[[STRUCT_SMALL]]* %[[COERCE]]
+// CHECK: call {{.*}}memcpy{{.*}} %[[RETVALP]]{{.*}} %[[COERCEP]]
+// CHECK: %[[COERCEDIVE1:.*]] = getelementptr{{.*}} %[[RETVAL]]
+// CHECK: %[[TMP:.*]] = load{{.*}} %[[COERCEDIVE1]]
+// CHECK: %[[COERCEVALPI:.*]] = ptrtoint{{.*}} %[[TMP]]
+// CHECK: ret{{.*}} %[[COERCEVALPI]]
+
+Small (*fp)() = []() -> Small { return Small(); };
Index: clang/lib/CodeGen/CGExprAgg.cpp
===
--- clang/lib/CodeGen/CGExprAgg.cpp
+++ clang/lib/CodeGen/CGExprAgg.cpp
@@ -2056,7 +2056,7 @@
   Record->hasTrivialCopyAssignment() ||
   Record->hasTrivialMoveConstructor() ||
   Record->hasTrivialMoveAssignment() ||
-  Record->isUnion()) &&
+  Record->hasAttr() || Record->isUnion()) &&
  "Trying to aggregate-copy a type without a trivial copy/move "
  "constructor or assignment operator");
   // Ignore empty classes in C++.


Index: clang/test/CodeGenCXX/trivial_abi.cpp
===
--- clang/test/CodeGenCXX/trivial_abi.cpp
+++ clang/test/CodeGenCXX/trivial_abi.cpp
@@ -262,3 +262,21 @@
 void testExceptionLarge() {
   calleeExceptionLarge(Large(), Large());
 }
+
+// PR42961
+
+// CHECK: define{{.*}} @"_ZN3$_08__invokeEv"()
+// CHECK: %[[RETVAL:.*]] = alloca %[[STRUCT_SMALL]], align 8
+// CHECK: %[[COERCE:.*]] = alloca %[[STRUCT_SMALL]], align 8
+// CHECK: %[[CALL:.*]] = call{{.*}} @"_ZNK3$_0clEv"
+// CHECK: %[[COERCEDIVE:.*]] = getelementptr{{.*}} %[[COERCE]]
+// CHECK: %[[COERCEVALIP:.*]] = inttoptr{{.*}} %[[CALL]]
+// CHECK: %[[RETVALP:.*]] = bitcast %[[STRUCT_SMALL]]* %[[RETVAL]]
+// CHECK: %[[COERCEP:.*]] = bitcast %[[STRUCT_SMALL]]* %[[COERCE]]
+// CHECK: call {{.*}}memcpy{{.*}} %[[RETVALP]]{{.*}} %[[COERCEP]]
+// CHECK: %[[COERCEDIVE1:.*]] = getelementptr{{.*}} %[[RETVAL]]
+// CHECK: %[[TMP:.*]] = load{{.*}} %[[COERCEDIVE1]]
+// CHECK: %[[COERCEVALPI:.*]] = ptrtoint{{.*}} %[[TMP]]
+// CHECK: ret{{.*}} %[[COERCEVALPI]]
+
+Small (*fp)() = []() -> Small { return Small(); };
Index: clang/lib/CodeGen/CGExprAgg.cpp
===
--- clang/lib/CodeGen/CGExprAgg.cpp
+++ clang/lib/CodeGen/CGExprAgg.cpp
@@ -2056,7 +2056,7 @@
   Record->hasTrivialCopyAssignment() ||
   Record->hasTrivialMoveConstructor() ||
   Record->hasTrivialMoveAssignment() ||
-  Record->isUnion()) &&
+  Record->hasAttr() || Record->isUnion()) &&
  "Trying to aggregate-copy a type without a trivial copy/move "
  "constructor or assignment operator");
   // Ignore empty classes in C++.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] c8227f0 - [clang] Don't assert in EmitAggregateCopy on trivial_abi types

2021-03-10 Thread Arthur Eubanks via cfe-commits

Author: Arthur Eubanks
Date: 2021-03-10T10:16:06-08:00
New Revision: c8227f06b3356cdc9cc757ddfb59a6d8ad89

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

LOG: [clang] Don't assert in EmitAggregateCopy on trivial_abi types

Fixes PR42961.

Reviewed By: rnk

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

Added: 


Modified: 
clang/lib/CodeGen/CGExprAgg.cpp
clang/test/CodeGenCXX/trivial_abi.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGExprAgg.cpp b/clang/lib/CodeGen/CGExprAgg.cpp
index 60ea1b2af037..f3ab91559d30 100644
--- a/clang/lib/CodeGen/CGExprAgg.cpp
+++ b/clang/lib/CodeGen/CGExprAgg.cpp
@@ -2056,7 +2056,7 @@ void CodeGenFunction::EmitAggregateCopy(LValue Dest, 
LValue Src, QualType Ty,
   Record->hasTrivialCopyAssignment() ||
   Record->hasTrivialMoveConstructor() ||
   Record->hasTrivialMoveAssignment() ||
-  Record->isUnion()) &&
+  Record->hasAttr() || Record->isUnion()) &&
  "Trying to aggregate-copy a type without a trivial copy/move "
  "constructor or assignment operator");
   // Ignore empty classes in C++.

diff  --git a/clang/test/CodeGenCXX/trivial_abi.cpp 
b/clang/test/CodeGenCXX/trivial_abi.cpp
index ac41f5cac086..a4222c100311 100644
--- a/clang/test/CodeGenCXX/trivial_abi.cpp
+++ b/clang/test/CodeGenCXX/trivial_abi.cpp
@@ -262,3 +262,21 @@ void calleeExceptionLarge(Large, Large);
 void testExceptionLarge() {
   calleeExceptionLarge(Large(), Large());
 }
+
+// PR42961
+
+// CHECK: define{{.*}} @"_ZN3$_08__invokeEv"()
+// CHECK: %[[RETVAL:.*]] = alloca %[[STRUCT_SMALL]], align 8
+// CHECK: %[[COERCE:.*]] = alloca %[[STRUCT_SMALL]], align 8
+// CHECK: %[[CALL:.*]] = call{{.*}} @"_ZNK3$_0clEv"
+// CHECK: %[[COERCEDIVE:.*]] = getelementptr{{.*}} %[[COERCE]]
+// CHECK: %[[COERCEVALIP:.*]] = inttoptr{{.*}} %[[CALL]]
+// CHECK: %[[RETVALP:.*]] = bitcast %[[STRUCT_SMALL]]* %[[RETVAL]]
+// CHECK: %[[COERCEP:.*]] = bitcast %[[STRUCT_SMALL]]* %[[COERCE]]
+// CHECK: call {{.*}}memcpy{{.*}} %[[RETVALP]]{{.*}} %[[COERCEP]]
+// CHECK: %[[COERCEDIVE1:.*]] = getelementptr{{.*}} %[[RETVAL]]
+// CHECK: %[[TMP:.*]] = load{{.*}} %[[COERCEDIVE1]]
+// CHECK: %[[COERCEVALPI:.*]] = ptrtoint{{.*}} %[[TMP]]
+// CHECK: ret{{.*}} %[[COERCEVALPI]]
+
+Small (*fp)() = []() -> Small { return Small(); };



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


[PATCH] D94973: [clang][OpenMP] Use OpenMPIRBuilder for workshare loops.

2021-03-10 Thread Michael Kruse via Phabricator via cfe-commits
Meinersbur added a comment.

Is is that same problem that D98265  addresses?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94973

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


[PATCH] D98193: [CUDA][HIP] Allow non-ODR use of host var in device

2021-03-10 Thread Artem Belevich via Phabricator via cfe-commits
tra added a reviewer: rsmith.
tra added a subscriber: rsmith.
tra added a comment.

LGTM

I've added @rsmith to double check that we're handling it correctly.


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

https://reviews.llvm.org/D98193

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


[PATCH] D98291: [compiler-rt] Fix stale incremental builds when using `LLVM_BUILD_EXTERNAL_COMPILER_RT=ON`.

2021-03-10 Thread Dan Liew via Phabricator via cfe-commits
delcypher added a comment.

In D98291#2616865 , @kubamracek wrote:

> Looks good!
>
> By the way, Apple Clang releases also build compiler-rt this way, so it would 
> be worth checking with @arphaman that he's okay with the change.

@kubamracek @arphaman Ah sorry I landed the change before I saw this message. 
It looks like the `LLVM_BUILD_EXTERNAL_COMPILER_RT` option is used as part of 
the 2-stage boot strap for AppleClang as it's set in 
`clang/cmake/caches/Apple-stage2.cmake`. I'll follow up offline.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98291

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


[PATCH] D96090: [analyzer] Replace StoreManager::CastRetrievedVal with SValBuilder::evalCast

2021-03-10 Thread Denys Petrov via Phabricator via cfe-commits
ASDenysPetrov updated this revision to Diff 329699.
ASDenysPetrov edited the summary of this revision.

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

https://reviews.llvm.org/D96090

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
  clang/lib/StaticAnalyzer/Core/RegionStore.cpp
  clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
  clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  clang/lib/StaticAnalyzer/Core/Store.cpp

Index: clang/lib/StaticAnalyzer/Core/Store.cpp
===
--- clang/lib/StaticAnalyzer/Core/Store.cpp
+++ clang/lib/StaticAnalyzer/Core/Store.cpp
@@ -394,48 +394,6 @@
   return UnknownVal();
 }
 
-static bool hasSameUnqualifiedPointeeType(QualType ty1, QualType ty2) {
-  return ty1->getPointeeType().getCanonicalType().getTypePtr() ==
- ty2->getPointeeType().getCanonicalType().getTypePtr();
-}
-
-/// CastRetrievedVal - Used by subclasses of StoreManager to implement
-///  implicit casts that arise from loads from regions that are reinterpreted
-///  as another region.
-SVal StoreManager::CastRetrievedVal(SVal V, const TypedValueRegion *R,
-QualType castTy) {
-  if (castTy.isNull() || V.isUnknownOrUndef())
-return V;
-
-  // The dispatchCast() call below would convert the int into a float.
-  // What we want, however, is a bit-by-bit reinterpretation of the int
-  // as a float, which usually yields nothing garbage. For now skip casts
-  // from ints to floats.
-  // TODO: What other combinations of types are affected?
-  if (castTy->isFloatingType()) {
-SymbolRef Sym = V.getAsSymbol();
-if (Sym && !Sym->getType()->isFloatingType())
-  return UnknownVal();
-  }
-
-  // When retrieving symbolic pointer and expecting a non-void pointer,
-  // wrap them into element regions of the expected type if necessary.
-  // SValBuilder::dispatchCast() doesn't do that, but it is necessary to
-  // make sure that the retrieved value makes sense, because there's no other
-  // cast in the AST that would tell us to cast it to the correct pointer type.
-  // We might need to do that for non-void pointers as well.
-  // FIXME: We really need a single good function to perform casts for us
-  // correctly every time we need it.
-  if (castTy->isPointerType() && !castTy->isVoidPointerType())
-if (const auto *SR = dyn_cast_or_null(V.getAsRegion())) {
-  QualType sr = SR->getSymbol()->getType();
-  if (!hasSameUnqualifiedPointeeType(sr, castTy))
-  return loc::MemRegionVal(castRegion(SR, castTy));
-}
-
-  return svalBuilder.dispatchCast(V, castTy);
-}
-
 SVal StoreManager::getLValueFieldOrIvar(const Decl *D, SVal Base) {
   if (Base.isUnknownOrUndef())
 return Base;
Index: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
===
--- clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -65,12 +65,12 @@
 // Transfer function for Casts.
 //===--===//
 
+// FIXME: This function should be eliminated and replaced with `evalCast`
 SVal SimpleSValBuilder::dispatchCast(SVal Val, QualType CastTy) {
-  assert(Val.getAs() || Val.getAs());
-  return Val.getAs() ? evalCastFromLoc(Val.castAs(), CastTy)
-   : evalCastFromNonLoc(Val.castAs(), CastTy);
+  return evalCast(Val, CastTy);
 }
 
+// FIXME: This function should be eliminated and replaced with `evalCast`
 SVal SimpleSValBuilder::evalCastFromNonLoc(NonLoc val, QualType castTy) {
   bool isLocType = Loc::isLocType(castTy);
   if (val.getAs())
@@ -127,6 +127,7 @@
 return makeIntVal(i);
 }
 
+// FIXME: This function should be eliminated and replaced with `evalCast`
 SVal SimpleSValBuilder::evalCastFromLoc(Loc val, QualType castTy) {
 
   // Casts from pointers -> pointers, just return the lval.
Index: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
===
--- clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
+++ clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
@@ -536,20 +536,40 @@
 // `evalCastKind` and `evalCastSubKind` are helpers
 //===--===//
 
-SVal SValBuilder::evalCast(SVal V, QualType CastTy, QualType OriginalTy) {
-  CastTy = Context.getCanonicalType(CastTy);
-  OriginalTy = Context.getCanonicalType(OriginalTy);
-  if (CastTy == OriginalTy)
+/// Cast a given SVal to another SVal using given QualType's.
+/// \param V -- SVal that should be casted.
+/// \param CastTy -- QualType that V should be casted according to.
+/// \param OriginalTy -- QualType which is associated to V. It provides
+/// additional information about what type the cast performs from. This is a
+/// default param. 

[PATCH] D98291: [compiler-rt] Fix stale incremental builds when using `LLVM_BUILD_EXTERNAL_COMPILER_RT=ON`.

2021-03-10 Thread Dan Liew 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 rGa159f91c8d06: [compiler-rt] Fix stale incremental builds 
when using… (authored by delcypher).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98291

Files:
  clang/runtime/CMakeLists.txt


Index: clang/runtime/CMakeLists.txt
===
--- clang/runtime/CMakeLists.txt
+++ clang/runtime/CMakeLists.txt
@@ -95,6 +95,8 @@
 USES_TERMINAL_CONFIGURE 1
 USES_TERMINAL_BUILD 1
 USES_TERMINAL_INSTALL 1
+# Always run the build command so that incremental builds are correct.
+BUILD_ALWAYS 1
 )
 
   get_ext_project_build_command(run_clean_compiler_rt clean)


Index: clang/runtime/CMakeLists.txt
===
--- clang/runtime/CMakeLists.txt
+++ clang/runtime/CMakeLists.txt
@@ -95,6 +95,8 @@
 USES_TERMINAL_CONFIGURE 1
 USES_TERMINAL_BUILD 1
 USES_TERMINAL_INSTALL 1
+# Always run the build command so that incremental builds are correct.
+BUILD_ALWAYS 1
 )
 
   get_ext_project_build_command(run_clean_compiler_rt clean)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

2021-03-10 Thread Alexey Bader via Phabricator via cfe-commits
bader added a comment.

In D89909#2606180 , @Anastasia wrote:

> In D89909#2600859 , @aaron.ballman 
> wrote:
>
>> Just a few minor nits from me, but I'm mostly wondering: where are we at 
>> with this and are there still substantive changes required? (I looked 
>> through the comments, but there's a lot of back-and-forth since Oct and I'm 
>> not certain what's holding the patch back currently.)
>
> To make it short, from my side I am not very clear about the overall design. 
> From the SYCL spec side, there is no indication of what compiler extensions 
> are needed and if at all. As a result, some of the design choices are unclear 
> to me - in particular why SPIR target would need a separate address space map 
> for SYCL. This is not how it was intended originally and I am worried that 
> this will create issues for the consumers of IR to handle two different 
> formats. But in general, if the community is now to maintain this code we 
> should at least have some deeper understanding of it.
>
> I would suggest starting from some high-level documentation that provides the 
> details of the compiler extension being implemented. Perhaps the 
> documentation that @bader has linked earlier could be used as a starting 
> point with some more details that would allow assessing and reviewing the 
> changes.

@Anastasia, do you suggest we copy 
https://github.com/intel/llvm/blob/sycl/sycl/doc/CompilerAndRuntimeDesign.md 
document to clang/docs within this patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89909

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


[clang] a159f91 - [compiler-rt] Fix stale incremental builds when using `LLVM_BUILD_EXTERNAL_COMPILER_RT=ON`.

2021-03-10 Thread Dan Liew via cfe-commits

Author: Dan Liew
Date: 2021-03-10T09:42:24-08:00
New Revision: a159f91c8d068cae660a200868b7fc28fcbcd3ff

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

LOG: [compiler-rt] Fix stale incremental builds when using 
`LLVM_BUILD_EXTERNAL_COMPILER_RT=ON`.

When building with `LLVM_BUILD_EXTERNAL_COMPILER_RT=ON` (e.g. Swift does
this) we do an "external" build of compiler-rt where we build
compiler-rt with the just built clang.

Unfortunately building in this mode had a bug where compiler-rt would
not get rebuilt if compiler-rt sources changed. This is problematic
for incremental builds because it meant that the compiler-rt binaries
were stale.

The fix is to use the `BUILD_ALWAYS` ExternalProject_Add option which
means the build command for compiler-rt is always run.

In principle if all of the following are true:

* compiler-rt has already been built.
* there are no compiler-rt source changes.
* the compiler hasn't changed.
* ninja is being used as the generator for the compiler-rt build.

then the overhead for always running the build command for incremental
builds is negligible.

However, in practice clang gets rebuilt everytime the HEAD commit
changes (due to commit hash being embedded in the output of `--version`)
which means all of compiler-rt will be rebuilt everytime this happens.
While this is annoying it's better to do the slow but correct thing
rather than the fast but incorrect thing.

rdar://75150660

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

Added: 


Modified: 
clang/runtime/CMakeLists.txt

Removed: 




diff  --git a/clang/runtime/CMakeLists.txt b/clang/runtime/CMakeLists.txt
index 61bbbf8faedd..1716c53b031e 100644
--- a/clang/runtime/CMakeLists.txt
+++ b/clang/runtime/CMakeLists.txt
@@ -95,6 +95,8 @@ if(LLVM_BUILD_EXTERNAL_COMPILER_RT AND EXISTS 
${COMPILER_RT_SRC_ROOT}/)
 USES_TERMINAL_CONFIGURE 1
 USES_TERMINAL_BUILD 1
 USES_TERMINAL_INSTALL 1
+# Always run the build command so that incremental builds are correct.
+BUILD_ALWAYS 1
 )
 
   get_ext_project_build_command(run_clean_compiler_rt clean)



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


[PATCH] D94973: [clang][OpenMP] Use OpenMPIRBuilder for workshare loops.

2021-03-10 Thread Alan Phipps via Phabricator via cfe-commits
alanphipps added a comment.

Hello! I am seeing a downstream build failure on Mac as a result of your 
changes:

  clang/lib/CodeGen/CGStmtOpenMP.cpp:1916:3: error: too few template arguments 
for class template 'SmallVector'
  SmallVector EffectiveArgs;
  ^
  clang/include/clang/Basic/LLVM.h:35:42: note: template is declared here
  template class SmallVector;


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94973

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


[PATCH] D98135: [OpenMP][InstrProfiling] Fix a missing instr profiling counter

2021-03-10 Thread Xun Li via Phabricator via cfe-commits
lxfind added a comment.

Thanks. I am landing it. 
But feel free to comment here if anything isn't right. @ABataev


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98135

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


  1   2   3   >