[PATCH] D72114: [MS] Overhaul how clang passes overaligned args on x86_32

[Anton Yudintsev via Phabricator via cfe-commits]

AntonYudintsev added inline comments.



Comment at: clang/test/CodeGen/x86_32-arguments-win32.c:77
+// CHECK-LABEL: define dso_local void @receive_vec_256(<8 x float> inreg %x, 
<8 x float> inreg %y, <8 x float> inreg %z, <8 x float>* %0, <8 x float>* %1)
+// CHECK-LABEL: define dso_local void @receive_vec_512(<16 x float> inreg %x, 
<16 x float> inreg %y, <16 x float> inreg %z, <16 x float>* %0, <16 x float>* 
%1)
+// CHECK-LABEL: define dso_local void @receive_vec_1024(<32 x float>* %0, <32 
x float>* %1, <32 x float>* %2, <32 x float>* %3, <32 x float>* %4)

rjmccall wrote:
> rnk wrote:
> > rjmccall wrote:
> > > rnk wrote:
> > > > craig.topper wrote:
> > > > > What happens in the backend with inreg if 512-bit vectors aren't 
> > > > > legal?
> > > > LLVM splits the vector up using the largest legal vector size. As many 
> > > > pieces as possible are passed in available XMM/YMM registers, and the 
> > > > rest are passed in memory. MSVC, of course, assumes the user wanted the 
> > > > larger vector size, and uses whatever vector instructions it needs to 
> > > > move the arguments around.
> > > > 
> > > > Previously, I advocated for a model where calling an Intel intrinsic 
> > > > function had the effect of implicitly marking the caller with the 
> > > > target attributes of the intrinsic. This falls down if the user tries 
> > > > to write a single function that conditionally branches between code 
> > > > that uses different instruction set extensions. You can imagine the 
> > > > SSE2 codepath accidentally using AVX instructions because the compiler 
> > > > thinks they are better. I'm told that ICC models CPU 
> > > > micro-architectural features in the CFG, but I don't ever expect that 
> > > > LLVM will do that. If we're stuck with per-function CPU feature 
> > > > settings, it seems like it would be nice to try to do what the user 
> > > > asked by default, and warn the user if we see them doing a cpuid check 
> > > > in a function that has been implicitly blessed with some target 
> > > > attributes. You could imagine doing a similar thing when large vector 
> > > > type variables are used: if a large vector argument or local is used, 
> > > > implicitly enable the appropriate target features to move vectors of 
> > > > that size around.
> > > > 
> > > > This idea didn't get anywhere, and the current situation has persisted.
> > > > 
> > > > ---
> > > > 
> > > > You know, maybe we should just keep clang the way it is, and just set 
> > > > up a warning in the backend that says "hey, I split your large vector. 
> > > > You probably didn't want that." And then we just continue doing what we 
> > > > do now. Nobody likes backend warnings, but it seems better than the 
> > > > current direction of the frontend knowing every detail of x86 vector 
> > > > extensions.
> > > If target attributes affect ABI, it seems really dangerous to implicitly 
> > > set attributes based on what intrinsics are called.
> > > 
> > > The local CPU-testing problem seems similar to the problems with local 
> > > `#pragma STDC FENV_ACCESS` blocks that the constrained-FP people are 
> > > looking into.  They both have a "this operation is normally fully 
> > > optimizable, but we might need to be more careful in specific functions" 
> > > aspect to them.  I wonder if there's a reasonable way to unify the 
> > > approaches, or at least benefit from lessons learned.
> > I agree, we wouldn't want intrinsic usage to change ABI. But, does anybody 
> > actually want the behavior that LLVM implements today where large vectors 
> > get split across registers and memory? I think most users who pass a 512 
> > bit vector want it to either be passed in ZMM registers or fail to compile. 
> > Why do we even support passing 1024 bit vectors? Could we make that an 
> > error?
> > 
> > Anyway, major redesigns aside, should clang do something when large vectors 
> > are passed? Maybe we should warn here? Passing by address is usually the 
> > safest way to pass something, so that's an option. Implementing splitting 
> > logic in clang doesn't seem worth it.
> > I agree, we wouldn't want intrinsic usage to change ABI. But, does anybody 
> > actually want the behavior that LLVM implements today where large vectors 
> > get split across registers and memory?
> 
> I take it you're implying that the actual (Windows-only?) platform ABI 
> doesn't say anything about this because other compilers don't allow large 
> vectors.  How large are the vectors we do have ABI rules for?  Do they have 
> the problem as the SysV ABI where the ABI rules are sensitive to compiler 
> flags?
> 
> Anyway, I didn't realize the i386 Windows ABI *ever* used registers for 
> arguments.  (Whether you can convince LLVM to do so  for a function signature 
> that Clang isn't supposed to emit for ABI-conforming functions is a separate 
> story.)   You're saying it uses them for vectors?  Presumably up to some 
> limit, and only when they're

[PATCH] D70764: build: reduce CMake handling for zlib

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai added a comment.

In D70764#1803559 , @raj.khem wrote:

> this is now in master, and I am seeing build failures in cross-building 
> clang, e.g. when building clang for arm on a x86_64 host. its resorting to 
> finding, libz from buildhost instead of target sysroot ( using --sysroot) and 
> failing in link step. e.g.
>
> FAILED: bin/llvm-config
>  ...
>   -o bin/llvm-config  -Wl,-rpath,"\$ORIGIN/../lib"  lib/libLLVMSupport.a  
> /usr/lib/libz.so  -lrt  -ldl  -ltinfo  -lm  lib/libLLVMDemangle.a
>  ...
>
> aarch64-yoe-linux-musl/aarch64-yoe-linux-musl-ld: /usr/lib/libz.so: error 
> adding symbols: file in wrong format
>  clang-10: error: linker command failed with exit code 1 (use -v to see 
> invocation)
>
> you can see that its adding /usr/lib/libz.so to linker cmdline while cross 
> linking.


Have you set `CMAKE_SYSROOT` appropriately?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70764



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

[PATCH] D72227: Add options for clang to align branches within 32B boundary

[annita.zhang via Phabricator via cfe-commits]

annita.zhang added inline comments.



Comment at: clang/include/clang/Driver/Options.td:2198
+   "should be a power of 2 and no less than 32. Branches will be "
+   "aligned within the boundary of specified size. "
+   "-x86-align-branch-boundary=0 doesn't align branches.">;

"aligned to prevent from being across or against the boundary of specified 
size."



Comment at: clang/include/clang/Driver/Options.td:2206
+  "Specify types of branches to align (plus separated list of "
+  "types). The branches's types is combination of jcc, fused, "
+  "jmp, call, ret, indirect."

"types). The branches' types are combination of jcc, fused, "



Comment at: clang/include/clang/Driver/Options.td:2208-2211
+  " jcc, which aligns conditional jumps; fused, which aligns fused "
+  "conditional jumps; jmp, which aligns unconditional jumps; call, "
+  "which aligns calls; ret, which aligns rets; indirect, which "
+  "aligns indirect jumps.">;

"jcc indicates conditional jumps, fused indicates fused conditional jumps,"
"jmp indicates unconditional jumps, call indicates direct and indirect calls,"
"ret indicates rets, indirect indicates indirect jumps."



Comment at: clang/include/clang/Driver/Options.td:2217
+  HelpText<"Specify the maximum number of prefixes on an instruction to "
+   "align branches. The number should be between 0 and 4.">;
+def mbranches_within_32B_boundaries

Is there an upbound for this parameter?



Comment at: clang/include/clang/Driver/Options.td:2227
+  "instruction. It is equivalent to -malign-branch-boundary=32, "
+  "-malign-branch=fused+jcc+jmp, -malign-branch-prefix-size=4.">;
+def mno_branches_within_32B_boundaries

-malign-branch-prefix-size=5.">;


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

https://reviews.llvm.org/D72227



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

[PATCH] D72227: Add options for clang to align branches within 32B boundary

[annita.zhang via Phabricator via cfe-commits]

annita.zhang added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2052
+CmdArgs.push_back("-mllvm");
+CmdArgs.push_back(Args.MakeArgString("-x86-align-branch-prefix-size=4"));
+  }

CmdArgs.push_back(Args.MakeArgString("-x86-align-branch-prefix-size=5"));


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

https://reviews.llvm.org/D72227



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

[PATCH] D72227: Add options for clang to align branches within 32B boundary

[annita.zhang via Phabricator via cfe-commits]

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

A general comment. All the tests have been done with 
-malign-branch-prefix-size=5. I don't see any explicit reason to change the 
default prefix size from 5 to 4.


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

https://reviews.llvm.org/D72227



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

[PATCH] D72304: Summary: Add OpenMP Directives (master and critical) to OMPBuilder, and use them in clang.

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

Some initial comments. Can we split it in two patches (master/critical)?




Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:250
+   llvm::ArrayType *KmpCriticalNameTy;
+   llvm::PointerType *KmpCriticalNamePtrTy;
+

If there is no good reason agains it, these should go into the 
`OMPKinds.def`/`OMPConstants.{h/cpp}`. We need support for array types but that 
is not a problem. 



Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:259
+
+public:
+

A lot of the functions below should not be public. We should expose as little 
as possible, mainly the `CreateDirective` methods.



Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:646
+
+   Directive OMPD = Directive::OMPD_master;
+   Constant *SrcLocStr = getOrCreateSrcLocStr(Loc);

You can just write `OMPD_master` below instead of `OMPD`.



Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:656
+   
Args, /*Conditional*/true, /*hasFinalize*/true);
+}
+

Make sure the patch is clang formatted. See also: 
https://clang.llvm.org/docs/ClangFormat.html#script-for-patch-reformatting



Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:850
+   return RTLFunc;
+}
+

Is this switch + if-cascade + `callEntry/Exit` really helpful?
I mean we can replace
  `Instruction *EntryCall = CreateEntryCall(OMPD, Args);`
with
  `Instruction *EntryCall = 
Builder.CreateCall(getOrCreateRuntimeFunction(OMPRTL___kmpc_omp_master), Args);`
right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72304



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

[PATCH] D72233: Add a new AST matcher 'optionally'.

[Rihan Yang via Phabricator via cfe-commits]

air20 updated this revision to Diff 236514.
air20 marked an inline comment as done.

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

https://reviews.llvm.org/D72233

Files:
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/include/clang/ASTMatchers/ASTMatchersInternal.h
  clang/lib/ASTMatchers/ASTMatchersInternal.cpp
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -1866,6 +1866,27 @@
   has(fieldDecl(hasName("b")).bind("v"));
 }
 
+TEST(Optionally, SubmatchersDoNotMatch) {
+  EXPECT_TRUE(matchAndVerifyResultFalse(
+  "class A { int a; int b; };",
+  recordDecl(optionally(has(fieldDecl(hasName("c")).bind("v")),
+has(fieldDecl(hasName("d")).bind("v",
+  std::make_unique>("v")));
+}
+
+TEST(Optionally, SubmatchersMatch) {
+  EXPECT_TRUE(matchAndVerifyResultTrue(
+  "class A { int a; int c; };",
+  recordDecl(optionally(has(fieldDecl(hasName("a")).bind("v")),
+has(fieldDecl(hasName("b")).bind("v",
+  std::make_unique>("v", 1)));
+  EXPECT_TRUE(matchAndVerifyResultTrue(
+  "class A { int c; int b; };",
+  recordDecl(optionally(has(fieldDecl(hasName("c")).bind("v")),
+has(fieldDecl(hasName("b")).bind("v",
+  std::make_unique>("v", 2)));
+}
+
 TEST(IsTemplateInstantiation, MatchesImplicitClassTemplateInstantiation) {
   // Make sure that we can both match the class by name (::X) and by the type
   // the template was instantiated with (via a field).
Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -278,8 +278,8 @@
   REGISTER_MATCHER(hasIncrement);
   REGISTER_MATCHER(hasIndex);
   REGISTER_MATCHER(hasInit);
-  REGISTER_MATCHER(hasInitializer);
   REGISTER_MATCHER(hasInitStatement);
+  REGISTER_MATCHER(hasInitializer);
   REGISTER_MATCHER(hasKeywordSelector);
   REGISTER_MATCHER(hasLHS);
   REGISTER_MATCHER(hasLocalQualifiers);
@@ -455,6 +455,7 @@
   REGISTER_MATCHER(on);
   REGISTER_MATCHER(onImplicitObjectArgument);
   REGISTER_MATCHER(opaqueValueExpr);
+  REGISTER_MATCHER(optionally);
   REGISTER_MATCHER(parameterCountIs);
   REGISTER_MATCHER(parenExpr);
   REGISTER_MATCHER(parenListExpr);
Index: clang/lib/ASTMatchers/ASTMatchersInternal.cpp
===
--- clang/lib/ASTMatchers/ASTMatchersInternal.cpp
+++ clang/lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -68,6 +68,11 @@
BoundNodesTreeBuilder *Builder,
ArrayRef InnerMatchers);
 
+bool OptionallyVariadicOperator(const ast_type_traits::DynTypedNode &DynNode,
+ASTMatchFinder *Finder,
+BoundNodesTreeBuilder *Builder,
+ArrayRef InnerMatchers);
+
 void BoundNodesTreeBuilder::visitMatches(Visitor *ResultVisitor) {
   if (Bindings.empty())
 Bindings.push_back(BoundNodesMap());
@@ -179,6 +184,11 @@
 SupportedKind, RestrictKind,
 new VariadicMatcher(std::move(InnerMatchers)));
 
+  case VO_Optionally:
+return DynTypedMatcher(SupportedKind, RestrictKind,
+   new VariadicMatcher(
+   std::move(InnerMatchers)));
+
   case VO_UnaryNot:
 // FIXME: Implement the Not operator to take a single matcher instead of a
 // vector.
@@ -342,6 +352,20 @@
   return false;
 }
 
+bool OptionallyVariadicOperator(const ast_type_traits::DynTypedNode &DynNode,
+ASTMatchFinder *Finder,
+BoundNodesTreeBuilder *Builder,
+ArrayRef InnerMatchers) {
+  BoundNodesTreeBuilder Result;
+  for (const DynTypedMatcher &InnerMatcher : InnerMatchers) {
+BoundNodesTreeBuilder BuilderInner(*Builder);
+if (InnerMatcher.matches(DynNode, Finder, &BuilderInner))
+  Result.addMatch(BuilderInner);
+  }
+  *Builder = std::move(Result);
+  return true;
+}
+
 inline static
 std::vector vectorFromRefs(ArrayRef NameRefs) {
   std::vector Names;
@@ -792,6 +816,9 @@
 const internal::VariadicOperatorMatcherFunc<
 2, std::numeric_limits::max()>
 allOf = {internal::DynTypedMatcher::VO_AllOf};
+const internal::VariadicOperatorMatcherFunc<
+1, std::numeric_limits::max()>
+optionally = {internal::DynTypedMatcher::VO_Optionally};
 const internal::VariadicFunction, StringRef,
  internal::hasAnyNameFunc>

[PATCH] D72233: Add a new AST matcher 'optionally'.

[Rihan Yang via Phabricator via cfe-commits]

air20 updated this revision to Diff 236513.
air20 added a comment.

Included base commits that was missing in the previous diff.


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

https://reviews.llvm.org/D72233

Files:
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/include/clang/ASTMatchers/ASTMatchersInternal.h
  clang/lib/ASTMatchers/ASTMatchersInternal.cpp
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -1866,6 +1866,27 @@
   has(fieldDecl(hasName("b")).bind("v"));
 }
 
+TEST(Optionally, SubmatchersDoNotMatch) {
+  EXPECT_TRUE(matchAndVerifyResultFalse(
+  "class A { int a; int b; };",
+  recordDecl(optionally(has(fieldDecl(hasName("c")).bind("v")),
+has(fieldDecl(hasName("d")).bind("v",
+  std::make_unique>("v")));
+}
+
+TEST(Optionally, SubmatchersMatch) {
+  EXPECT_TRUE(matchAndVerifyResultTrue(
+  "class A { int a; int c; };",
+  recordDecl(optionally(has(fieldDecl(hasName("a")).bind("v")),
+has(fieldDecl(hasName("b")).bind("v",
+  std::make_unique>("v", 1)));
+  EXPECT_TRUE(matchAndVerifyResultTrue(
+  "class A { int c; int b; };",
+  recordDecl(optionally(has(fieldDecl(hasName("c")).bind("v")),
+has(fieldDecl(hasName("b")).bind("v",
+  std::make_unique>("v", 2)));
+}
+
 TEST(IsTemplateInstantiation, MatchesImplicitClassTemplateInstantiation) {
   // Make sure that we can both match the class by name (::X) and by the type
   // the template was instantiated with (via a field).
Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -278,8 +278,8 @@
   REGISTER_MATCHER(hasIncrement);
   REGISTER_MATCHER(hasIndex);
   REGISTER_MATCHER(hasInit);
-  REGISTER_MATCHER(hasInitializer);
   REGISTER_MATCHER(hasInitStatement);
+  REGISTER_MATCHER(hasInitializer);
   REGISTER_MATCHER(hasKeywordSelector);
   REGISTER_MATCHER(hasLHS);
   REGISTER_MATCHER(hasLocalQualifiers);
@@ -455,6 +455,7 @@
   REGISTER_MATCHER(on);
   REGISTER_MATCHER(onImplicitObjectArgument);
   REGISTER_MATCHER(opaqueValueExpr);
+  REGISTER_MATCHER(optionally);
   REGISTER_MATCHER(parameterCountIs);
   REGISTER_MATCHER(parenExpr);
   REGISTER_MATCHER(parenListExpr);
Index: clang/lib/ASTMatchers/ASTMatchersInternal.cpp
===
--- clang/lib/ASTMatchers/ASTMatchersInternal.cpp
+++ clang/lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -68,6 +68,11 @@
BoundNodesTreeBuilder *Builder,
ArrayRef InnerMatchers);
 
+bool OptionallyVariadicOperator(const ast_type_traits::DynTypedNode &DynNode,
+ASTMatchFinder *Finder,
+BoundNodesTreeBuilder *Builder,
+ArrayRef InnerMatchers);
+
 void BoundNodesTreeBuilder::visitMatches(Visitor *ResultVisitor) {
   if (Bindings.empty())
 Bindings.push_back(BoundNodesMap());
@@ -179,6 +184,11 @@
 SupportedKind, RestrictKind,
 new VariadicMatcher(std::move(InnerMatchers)));
 
+  case VO_Optionally:
+return DynTypedMatcher(SupportedKind, RestrictKind,
+   new VariadicMatcher(
+   std::move(InnerMatchers)));
+
   case VO_UnaryNot:
 // FIXME: Implement the Not operator to take a single matcher instead of a
 // vector.
@@ -342,6 +352,21 @@
   return false;
 }
 
+bool OptionallyVariadicOperator(const ast_type_traits::DynTypedNode &DynNode,
+ASTMatchFinder *Finder,
+BoundNodesTreeBuilder *Builder,
+ArrayRef InnerMatchers) {
+  BoundNodesTreeBuilder Result;
+  for (const DynTypedMatcher &InnerMatcher : InnerMatchers) {
+BoundNodesTreeBuilder BuilderInner(*Builder);
+if (InnerMatcher.matches(DynNode, Finder, &BuilderInner)) {
+  Result.addMatch(BuilderInner);
+}
+  }
+  *Builder = std::move(Result);
+  return true;
+}
+
 inline static
 std::vector vectorFromRefs(ArrayRef NameRefs) {
   std::vector Names;
@@ -792,6 +817,9 @@
 const internal::VariadicOperatorMatcherFunc<
 2, std::numeric_limits::max()>
 allOf = {internal::DynTypedMatcher::VO_AllOf};
+const internal::VariadicOperatorMatcherFunc<
+1, std::numeric_limits::max()>
+optionally = {internal::DynTypedMatcher::VO_Optionally};
 const internal::VariadicFunction, StringRef,

[PATCH] D71365: expand printf when compiling HIP to AMDGPU

[Sameer Sahasrabuddhe via Phabricator via cfe-commits]

sameerds added inline comments.



Comment at: clang/test/CodeGenHIP/printf.cpp:18
+}
+
+// CHECK: [[BEGIN:%.*]]   = call i64 @__ockl_printf_begin(i64 0)

arsenm wrote:
> This could use a lot more testcases. Can you add some half, float, and double 
> as well as pointers (including different address spaces) and vectors?
I am not sure what exactly should be tested. The validity of this expansion 
depends on the signature of the builtin printf function. Since printf is 
variadic, the "default argument promotions" in the C/C++ spec guarantee that 
the arguments are 32/64 bit integers or doubles if they are not pointers. The 
printf signature as well as the device library functions are defined using only 
generic pointers, so the address space does not matter. Non-scalar arguments 
are not supported, which is checked by another test using a struct. I could add 
a vector there, but that doesn't seem to be adding any value.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71365



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

[PATCH] D72233: Add a new AST matcher 'optionally'.

[Rihan Yang via Phabricator via cfe-commits]

air20 updated this revision to Diff 236510.
air20 added a comment.

Fixed Registry.cpp and documentation as per comments.


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

https://reviews.llvm.org/D72233

Files:
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/ASTMatchers/Dynamic/Registry.cpp

Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -278,8 +278,8 @@
   REGISTER_MATCHER(hasIncrement);
   REGISTER_MATCHER(hasIndex);
   REGISTER_MATCHER(hasInit);
-  REGISTER_MATCHER(hasInitializer);
   REGISTER_MATCHER(hasInitStatement);
+  REGISTER_MATCHER(hasInitializer);
   REGISTER_MATCHER(hasKeywordSelector);
   REGISTER_MATCHER(hasLHS);
   REGISTER_MATCHER(hasLocalQualifiers);
@@ -455,6 +455,7 @@
   REGISTER_MATCHER(on);
   REGISTER_MATCHER(onImplicitObjectArgument);
   REGISTER_MATCHER(opaqueValueExpr);
+  REGISTER_MATCHER(optionally);
   REGISTER_MATCHER(parameterCountIs);
   REGISTER_MATCHER(parenExpr);
   REGISTER_MATCHER(parenListExpr);
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -2532,7 +2532,27 @@
 /// Matches any node regardless of the submatchers.
 ///
 /// However, \c optionally will generate a result binding for each matching
-/// submatchers.
+/// submatcher.
+/// 
+/// Useful when additional information which may or may not present about a
+/// main matching node is desired.
+///
+/// For example, in:
+/// \code
+///   class Foo {
+/// int bar;
+///   }
+/// \endcode
+/// The matcher:
+/// \code
+///   cxxRecordDecl(
+/// optionally(has(
+///   fieldDecl(hasName("bar")).bind("var")
+///   ))).bind("record")
+/// \endcode
+/// will produce a result binding for both "record" and "var".
+/// The matcher will produce a "record" binding for even if there is no data
+/// member named "bar" in that class.
 ///
 /// Usable as: Any Matcher
 extern const internal::VariadicOperatorMatcherFunc<
Index: clang/docs/LibASTMatchersReference.html
===
--- clang/docs/LibASTMatchersReference.html
+++ clang/docs/LibASTMatchersReference.html
@@ -4689,6 +4689,32 @@
 
 
 
+Matcher<*>optionallyMatcher<*>, ..., Matcher<*>
+Matches any node regardless of the submatchers.
+
+However, optionally will generate a result binding for each matching
+submatcher.
+
+Useful when additional information which may or may not present about a
+main matching node is desired.
+
+For example, in:
+  class Foo {
+int bar;
+  }
+The matcher:
+  cxxRecordDecl(
+optionally(has(
+  fieldDecl(hasName("bar")).bind("var")
+  ))).bind("record")
+will produce a result binding for both "record" and "var".
+The matcher will produce a "record" binding for even if there is no data
+member named "bar" in that class.
+
+Usable as: Any Matcher
+
+
+
 MatcherAbstractConditionalOperator>hasConditionMatcherExpr> InnerMatcher
 Matches the condition expression of an if statement, for loop,
 switch statement or conditional operator.
@@ -5098,15 +5124,15 @@
 Matches selection statements with initializer.
 
 Given:
- void foo() { 
+ void foo() {
if (int i = foobar(); i > 0) {}
switch (int i = foobar(); i) {}
-   for (auto& a = get_range(); auto& x : a) {} 
+   for (auto& a = get_range(); auto& x : a) {}
  }
- void bar() { 
+ void bar() {
if (foobar() > 0) {}
switch (foobar()) {}
-   for (auto& x : get_range()) {} 
+   for (auto& x : get_range()) {}
  }
 ifStmt(hasInitStatement(anything()))
   matches the if statement in foo but not in bar.
@@ -6245,15 +6271,15 @@
 Matches selection statements with initializer.
 
 Given:
- void foo() { 
+ void foo() {
if (int i = foobar(); i > 0) {}
switch (int i = foobar(); i) {}
-   for (auto& a = get_range(); auto& x : a) {} 
+   for (auto& a = get_range(); auto& x : a) {}
  }
- void bar() { 
+ void bar() {
if (foobar() > 0) {}
switch (foobar()) {}
-   for (auto& x : get_range()) {} 
+   for (auto& x : get_range()) {}
  }
 ifStmt(hasInitStatement(anything()))
   matches the if statement in foo but not in bar.
@@ -7005,15 +7031,15 @@
 Matches selection statements with initializer.
 
 Given:
- void foo() { 
+ void foo() {
if (int i = foobar(); i > 0) {}
switch (int i = foobar(); i) {}
-   for (auto& a = get_range(); auto& x : a) {} 
+   for (auto& a = get_range(); auto& x : a) {}
  }
- void bar() { 
+ void bar() {
if (foobar() > 0) {}
switch (foobar()) {}
-   for (auto& x : get_range()) {} 
+   for (auto& x : get_range()) {}
  }
 ifStmt(hasInitStatement(an

[PATCH] D68101: [MC][ELF] Prevent globals with an explicit section from being mergeable

[ben via Phabricator via cfe-commits]

bd1976llvm added a comment.

Thanks for the feedback. I'm sorry that I am being slow on this... v.busy at 
the moment. Will try to finish the patch in the next few days.


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

https://reviews.llvm.org/D68101



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

[PATCH] D72217: [clang-tidy] Added readability-qualified-auto check

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 236486.
njames93 added a comment.

I have tried to make the code more robust against macro decls. Also target the 
actual type specifier rather than everything before the name when doing the 
replacements. This should leave any other qualifiers or attributes in tact. The 
test cases have been updated to add checks for using and typedefs alias. There 
are also tests for function pointers and lambdas,  with the current behaviour 
to add pointer qual for function pointer but not for lambdas(as they aren't 
function pointers under the hood). Not sure I want to implement support for 
pointers to pointers as that's not going to help readability(Are you a pointer 
to an array of arrays, array of pointers, pointer to array or pointer to 
pointer). As for the alias whats the correct procedure for creating an alias


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

https://reviews.llvm.org/D72217

Files:
  clang-tools-extra/clang-tidy/readability/CMakeLists.txt
  clang-tools-extra/clang-tidy/readability/QualifiedAutoCheck.cpp
  clang-tools-extra/clang-tidy/readability/QualifiedAutoCheck.h
  clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/readability-qualified-auto.rst
  clang-tools-extra/test/clang-tidy/checkers/readability-qualified-auto.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-qualified-auto.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability-qualified-auto.cpp
@@ -0,0 +1,186 @@
+// RUN: %check_clang_tidy %s readability-qualified-auto %t -- -- -std=c++17
+
+namespace typedefs {
+typedef int *MyPtr;
+typedef int &MyRef;
+typedef const int *CMyPtr;
+typedef const int &CMyRef;
+
+MyPtr getPtr();
+MyRef getRef();
+CMyPtr getCPtr();
+CMyRef getCRef();
+
+void foo() {
+  auto TdNakedPtr = getPtr();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'auto TdNakedPtr' can be declared as 'auto *TdNakedPtr'
+  // CHECK-FIXES: {{^}}  auto *TdNakedPtr = getPtr();
+  auto &TdNakedRef = getRef();
+  auto TdNakedRefDeref = getRef();
+  auto TdNakedCPtr = getCPtr();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'auto TdNakedCPtr' can be declared as 'const auto *TdNakedCPtr'
+  // CHECK-FIXES: {{^}}  const auto *TdNakedCPtr = getCPtr();
+  auto &TdNakedCRef = getCRef();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'auto &TdNakedCRef' can be declared as 'const auto &TdNakedCRef'
+  // CHECK-FIXES: {{^}}  const auto &TdNakedCRef = getCRef();
+  auto TdNakedCRefDeref = getCRef();
+}
+
+}; // namespace typedefs
+
+namespace usings {
+using MyPtr = int *;
+using MyRef = int &;
+using CMyPtr = const int *;
+using CMyRef = const int &;
+
+MyPtr getPtr();
+MyRef getRef();
+CMyPtr getCPtr();
+CMyRef getCRef();
+
+void foo() {
+  auto UNakedPtr = getPtr();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'auto UNakedPtr' can be declared as 'auto *UNakedPtr'
+  // CHECK-FIXES: {{^}}  auto *UNakedPtr = getPtr();
+  auto &UNakedRef = getRef();
+  auto UNakedRefDeref = getRef();
+  auto UNakedCPtr = getCPtr();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'auto UNakedCPtr' can be declared as 'const auto *UNakedCPtr'
+  // CHECK-FIXES: {{^}}  const auto *UNakedCPtr = getCPtr();
+  auto &UNakedCRef = getCRef();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'auto &UNakedCRef' can be declared as 'const auto &UNakedCRef'
+  // CHECK-FIXES: {{^}}  const auto &UNakedCRef = getCRef();
+  auto UNakedCRefDeref = getCRef();
+}
+
+}; // namespace usings
+
+int getInt();
+int *getIntPtr();
+const int *getCIntPtr();
+
+void foo() {
+  // make sure check disregards named types
+  int TypedInt = getInt();
+  int *TypedPtr = getIntPtr();
+  const int *TypedConstPtr = getCIntPtr();
+  int &TypedRef = *getIntPtr();
+  const int &TypedConstRef = *getCIntPtr();
+
+  // make sure check disregards auto types that aren't pointers or references
+  auto AutoInt = getInt();
+
+  auto NakedPtr = getIntPtr();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'auto NakedPtr' can be declared as 'auto *NakedPtr'
+  // CHECK-FIXES: {{^}}  auto *NakedPtr = getIntPtr();
+  auto NakedCPtr = getCIntPtr();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'auto NakedCPtr' can be declared as 'const auto *NakedCPtr'
+  // CHECK-FIXES: {{^}}  const auto *NakedCPtr = getCIntPtr();
+
+  const auto ConstPtr = getIntPtr();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'const auto ConstPtr' can be declared as 'auto *const ConstPtr'
+  // CHECK-FIXES: {{^}}  auto *const ConstPtr = getIntPtr();
+  const auto ConstCPtr = getCIntPtr();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'const auto ConstCPtr' can be declared as 'const auto *const ConstCPtr'
+  // CHECK-FIXES: {{^}}  const auto *const ConstCPtr = getCIntPtr();
+
+  auto *QualPtr = getIntPtr();
+  auto *QualCPtr = 

[PATCH] D72121: [clang-tidy] Fix readability-identifier-naming missing member variables

[Nathan James via Phabricator via cfe-commits]

njames93 marked 2 inline comments as done.
njames93 added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-member-decl-usage.cpp:137
+}
+}; // namespace CtorInits

aaron.ballman wrote:
> I think this is one more test case to add: base class specifiers. e.g.,
> ```
> struct foo_bar {}; // Should be renamed
> 
> struct Baz : foo_bar {}; // Should rename foo_bar
> ```
That test case already exists in the readability-identifier-naming.cpp file

```
class my_derived_class : public virtual my_class {};
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for class 
'my_derived_class'
// CHECK-FIXES: {{^}}class CMyDerivedClass : public virtual CMyClass {};{{$}}
```


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

https://reviews.llvm.org/D72121



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

[PATCH] D72212: [Sema] Improve -Wrange-loop-analysis warnings

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

Thanks for the patch. Will take some time to review. Without this, adding 
-Wrange-loop-analysis to -Wall will make lots of projects fail to build if they 
have -Werror. For example, `for (const absl::string_review : ...)` or `const 
std::string_view` is common.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72212



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

[clang] 907cefe - Always deduce the lengths of contained parameter packs when deducing a

[Richard Smith via cfe-commits]

Author: Richard Smith
Date: 2020-01-06T17:24:29-08:00
New Revision: 907cefe721437fa8950c1b6c1c028038b175f921

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

LOG: Always deduce the lengths of contained parameter packs when deducing a
pack expansion.

Previously, if all parameter / argument pairs for a pack expansion
deduction were non-deduced contexts, we would not deduce the arity of
the pack, and could end up deducing a different arity (leading to
failures during substitution) or defaulting to an arity of 0 (leading to
bad diagnostics about passing the wrong number of arguments to a
variadic function). Instead, we now always deduce the arity for all
involved packs any time we deduce a pack expansion.

This will result in less substitution happening in some cases, which
could avoid non-SFINAEable errors, and should generally improve the
quality of diagnostics when passing initializer lists to variadic
functions.

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaOverload.cpp
clang/lib/Sema/SemaTemplateDeduction.cpp
clang/test/CXX/drs/dr13xx.cpp
clang/test/CXX/temp/temp.fct.spec/temp.deduct/temp.deduct.type/p5-0x.cpp
clang/test/SemaTemplate/alias-templates.cpp
clang/test/SemaTemplate/deduction.cpp
clang/test/SemaTemplate/pack-deduction.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 99ce42dd7533..a8f49fef9f1f 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3875,12 +3875,14 @@ def note_ovl_candidate_bad_deduction : Note<
 "candidate template ignored: failed template argument deduction">;
 def note_ovl_candidate_incomplete_deduction : Note<"candidate template 
ignored: "
 "couldn't infer template argument %0">;
-def note_ovl_candidate_incomplete_deduction_pack : Note<"candidate template 
ignored: "
+def note_ovl_candidate_incomplete_deduction_pack : Note<
+"candidate template ignored: "
 "deduced too few arguments for expanded pack %0; no argument for %ordinal1 
"
 "expanded parameter in deduced argument pack %2">;
 def note_ovl_candidate_inconsistent_deduction : Note<
-"candidate template ignored: deduced conflicting %select{types|values|"
-"templates}0 for parameter %1%
diff { ($ vs. $)|}2,3">;
+"candidate template ignored: deduced %select{conflicting types|"
+"conflicting values|conflicting templates|packs of 
diff erent lengths}0 "
+"for parameter %1%
diff { ($ vs. $)|}2,3">;
 def note_ovl_candidate_inconsistent_deduction_types : Note<
 "candidate template ignored: deduced values %
diff {"
 "of conflicting types for parameter %0 (%1 of type $ vs. %3 of type $)|"

diff  --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index 74a0bc7c78ff..83b7f497f99d 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -10348,6 +10348,16 @@ static void DiagnoseBadDeduction(Sema &S, NamedDecl 
*Found, Decl *Templated,
   which = 2;
 }
 
+// Tweak the diagnostic if the problem is that we deduced packs of
+// 
diff erent arities. We'll print the actual packs anyway in case that
+// includes additional useful information.
+if (DeductionFailure.getFirstArg()->getKind() == TemplateArgument::Pack &&
+DeductionFailure.getSecondArg()->getKind() == TemplateArgument::Pack &&
+DeductionFailure.getFirstArg()->pack_size() !=
+DeductionFailure.getSecondArg()->pack_size()) {
+  which = 3;
+}
+
 S.Diag(Templated->getLocation(),
diag::note_ovl_candidate_inconsistent_deduction)
 << which << ParamD->getDeclName() << *DeductionFailure.getFirstArg()
@@ -10385,6 +10395,8 @@ static void DiagnoseBadDeduction(Sema &S, NamedDecl 
*Found, Decl *Templated,
 TemplateArgString = " ";
 TemplateArgString += S.getTemplateArgumentBindingsText(
 getDescribedTemplate(Templated)->getTemplateParameters(), *Args);
+if (TemplateArgString.size() == 1)
+  TemplateArgString.clear();
 S.Diag(Templated->getLocation(),
diag::note_ovl_candidate_unsatisfied_constraints)
 << TemplateArgString;
@@ -10412,6 +10424,8 @@ static void DiagnoseBadDeduction(Sema &S, NamedDecl 
*Found, Decl *Templated,
   TemplateArgString = " ";
   TemplateArgString += S.getTemplateArgumentBindingsText(
   getDescribedTemplate(Templated)->getTemplateParameters(), *Args);
+  if (TemplateArgString.size() == 1)
+TemplateArgString.clear();
 }
 
 // If this candidate was disabled by enable_if, say so.
@@ -10461,6 +10475,8 @@ static void DiagnoseBadDeduction(Sema &S, NamedDec

[PATCH] D72284: [clang-tidy] Factor out renaming logic from readability-identifier-naming

[Logan Smith via Phabricator via cfe-commits]

logan-5 updated this revision to Diff 236483.
logan-5 marked 3 inline comments as done.
logan-5 added a comment.

Consistent-ified single-statement-body control flow statements to not use 
braces.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72284

Files:
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
  clang-tools-extra/clang-tidy/utils/CMakeLists.txt
  clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
  clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h

Index: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h
@@ -0,0 +1,150 @@
+//===--- RenamderClangTidyCheck.h - clang-tidy --*- 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
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_RENAMERCLANGTIDYCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_RENAMERCLANGTIDYCHECK_H
+
+#include "../ClangTidyCheck.h"
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/Optional.h"
+#include 
+#include 
+
+namespace clang {
+
+class MacroInfo;
+
+namespace tidy {
+
+/// Base class for clang-tidy checks that want to flag declarations and/or
+/// macros for renaming based on customizable criteria.
+class RenamerClangTidyCheck : public ClangTidyCheck {
+public:
+  RenamerClangTidyCheck(StringRef CheckName, ClangTidyContext *Context);
+  ~RenamerClangTidyCheck();
+
+  /// Derived classes should not implement any matching logic themselves; this
+  /// class will do the matching and call the derived class'
+  /// GetDeclFailureInfo() and GetMacroFailureInfo() for determining whether a
+  /// given identifier passes or fails the check.
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override final;
+  void
+  check(const ast_matchers::MatchFinder::MatchResult &Result) override final;
+  void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
+   Preprocessor *ModuleExpanderPP) override final;
+  void onEndOfTranslationUnit() override final;
+
+  /// This enum will be used in %select of the diagnostic message.
+  /// Each value below IgnoreFailureThreshold should have an error message.
+  enum class ShouldFixStatus {
+ShouldFix,
+
+/// The fixup will conflict with a language keyword,
+/// so we can't fix it automatically.
+ConflictsWithKeyword,
+
+/// The fixup will conflict with a macro
+/// definition, so we can't fix it
+/// automatically.
+ConflictsWithMacroDefinition,
+
+/// Values pass this threshold will be ignored completely
+/// i.e no message, no fixup.
+IgnoreFailureThreshold,
+
+/// If the identifier was used or declared within a macro we
+/// won't offer a fixup for safety reasons.
+InsideMacro,
+  };
+
+  /// Information describing a failed check
+  struct FailureInfo {
+std::string KindName; // Tag or misc info to be used as derived classes need
+std::string Fixup;// The name that will be proposed as a fix-it hint
+  };
+
+  /// Holds an identifier name check failure, tracking the kind of the
+  /// identifier, its possible fixup and the starting locations of all the
+  /// identifier usages.
+  struct NamingCheckFailure {
+FailureInfo Info;
+
+/// Whether the failure should be fixed or not.
+///
+/// e.g.: if the identifier was used or declared within a macro we won't
+/// offer a fixup for safety reasons.
+bool ShouldFix() const {
+  return FixStatus == ShouldFixStatus::ShouldFix && !Info.Fixup.empty();
+}
+
+bool ShouldNotify() const {
+  return FixStatus < ShouldFixStatus::IgnoreFailureThreshold;
+}
+
+ShouldFixStatus FixStatus = ShouldFixStatus::ShouldFix;
+
+/// A set of all the identifier usages starting SourceLocation, in
+/// their encoded form.
+llvm::DenseSet RawUsageLocs;
+
+NamingCheckFailure() = default;
+  };
+
+  using NamingCheckId = std::pair;
+
+  using NamingCheckFailureMap =
+  llvm::DenseMap;
+
+  /// Check Macros for style violations.
+  void checkMacro(SourceManager &sourceMgr, const Token &MacroNameTok,
+  const MacroInfo *MI);
+
+  /// Add a usage of a macro if it already has a violation.
+  void expandMacro(const Token &MacroNameTok, const MacroInfo *MI);
+
+protected:
+  /// Overridden by derived classes, returns information about if and how a Decl
+  /// failed the check. A 'None' result means the Decl did not fail the check.
+  virtual llvm::Optional
+ 

[PATCH] D69876: Allow output constraints on "asm goto"

[Bill Wendling via Phabricator via cfe-commits]

void updated this revision to Diff 236482.
void marked 2 inline comments as done.
void added a comment.

Reword the extension explanation to be more readable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69876

Files:
  llvm/docs/LangRef.rst
  llvm/include/llvm/CodeGen/MachineBasicBlock.h
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/CodeGen/MachineBasicBlock.cpp
  llvm/lib/CodeGen/MachineVerifier.cpp
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/test/CodeGen/X86/callbr-asm-outputs.ll
  llvm/test/CodeGen/X86/callbr-asm.ll

Index: llvm/test/CodeGen/X86/callbr-asm.ll
===
--- llvm/test/CodeGen/X86/callbr-asm.ll
+++ llvm/test/CodeGen/X86/callbr-asm.ll
@@ -1,5 +1,5 @@
 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
-; RUN: llc < %s -mtriple=i686-- -O3 | FileCheck %s
+; RUN: llc < %s -mtriple=i686-- -O3 -verify-machineinstrs | FileCheck %s
 
 ; Tests for using callbr as an asm-goto wrapper
 
@@ -159,3 +159,54 @@
 cleanup:  ; preds = %asm.fallthrough, %quux
   ret void
 }
+
+define i32 @test5(i32 %out1, i32 %out2) {
+; Test 5 - asm-goto with output constraints.
+; CHECK-LABEL: test5:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:  movl $-1, %eax
+; CHECK-NEXT:  movl 4(%esp), %ecx
+; CHECK-NEXT:  #APP
+; CHECK-NEXT:  testl %ecx, %ecx
+; CHECK-NEXT:  testl %edx, %ecx
+; CHECK-NEXT:  jne .Ltmp6
+; CHECK-NEXT:  #NO_APP
+; CHECK-NEXT:  .LBB4_1:
+; CHECK-NEXT:  #APP
+; CHECK-NEXT:  testl %ecx, %edx
+; CHECK-NEXT:  testl %ecx, %edx
+; CHECK-NEXT:  jne .Ltmp7
+; CHECK-NEXT:  #NO_APP
+; CHECK-NEXT:  .LBB4_2:
+; CHECK-NEXT:  addl %edx, %ecx
+; CHECK-NEXT:  movl %ecx, %eax
+; CHECK-NEXT:  retl
+; CHECK-NEXT:  .Ltmp6:
+; CHECK-NEXT:  .LBB4_3:
+; CHECK-NEXT:  movl $-2, %eax
+; CHECK-NEXT:  .Ltmp7:
+; CHECK-NEXT:  .LBB4_4:
+; CHECK-NEXT:  retl
+entry:
+  %0 = callbr { i32, i32 } asm sideeffect "testl $0, $0; testl $1, $2; jne ${3:l}", "=r,=r,r,X,X,~{dirflag},~{fpsr},~{flags}"(i32 %out1, i8* blockaddress(@test5, %label_true), i8* blockaddress(@test5, %return))
+  to label %asm.fallthrough [label %label_true, label %return]
+
+asm.fallthrough:  ; preds = %entry
+  %asmresult = extractvalue { i32, i32 } %0, 0
+  %asmresult1 = extractvalue { i32, i32 } %0, 1
+  %1 = callbr { i32, i32 } asm sideeffect "testl $0, $1; testl $2, $3; jne ${5:l}", "=r,=r,r,r,X,X,~{dirflag},~{fpsr},~{flags}"(i32 %asmresult, i32 %asmresult1, i8* blockaddress(@test5, %label_true), i8* blockaddress(@test5, %return))
+  to label %asm.fallthrough2 [label %label_true, label %return]
+
+asm.fallthrough2: ; preds = %asm.fallthrough
+  %asmresult3 = extractvalue { i32, i32 } %1, 0
+  %asmresult4 = extractvalue { i32, i32 } %1, 1
+  %add = add nsw i32 %asmresult3, %asmresult4
+  br label %return
+
+label_true:   ; preds = %asm.fallthrough, %entry
+  br label %return
+
+return:   ; preds = %entry, %asm.fallthrough, %label_true, %asm.fallthrough2
+  %retval.0 = phi i32 [ %add, %asm.fallthrough2 ], [ -2, %label_true ], [ -1, %asm.fallthrough ], [ -1, %entry ]
+  ret i32 %retval.0
+}
Index: llvm/test/CodeGen/X86/callbr-asm-outputs.ll
===
--- llvm/test/CodeGen/X86/callbr-asm-outputs.ll
+++ llvm/test/CodeGen/X86/callbr-asm-outputs.ll
@@ -1,18 +1,118 @@
-; RUN: not llc -mtriple=i686-- < %s 2> %t
-; RUN: FileCheck %s < %t
+; RUN: llc -mtriple=i686-- -verify-machineinstrs < %s | FileCheck %s
 
-; CHECK: error: asm-goto outputs not supported
+; A test for asm-goto output
 
-; A test for asm-goto output prohibition
-
-define i32 @test(i32 %a) {
+; CHECK-LABEL: test1:
+; CHECK:   movl 4(%esp), %eax
+; CHECK-NEXT:  addl $4, %eax
+; CHECK-NEXT:  #APP
+; CHECK-NEXT:  xorl %eax, %eax
+; CHECK-NEXT:  jmp .Ltmp0
+; CHECK-NEXT:  #NO_APP
+; CHECK-NEXT:  .LBB0_1:
+; CHECK-NEXT:  retl
+; CHECK-NEXT:  .Ltmp0:
+define i32 @test1(i32 %x) {
 entry:
-  %0 = add i32 %a, 4
-  %1 = callbr i32 asm "xorl $1, $1; jmp ${1:l}", "=&r,r,X,~{dirflag},~{fpsr},~{flags}"(i32 %0, i8* blockaddress(@test, %fail)) to label %normal [label %fail]
+  %add = add nsw i32 %x, 4
+  %ret = callbr i32 asm "xorl $1, $0; jmp ${2:l}", "=r,r,X,~{dirflag},~{fpsr},~{flags}"(i32 %add, i8* blockaddress(@test1, %abnormal))
+  to label %normal [label %abnormal]
 
 normal:
-  ret i32 %1
+  ret i32 %ret
 
-fail:
+abnormal:
   ret i32 1
 }
+
+; CHECK-LABEL: test2:
+; CHECK:   # %bb.1: # %if.then
+; CHECK-NEXT:  #APP
+; CHECK-NEXT:  testl %esi, %esi
+; CHECK-NEXT:  testl %edi, %esi
+; CHECK-NEXT:  jne .Ltmp1
+; CHECK-NEXT:  #NO_APP
+; CHECK-NEXT:  .L

[PATCH] D69876: Allow output constraints on "asm goto"

[Bill Wendling via Phabricator via cfe-commits]

void added inline comments.



Comment at: clang/lib/AST/Stmt.cpp:646-648
+  // Labels are placed after "InOut" operands. Adjust accordingly.
+  if (IsLabel)
+N += getNumPlusOperands();

jyknight wrote:
> I'm confused about this part. Why isn't the "N" specified in the assembly 
> string already the correct value for the labels? Is the ordering we use 
> internally and that users use externally not the same? I'm assuming your code 
> here is correct, just I'm not understanding, so probably an improved comment 
> would be nice.
The LLVM-specific ordering that I saw was something like:

  ``

The `` is empty when there are no output constraints. This 
just makes up for this. It's probably better to reverse the `` and 
`` parts, but I didn't know how pervasive the order's 
assumption is in the code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69876



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

[PATCH] D72306: [PowerPC] FreeBSD >= 13 default ABI is ELFv2

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

Are you still using -target powerpc64-unknown-freebsd12.0-elfv2 or -target 
powerpc64-unknown-freebsd12.0-elfv1 added in https://reviews.llvm.org/rL361355 ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72306



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

[PATCH] D72306: [PowerPC] FreeBSD >= 13 default ABI is ELFv2

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/lib/Basic/Targets/PPC.h:389
+if (Triple.getEnvironment() != llvm::Triple::UnknownEnvironment) {
+  switch (Triple.getEnvironment()){
+case llvm::Triple::ELFv1:

Which ABI does `powerpc64le-linux-gnu` use after this change?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72306



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

[PATCH] D72284: [clang-tidy] Factor out renaming logic from readability-identifier-naming

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp:272
+  if (const auto *Typedef =
+  Value->getReturnType().getTypePtr()->getAs()) {
+addUsage(NamingCheckFailures, Typedef->getDecl(),

logan-5 wrote:
> Eugene.Zelenko wrote:
> > Please elide braces.
> Would you mind pointing me toward a resource for these formatting nits? I 
> don't see anything about requiring omitting braces for single-statement if()s 
> in the official LLVM coding standards (and I happen to think the braces 
> actually help readability here). If this stuff is explicitly documented 
> somewhere and painstakingly enforced like this, I'd rather get it right the 
> first time.
I looked through the coding standard and it does not suggest this, which I am 
really surprised to see (I feel like the standard used to say it at some point, 
but it likely got removed and I did not notice). This is a common pattern 
that's used all over the code base (likely from fairly consistent review 
guidance over the years), and so it's best to be consistent with the style used 
locally, but that's a pretty weak argument for new code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72284



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

[PATCH] D72284: [clang-tidy] Factor out renaming logic from readability-identifier-naming

[Logan Smith via Phabricator via cfe-commits]

logan-5 updated this revision to Diff 236478.
logan-5 marked 3 inline comments as done.
logan-5 added a comment.

More nits.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72284

Files:
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
  clang-tools-extra/clang-tidy/utils/CMakeLists.txt
  clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
  clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h

Index: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h
@@ -0,0 +1,150 @@
+//===--- RenamderClangTidyCheck.h - clang-tidy --*- 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
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_RENAMERCLANGTIDYCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_RENAMERCLANGTIDYCHECK_H
+
+#include "../ClangTidyCheck.h"
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/Optional.h"
+#include 
+#include 
+
+namespace clang {
+
+class MacroInfo;
+
+namespace tidy {
+
+/// Base class for clang-tidy checks that want to flag declarations and/or
+/// macros for renaming based on customizable criteria.
+class RenamerClangTidyCheck : public ClangTidyCheck {
+public:
+  RenamerClangTidyCheck(StringRef CheckName, ClangTidyContext *Context);
+  ~RenamerClangTidyCheck();
+
+  /// Derived classes should not implement any matching logic themselves; this
+  /// class will do the matching and call the derived class'
+  /// GetDeclFailureInfo() and GetMacroFailureInfo() for determining whether a
+  /// given identifier passes or fails the check.
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override final;
+  void
+  check(const ast_matchers::MatchFinder::MatchResult &Result) override final;
+  void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
+   Preprocessor *ModuleExpanderPP) override final;
+  void onEndOfTranslationUnit() override final;
+
+  /// This enum will be used in %select of the diagnostic message.
+  /// Each value below IgnoreFailureThreshold should have an error message.
+  enum class ShouldFixStatus {
+ShouldFix,
+
+/// The fixup will conflict with a language keyword,
+/// so we can't fix it automatically.
+ConflictsWithKeyword,
+
+/// The fixup will conflict with a macro
+/// definition, so we can't fix it
+/// automatically.
+ConflictsWithMacroDefinition,
+
+/// Values pass this threshold will be ignored completely
+/// i.e no message, no fixup.
+IgnoreFailureThreshold,
+
+/// If the identifier was used or declared within a macro we
+/// won't offer a fixup for safety reasons.
+InsideMacro,
+  };
+
+  /// Information describing a failed check
+  struct FailureInfo {
+std::string KindName; // Tag or misc info to be used as derived classes need
+std::string Fixup;// The name that will be proposed as a fix-it hint
+  };
+
+  /// Holds an identifier name check failure, tracking the kind of the
+  /// identifier, its possible fixup and the starting locations of all the
+  /// identifier usages.
+  struct NamingCheckFailure {
+FailureInfo Info;
+
+/// Whether the failure should be fixed or not.
+///
+/// e.g.: if the identifier was used or declared within a macro we won't
+/// offer a fixup for safety reasons.
+bool ShouldFix() const {
+  return FixStatus == ShouldFixStatus::ShouldFix && !Info.Fixup.empty();
+}
+
+bool ShouldNotify() const {
+  return FixStatus < ShouldFixStatus::IgnoreFailureThreshold;
+}
+
+ShouldFixStatus FixStatus = ShouldFixStatus::ShouldFix;
+
+/// A set of all the identifier usages starting SourceLocation, in
+/// their encoded form.
+llvm::DenseSet RawUsageLocs;
+
+NamingCheckFailure() = default;
+  };
+
+  using NamingCheckId = std::pair;
+
+  using NamingCheckFailureMap =
+  llvm::DenseMap;
+
+  /// Check Macros for style violations.
+  void checkMacro(SourceManager &sourceMgr, const Token &MacroNameTok,
+  const MacroInfo *MI);
+
+  /// Add a usage of a macro if it already has a violation.
+  void expandMacro(const Token &MacroNameTok, const MacroInfo *MI);
+
+protected:
+  /// Overridden by derived classes, returns information about if and how a Decl
+  /// failed the check. A 'None' result means the Decl did not fail the check.
+  virtual llvm::Optional
+  GetDeclFailureInfo(const NamedDecl *Decl, const SourceManager &SM) cons

[PATCH] D72284: [clang-tidy] Factor out renaming logic from readability-identifier-naming

[Logan Smith via Phabricator via cfe-commits]

logan-5 added inline comments.



Comment at: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp:272
+  if (const auto *Typedef =
+  Value->getReturnType().getTypePtr()->getAs()) {
+addUsage(NamingCheckFailures, Typedef->getDecl(),

Eugene.Zelenko wrote:
> Please elide braces.
Would you mind pointing me toward a resource for these formatting nits? I don't 
see anything about requiring omitting braces for single-statement if()s in the 
official LLVM coding standards (and I happen to think the braces actually help 
readability here). If this stuff is explicitly documented somewhere and 
painstakingly enforced like this, I'd rather get it right the first time.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72284



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

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

[Florin Iucha via Phabricator via cfe-commits]

0x8000- added a comment.

In D54943#1805439 , @JonasToth wrote:

> > This last version too builds with RelWithDebInfo and fails with Debug 
> > configuration (both clean builds).
>
> So it only fails in debug-build?


Correct; I can build 'Release' or 'RelWithDebInfo' but not 'Debug'.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D54943



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

[clang] 20f005d - [CodeGen][ObjC] Push the properties of a protocol before pushing the

[Akira Hatanaka via cfe-commits]

Author: Akira Hatanaka
Date: 2020-01-06T16:16:02-08:00
New Revision: 20f005d25f488fa1dc69d6792700e014c6a5d165

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

LOG: [CodeGen][ObjC] Push the properties of a protocol before pushing the
properties of the protocol it inherits

This fixes a bug where the type string for a @dynamic property of an
@implementation didn't have 'D' in it when the protocol it conforms to
redeclares the property declared in the base protocol.

rdar://problem/45503561

Added: 


Modified: 
clang/lib/CodeGen/CGObjCMac.cpp
clang/test/CodeGenObjC/encode-test-2.m

Removed: 




diff  --git a/clang/lib/CodeGen/CGObjCMac.cpp b/clang/lib/CodeGen/CGObjCMac.cpp
index e18105fe0193..f36c28a85a68 100644
--- a/clang/lib/CodeGen/CGObjCMac.cpp
+++ b/clang/lib/CodeGen/CGObjCMac.cpp
@@ -3245,9 +3245,6 @@ PushProtocolProperties(llvm::SmallPtrSet &PropertySet,
SmallVectorImpl &Properties,
const ObjCProtocolDecl *Proto,
bool IsClassProperty) {
-  for (const auto *P : Proto->protocols())
-PushProtocolProperties(PropertySet, Properties, P, IsClassProperty);
-
   for (const auto *PD : Proto->properties()) {
 if (IsClassProperty != PD->isClassProperty())
   continue;
@@ -3255,6 +3252,9 @@ PushProtocolProperties(llvm::SmallPtrSet &PropertySet,
   continue;
 Properties.push_back(PD);
   }
+
+  for (const auto *P : Proto->protocols())
+PushProtocolProperties(PropertySet, Properties, P, IsClassProperty);
 }
 
 /*

diff  --git a/clang/test/CodeGenObjC/encode-test-2.m 
b/clang/test/CodeGenObjC/encode-test-2.m
index 2985fbda1862..2d9593d65b0c 100644
--- a/clang/test/CodeGenObjC/encode-test-2.m
+++ b/clang/test/CodeGenObjC/encode-test-2.m
@@ -6,6 +6,9 @@
 // CHECK: private unnamed_addr constant [16 x i8] c"@\22Foo\22\00",
 // CHECK: private unnamed_addr constant [13 x i8] c"{Intf=#}\00",
 
+// CHECK: @[[PROP_NAME_ATTR:.*]] = private unnamed_addr constant [5 x i8] 
c"T@,D\00",
+// CHECK: @"_OBJC_$_PROP_LIST_C0" = internal global { i32, i32, [1 x %{{.*}}] 
} { i32 8, i32 1, [1 x %{{.*}}] [%{{.*}} { {{.*}}, i8* getelementptr inbounds 
([5 x i8], [5 x i8]* @[[PROP_NAME_ATTR]], i32 0, i32 0) }] },
+
 @protocol X, Y, Z;
 @class Foo;
 
@@ -29,3 +32,18 @@ int main()
 {
const char * en = @encode(Intf);
 }
+
+@protocol P0
+@property id prop0;
+@end
+
+@protocol P1 
+@property id prop0;
+@end
+
+@interface C0 
+@end
+
+@implementation C0
+@dynamic prop0;
+@end



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

[PATCH] D72247: Add Triple::isX86()

[Fangrui Song via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG6904cd948674: Add Triple::isX86() (authored by MaskRay).
Herald added subscribers: fedor.sergeev, emaste.

Changed prior to commit:
  https://reviews.llvm.org/D72247?vs=236300&id=236477#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72247

Files:
  clang/lib/AST/MicrosoftMangle.cpp
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/lib/Driver/ToolChains/FreeBSD.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Parse/ParseStmtAsm.cpp
  llvm/include/llvm/ADT/Triple.h
  llvm/lib/IR/AutoUpgrade.cpp
  llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp

Index: llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
===
--- llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
+++ llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
@@ -1253,8 +1253,7 @@
 
 bool DevirtModule::shouldExportConstantsAsAbsoluteSymbols() {
   Triple T(M.getTargetTriple());
-  return (T.getArch() == Triple::x86 || T.getArch() == Triple::x86_64) &&
- T.getObjectFormat() == Triple::ELF;
+  return T.isX86() && T.getObjectFormat() == Triple::ELF;
 }
 
 void DevirtModule::exportGlobal(VTableSlot Slot, ArrayRef Args,
Index: llvm/lib/IR/AutoUpgrade.cpp
===
--- llvm/lib/IR/AutoUpgrade.cpp
+++ llvm/lib/IR/AutoUpgrade.cpp
@@ -4163,9 +4163,7 @@
 
   // If X86, and the datalayout matches the expected format, add pointer size
   // address spaces to the datalayout.
-  Triple::ArchType Arch = Triple(TT).getArch();
-  if ((Arch != llvm::Triple::x86 && Arch != llvm::Triple::x86_64) ||
-  DL.contains(AddrSpaces))
+  if (!Triple(TT).isX86() || DL.contains(AddrSpaces))
 return DL;
 
   SmallVector Groups;
Index: llvm/include/llvm/ADT/Triple.h
===
--- llvm/include/llvm/ADT/Triple.h
+++ llvm/include/llvm/ADT/Triple.h
@@ -730,6 +730,11 @@
 return getArch() == Triple::riscv32 || getArch() == Triple::riscv64;
   }
 
+  /// Tests whether the target is x86 (32- or 64-bit).
+  bool isX86() const {
+return getArch() == Triple::x86 || getArch() == Triple::x86_64;
+  }
+
   /// Tests whether the target supports comdat
   bool supportsCOMDAT() const {
 return !isOSBinFormatMachO();
Index: clang/lib/Parse/ParseStmtAsm.cpp
===
--- clang/lib/Parse/ParseStmtAsm.cpp
+++ clang/lib/Parse/ParseStmtAsm.cpp
@@ -547,12 +547,9 @@
 
   // We need an actual supported target.
   const llvm::Triple &TheTriple = Actions.Context.getTargetInfo().getTriple();
-  llvm::Triple::ArchType ArchTy = TheTriple.getArch();
   const std::string &TT = TheTriple.getTriple();
   const llvm::Target *TheTarget = nullptr;
-  bool UnsupportedArch =
-  (ArchTy != llvm::Triple::x86 && ArchTy != llvm::Triple::x86_64);
-  if (UnsupportedArch) {
+  if (!TheTriple.isX86()) {
 Diag(AsmLoc, diag::err_msasm_unsupported_arch) << TheTriple.getArchName();
   } else {
 std::string Error;
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -2983,7 +2983,7 @@
  Arch != llvm::Triple::x86;
 emitError |= (DefaultCC == LangOptions::DCC_VectorCall ||
   DefaultCC == LangOptions::DCC_RegCall) &&
- !(Arch == llvm::Triple::x86 || Arch == llvm::Triple::x86_64);
+ !T.isX86();
 if (emitError)
   Diags.Report(diag::err_drv_argument_not_allowed_with)
   << A->getSpelling() << T.getTriple();
Index: clang/lib/Driver/ToolChains/FreeBSD.cpp
===
--- clang/lib/Driver/ToolChains/FreeBSD.cpp
+++ clang/lib/Driver/ToolChains/FreeBSD.cpp
@@ -170,11 +170,10 @@
   CmdArgs.push_back("-dynamic-linker");
   CmdArgs.push_back("/libexec/ld-elf.so.1");
 }
-if (ToolChain.getTriple().getOSMajorVersion() >= 9) {
-  if (Arch == llvm::Triple::arm || Arch == llvm::Triple::sparc ||
-  Arch == llvm::Triple::x86 || Arch == llvm::Triple::x86_64) {
+const llvm::Triple &T = ToolChain.getTriple();
+if (T.getOSMajorVersion() >= 9) {
+  if (Arch == llvm::Triple::arm || Arch == llvm::Triple::sparc || T.isX86())
 CmdArgs.push_back("--hash-style=both");
-  }
 }
 CmdArgs.push_back("--enable-new-dtags");
   }
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -2491,7 +2491,7 @@
 
 bool MachO::SupportsProfiling() const {
   // Profiling instrumentation is only supported 

[clang] c6fd16a - Use FileCheck instead of grep

[Akira Hatanaka via cfe-commits]

Author: Akira Hatanaka
Date: 2020-01-06T15:50:23-08:00
New Revision: c6fd16af2be98b49d663285e3808ecde61bec614

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

LOG: Use FileCheck instead of grep

Added: 


Modified: 
clang/test/CodeGenObjC/encode-test-2.m

Removed: 




diff  --git a/clang/test/CodeGenObjC/encode-test-2.m 
b/clang/test/CodeGenObjC/encode-test-2.m
index 9e1423755a79..2985fbda1862 100644
--- a/clang/test/CodeGenObjC/encode-test-2.m
+++ b/clang/test/CodeGenObjC/encode-test-2.m
@@ -1,9 +1,10 @@
-// RUN: %clang_cc1 -triple=i686-apple-darwin9 -emit-llvm -o %t %s
-// RUN: grep -e "@\\\22\\\22" %t
-// RUN: grep -e "@\\\22\\\22" %t
-// RUN: grep -e "@\\\22\\\22" %t
-// RUN: grep -e "@\\\22Foo\\\22" %t
-// RUN: grep -e "{Intf=#}" %t  
+// RUN: %clang_cc1 -triple=i686-apple-darwin9 -emit-llvm -o - %s | FileCheck %s
+
+// CHECK: private unnamed_addr constant [7 x i8] c"@\22\22\00",
+// CHECK: private unnamed_addr constant [10 x i8] c"@\22\22\00",
+// CHECK: private unnamed_addr constant [13 x i8] c"@\22\22\00",
+// CHECK: private unnamed_addr constant [16 x i8] c"@\22Foo\22\00",
+// CHECK: private unnamed_addr constant [13 x i8] c"{Intf=#}\00",
 
 @protocol X, Y, Z;
 @class Foo;



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

[clang] 6904cd9 - Add Triple::isX86()

[Fangrui Song via cfe-commits]

Author: Fangrui Song
Date: 2020-01-06T15:51:02-08:00
New Revision: 6904cd948674df7f55843519695dbc95157a9429

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

LOG: Add Triple::isX86()

Reviewed By: craig.topper, skan

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

Added: 


Modified: 
clang/lib/AST/MicrosoftMangle.cpp
clang/lib/CodeGen/CGBuiltin.cpp
clang/lib/Driver/ToolChains/Darwin.cpp
clang/lib/Driver/ToolChains/FreeBSD.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Parse/ParseStmtAsm.cpp
llvm/include/llvm/ADT/Triple.h
llvm/lib/IR/AutoUpgrade.cpp
llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp

Removed: 




diff  --git a/clang/lib/AST/MicrosoftMangle.cpp 
b/clang/lib/AST/MicrosoftMangle.cpp
index 6b984955849a..91413ae28e9c 100644
--- a/clang/lib/AST/MicrosoftMangle.cpp
+++ b/clang/lib/AST/MicrosoftMangle.cpp
@@ -2701,9 +2701,7 @@ void MicrosoftCXXNameMangler::mangleType(const VectorType 
*T, Qualifiers Quals,
   // doesn't match the Intel types uses a custom mangling below.
   size_t OutSizeBefore = Out.tell();
   if (!isa(T)) {
-llvm::Triple::ArchType AT =
-getASTContext().getTargetInfo().getTriple().getArch();
-if (AT == llvm::Triple::x86 || AT == llvm::Triple::x86_64) {
+if (getASTContext().getTargetInfo().getTriple().isX86()) {
   if (Width == 64 && ET->getKind() == BuiltinType::LongLong) {
 mangleArtificialTagType(TTK_Union, "__m64");
   } else if (Width >= 128) {

diff  --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index 12517709573a..4b89b1b83a6a 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -821,8 +821,7 @@ static llvm::Value *EmitBitTestIntrinsic(CodeGenFunction 
&CGF,
 
   // X86 has special BT, BTC, BTR, and BTS instructions that handle the array
   // indexing operation internally. Use them if possible.
-  llvm::Triple::ArchType Arch = CGF.getTarget().getTriple().getArch();
-  if (Arch == llvm::Triple::x86 || Arch == llvm::Triple::x86_64)
+  if (CGF.getTarget().getTriple().isX86())
 return EmitX86BitTestIntrinsic(CGF, BT, E, BitBase, BitPos);
 
   // Otherwise, use generic code to load one byte and test the bit. Use all but

diff  --git a/clang/lib/Driver/ToolChains/Darwin.cpp 
b/clang/lib/Driver/ToolChains/Darwin.cpp
index 5725a326ca5f..a2ac532c0158 100644
--- a/clang/lib/Driver/ToolChains/Darwin.cpp
+++ b/clang/lib/Driver/ToolChains/Darwin.cpp
@@ -2491,7 +2491,7 @@ bool MachO::isPICDefaultForced() const {
 
 bool MachO::SupportsProfiling() const {
   // Profiling instrumentation is only supported on x86.
-  return getArch() == llvm::Triple::x86 || getArch() == llvm::Triple::x86_64;
+  return getTriple().isX86();
 }
 
 void Darwin::addMinVersionArgs(const ArgList &Args,

diff  --git a/clang/lib/Driver/ToolChains/FreeBSD.cpp 
b/clang/lib/Driver/ToolChains/FreeBSD.cpp
index c10d991845d1..c5c6f530f48c 100644
--- a/clang/lib/Driver/ToolChains/FreeBSD.cpp
+++ b/clang/lib/Driver/ToolChains/FreeBSD.cpp
@@ -170,11 +170,10 @@ void freebsd::Linker::ConstructJob(Compilation &C, const 
JobAction &JA,
   CmdArgs.push_back("-dynamic-linker");
   CmdArgs.push_back("/libexec/ld-elf.so.1");
 }
-if (ToolChain.getTriple().getOSMajorVersion() >= 9) {
-  if (Arch == llvm::Triple::arm || Arch == llvm::Triple::sparc ||
-  Arch == llvm::Triple::x86 || Arch == llvm::Triple::x86_64) {
+const llvm::Triple &T = ToolChain.getTriple();
+if (T.getOSMajorVersion() >= 9) {
+  if (Arch == llvm::Triple::arm || Arch == llvm::Triple::sparc || 
T.isX86())
 CmdArgs.push_back("--hash-style=both");
-  }
 }
 CmdArgs.push_back("--enable-new-dtags");
   }

diff  --git a/clang/lib/Frontend/CompilerInvocation.cpp 
b/clang/lib/Frontend/CompilerInvocation.cpp
index 289c58e3eb9d..5f332aff75c2 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -2983,7 +2983,7 @@ static void ParseLangArgs(LangOptions &Opts, ArgList 
&Args, InputKind IK,
  Arch != llvm::Triple::x86;
 emitError |= (DefaultCC == LangOptions::DCC_VectorCall ||
   DefaultCC == LangOptions::DCC_RegCall) &&
- !(Arch == llvm::Triple::x86 || Arch == llvm::Triple::x86_64);
+ !T.isX86();
 if (emitError)
   Diags.Report(diag::err_drv_argument_not_allowed_with)
   << A->getSpelling() << T.getTriple();

diff  --git a/clang/lib/Parse/ParseStmtAsm.cpp 
b/clang/lib/Parse/ParseStmtAsm.cpp
index 98133aaf67af..d35973df921b 100644
--- a/clang/lib/Parse/ParseStmtAsm.cpp
+++ b/clang/lib/Parse/ParseStmtAsm.cpp
@@ -547,12 +547,9 @@ StmtResult 
Parser::ParseMicrosoftAsmStatement(SourceLocation AsmLoc) {
 
   // We nee

Re: [clang] 24ab9b5 - Generalize the pass registration mechanism used by Polly to any third-party tool

[Eric Christopher via cfe-commits]

Hi Serge,

I have a few questions here about this:

In general this appears to be a lot more complex than the existing plugin
solutions and requires quite a lot of custom cmake rules that are difficult
to maintain. Why do we want this in general for the project? I understand
the desire to make polly less of a special case, but I don't think that the
overall complexity is worth the added ability to register polly plugins.
Perhaps disabling the polly plugin registration scheme is a better option?

That being said, perhaps it is worth it? But I think we need to call out
why we want it. I would also have expected something to llvm-dev for a
change of this magnitude. I didn't see anyone from the pass manager
hierarchy on the reviews and the final reviewer wasn't someone who
contributes to these areas typically.

In addition, what's with the OSX failure? It's currently turned off and was
breaking the bots, but does it mean that you don't expect this machinery to
work on OSX? That seems like a severely limiting factor for the project.

+One first needs to create an independent project and add it to either
``tools/``
+or, using the MonoRepo layout, at the root of the repo alongside other
projects.

This appears to be from an earlier incarnation of the patch? There's only
one layout now.


 ; CHECK-EP-VECTORIZER-START-NEXT: Running pass: NoOpFunctionPass
+; CHECK-EXT: Running pass: {{.*}}::Bye on foo

Why is this running on every test of the pass manager? It should be an
example run in the examples directory and not on by default? Same for every
other PM test. This seems like a bug?

Thanks!

-eric


On Thu, Jan 2, 2020 at 7:52 AM via cfe-commits 
wrote:

>
> Author: serge_sans_paille
> Date: 2020-01-02T16:45:31+01:00
> New Revision: 24ab9b537e61b3fe5e6a1019492ff6530d82a3ee
>
> URL:
> https://github.com/llvm/llvm-project/commit/24ab9b537e61b3fe5e6a1019492ff6530d82a3ee
> DIFF:
> https://github.com/llvm/llvm-project/commit/24ab9b537e61b3fe5e6a1019492ff6530d82a3ee.diff
>
> LOG: Generalize the pass registration mechanism used by Polly to any
> third-party tool
>
> There's quite a lot of references to Polly in the LLVM CMake codebase.
> However
> the registration pattern used by Polly could be useful to other external
> projects: thanks to that mechanism it would be possible to develop LLVM
> extension without touching the LLVM code base.
>
> This patch has two effects:
>
> 1. Remove all code specific to Polly in the llvm/clang codebase,
> replaicing it
>with a generic mechanism
>
> 2. Provide a generic mechanism to register compiler extensions.
>
> A compiler extension is similar to a pass plugin, with the notable
> difference
> that the compiler extension can be configured to be built dynamically (like
> plugins) or statically (like regular passes).
>
> As a result, people willing to add extra passes to clang/opt can do it
> using a
> separate code repo, but still have their pass be linked in clang/opt as
> built-in
> passes.
>
> Differential Revision: https://reviews.llvm.org/D61446
>
> Added:
> llvm/examples/Bye/Bye.cpp
> llvm/examples/Bye/CMakeLists.txt
> llvm/test/Feature/load_extension.ll
> polly/lib/Plugin/Polly.cpp
>
> Modified:
> clang/lib/CodeGen/BackendUtil.cpp
> clang/lib/CodeGen/CMakeLists.txt
> clang/tools/driver/CMakeLists.txt
> clang/tools/driver/cc1_main.cpp
> llvm/CMakeLists.txt
> llvm/cmake/modules/AddLLVM.cmake
> llvm/docs/WritingAnLLVMPass.rst
> llvm/examples/CMakeLists.txt
> llvm/include/llvm/Config/llvm-config.h.cmake
> llvm/test/Other/new-pm-defaults.ll
> llvm/test/Other/new-pm-thinlto-defaults.ll
> llvm/test/Other/opt-O0-pipeline.ll
> llvm/test/Other/opt-O2-pipeline.ll
> llvm/test/Other/opt-O3-pipeline.ll
> llvm/test/Other/opt-Os-pipeline.ll
> llvm/test/lit.cfg.py
> llvm/test/lit.site.cfg.py.in
> llvm/tools/CMakeLists.txt
> llvm/tools/bugpoint/CMakeLists.txt
> llvm/tools/bugpoint/bugpoint.cpp
> llvm/tools/opt/CMakeLists.txt
> llvm/tools/opt/NewPMDriver.cpp
> llvm/tools/opt/opt.cpp
> llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
> polly/include/polly/RegisterPasses.h
> polly/lib/CMakeLists.txt
> polly/lib/Support/RegisterPasses.cpp
> polly/test/Unit/lit.site.cfg.in
> polly/test/lit.site.cfg.in
> polly/test/update_check.py
>
> Removed:
> polly/lib/Polly.cpp
>
>
>
> 
> diff  --git a/clang/lib/CodeGen/BackendUtil.cpp
> b/clang/lib/CodeGen/BackendUtil.cpp
> index 645ef0165a53..ed881f2ddf68 100644
> --- a/clang/lib/CodeGen/BackendUtil.cpp
> +++ b/clang/lib/CodeGen/BackendUtil.cpp
> @@ -75,6 +75,10 @@
>  using namespace clang;
>  using namespace llvm;
>
> +#define HANDLE_EXTENSION(Ext)
>   \
> +  llvm::PassPluginLibraryInfo get##Ext##PluginInfo();
> +#include "llvm/Support/Extension.def"
> +
>  namespace {
>
>  // Default filename used for profile generation.
> @@ -

[PATCH] D72221: Support function attribute patchable_function_entry

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

{icon times-circle color=red} Unit tests: fail. 61271 tests passed, 1 failed 
and 736 were skipped.

  failed: Clang.CodeGen/patchable-function-entry.c

{icon times-circle color=red} clang-tidy: fail. Please fix clang-tidy findings 
.

{icon times-circle color=red} clang-format: fail. Please format your changes 
with clang-format by running `git-clang-format HEAD^` or applying this patch 
.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72221



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

[PATCH] D72247: Add Triple::isX86()

[Craig Topper via Phabricator via cfe-commits]

craig.topper accepted this revision.
craig.topper added a comment.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72247



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

[PATCH] D72247: Add Triple::isX86()

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

@craig.topper Good to go? :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72247



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

[PATCH] D72306: [PowerPC] FreeBSD >= 13 default ABI is ELFv2

[Alfredo Dal'Ava Júnior via Phabricator via cfe-commits]

adalava created this revision.
adalava added reviewers: dim, MaskRay.
adalava added a project: PowerPC.
Herald added subscribers: llvm-commits, cfe-commits, steven.zhang, shchenz, 
jsji, kbarton, hiraditya, krytarowski, arichardson, nemanjai, emaste.
Herald added projects: clang, LLVM.

FreeBSD for PowerPC* transitioned[1][2] from gcc4.2 to LLVM9 on Dec 26, 2019 
(starting from r356111). The PowerPC64 target also transitioned to ELFv2 ABI at 
same time,  so this makes it default for FreeBSD releases >= 13

*Also would like to have this back ported to LLVM 9 on upstream if possible

[1] https://wiki.freebsd.org/powerpc/llvm-elfv2
[2] https://lists.freebsd.org/pipermail/freebsd-ppc/2019-December/011042.html


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72306

Files:
  clang/lib/Basic/Targets/PPC.h
  llvm/lib/Target/PowerPC/PPCTargetMachine.cpp
  llvm/test/CodeGen/PowerPC/ppc64-elf-abi.ll

Index: llvm/test/CodeGen/PowerPC/ppc64-elf-abi.ll
===
--- llvm/test/CodeGen/PowerPC/ppc64-elf-abi.ll
+++ llvm/test/CodeGen/PowerPC/ppc64-elf-abi.ll
@@ -5,6 +5,30 @@
 ; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu -target-abi elfv1 < %s | FileCheck %s -check-prefix=CHECK-ELFv1
 ; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu -target-abi elfv2 < %s | FileCheck %s -check-prefix=CHECK-ELFv2
 
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-freebsd11 < %s | FileCheck %s -check-prefix=CHECK-ELFv1
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-freebsd12 < %s | FileCheck %s -check-prefix=CHECK-ELFv1
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-freebsd13 < %s | FileCheck %s -check-prefix=CHECK-ELFv2
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-freebsd14 < %s | FileCheck %s -check-prefix=CHECK-ELFv2
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-freebsd11 -target-abi elfv1 < %s | FileCheck %s -check-prefix=CHECK-ELFv1
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-freebsd11 -target-abi elfv2 < %s | FileCheck %s -check-prefix=CHECK-ELFv2
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-freebsd12 -target-abi elfv1 < %s | FileCheck %s -check-prefix=CHECK-ELFv1
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-freebsd12 -target-abi elfv2 < %s | FileCheck %s -check-prefix=CHECK-ELFv2
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-freebsd13 -target-abi elfv1 < %s | FileCheck %s -check-prefix=CHECK-ELFv1
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-freebsd13 -target-abi elfv2 < %s | FileCheck %s -check-prefix=CHECK-ELFv2
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-freebsd14 -target-abi elfv1 < %s | FileCheck %s -check-prefix=CHECK-ELFv1
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-freebsd14 -target-abi elfv2 < %s | FileCheck %s -check-prefix=CHECK-ELFv2
+
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-freebsd11-elfv1 < %s | FileCheck %s -check-prefix=CHECK-ELFv1
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-freebsd11-elfv2 < %s | FileCheck %s -check-prefix=CHECK-ELFv2
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-freebsd12-elfv1 < %s | FileCheck %s -check-prefix=CHECK-ELFv1
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-freebsd12-elfv2 < %s | FileCheck %s -check-prefix=CHECK-ELFv2
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-freebsd13-elfv1 < %s | FileCheck %s -check-prefix=CHECK-ELFv1
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-freebsd13-elfv2 < %s | FileCheck %s -check-prefix=CHECK-ELFv2
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-freebsd14-elfv1 < %s | FileCheck %s -check-prefix=CHECK-ELFv1
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-freebsd14-elfv2 < %s | FileCheck %s -check-prefix=CHECK-ELFv2
+
+; FreeBSD target triple without OS version number is incorrect, but checking for regressions anyway
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-freebsd < %s | FileCheck %s -check-prefix=CHECK-ELFv1
 ; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-freebsd < %s | FileCheck %s -check-prefix=CHECK-ELFv1
 ; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-freebsd -target-abi elfv1 < %s | FileCheck %s -check-prefix=CHECK-ELFv1
 ; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-freebsd -target-abi elfv2 < %s | FileCheck %s -check-prefix=CHECK-ELFv2
Index: llvm/lib/Target/PowerPC/PPCTargetMachine.cpp
===
--- llvm/lib/Target/PowerPC/PPCTargetMachine.cpp
+++ llvm/lib/Target/PowerPC/PPCTargetMachine.cpp
@@ -199,7 +199,7 @@
  const TargetOptions &Options) {
   if (TT.isOSDarwin())
 report_fatal_error("Darwin is no longer supported for PowerPC");
-  
+
   if (Options.MCOpt

[PATCH] D72221: Support function attribute patchable_function_entry

[Fangrui Song via Phabricator via cfe-commits]

MaskRay updated this revision to Diff 236468.
MaskRay marked 13 inline comments as done.
MaskRay added a comment.

Address review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72221

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/patchable-function-entry.c
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/Sema/patchable-function-entry-attr.c
  clang/test/Sema/patchable-function-entry-attr.cpp

Index: clang/test/Sema/patchable-function-entry-attr.cpp
===
--- /dev/null
+++ clang/test/Sema/patchable-function-entry-attr.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -triple aarch64 -fsyntax-only -verify=silence %s
+// RUN: %clang_cc1 -triple i386 -fsyntax-only -verify=silence %s
+// RUN: %clang_cc1 -triple x86_64 -fsyntax-only -verify=silence %s
+// RUN: %clang_cc1 -triple ppc64le -fsyntax-only -verify %s
+
+// silence-no-diagnostics
+
+// expected-warning@+1 {{unknown attribute 'patchable_function_entry' ignored}}
+[[gnu::patchable_function_entry(0)]] void f();
Index: clang/test/Sema/patchable-function-entry-attr.c
===
--- /dev/null
+++ clang/test/Sema/patchable-function-entry-attr.c
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -triple aarch64 -fsyntax-only -verify %s
+
+// expected-error@+1 {{'patchable_function_entry' attribute takes at least 1 argument}}
+__attribute__((patchable_function_entry)) void f();
+
+// expected-error@+1 {{'patchable_function_entry' attribute takes no more than 2 arguments}}
+__attribute__((patchable_function_entry(0, 0, 0))) void f();
+
+// expected-error@+1 {{'patchable_function_entry' attribute requires a non-negative integral compile time constant expression}}
+__attribute__((patchable_function_entry(-1))) void f();
+
+int i;
+// expected-error@+1 {{'patchable_function_entry' attribute requires parameter 0 to be an integer constant}}
+__attribute__((patchable_function_entry(i))) void f();
+
+// expected-error@+1 {{'patchable_function_entry' attribute requires integer constant between 0 and 0 inclusive}}
+__attribute__((patchable_function_entry(1, 1))) void f();
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -128,6 +128,7 @@
 // CHECK-NEXT: Owner (SubjectMatchRule_record_not_is_union)
 // CHECK-NEXT: ParamTypestate (SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: PassObjectSize (SubjectMatchRule_variable_is_parameter)
+// CHECK-NEXT: PatchableFunctionEntry (SubjectMatchRule_function, SubjectMatchRule_objc_method)
 // CHECK-NEXT: Pointer (SubjectMatchRule_record_not_is_union)
 // CHECK-NEXT: ReleaseHandle (SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: RenderScriptKernel (SubjectMatchRule_function)
Index: clang/test/CodeGen/patchable-function-entry.c
===
--- /dev/null
+++ clang/test/CodeGen/patchable-function-entry.c
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -triple aarch64 -emit-llvm %s -o - | FileCheck %s
+
+// CHECK: define void @f0() #0
+__attribute__((patchable_function_entry(0))) void f0() {}
+
+// CHECK: define void @f00() #0
+__attribute__((patchable_function_entry(0, 0))) void f00() {}
+
+// CHECK: define void @f2() #1
+__attribute__((patchable_function_entry(2))) void f2() {}
+
+// CHECK: define void @f20() #1
+__attribute__((patchable_function_entry(2, 0))) void f20() {}
+
+// CHECK: define void @f20decl() #1
+__attribute__((patchable_function_entry(2, 0))) void f20decl();
+void f20decl() {}
+
+/// M in patchable_function_entry(N,M) is currently ignored.
+// CHECK: attributes #0 = { {{.*}} "patchable-function-entry"="0"
+// CHECK: attributes #1 = { {{.*}} "patchable-function-entry"="2"
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -4917,6 +4917,25 @@
  XRayLogArgsAttr(S.Context, AL, ArgCount.getSourceIndex()));
 }
 
+static void handlePatchableFunctionEntryAttr(Sema &S, Decl *D,
+ const ParsedAttr &AL) {
+  uint32_t Count = 0, Offset = 0;
+  if (!checkUInt32Argument(S, AL, AL.getArgAsExpr(0), Count, 0, true))
+return;
+  if (AL.getNumArgs() == 2) {
+Expr *Arg = AL.getArgAsExpr(1);
+if (!checkUInt32Argument(S, AL, Arg, Offset, 1, true))
+  return;
+if (Offset) {
+  S.Diag(getAttrLoc(AL), diag::err_attribute_argument_out_of_range)
+  << &AL << 0 << 0 << Arg->getBeginLoc();
+  ret

[PATCH] D72221: Support function attribute patchable_function_entry

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:686
 
+def PatchableFunctionEntry : InheritableAttr {
+  let Spellings = [GCC<"patchable_function_entry">];

aaron.ballman wrote:
> Should this be inheriting from `TargetSpecificAttr` as well given that the 
> attribute is only supported on some architectures?
Thanks for the suggestion. Didn't know TargetSpecificAttr.



Comment at: clang/include/clang/Basic/AttrDocs.td:3995
+``__attribute__((patchable_function_entry(N,M)))`` is used to generate M NOPs
+before the function entry and N-M NOPs after the function entry. This 
attributes
+takes precedence over command line option ``-fpatchable-function-entry=N,M``.

ostannard wrote:
> Grammar nits:
> s/attributes/attribute/
> over _the_ command line option
Thanks. (I am a non-native English speaker and any advice will be highly 
appreciated.)

(So, I believe GCC's documentation misses _the_)



Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:826
+  "patchable-function-entry",
+  (Twine(Attr->getSize()) + "," + Twine(Attr->getStart())).str());
   }

nickdesaulniers wrote:
> ostannard wrote:
> > I think using two function attributes here would be better, to avoid 
> > needing to parse this again later.
> In that case, it would not make sense to have start without a size, and thus 
> should be checked for in the verification.
Agreed. Updated D72215 (semantics of the IR function attribute 
"patchable-function-entry") as well.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:4925
+  T.getArch() != llvm::Triple::x86_64) {
+S.Diag(getAttrLoc(AL), diag::err_attribute_unsupported) << AL;
+return;

nickdesaulniers wrote:
> nickdesaulniers wrote:
> > Why is the target arch also checked in 
> > `clang/lib/Driver/ToolChains/Clang.cpp` in https://reviews.llvm.org/D7?
> Ah, https://reviews.llvm.org/D72221 is checking the `__attribute__` syntax, 
> https://reviews.llvm.org/D7 is checking the command line `-f` syntax.
Yes. D72221 is for the Clang function attribute while D7 is for 
`-fpatchable-function-entry=`.





Comment at: clang/test/CodeGen/patchable-function-entry.c:20
+// CHECK: attributes #0 = { {{.*}} "patchable-function-entry"="0,0"
+// CHECK: attributes #1 = { {{.*}} "patchable-function-entry"="2,0"

aaron.ballman wrote:
> Can you also add a test verifying that this attribute works with 
> redeclarations as well? Something like:
> ```
> [[gnu::patchable_function_entry(12, 4)]] void func(void);
> 
> void func(void) { // Definition should have the noop sled
> }
> ```
```
__attribute__((patchable_function_entry(2,0))) void f20decl();
__attribute__((noinline)) void f20decl() {}
```

Checked this case. I'll delete `__attribute__((noinline))` since it does not 
seem to be clear what it intends to do.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72221



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

[PATCH] D72304: Summary: Add OpenMP Directives (master and critical) to OMPBuilder, and use them in clang.

[Fady Ghanim via Phabricator via cfe-commits]

fghanim created this revision.
fghanim added a reviewer: jdoerfert.
Herald added subscribers: llvm-commits, cfe-commits, guansong, hiraditya.
Herald added projects: clang, LLVM.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72304

Files:
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
  llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp

Index: llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
===
--- llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
+++ llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
@@ -613,4 +613,180 @@
   }
 }
 
+TEST_F(OpenMPIRBuilderTest, MasterDirective) {
+	using InsertPointTy = OpenMPIRBuilder::InsertPointTy;
+	OpenMPIRBuilder OMPBuilder(*M);
+	OMPBuilder.initialize();
+	F->setName("func");
+	IRBuilder<> Builder(BB);
+
+	OpenMPIRBuilder::LocationDescription Loc( { Builder.saveIP(), DL });
+
+	AllocaInst *PrivAI = nullptr;
+
+	BasicBlock *EntryBB = nullptr;
+	BasicBlock *FinalBB = nullptr;
+	BasicBlock *ExitBB = nullptr;
+	BasicBlock *ThenBB = nullptr;
+
+	auto BodyGenCB = [&](InsertPointTy AllocaIP, InsertPointTy CodeGenIP,
+			BasicBlock &FiniBB) {
+		if (AllocaIP.isSet())
+			Builder.restoreIP(AllocaIP);
+		else
+			Builder.SetInsertPoint(&*(F->getEntryBlock().getFirstInsertionPt()));
+		PrivAI = Builder.CreateAlloca(F->arg_begin()->getType());
+		Builder.CreateStore(F->arg_begin(), PrivAI);
+
+		llvm::BasicBlock *CodeGenIPBB = CodeGenIP.getBlock();
+		llvm::Instruction *CodeGenIPInst = &*CodeGenIP.getPoint();
+		EXPECT_EQ(CodeGenIPBB->getTerminator(), CodeGenIPInst);
+
+		Builder.restoreIP(CodeGenIP);
+
+		//collect some info for checks later
+		FinalBB = &FiniBB;
+		ExitBB = FiniBB.getUniqueSuccessor();
+		ThenBB = Builder.GetInsertBlock();
+		EntryBB = ThenBB->getUniquePredecessor();
+
+		//simple instructions for body
+		Value *PrivLoad = Builder.CreateLoad(PrivAI, "local.use");
+		Builder.CreateICmpNE(F->arg_begin(), PrivLoad);
+	};
+
+	auto FiniCB = [&](InsertPointTy IP) {
+		BasicBlock *IPBB = IP.getBlock();
+		EXPECT_NE(IPBB->end(), IP.getPoint());
+	};
+
+	Builder.restoreIP(OMPBuilder.CreateMaster(Builder, BodyGenCB, FiniCB));
+	Value *EntryBBTI = EntryBB->getTerminator();
+	EXPECT_NE(EntryBBTI, nullptr);
+	EXPECT_TRUE(isa(EntryBBTI));
+	BranchInst *EntryBr = cast(EntryBB->getTerminator());
+	EXPECT_TRUE(EntryBr->isConditional());
+	EXPECT_EQ(EntryBr->getSuccessor(0), ThenBB);
+	EXPECT_EQ(ThenBB->getUniqueSuccessor(), FinalBB);
+	EXPECT_EQ(FinalBB->getUniqueSuccessor(), ExitBB);
+	EXPECT_EQ(EntryBr->getSuccessor(1), ExitBB);
+
+	CmpInst *CondInst = cast(EntryBr->getCondition());
+	EXPECT_TRUE(isa(CondInst->getOperand(0)));
+
+	CallInst *MasterEntryCI = cast(CondInst->getOperand(0));
+	EXPECT_EQ(MasterEntryCI->getNumArgOperands(), 2U);
+	EXPECT_EQ(MasterEntryCI->getCalledFunction()->getName(), "__kmpc_omp_master");
+	EXPECT_TRUE(isa(MasterEntryCI->getArgOperand(0)));
+
+	CallInst *MasterEndCI = nullptr;
+	for (auto FI = FinalBB->begin(); FI != FinalBB->end(); ++FI) {
+		Instruction *cur = &*FI;
+		if (isa(cur)) {
+			MasterEndCI = cast(cur);
+			if (MasterEndCI->getCalledFunction()->getName()
+	== "__kmpc_omp_end_master")
+break;
+			else
+MasterEndCI = nullptr;
+		}
+	}
+	EXPECT_NE(MasterEndCI, nullptr);
+	EXPECT_EQ(MasterEndCI->getNumArgOperands(), 2U);
+	EXPECT_TRUE(isa(MasterEndCI->getArgOperand(0)));
+	EXPECT_EQ(MasterEndCI->getArgOperand(1), MasterEntryCI->getArgOperand(1));
+}
+
+TEST_F(OpenMPIRBuilderTest, CriticalDirective) {
+	using InsertPointTy = OpenMPIRBuilder::InsertPointTy;
+	OpenMPIRBuilder OMPBuilder(*M);
+	OMPBuilder.initialize();
+	F->setName("func");
+	IRBuilder<> Builder(BB);
+
+	OpenMPIRBuilder::LocationDescription Loc( { Builder.saveIP(), DL });
+
+	AllocaInst *PrivAI = Builder.CreateAlloca(F->arg_begin()->getType());
+
+	BasicBlock *EntryBB = nullptr;
+	BasicBlock *FinalBB = nullptr;
+	BasicBlock *ExitBB = nullptr;
+
+	auto BodyGenCB = [&](InsertPointTy AllocaIP, InsertPointTy CodeGenIP,
+			BasicBlock &FiniBB) {
+		//collect some info for checks later
+		FinalBB = &FiniBB;
+		ExitBB = FiniBB.getUniqueSuccessor();
+		EntryBB = FinalBB->getUniquePredecessor();
+
+		//actual start for bodyCB
+		llvm::BasicBlock *CodeGenIPBB = CodeGenIP.getBlock();
+		llvm::Instruction *CodeGenIPInst = &*CodeGenIP.getPoint();
+		EXPECT_EQ(CodeGenIPBB->getTerminator(), CodeGenIPInst);
+		EXPECT_EQ(EntryBB, CodeGenIPBB);
+
+		//body begin
+		Builder.restoreIP(CodeGenIP);
+		Builder.CreateStore(F->arg_begin(), PrivAI);
+		Value *PrivLoad = Builder.CreateLoad(PrivAI, "local.use");
+		Builder.CreateICmpNE(F->arg_begin(), PrivLoad);
+	};
+
+	auto FiniCB = [&](InsertPointTy IP) {
+		BasicBlock *IPBB = IP.getBlock();
+		EXPECT_NE(IPBB->end(), IP.getPoint());
+	};
+
+	Builder.restoreIP(
+			OMPBuilder.CreateCritical(Builder, BodyGenCB, FiniCB, "testCRT",
+	

[PATCH] D72053: [RFC] Handling implementation limits

[Mark de Wever via Phabricator via cfe-commits]

Mordante marked 6 inline comments as done.
Mordante added a comment.

Thanks for the feedback!




Comment at: clang/include/clang/AST/Decl.h:903
 /// declared.
-unsigned ScopeDepthOrObjCQuals : NumScopeDepthOrObjCQualsBits;
+unsigned ScopeDepthOrObjCQuals : IQ::NumScopeDepthOrObjCQualsBits;
 

rsmith wrote:
> rsmith wrote:
> > These bitfields are assumed to fit into an `unsigned`, and this can no 
> > longer be verified locally (changes to the `.td` file can break this 
> > assumption). Please add a `static_assert(sizeof(ParmVarDeclBitfields) <= 
> > sizeof(unsigned), "...")` next to the `union` below (and likewise for 
> > `NonParmVarDeclBitfields`).
> I think `IQ` is too terse. We don't need a really short name for this; how 
> about `ImpLimits::` or similar?
I feared ImpLimits would be considered too verbose, but I'll change it in the 
next revision.



Comment at: clang/include/clang/AST/Decl.h:903
 /// declared.
-unsigned ScopeDepthOrObjCQuals : NumScopeDepthOrObjCQualsBits;
+unsigned ScopeDepthOrObjCQuals : IQ::NumScopeDepthOrObjCQualsBits;
 

Mordante wrote:
> rsmith wrote:
> > rsmith wrote:
> > > These bitfields are assumed to fit into an `unsigned`, and this can no 
> > > longer be verified locally (changes to the `.td` file can break this 
> > > assumption). Please add a `static_assert(sizeof(ParmVarDeclBitfields) <= 
> > > sizeof(unsigned), "...")` next to the `union` below (and likewise for 
> > > `NonParmVarDeclBitfields`).
> > I think `IQ` is too terse. We don't need a really short name for this; how 
> > about `ImpLimits::` or similar?
> I feared ImpLimits would be considered too verbose, but I'll change it in the 
> next revision.
I'm not too fond to use the `static_assert(sizeof(ParamVarDeclBitFields <= 
sizeof(unsigned), "...");` here and use a hardcoded value and a  
`static_assert` in other places. If it's important to see the size of bit-field 
directly I prefer the style you suggested later: "Please keep the hard-coded 15 
here and add an adjacent `static_assert(IQ::BitFieldBits <= 15, "...")` 
instead, or use some similar strategy so that the bit-field allocation and 
layout remains locally obvious."

I'll give it some more thoughts before updating the proposal.



Comment at: clang/include/clang/Basic/ImplementationQuantities.td:107-111
+  bit isBitLimit = 0;
+  bit isValueLimit = 0;
+  bit isCompilerFlagLimit = 0;
+  bit isDescriptionLimit = 0;
+  bit hasRemark = 0;

rsmith wrote:
> Switching between upper- and lower-case names for these fields seems 
> surprising. Please pick a consistent capitalization scheme.
This style is used in other .td files. But I'll switch to capitalize the is/has 
in the next revision. 


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

https://reviews.llvm.org/D72053



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

[PATCH] D72202: [Diagnostic] make Wmisleading-indendation not warn about labels

[Tyker via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf5329bfc76bb: [Diagnostic] make Wmisleading-indendation not 
warn about labels (authored by Tyker).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72202

Files:
  clang/lib/Parse/ParseStmt.cpp
  clang/test/Parser/warn-misleading-indentation.cpp


Index: clang/test/Parser/warn-misleading-indentation.cpp
===
--- clang/test/Parser/warn-misleading-indentation.cpp
+++ clang/test/Parser/warn-misleading-indentation.cpp
@@ -305,3 +305,10 @@
 fail:;
 }
 
+void f_label(int b) {
+  if (b)
+return;
+a:
+  return;
+  goto a;
+}
Index: clang/lib/Parse/ParseStmt.cpp
===
--- clang/lib/Parse/ParseStmt.cpp
+++ clang/lib/Parse/ParseStmt.cpp
@@ -1272,10 +1272,12 @@
 
 if (PrevColNum != 0 && CurColNum != 0 && StmtColNum != 0 &&
 ((PrevColNum > StmtColNum && PrevColNum == CurColNum) ||
- !Tok.isAtStartOfLine()) && SM.getPresumedLineNumber(StmtLoc) !=
-  SM.getPresumedLineNumber(Tok.getLocation())) {
-  P.Diag(Tok.getLocation(), diag::warn_misleading_indentation)
-  << Kind;
+ !Tok.isAtStartOfLine()) &&
+SM.getPresumedLineNumber(StmtLoc) !=
+SM.getPresumedLineNumber(Tok.getLocation()) &&
+(Tok.isNot(tok::identifier) ||
+ P.getPreprocessor().LookAhead(0).isNot(tok::colon))) {
+  P.Diag(Tok.getLocation(), diag::warn_misleading_indentation) << Kind;
   P.Diag(StmtLoc, diag::note_previous_statement);
 }
   }


Index: clang/test/Parser/warn-misleading-indentation.cpp
===
--- clang/test/Parser/warn-misleading-indentation.cpp
+++ clang/test/Parser/warn-misleading-indentation.cpp
@@ -305,3 +305,10 @@
 fail:;
 }
 
+void f_label(int b) {
+  if (b)
+return;
+a:
+  return;
+  goto a;
+}
Index: clang/lib/Parse/ParseStmt.cpp
===
--- clang/lib/Parse/ParseStmt.cpp
+++ clang/lib/Parse/ParseStmt.cpp
@@ -1272,10 +1272,12 @@
 
 if (PrevColNum != 0 && CurColNum != 0 && StmtColNum != 0 &&
 ((PrevColNum > StmtColNum && PrevColNum == CurColNum) ||
- !Tok.isAtStartOfLine()) && SM.getPresumedLineNumber(StmtLoc) !=
-  SM.getPresumedLineNumber(Tok.getLocation())) {
-  P.Diag(Tok.getLocation(), diag::warn_misleading_indentation)
-  << Kind;
+ !Tok.isAtStartOfLine()) &&
+SM.getPresumedLineNumber(StmtLoc) !=
+SM.getPresumedLineNumber(Tok.getLocation()) &&
+(Tok.isNot(tok::identifier) ||
+ P.getPreprocessor().LookAhead(0).isNot(tok::colon))) {
+  P.Diag(Tok.getLocation(), diag::warn_misleading_indentation) << Kind;
   P.Diag(StmtLoc, diag::note_previous_statement);
 }
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] f5329bf - [Diagnostic] make Wmisleading-indendation not warn about labels

[via cfe-commits]

Author: Tyker
Date: 2020-01-06T23:22:27+01:00
New Revision: f5329bfc76bb6fc30a589e8238aabc005c52e5d6

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

LOG: [Diagnostic] make Wmisleading-indendation not warn about labels

Reviewers: aaron.ballman, xbolva00

Reviewed By: aaron.ballman

Subscribers: nickdesaulniers, nathanchance

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

Added: 


Modified: 
clang/lib/Parse/ParseStmt.cpp
clang/test/Parser/warn-misleading-indentation.cpp

Removed: 




diff  --git a/clang/lib/Parse/ParseStmt.cpp b/clang/lib/Parse/ParseStmt.cpp
index b79654015653..0339328ca513 100644
--- a/clang/lib/Parse/ParseStmt.cpp
+++ b/clang/lib/Parse/ParseStmt.cpp
@@ -1272,10 +1272,12 @@ struct MisleadingIndentationChecker {
 
 if (PrevColNum != 0 && CurColNum != 0 && StmtColNum != 0 &&
 ((PrevColNum > StmtColNum && PrevColNum == CurColNum) ||
- !Tok.isAtStartOfLine()) && SM.getPresumedLineNumber(StmtLoc) !=
-  SM.getPresumedLineNumber(Tok.getLocation())) {
-  P.Diag(Tok.getLocation(), diag::warn_misleading_indentation)
-  << Kind;
+ !Tok.isAtStartOfLine()) &&
+SM.getPresumedLineNumber(StmtLoc) !=
+SM.getPresumedLineNumber(Tok.getLocation()) &&
+(Tok.isNot(tok::identifier) ||
+ P.getPreprocessor().LookAhead(0).isNot(tok::colon))) {
+  P.Diag(Tok.getLocation(), diag::warn_misleading_indentation) << Kind;
   P.Diag(StmtLoc, diag::note_previous_statement);
 }
   }

diff  --git a/clang/test/Parser/warn-misleading-indentation.cpp 
b/clang/test/Parser/warn-misleading-indentation.cpp
index 4b9d45b19ca6..8339f21099c3 100644
--- a/clang/test/Parser/warn-misleading-indentation.cpp
+++ b/clang/test/Parser/warn-misleading-indentation.cpp
@@ -305,3 +305,10 @@ int main(int argc, char* argv[]) {
 fail:;
 }
 
+void f_label(int b) {
+  if (b)
+return;
+a:
+  return;
+  goto a;
+}



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

[PATCH] D72282: [clang-tidy] Add `bugprone-unintended-adl`

[Logan Smith via Phabricator via cfe-commits]

logan-5 marked an inline comment as done.
logan-5 added inline comments.



Comment at: clang-tools-extra/clang-tidy/bugprone/UnintendedADLCheck.cpp:43
+  Whitelist(
+  utils::options::parseStringList(Options.get("Whitelist", "swap"))) {}
+

JonasToth wrote:
> do you mean `std::swap`? If you it should be fully qualified.
> Doesn't `std::error_code` rely on adl, too? I think `std::cout <<` and other 
> streams of the STL rely on it too, and probably many more code-constructs 
> that are commonly used.
> 
> That means, the list should be extended to at least all standard-library 
> facilities that basically required ADL to work. And then we need data on 
> different code bases (e.g. LLVM is a good start) how much noise gets 
> generated.
I distinctly //don't// mean `std::swap` -- I want to whitelist any unqualified 
function call spelled simply `swap`.

Overloaded operators are the poster child for ADL's usefulness, so that's why 
this check has a special default-on `IgnoreOverloadedOperators` option. That 
whitelists a whole ton of legitimate stuff including `std::cout << x` and 
friends.

I don't see a ton of discussion online about `error_code`/`make_error_code` and 
ADL being very much intertwined. I'm not particularly familiar with those 
constructs myself though, and I could just be out of the loop. I do see a fair 
number of unqualified calls to `make_error_code` within LLVM, though most of 
those resolve to `llvm::make_error_code`, the documentation for which says it 
exists because `std::make_error_code` can't be reliably/portably used with ADL. 
That makes me think `make_error_code` would belong in LLVM's project-specific 
whitelist configuration, not the check's default.

Speaking of which, I did run this check over LLVM while developing and found it 
not particularly noisy as written. That is, it generated a fair number of 
warnings, but only on constructs that, when examined closely, //were// a little 
suspicious or non-obvious.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72282



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

[PATCH] D72284: [clang-tidy] Factor out renaming logic from readability-identifier-naming

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp:272
+  if (const auto *Typedef =
+  Value->getReturnType().getTypePtr()->getAs()) {
+addUsage(NamingCheckFailures, Typedef->getDecl(),

Please elide braces.



Comment at: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp:280
+ .getTypePtr()
+ ->getAs()) {
+  addUsage(NamingCheckFailures, Typedef->getDecl(),

Please elide braces.



Comment at: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h:47
+ShouldFix,
+ConflictsWithKeyword, /// The fixup will conflict with a language keyword,
+  /// so we can't fix it automatically.

It'll be reasonable to place all comments consistently: before definition or 
after.



Comment at: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h:94
+
+  typedef std::pair NamingCheckId;
+

Please use using. Same below. See modernize-use-using.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72284



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

[PATCH] D72284: [clang-tidy] Factor out renaming logic from readability-identifier-naming

[Logan Smith via Phabricator via cfe-commits]

logan-5 updated this revision to Diff 236449.
logan-5 marked 3 inline comments as done.
logan-5 added a comment.

Addressed some nits. If the rest looks good, I'll need someone with commit 
access to help me wrap this up.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72284

Files:
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
  clang-tools-extra/clang-tidy/utils/CMakeLists.txt
  clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
  clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h

Index: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h
@@ -0,0 +1,145 @@
+//===--- RenamderClangTidyCheck.h - clang-tidy --*- 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
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_RENAMERCLANGTIDYCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_RENAMERCLANGTIDYCHECK_H
+
+#include "../ClangTidyCheck.h"
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/Optional.h"
+#include 
+#include 
+
+namespace clang {
+
+class MacroInfo;
+
+namespace tidy {
+
+/// Base class for clang-tidy checks that want to flag declarations and/or
+/// macros for renaming based on customizable criteria.
+class RenamerClangTidyCheck : public ClangTidyCheck {
+public:
+  RenamerClangTidyCheck(StringRef CheckName, ClangTidyContext *Context);
+  ~RenamerClangTidyCheck();
+
+  /// Derived classes should not implement any matching logic themselves; this
+  /// class will do the matching and call the derived class'
+  /// GetDeclFailureInfo() and GetMacroFailureInfo() for determining whether a
+  /// given identifier passes or fails the check.
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override final;
+  void
+  check(const ast_matchers::MatchFinder::MatchResult &Result) override final;
+  void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
+   Preprocessor *ModuleExpanderPP) override final;
+  void onEndOfTranslationUnit() override final;
+
+  /// This enum will be used in %select of the diagnostic message.
+  /// Each value below IgnoreFailureThreshold should have an error message.
+  enum class ShouldFixStatus {
+ShouldFix,
+ConflictsWithKeyword, /// The fixup will conflict with a language keyword,
+  /// so we can't fix it automatically.
+ConflictsWithMacroDefinition, /// The fixup will conflict with a macro
+  /// definition, so we can't fix it
+  /// automatically.
+
+/// Values pass this threshold will be ignored completely
+/// i.e no message, no fixup.
+IgnoreFailureThreshold,
+
+InsideMacro, /// If the identifier was used or declared within a macro we
+ /// won't offer a fixup for safety reasons.
+  };
+
+  /// Information describing a failed check
+  struct FailureInfo {
+std::string KindName; // Tag or misc info to be used as derived classes need
+std::string Fixup;// The name that will be proposed as a fix-it hint
+  };
+
+  /// Holds an identifier name check failure, tracking the kind of the
+  /// identifier, its possible fixup and the starting locations of all the
+  /// identifier usages.
+  struct NamingCheckFailure {
+FailureInfo Info;
+
+/// Whether the failure should be fixed or not.
+///
+/// e.g.: if the identifier was used or declared within a macro we won't
+/// offer a fixup for safety reasons.
+bool ShouldFix() const {
+  return FixStatus == ShouldFixStatus::ShouldFix && !Info.Fixup.empty();
+}
+
+bool ShouldNotify() const {
+  return FixStatus < ShouldFixStatus::IgnoreFailureThreshold;
+}
+
+ShouldFixStatus FixStatus = ShouldFixStatus::ShouldFix;
+
+/// A set of all the identifier usages starting SourceLocation, in
+/// their encoded form.
+llvm::DenseSet RawUsageLocs;
+
+NamingCheckFailure() = default;
+  };
+
+  typedef std::pair NamingCheckId;
+
+  typedef llvm::DenseMap
+  NamingCheckFailureMap;
+
+  /// Check Macros for style violations.
+  void checkMacro(SourceManager &sourceMgr, const Token &MacroNameTok,
+  const MacroInfo *MI);
+
+  /// Add a usage of a macro if it already has a violation.
+  void expandMacro(const Token &MacroNameTok, const MacroInfo *MI);
+
+protected:
+  /// Overridden by derived classes, returns information about if and how a Decl
+  /// 

[PATCH] D71714: [Sema] Fix -Warray-bounds false negative when casting an out-of-bounds array item

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

In D71714#1795313 , @ilya wrote:

> While I agree with the general notion about the flaws with the current 
> implementation, I feel that the more major refactoring proposed in this 
> review is out of the scope of this minor fix.


I'm OK with making the minor fix here and deferring the larger rework.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71714



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

[PATCH] D71714: [Sema] Fix -Warray-bounds false negative when casting an out-of-bounds array item

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.



Comment at: clang/lib/Sema/SemaChecking.cpp:13384
   case Stmt::MemberExprClass: {
 expr = cast(expr)->getBase();
 break;

ilya wrote:
> rsmith wrote:
> > Hmm, don't we need to do different things for dot and arrow in this case?
> There are several test cases for an out of bounds access on an array member 
> using dot and arrow operators in array-bounds.cpp. Do you have a specific 
> test case for which you think the code is broken?
> There are several test cases for an out of bounds access on an array member 
> using dot and arrow operators in array-bounds.cpp. Do you have a specific 
> test case for which you think the code is broken?

Sure. There's a false negative for this:

```
struct A { int n; };
A *a[4];
int *n = &a[4]->n;
```

... because we incorrectly visit the left-hand side of the `->` with 
`AllowOnePastEnd == 1`. The left-hand side of `->` is subject to 
lvalue-to-rvalue conversion, so can't be one-past-the-end regardless of the 
context in which the `->` appears.



Comment at: clang/test/SemaCXX/array-bounds.cpp:331
+  Base baseArr[2]; // expected-note {{array 'baseArr' declared here}}
+  Derived *d1 = dynamic_cast(&baseArr[2]); // no warning for 
one-past-end element's address retrieval
+  Derived &d2 = dynamic_cast(baseArr[2]); // expected-warning 
{{array index 2 is past the end of the array (which contains 2 elements)}}

This case should warn; `dynamic_cast` will access the object's vptr. Please at 
least add a FIXME.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71714



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

[clang] 7b518dc - [OPENMP50]Support lastprivate conditional updates in inc/dec unary ops.

[Alexey Bataev via cfe-commits]

Author: Alexey Bataev
Date: 2020-01-06T16:37:01-05:00
New Revision: 7b518dcb291e740c3e957d93c2b4046bc8a97f00

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

LOG: [OPENMP50]Support lastprivate conditional updates in inc/dec unary ops.

Added support for checking of updates of variables used in unary
pre(pos) inc/dec expressions.

Added: 


Modified: 
clang/lib/CodeGen/CGExpr.cpp
clang/lib/CodeGen/CGExprScalar.cpp
clang/lib/CodeGen/CGOpenMPRuntime.cpp
clang/lib/CodeGen/CGOpenMPRuntime.h
clang/lib/CodeGen/CGStmtOpenMP.cpp
clang/test/OpenMP/for_lastprivate_codegen.cpp
clang/test/OpenMP/sections_lastprivate_codegen.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index e43ed5030bc2..c1a0d5639d12 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -1011,6 +1011,9 @@ EmitComplexPrePostIncDec(const UnaryOperator *E, LValue 
LV,
 
   // Store the updated result through the lvalue.
   EmitStoreOfComplex(IncVal, LV, /*init*/ false);
+  if (getLangOpts().OpenMP)
+CGM.getOpenMPRuntime().checkAndEmitLastprivateConditional(*this,
+  E->getSubExpr());
 
   // If this is a postinc, return the value read from memory, otherwise use the
   // updated value.

diff  --git a/clang/lib/CodeGen/CGExprScalar.cpp 
b/clang/lib/CodeGen/CGExprScalar.cpp
index d759d3682cef..4adaca8ae571 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -2356,10 +2356,29 @@ llvm::Value 
*ScalarExprEmitter::EmitIncDecConsiderOverflowBehavior(
   llvm_unreachable("Unknown SignedOverflowBehaviorTy");
 }
 
+namespace {
+/// Handles check and update for lastprivate conditional variables.
+class OMPLastprivateConditionalUpdateRAII {
+private:
+  CodeGenFunction &CGF;
+  const UnaryOperator *E;
+
+public:
+  OMPLastprivateConditionalUpdateRAII(CodeGenFunction &CGF,
+  const UnaryOperator *E)
+  : CGF(CGF), E(E) {}
+  ~OMPLastprivateConditionalUpdateRAII() {
+if (CGF.getLangOpts().OpenMP)
+  CGF.CGM.getOpenMPRuntime().checkAndEmitLastprivateConditional(
+  CGF, E->getSubExpr());
+  }
+};
+} // namespace
+
 llvm::Value *
 ScalarExprEmitter::EmitScalarPrePostIncDec(const UnaryOperator *E, LValue LV,
bool isInc, bool isPre) {
-
+  OMPLastprivateConditionalUpdateRAII OMPRegion(CGF, E);
   QualType type = E->getSubExpr()->getType();
   llvm::PHINode *atomicPHI = nullptr;
   llvm::Value *value;

diff  --git a/clang/lib/CodeGen/CGOpenMPRuntime.cpp 
b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
index 735cacf0b7d1..b3b16befeea4 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -11447,14 +11447,6 @@ 
CGOpenMPRuntime::LastprivateConditionalRAII::LastprivateConditionalRAII(
   OS << "$pl_cond_" << ID.getDevice() << "_" << ID.getFile() << "_"
  << PLoc.getLine() << "_" << PLoc.getColumn() << "$iv";
   Data.IVName = OS.str();
-
-  // Global loop counter. Required to handle inner parallel-for regions.
-  // global_iv = &iv;
-  QualType PtrIVTy = CGM.getContext().getPointerType(IVLVal.getType());
-  Address GlobIVAddr = CGM.getOpenMPRuntime().getAddrOfArtificialThreadPrivate(
-  CGF, PtrIVTy, Data.IVName);
-  LValue GlobIVLVal = CGF.MakeAddrLValue(GlobIVAddr, PtrIVTy);
-  CGF.EmitStoreOfScalar(IVLVal.getPointer(CGF), GlobIVLVal);
 }
 
 CGOpenMPRuntime::LastprivateConditionalRAII::~LastprivateConditionalRAII() {
@@ -11463,6 +11455,27 @@ 
CGOpenMPRuntime::LastprivateConditionalRAII::~LastprivateConditionalRAII() {
   CGM.getOpenMPRuntime().LastprivateConditionalStack.pop_back();
 }
 
+void CGOpenMPRuntime::initLastprivateConditionalCounter(
+CodeGenFunction &CGF, const OMPExecutableDirective &S) {
+  if (CGM.getLangOpts().OpenMPSimd ||
+  !llvm::any_of(S.getClausesOfKind(),
+[](const OMPLastprivateClause *C) {
+  return C->getKind() == OMPC_LASTPRIVATE_conditional;
+}))
+return;
+  const CGOpenMPRuntime::LastprivateConditionalData &Data =
+  LastprivateConditionalStack.back();
+  if (Data.UseOriginalIV)
+return;
+  // Global loop counter. Required to handle inner parallel-for regions.
+  // global_iv = iv;
+  Address GlobIVAddr = CGM.getOpenMPRuntime().getAddrOfArtificialThreadPrivate(
+  CGF, Data.IVLVal.getType(), Data.IVName);
+  LValue GlobIVLVal = CGF.MakeAddrLValue(GlobIVAddr, Data.IVLVal.getType());
+  llvm::Value *IVVal = CGF.EmitLoadOfScalar(Data.IVLVal, S.getBeginLoc());
+  CGF.EmitStoreOfScalar(IVVal, GlobIVLVal);
+}
+
 namespace {
 /// Checks if the lastprivate conditional variable is referenced in LHS.
 class

[PATCH] D72097: [LifetimeAnalysis] Do not forbid void deref type in gsl::Pointer/gsl::Owner annotations

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM!


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

https://reviews.llvm.org/D72097



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

[clang] 02f694b - [NFC] Test commit, revert whitespace change

[via cfe-commits]

Author: stevewan
Date: 2020-01-06T16:28:13-05:00
New Revision: 02f694b69a8b30db7b5d43670da5ab3b9f31bb81

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

LOG:  [NFC] Test commit, revert whitespace change

As per the Developer Policy, upon obtaining commit access.

Added: 


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

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/AIX.cpp 
b/clang/lib/Driver/ToolChains/AIX.cpp
index e8d2a14f960f..6fbff61f7656 100644
--- a/clang/lib/Driver/ToolChains/AIX.cpp
+++ b/clang/lib/Driver/ToolChains/AIX.cpp
@@ -6,7 +6,6 @@
 //
 
//===--===//
 
-
 #include "AIX.h"
 #include "Arch/PPC.h"
 #include "CommonArgs.h"



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

[clang] b73fea6 - [NFC] Test commit, whitespace change

[via cfe-commits]

Author: stevewan
Date: 2020-01-06T16:24:27-05:00
New Revision: b73fea6a7cfd87fe07b9c05ba153042198b5d873

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

LOG: [NFC] Test commit, whitespace change

As per the Developer Policy, upon obtaining commit access.

Added: 


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

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/AIX.cpp 
b/clang/lib/Driver/ToolChains/AIX.cpp
index 6fbff61f7656..e8d2a14f960f 100644
--- a/clang/lib/Driver/ToolChains/AIX.cpp
+++ b/clang/lib/Driver/ToolChains/AIX.cpp
@@ -6,6 +6,7 @@
 //
 
//===--===//
 
+
 #include "AIX.h"
 #include "Arch/PPC.h"
 #include "CommonArgs.h"



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

[PATCH] D72282: [clang-tidy] Add `bugprone-unintended-adl`

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.



Comment at: clang-tools-extra/clang-tidy/bugprone/UnintendedADLCheck.cpp:43
+  Whitelist(
+  utils::options::parseStringList(Options.get("Whitelist", "swap"))) {}
+

do you mean `std::swap`? If you it should be fully qualified.
Doesn't `std::error_code` rely on adl, too? I think `std::cout <<` and other 
streams of the STL rely on it too, and probably many more code-constructs that 
are commonly used.

That means, the list should be extended to at least all standard-library 
facilities that basically required ADL to work. And then we need data on 
different code bases (e.g. LLVM is a good start) how much noise gets generated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72282



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

[PATCH] D72284: [clang-tidy] Factor out renaming logic from readability-identifier-naming

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

I guess all the tests run? 
I think this LGTM after the nits are adressed.




Comment at: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp:203
+  Decl = Ref.getDecl();
+}
+

What about the else? If it is unreachable it should have a 
`llvm_unreachable("Reason");`, if not, i think a little fallthrough comment 
wouldn't hurt.



Comment at: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp:259
+
+// Fix type aliases in value declarations
+if (const auto *Value = Result.Nodes.getNodeAs("decl")) {

Please add punctuation here and in the comments below.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72284



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

[PATCH] D72053: [RFC] Handling implementation limits

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.



Comment at: clang/include/clang/AST/Decl.h:903
 /// declared.
-unsigned ScopeDepthOrObjCQuals : NumScopeDepthOrObjCQualsBits;
+unsigned ScopeDepthOrObjCQuals : IQ::NumScopeDepthOrObjCQualsBits;
 

These bitfields are assumed to fit into an `unsigned`, and this can no longer 
be verified locally (changes to the `.td` file can break this assumption). 
Please add a `static_assert(sizeof(ParmVarDeclBitfields) <= sizeof(unsigned), 
"...")` next to the `union` below (and likewise for `NonParmVarDeclBitfields`).



Comment at: clang/include/clang/AST/Decl.h:903
 /// declared.
-unsigned ScopeDepthOrObjCQuals : NumScopeDepthOrObjCQualsBits;
+unsigned ScopeDepthOrObjCQuals : IQ::NumScopeDepthOrObjCQualsBits;
 

rsmith wrote:
> These bitfields are assumed to fit into an `unsigned`, and this can no longer 
> be verified locally (changes to the `.td` file can break this assumption). 
> Please add a `static_assert(sizeof(ParmVarDeclBitfields) <= sizeof(unsigned), 
> "...")` next to the `union` below (and likewise for 
> `NonParmVarDeclBitfields`).
I think `IQ` is too terse. We don't need a really short name for this; how 
about `ImpLimits::` or similar?



Comment at: clang/include/clang/Basic/ImplementationQuantities.td:107-111
+  bit isBitLimit = 0;
+  bit isValueLimit = 0;
+  bit isCompilerFlagLimit = 0;
+  bit isDescriptionLimit = 0;
+  bit hasRemark = 0;

Switching between upper- and lower-case names for these fields seems 
surprising. Please pick a consistent capitalization scheme.



Comment at: clang/lib/CodeGen/CGRecordLayout.h:72
   /// The total size of the bit-field, in bits.
-  unsigned Size : 15;
+  unsigned Size : IQ::BitFieldBits;
 

These three bitfields no longer obviously fit in 32 bits. Please keep the 
hard-coded 15 here and add an adjacent `static_assert(IQ::BitFieldBits <= 15, 
"...")` instead, or use some similar strategy so that the bit-field allocation 
and layout remains locally obvious.


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

https://reviews.llvm.org/D72053



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

[PATCH] D72121: [clang-tidy] Fix readability-identifier-naming missing member variables

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-member-decl-usage.cpp:137
+}
+}; // namespace CtorInits

I think this is one more test case to add: base class specifiers. e.g.,
```
struct foo_bar {}; // Should be renamed

struct Baz : foo_bar {}; // Should rename foo_bar
```


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

https://reviews.llvm.org/D72121



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

[PATCH] D70926: [clang-format] Add option for not breaking line before ObjC params

[Kanglei Fang via Phabricator via cfe-commits]

ghvg1313 added a comment.

In D70926#1790848 , @MyDeveloperDay 
wrote:

> Thanks for the patch.


Happy new year!
Could you help landing this diff? I don't know exactly what to do from here


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70926



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

[PATCH] D70689: [analyzer] Fix SARIF column locations

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM! I think we've had sufficient time for other reviewers to lodge concerns 
and we can deal with any other issues post-commit. Do you need me to commit on 
your behalf?




Comment at: clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp:157
+  const MemoryBuffer *Buf = SM.getBuffer(LocInfo.first);
+  assert(Buf && "got a NULL buffer for the location's file");
+  assert(Buf->getBufferSize() >= (LocInfo.second + TokenLen) &&

jranieri-grammatech wrote:
> aaron.ballman wrote:
> > I don't think this API is able to return a null buffer, but it can return 
> > an invalid buffer under legitimate circumstances, I believe. For instance, 
> > if the source location is in the scratch space used for things like macros 
> > defined on the command line with `-D`. I think you should pass the optional 
> > `invalid` parameter to the call to `getBuffer()` and then do no adjustments 
> > if the buffer is invalid. WDYT?
> Good catch that this can't return NULL.
> 
> I tested out defining things on the command line, but this code deals with 
> expansion locations and wasn't affected by it. Unless you can think of a way 
> to reproduce it, I think I'd rather pass in the `invalid` argument and assert 
> that it isn't invalid.
> I think I'd rather pass in the invalid argument and assert that it isn't 
> invalid.

That's fine by me -- I can't think of another way to reproduce it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70689



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

[PATCH] D72289: [analyzer] Update help text to reflect sarif support

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM, thank you for this!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72289



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

[PATCH] D72233: Add a new AST matcher 'optionally'.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Just to make sure I understand the purpose -- the goal here is to optionally 
match one or more inner matchers without failing even if none of the inner 
matchers match anything, and this is a different use case than `anyOf()` 
because that would fail when none of the inner matchers match?




Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:2538
+/// Usable as: Any Matcher
+extern const internal::VariadicOperatorMatcherFunc<
+1, std::numeric_limits::max()>

This needs to be registered in the dynamic matcher registry (Registry.cpp) as 
well. Also, you should regenerate the documentation by running 
`clang\docs\tools\dump_ast_matchers.py`.

I think this matcher could use some example uses in the documentation as well.



Comment at: clang/lib/ASTMatchers/ASTMatchersInternal.cpp:362
+BoundNodesTreeBuilder BuilderInner(*Builder);
+if (InnerMatcher.matches(DynNode, Finder, &BuilderInner)) {
+  Result.addMatch(BuilderInner);

You can elide the braces here.


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

https://reviews.llvm.org/D72233



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

[PATCH] D69878: Consoldiate internal denormal flushing controls

[Andy Kaylor via Phabricator via cfe-commits]

andrew.w.kaylor added inline comments.



Comment at: llvm/docs/LangRef.rst:1837
+   are present, this overrides ``"denormal-fp-math"``. Not all targets
+   support separately setting the denormal mode per type.
+

arsenm wrote:
> andrew.w.kaylor wrote:
> > Can you document which targets do support the option? What happens if I try 
> > to use the option on a target where it is not supported?
> I'm not sure where to document this, or if/how/where to diagnose it. I don't 
> think the high level LangRef description is the right place to discuss 
> specific target handling.
> 
> Currently it won't error or anything. Code checking the denorm mode will see 
> the f32 specific mode, even if the target in the end isn't really going to 
> respect this.
> 
> One problem is this potentially does require coordination with other 
> toolchain components. For AMDGPU, the compiler can directly tell the driver 
> what FP mode to set on each entry point, but for x86 it requires linking in 
> crtfastmath to set the default mode bits. If another target had a similar 
> runtime environment requirement, I don't think we can be sure the attribute 
> is correct or not.
There is precedent for describing target-specific behavior in LangRef. It just 
doesn't seem useful to say that not all targets support the attribute without 
saying which ones do. We should also say what is expected if a target doesn't 
support the attribute. It seems reasonable for the function attribute to be 
silently ignored.

> One problem is this potentially does require coordination with other 
> toolchain components. For AMDGPU, the compiler can directly tell the driver 
> what FP mode to set on each entry point, but for x86 it requires linking in 
> crtfastmath to set the default mode bits.

This is a point I'm interested in. I don't like the current crtfastmath.o 
handling. It feels almost accidental when FTZ works as expected. My 
understanding is we link crtfastmath.o if we find it but if not everything just 
goes about its business. The Intel compiler injects code into main() to 
explicitly set the FTZ/DAZ control modes. That obviously has problems too, but 
it's at least consistent and predictable. As I understand it, crtfastmath.o 
sets these modes from a static initializer, but I'm not sure anything is done 
to determine the order of that initializer relative to others.

How does the compiler identify entry points for AMDGPU? And does it emit code 
to set FTZ based on the function attribute here?


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

https://reviews.llvm.org/D69878



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

[PATCH] D72221: Support function attribute patchable_function_entry

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:686
 
+def PatchableFunctionEntry : InheritableAttr {
+  let Spellings = [GCC<"patchable_function_entry">];

Should this be inheriting from `TargetSpecificAttr` as well given that the 
attribute is only supported on some architectures?



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:4923
+  const llvm::Triple &T = S.Context.getTargetInfo().getTriple();
+  if (!T.isAArch64() && T.getArch() != llvm::Triple::x86 &&
+  T.getArch() != llvm::Triple::x86_64) {

If you make the attribute target-specific, this code can be removed.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:4930
+  uint32_t Size = 0, Start = 0;
+  if (AL.getNumArgs()) {
+if (!checkUInt32Argument(S, AL, AL.getArgAsExpr(0), Size, 0, true))

Should not be a need to check this -- it will always have at least one argument 
because the common attribute handling code will diagnose otherwise given the 
subject list.



Comment at: clang/test/CodeGen/patchable-function-entry.c:20
+// CHECK: attributes #0 = { {{.*}} "patchable-function-entry"="0,0"
+// CHECK: attributes #1 = { {{.*}} "patchable-function-entry"="2,0"

Can you also add a test verifying that this attribute works with redeclarations 
as well? Something like:
```
[[gnu::patchable_function_entry(12, 4)]] void func(void);

void func(void) { // Definition should have the noop sled
}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72221



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

[PATCH] D68101: [MC][ELF] Prevent globals with an explicit section from being mergeable

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

In D68101#1806213 , @rjmccall wrote:

> In D68101#1806135 , @nickdesaulniers 
> wrote:
>
> > In D68101#1802280 , @rjmccall 
> > wrote:
> >
> > > I laid out a series of three options before:
> > >
> > > - Emit different object-file sections for different mergeability settings.
> > > - Only mark an object-file section as mergeable if all the symbols in it 
> > > would have the same mergeability settings.
> > > - Stop implicitly using mergeability for "ordinary" sections (i.e. 
> > > sections other than the string section).
> >
> >
> > Of the above, I really think #2 is the only complete solution.
>
>
> Can you explain why you think #1 is not "complete"?  All three seem to 
> establish correctness; I can see how giving up on the optimization (#3) is 
> not a great solution, but #1 doesn't have that problem and actually preserves 
> it in more places.  To be clear, this is just taking advantage of the ability 
> to have multiple sections with the same name in an ELF object file; they will 
> still be assembled into a single section in the linked image.
>
> My understanding is that changing the flags on an MCSection retroactively is 
> pretty counter to the architecture, so I'm not sure how we'd actually achieve 
> #2.


Ah, ok, I reread  https://reviews.llvm.org/D72194 and see that it's creating 
non-contiguous sections (option 1), with unique entity sizes.  That should be 
fine. Dissenting opinion retracted.  We should prefer 
https://reviews.llvm.org/D72194 with the commit message updated.


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

https://reviews.llvm.org/D68101



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

[PATCH] D72282: [clang-tidy] Add `bugprone-unintended-adl`

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/bugprone-unintended-adl.rst:39
+
+   Semicolon-separated list of names that the check ignores. Default is `swap`.

logan-5 wrote:
> Eugene.Zelenko wrote:
> > Indentation. Please use double back-ticks for swap (or std::swap?).
> Since the 'swap' here is referring to the default value of the option string, 
> shouldn't it just be single backticks?
It was about swap above, not about option default value.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72282



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

[PATCH] D72282: [clang-tidy] Add `bugprone-unintended-adl`

[Logan Smith via Phabricator via cfe-commits]

logan-5 updated this revision to Diff 236420.
logan-5 marked 5 inline comments as done.
logan-5 added a comment.

Addressed some formatting stuff.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72282

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/UnintendedADLCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/UnintendedADLCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone-unintended-adl.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone-unintended-adl.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-unintended-adl.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-unintended-adl.cpp
@@ -0,0 +1,75 @@
+// RUN: %check_clang_tidy %s bugprone-unintended-adl %t
+
+namespace aspace {
+struct A {};
+void func(const A &);
+} // namespace aspace
+
+namespace bspace {
+void func(int);
+void test() {
+  aspace::A a;
+  func(5);
+  func(a);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: expression calls 'aspace::func' through ADL [bugprone-unintended-adl]
+}
+} // namespace bspace
+
+namespace ops {
+struct Stream {
+} stream;
+Stream &operator<<(Stream &s, int) {
+  return s;
+}
+void smooth_operator(Stream);
+} // namespace ops
+
+void test() {
+  ops::stream << 5;
+  operator<<(ops::stream, 5);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: expression calls 'ops::operator<<' through ADL [bugprone-unintended-adl]
+  smooth_operator(ops::stream);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: expression calls 'ops::smooth_operator' through ADL [bugprone-unintended-adl]
+}
+
+namespace ns {
+struct Swappable {};
+void swap(Swappable &a, Swappable &b); // whitelisted by default
+
+void move(Swappable) {} // non-whitelisted
+template 
+void forward(T) {} // non-whitelisted
+
+struct Swappable2 {};
+} // namespace ns
+
+void move(ns::Swappable2);
+auto ref = [](ns::Swappable2) {};
+
+void test2() {
+  ns::Swappable a, b;
+  using namespace std;
+  swap(a, b);
+  move(a);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: expression calls 'ns::move' through ADL [bugprone-unintended-adl]
+  forward(a);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: expression calls 'ns::forward' through ADL [bugprone-unintended-adl]
+}
+
+template 
+void templateFunction(T t) {
+  swap(t, t);
+
+  move(t);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: unqualified call to 'move' may be resolved through ADL [bugprone-unintended-adl]
+
+  ns::move(t);
+  ::move(t);
+
+  ref(t);
+  ::ref(t);
+
+  t << 5;
+  operator<<(t, 5);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: unqualified call to 'operator<<' may be resolved through ADL [bugprone-unintended-adl]
+}
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone-unintended-adl.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone-unintended-adl.rst
@@ -0,0 +1,47 @@
+.. title:: clang-tidy - bugprone-unintended-adl
+
+bugprone-unintended-adl
+===
+
+Finds usages of ADL (argument-dependent lookup), or potential ADL in the case 
+of templates, that are not on the provided whitelist.
+
+.. code-block:: c++
+
+  namespace aspace {
+  struct A {};
+  void func(const A &);
+  } // namespace aspace
+  
+  namespace bspace {
+  void func(int);
+  void test() {
+aspace::A a;
+func(5);
+func(a); // calls 'aspace::func' through ADL
+  }
+  } // namespace bspace
+
+(Example respectfully borrowed from `Abseil TotW #49 `_.)
+
+ADL can be surprising, and can lead to `subtle bugs 
+`_ without the utmost attention. 
+However, it very is useful for lookup of overloaded operators, and for 
+customization points within libraries (e.g. ``swap`` in the C++ standard 
+library). As such, this check can be configured to ignore calls to overloaded 
+operators as well as other legitimate uses of ADL specified in a whitelist.
+
+This check does not suggest any fixes.
+
+Options
+---
+
+.. option:: IgnoreOverloadedOperators
+
+   If non-zero, ignores calls to overloaded operators using operator syntax 
+   (e.g. ``a + b``), but not function call syntax (e.g. ``operator+(a, b)``). 
+   Default is `1`.
+
+.. option:: Whitelist
+
+   Semicolon-separated list of names that the check ignores. Default is `swap`.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -104,6 +104,12 @@
   
   Violating the naming rules above results in undefined behavior.
 
+- New :doc:`bugprone-unintended-adl
+  ` check.
+
+  Finds usages of ADL (argument-

[PATCH] D71227: [cuda][hip] Fix function overload resolution in the global initiailizer.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.



Comment at: clang/include/clang/Sema/Sema.h:11190
   /// combination, based on their host/device attributes.
-  /// \param Caller function which needs address of \p Callee.
-  ///   nullptr in case of global context.
-  /// \param Callee target function
+  /// \param CallContextDecl the context decl which needs address of \p Callee.
+  ///nullptr in case of global context.

Please capitalize the first word of each of these parameter descriptions, to 
match the style used elsewhere in Clang.



Comment at: clang/include/clang/Sema/Sema.h:11191
+  /// \param CallContextDecl the context decl which needs address of \p Callee.
+  ///nullptr in case of global context.
+  /// \param Callee  target function

"Null" not "nullptr".



Comment at: clang/include/clang/Sema/Sema.h:11198-11206
+  SmallVector CUDANonLocalVariableStack;
+
+  void pushCUDANonLocalVariable(const Decl *D);
+  void popCUDANonLocalVariable(const Decl *D);
+
+  const Decl *getCUDACurrentNonLocalVariable() const {
+return CUDANonLocalVariableStack.empty() ? nullptr

Does this really need to be CUDA-specific?

This is (at least) the third time we've needed this. We currently have a 
`ManglingContextDecl` on `ExpressionEvaluationContextRecord` that tracks the 
non-local variable whose initializer we're parsing. In addition to using this 
as a lambda context declaration, we also (hackily) use it as the context 
declaration for `DiagRuntimeBehavior`. It would seem sensible to use that 
mechanism here too (and rename it to remove any suggestion that this is 
specific to lambdas or mangling).

I think we only currently push `ExpressionEvaluationContext`s for variable 
initializers in C++. That's presumably fine for CUDA's purposes.



Comment at: clang/lib/Parse/ParseDecl.cpp:2336
 
+  Actions.pushCUDANonLocalVariable(ThisDecl);
+

tra wrote:
> @rsmith -- is this sufficient to catch all attempts to call an initializer 
> for a global?
> I wonder if there are other sneaky ways to call an initializer. 
No, this is not sufficient; it's missing (at least) the template instantiation 
case. (The `ExpressionEvaluationContextRecord` mechanism does handle that case 
properly.)

You should also consider what should happen in default arguments (which are 
sometimes parsed before we form a `FunctionDecl` for the function for which 
they are parameters) and default member initializers (which are parsed after we 
know whether the enclosing class has a user-declared default constructor, so 
you could in principle consider the CUDA function kind of the declared 
constructors, I suppose -- but the constructor bodies are not yet available, so 
you can't tell which constructors would actually use the initializers). Both of 
those cases are also tracked by the `ExpressionEvaluationContextRecord` 
mechanism, though you may need to track additional information to process 
default arguments in the same mode as the function for which they are supplied.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71227



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

[PATCH] D72282: [clang-tidy] Add `bugprone-unintended-adl`

[Logan Smith via Phabricator via cfe-commits]

logan-5 added inline comments.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/bugprone-unintended-adl.rst:35
+
+   If non-zero, ignores calls to overloaded operators using the operator 
syntax (e.g. `a + b`), but not the function call syntax (e.g. `operator+(a, 
b)`). Default is `1`.
+

Eugene.Zelenko wrote:
> Indentation. Please use double back-ticks for a + b and operator+(a, b).
I'm not sure what you would like me to change about the indentation. This 
follows the same format (three spaces) I've found in the documentation for 
other checks.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/bugprone-unintended-adl.rst:39
+
+   Semicolon-separated list of names that the check ignores. Default is `swap`.

Eugene.Zelenko wrote:
> Indentation. Please use double back-ticks for swap (or std::swap?).
Since the 'swap' here is referring to the default value of the option string, 
shouldn't it just be single backticks?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72282



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

[PATCH] D69878: Consoldiate internal denormal flushing controls

[Matt Arsenault via Phabricator via cfe-commits]

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



Comment at: llvm/docs/LangRef.rst:1837
+   are present, this overrides ``"denormal-fp-math"``. Not all targets
+   support separately setting the denormal mode per type.
+

andrew.w.kaylor wrote:
> Can you document which targets do support the option? What happens if I try 
> to use the option on a target where it is not supported?
I'm not sure where to document this, or if/how/where to diagnose it. I don't 
think the high level LangRef description is the right place to discuss specific 
target handling.

Currently it won't error or anything. Code checking the denorm mode will see 
the f32 specific mode, even if the target in the end isn't really going to 
respect this.

One problem is this potentially does require coordination with other toolchain 
components. For AMDGPU, the compiler can directly tell the driver what FP mode 
to set on each entry point, but for x86 it requires linking in crtfastmath to 
set the default mode bits. If another target had a similar runtime environment 
requirement, I don't think we can be sure the attribute is correct or not.


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

https://reviews.llvm.org/D69878



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

[PATCH] D68720: Support -fstack-clash-protection for x86

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: clang/docs/ClangCommandLineReference.rst:1903
 
+.. option:: -fstack-clash-protection
+

Probably need a -fno-stack-class-protection as well. Looks like gcc has it. 
You'll need to update the handling in the driver to make sure the last polarity 
wins. For cc1 you might be able to support just the positive variant. But try 
to see what we usually do.



Comment at: clang/lib/Basic/Targets/X86.h:152
 
+  const char *getSPRegName() const override { return "rsp"; }
+

What about 32-bit mode where the register name is "esp"?



Comment at: clang/lib/CodeGen/CGStmt.cpp:2254
+CGM.getDiags().Report(S.getAsmLoc(),
+  diag::warn_fe_stack_clash_protection_inline_asm);
+  }

Why is this in the frontend diagnostic list?



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2573
+
+  switch (EffectiveTriple.getArch()) {
+  default:

Can we use EffectiveTriple.isX86() that was just introduced yesterday?



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2582
+  for (const Arg *A : Args) {
+switch (A->getOption().getID()) {
+default:

Seems like this should just be an if? Or maybe use Args.filtered or args 
getLastArg?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68720



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

[PATCH] D72221: Support function attribute patchable_function_entry

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:826
+  "patchable-function-entry",
+  (Twine(Attr->getSize()) + "," + Twine(Attr->getStart())).str());
   }

ostannard wrote:
> I think using two function attributes here would be better, to avoid needing 
> to parse this again later.
In that case, it would not make sense to have start without a size, and thus 
should be checked for in the verification.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72221



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

[PATCH] D72282: [clang-tidy] Add `bugprone-unintended-adl`

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tools-extra/clang-tidy/bugprone/UnintendedADLCheck.h:13
+#include "../ClangTidyCheck.h"
+
+#include 

Unnecessary empty line.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/bugprone-unintended-adl.rst:6
+
+Finds usages of ADL (argument-dependent lookup), or potential ADL in the case 
of templates, that are not on the provided whitelist.
+

Please follow 80 character length limit. Same below.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/bugprone-unintended-adl.rst:26
+
+ADL can be surprising, and can lead to `subtle bugs 
`_ without the utmost attention. 
However, it very is useful for lookup of overloaded operators, and for 
customization points within libraries (e.g. `swap` in the C++ standard 
library). As such, this check can be configured to ignore calls to overloaded 
operators as well as other legitimate uses of ADL specified in a whitelist.
+

Please use double back-ticks for swap (or std::swap?).



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/bugprone-unintended-adl.rst:35
+
+   If non-zero, ignores calls to overloaded operators using the operator 
syntax (e.g. `a + b`), but not the function call syntax (e.g. `operator+(a, 
b)`). Default is `1`.
+

Indentation. Please use double back-ticks for a + b and operator+(a, b).



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/bugprone-unintended-adl.rst:39
+
+   Semicolon-separated list of names that the check ignores. Default is `swap`.

Indentation. Please use double back-ticks for swap (or std::swap?).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72282



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

[PATCH] D72289: [analyzer] Update help text to reflect sarif support

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun created this revision.
xazax.hun added reviewers: aaron.ballman, NoQ.
xazax.hun added a project: clang.
Herald added subscribers: Charusso, gamesh411, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72289

Files:
  clang/include/clang/Driver/Options.td


Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -2858,7 +2858,7 @@
 def _all_warnings : Flag<["--"], "all-warnings">, Alias;
 def _analyzer_no_default_checks : Flag<["--"], "analyzer-no-default-checks">, 
Flags<[DriverOption]>;
 def _analyzer_output : JoinedOrSeparate<["--"], "analyzer-output">, 
Flags<[DriverOption]>,
-  HelpText<"Static analyzer report output format 
(html|plist|plist-multi-file|plist-html|text).">;
+  HelpText<"Static analyzer report output format 
(html|plist|plist-multi-file|plist-html|sarif|text).">;
 def _analyze : Flag<["--"], "analyze">, Flags<[DriverOption, CoreOption]>,
   HelpText<"Run the static analyzer">;
 def _assemble : Flag<["--"], "assemble">, Alias;


Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -2858,7 +2858,7 @@
 def _all_warnings : Flag<["--"], "all-warnings">, Alias;
 def _analyzer_no_default_checks : Flag<["--"], "analyzer-no-default-checks">, Flags<[DriverOption]>;
 def _analyzer_output : JoinedOrSeparate<["--"], "analyzer-output">, Flags<[DriverOption]>,
-  HelpText<"Static analyzer report output format (html|plist|plist-multi-file|plist-html|text).">;
+  HelpText<"Static analyzer report output format (html|plist|plist-multi-file|plist-html|sarif|text).">;
 def _analyze : Flag<["--"], "analyze">, Flags<[DriverOption, CoreOption]>,
   HelpText<"Run the static analyzer">;
 def _assemble : Flag<["--"], "assemble">, Alias;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71848: Allow the discovery of Android NDK's triple-prefixed binaries.

[Brian Ledger via Phabricator via cfe-commits]

brianpl added a comment.

> Just to clarify, this is needed for the triple-prefixed tools, but the 
> triple-specific directory worked fine before this patch? If I'm understanding 
> that correctly then LGTM, otherwise I'm confused as to why I haven't seen 
> this problem before.

Yes, that's right. Without the patch, Clang will overlook the triple-prefixed 
tools, but it will correctly find tools in the triple-specific directory either 
way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71848



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

[PATCH] D72221: Support function attribute patchable_function_entry

[Nick Desaulniers via Phabricator via cfe-commits]

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



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:4925
+  T.getArch() != llvm::Triple::x86_64) {
+S.Diag(getAttrLoc(AL), diag::err_attribute_unsupported) << AL;
+return;

nickdesaulniers wrote:
> Why is the target arch also checked in 
> `clang/lib/Driver/ToolChains/Clang.cpp` in https://reviews.llvm.org/D7?
Ah, https://reviews.llvm.org/D72221 is checking the `__attribute__` syntax, 
https://reviews.llvm.org/D7 is checking the command line `-f` syntax.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72221



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

[PATCH] D69878: Consoldiate internal denormal flushing controls

[Andy Kaylor via Phabricator via cfe-commits]

andrew.w.kaylor added inline comments.



Comment at: llvm/docs/LangRef.rst:1837
+   are present, this overrides ``"denormal-fp-math"``. Not all targets
+   support separately setting the denormal mode per type.
+

Can you document which targets do support the option? What happens if I try to 
use the option on a target where it is not supported?


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

https://reviews.llvm.org/D69878



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

[PATCH] D72221: Support function attribute patchable_function_entry

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:4925
+  T.getArch() != llvm::Triple::x86_64) {
+S.Diag(getAttrLoc(AL), diag::err_attribute_unsupported) << AL;
+return;

Why is the target arch also checked in `clang/lib/Driver/ToolChains/Clang.cpp` 
in https://reviews.llvm.org/D7?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72221



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

[PATCH] D72284: [clang-tidy] Factor out renaming logic from readability-identifier-naming

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp:351
+const std::string &Fixup) {
+  if (Fixup.empty()) {
+return "; cannot be fixed automatically";

Please elide braces. Same below.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72284



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

[PATCH] D71714: [Sema] Fix -Warray-bounds false negative when casting an out-of-bounds array item

[Ilya Mirsky via Phabricator via cfe-commits]

ilya added a comment.

Kind ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71714



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

[PATCH] D68101: [MC][ELF] Prevent globals with an explicit section from being mergeable

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D68101#1806135 , @nickdesaulniers 
wrote:

> In D68101#1802220 , @bd1976llvm 
> wrote:
>
> > Below is the code comment from the new patch explaining the new approach, 
> > please take a look and see if you have any questions/comments:
> >
> >   // If two globals with differing sizes end up in the same mergeable
> >   // section that section can be assigned an incorrect entry size. Normally,
> >   // the assembler avoids this by putting incompatible globals into
> >   // differently named sections. However, globals can be explicitly assigned
> >   // to a section by specifying the section name. In this case, if unique
> >   // section names are available (-unique-section-names in LLVM) then we
> >   // bin compatible globals into different mergeable sections with the same 
> > name.
>
>
> Looks good up to here.
>
> >   // Otherwise, if incompatible globals have been explicitly assigned to 
> > section by a
> >   // fine-grained/per-symbol mechanism (e.g. via  
> > _attribute_((section(“myname” then
> >   // we issue an error and the user can then change the section assignment. 
> > If the
>
> No bueno.  The Linux kernel has code where there are 2D global arrays of 
> different entity sizes explicitly placed in different sections (together). 
> GCC does not error for this, (it doesn't mark the section merge-able), 
> neither should we.
>
> In D68101#1802280 , @rjmccall wrote:
>
> > I laid out a series of three options before:
> >
> > - Emit different object-file sections for different mergeability settings.
> > - Only mark an object-file section as mergeable if all the symbols in it 
> > would have the same mergeability settings.
> > - Stop implicitly using mergeability for "ordinary" sections (i.e. sections 
> > other than the string section).
>
>
> Of the above, I really think #2 is the only complete solution.


Can you explain why you think #1 is not "complete"?  All three seem to 
establish correctness; I can see how giving up on the optimization (#3) is not 
a great solution, but #1 doesn't have that problem and actually preserves it in 
more places.  To be clear, this is just taking advantage of the ability to have 
multiple sections with the same name in an ELF object file; they will still be 
assembled into a single section in the linked image.

My understanding is that changing the flags on an MCSection retroactively is 
pretty counter to the architecture, so I'm not sure how we'd actually achieve 
#2.


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

https://reviews.llvm.org/D68101



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

[PATCH] D69876: Allow output constraints on "asm goto"

[James Y Knight via Phabricator via cfe-commits]

jyknight accepted this revision.
jyknight added a comment.

LGTM modulo some minor wording.




Comment at: clang/docs/LanguageExtensions.rst:1256-1258
+Clang provides support for the `goto form of GCC's extended
+assembly`_ with output
+constraints.

I find that sentence confusing. Maybe,
"In addition to the functionality provided by , Clang 
also supports output constraints with the goto form."



Comment at: clang/lib/AST/Stmt.cpp:646-648
+  // Labels are placed after "InOut" operands. Adjust accordingly.
+  if (IsLabel)
+N += getNumPlusOperands();

I'm confused about this part. Why isn't the "N" specified in the assembly 
string already the correct value for the labels? Is the ordering we use 
internally and that users use externally not the same? I'm assuming your code 
here is correct, just I'm not understanding, so probably an improved comment 
would be nice.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69876



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

[PATCH] D72281: [Matrix] Add matrix type to Clang (WIP).

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

{icon times-circle color=red} Unit tests: fail. 61254 tests passed, 1 failed 
and 736 were skipped.

  failed: Clang.CodeGen/pch-dllexport.cpp

{icon times-circle color=red} clang-tidy: fail. Please fix clang-tidy findings 
.

{icon times-circle color=red} clang-format: fail. Please format your changes 
with clang-format by running `git-clang-format HEAD^` or applying this patch 
.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72281



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

[PATCH] D71848: Allow the discovery of Android NDK's triple-prefixed binaries.

[Dan Albert via Phabricator via cfe-commits]

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

Just to clarify, this is needed for the triple-prefixed tools, but the 
triple-specific directory worked fine before this patch? If I'm understanding 
that correctly then LGTM, otherwise I'm confused as to why I haven't seen this 
problem before.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71848



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

[PATCH] D72271: [Clang] Handle target-specific builtins returning aggregates.

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:4333
+if (!ReturnValue.isNull() &&
+
ReturnValue.getValue().getType()->getPointerElementType()->isStructTy())
+  return RValue::getAggregate(ReturnValue.getValue(),

The requirement is that we construct an `RValue` of the right kind for the 
expression's result type, so the correct approach here is to switch over 
`getEvaluationKind(E->getType())`.  This is also a more reliable signal than 
the presence of the return value slot.

We then need to think about how the return value will be returned to us in each 
case:

- For scalars, `RValue::get(V)` is right.

- We can assert for now that there are no target builtins returning a complex 
type; if we ever need to add this, we'll probably have to break apart an IR 
struct.

- So that just leaves aggregates.  I believe we can't rely on `ReturnValue` 
being non-null on entry to this function; for example, IIRC we'll pass down a 
null slot if the code does an immediate member access into the result, e.g. 
`vld2q(...).val[0]`.  It looks like the custom logic for VLD24 does not do 
anything reasonable in this case: it returns a first-class struct, which the 
rest of IRGen will definitely not expect.  Probably the most reasonable thing 
would be to create a slot if we have an aggregate result and the caller didn't 
give us a slot (out here in `EmitBuiltinExpr`), and then we can just require 
`EmitTargetBuiltinExpr` to return null or something else that makes it clear 
that they've done the right thing with the slot that was passed in.  You can 
create a slot with `CreateMemTemp`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72271



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

[PATCH] D71314: Emit a warning if a variable is uninitialized in indirect ASM goto destination.

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: clang/lib/Analysis/UninitializedValues.cpp:856
+vals[VD] = MayUninitialized;
 }
 

Can you walk me through the logic of this function?

I would assume for changes to `asm goto`, the above early `return` would have 
been removed? Otherwise we seem to be changing the logic of the non-`asm goto` 
case?



Comment at: clang/test/Analysis/uninit-asm-goto.cpp:57
+  return y;
+indirect:
+  return -2;

nickdesaulniers wrote:
> I think if you left out `indirect`, it would be clearer what this test is 
> testing.
bump


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71314



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

[PATCH] D72213: [clang-tidy] Add `bugprone-reserved-identifier` to flag usages of reserved C++ names

[Logan Smith via Phabricator via cfe-commits]

logan-5 added a comment.

I'm taking @JonasToth's suggestion and splitting this into two patches, one for 
the refactor, followed by one for the new check. The refactor patch is here: 
https://reviews.llvm.org/D72284. Thanks everyone for your feedback so far.


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

https://reviews.llvm.org/D72213



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

[PATCH] D72284: [clang-tidy] Factor out renaming logic from readability-identifier-naming

[Logan Smith via Phabricator via cfe-commits]

logan-5 created this revision.
logan-5 added reviewers: alexfh, hokein, aaron.ballman.
logan-5 added a project: clang-tools-extra.
Herald added subscribers: cfe-commits, xazax.hun, mgorny.
Herald added a project: clang.

Before this patch, `readability-identifier-naming` contained a significant 
amount of logic for (a) checking the style of identifiers, followed by (b) 
renaming/applying fix-its. This patch factors out (b) into a separate base 
class so that it can be reused by other checks that want to do renaming. This 
also cleans up `readability-identifier-naming` significantly, since now it only 
needs to be concerned with the interesting details of (a).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72284

Files:
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
  clang-tools-extra/clang-tidy/utils/CMakeLists.txt
  clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
  clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h

Index: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h
@@ -0,0 +1,145 @@
+//===--- RenamderClangTidyCheck.h - clang-tidy --*- 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
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_RENAMERCLANGTIDYCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_RENAMERCLANGTIDYCHECK_H
+
+#include "../ClangTidyCheck.h"
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/Optional.h"
+#include 
+#include 
+
+namespace clang {
+
+class MacroInfo;
+
+namespace tidy {
+
+/// Base class for clang-tidy checks that want to flag declarations and/or
+/// macros for renaming based on customizable criteria.
+class RenamerClangTidyCheck : public ClangTidyCheck {
+public:
+  RenamerClangTidyCheck(StringRef CheckName, ClangTidyContext *Context);
+  ~RenamerClangTidyCheck();
+
+  /// Derived classes should not implement any matching logic themselves; this
+  /// class will do the matching and call the derived class'
+  /// GetDeclFailureInfo() and GetMacroFailureInfo() for determining whether a
+  /// given identifier passes or fails the check.
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override final;
+  void
+  check(const ast_matchers::MatchFinder::MatchResult &Result) override final;
+  void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
+   Preprocessor *ModuleExpanderPP) override final;
+  void onEndOfTranslationUnit() override final;
+
+  /// This enum will be used in %select of the diagnostic message.
+  /// Each value below IgnoreFailureThreshold should have an error message.
+  enum class ShouldFixStatus {
+ShouldFix,
+ConflictsWithKeyword, /// The fixup will conflict with a language keyword,
+  /// so we can't fix it automatically.
+ConflictsWithMacroDefinition, /// The fixup will conflict with a macro
+  /// definition, so we can't fix it
+  /// automatically.
+
+/// Values pass this threshold will be ignored completely
+/// i.e no message, no fixup.
+IgnoreFailureThreshold,
+
+InsideMacro, /// If the identifier was used or declared within a macro we
+ /// won't offer a fixup for safety reasons.
+  };
+
+  /// Information describing a failed check
+  struct FailureInfo {
+std::string KindName; // Tag or misc info to be used as derived classes need
+std::string Fixup;// The name that will be proposed as a fix-it hint
+  };
+
+  /// Holds an identifier name check failure, tracking the kind of the
+  /// identifier, its possible fixup and the starting locations of all the
+  /// identifier usages.
+  struct NamingCheckFailure {
+FailureInfo Info;
+
+/// Whether the failure should be fixed or not.
+///
+/// e.g.: if the identifier was used or declared within a macro we won't
+/// offer a fixup for safety reasons.
+bool ShouldFix() const {
+  return FixStatus == ShouldFixStatus::ShouldFix && !Info.Fixup.empty();
+}
+
+bool ShouldNotify() const {
+  return FixStatus < ShouldFixStatus::IgnoreFailureThreshold;
+}
+
+ShouldFixStatus FixStatus = ShouldFixStatus::ShouldFix;
+
+/// A set of all the identifier usages starting SourceLocation, in
+/// their encoded form.
+llvm::DenseSet RawUsageLocs;
+
+NamingCheckFailure() = default;
+  };
+
+  typedef std::pair NamingCheckId;
+
+  typedef llvm::DenseMap
+  NamingCheckFailureMap;
+
+  /// Che

[PATCH] D71174: [clang-tidy] new check: bugprone-signed-char-misuse

[Tamás Zolnai via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG350da402ef6b: [clang-tidy] new check: 
bugprone-signed-char-misuse (authored by ztamas).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71174

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone-signed-char-misuse.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-signed-char-misuse-fsigned-char.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-signed-char-misuse-funsigned-char.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-signed-char-misuse-with-option.cpp
  clang-tools-extra/test/clang-tidy/checkers/bugprone-signed-char-misuse.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-signed-char-misuse.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-signed-char-misuse.cpp
@@ -0,0 +1,123 @@
+// RUN: %check_clang_tidy %s bugprone-signed-char-misuse %t
+
+///
+/// Test cases correctly caught by the check.
+
+int SimpleVarDeclaration() {
+  signed char CCharacter = -5;
+  int NCharacter = CCharacter;
+  // CHECK-MESSAGES: [[@LINE-1]]:20: warning: 'signed char' to 'int' conversion; consider casting to 'unsigned char' first. [bugprone-signed-char-misuse]
+
+  return NCharacter;
+}
+
+int SimpleAssignment() {
+  signed char CCharacter = -5;
+  int NCharacter;
+  NCharacter = CCharacter;
+  // CHECK-MESSAGES: [[@LINE-1]]:16: warning: 'signed char' to 'int' conversion; consider casting to 'unsigned char' first. [bugprone-signed-char-misuse]
+
+  return NCharacter;
+}
+
+int CStyleCast() {
+  signed char CCharacter = -5;
+  int NCharacter;
+  NCharacter = (int)CCharacter;
+  // CHECK-MESSAGES: [[@LINE-1]]:21: warning: 'signed char' to 'int' conversion; consider casting to 'unsigned char' first. [bugprone-signed-char-misuse]
+
+  return NCharacter;
+}
+
+int StaticCast() {
+  signed char CCharacter = -5;
+  int NCharacter;
+  NCharacter = static_cast(CCharacter);
+  // CHECK-MESSAGES: [[@LINE-1]]:33: warning: 'signed char' to 'int' conversion; consider casting to 'unsigned char' first. [bugprone-signed-char-misuse]
+
+  return NCharacter;
+}
+
+int FunctionalCast() {
+  signed char CCharacter = -5;
+  int NCharacter;
+  NCharacter = int(CCharacter);
+  // CHECK-MESSAGES: [[@LINE-1]]:20: warning: 'signed char' to 'int' conversion; consider casting to 'unsigned char' first. [bugprone-signed-char-misuse]
+
+  return NCharacter;
+}
+
+int NegativeConstValue() {
+  const signed char CCharacter = -5;
+  int NCharacter = CCharacter;
+  // CHECK-MESSAGES: [[@LINE-1]]:20: warning: 'signed char' to 'int' conversion; consider casting to 'unsigned char' first. [bugprone-signed-char-misuse]
+
+  return NCharacter;
+}
+
+int CharPointer(signed char *CCharacter) {
+  int NCharacter = *CCharacter;
+  // CHECK-MESSAGES: [[@LINE-1]]:20: warning: 'signed char' to 'int' conversion; consider casting to 'unsigned char' first. [bugprone-signed-char-misuse]
+
+  return NCharacter;
+}
+
+///
+/// Test cases correctly ignored by the check.
+
+int UnsignedCharCast() {
+  unsigned char CCharacter = 'a';
+  int NCharacter = CCharacter;
+
+  return NCharacter;
+}
+
+int PositiveConstValue() {
+  const signed char CCharacter = 5;
+  int NCharacter = CCharacter;
+
+  return NCharacter;
+}
+
+// singed char -> integer cast is not the direct child of declaration expression.
+int DescendantCast() {
+  signed char CCharacter = 'a';
+  int NCharacter = 10 + CCharacter;
+
+  return NCharacter;
+}
+
+// singed char -> integer cast is not the direct child of assignment expression.
+int DescendantCastAssignment() {
+  signed char CCharacter = 'a';
+  int NCharacter;
+  NCharacter = 10 + CCharacter;
+
+  return NCharacter;
+}
+
+// bool is an integer type in clang; make sure to ignore it.
+bool BoolVarDeclaration() {
+  signed char CCharacter = 'a';
+  bool BCharacter = CCharacter == 'b';
+
+  return BCharacter;
+}
+
+// bool is an integer type in clang; make sure to ignore it.
+bool BoolAssignment() {
+  signed char CCharacter = 'a';
+  bool BCharacter;
+  BCharacter = CCharacter == 'b';
+
+  return BCharacter;
+}
+
+// char is an integer type in clang; make sure to ignore it.
+unsigned char CharToCharCast() {
+  signed char SCCharacter = 'a';
+  unsigned char USCharacter;
+  USCharacter = SCCharacter;
+
+  return USCharacter;
+}
Index: clang-tools-extra/test/clang-tidy/checkers/bugpro

[PATCH] D69876: Allow output constraints on "asm goto"

[Nick Desaulniers via Phabricator via cfe-commits]

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

No, thanks for the work on this @void !


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69876



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

[PATCH] D68101: [MC][ELF] Prevent globals with an explicit section from being mergeable

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

In D68101#1802220 , @bd1976llvm wrote:

> Below is the code comment from the new patch explaining the new approach, 
> please take a look and see if you have any questions/comments:
>
>   // If two globals with differing sizes end up in the same mergeable
>   // section that section can be assigned an incorrect entry size. Normally,
>   // the assembler avoids this by putting incompatible globals into
>   // differently named sections. However, globals can be explicitly assigned
>   // to a section by specifying the section name. In this case, if unique
>   // section names are available (-unique-section-names in LLVM) then we
>   // bin compatible globals into different mergeable sections with the same 
> name.


Looks good up to here.

>   // Otherwise, if incompatible globals have been explicitly assigned to 
> section by a
>   // fine-grained/per-symbol mechanism (e.g. via  
> _attribute_((section(“myname” then
>   // we issue an error and the user can then change the section assignment. 
> If the

No bueno.  The Linux kernel has code where there are 2D global arrays of 
different entity sizes explicitly placed in different sections (together). GCC 
does not error for this, (it doesn't mark the section merge-able), neither 
should we.

In D68101#1802280 , @rjmccall wrote:

> I laid out a series of three options before:
>
> - Emit different object-file sections for different mergeability settings.
> - Only mark an object-file section as mergeable if all the symbols in it 
> would have the same mergeability settings.
> - Stop implicitly using mergeability for "ordinary" sections (i.e. sections 
> other than the string section).


Of the above, I really think #2 is the only complete solution.


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

https://reviews.llvm.org/D68101



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

[PATCH] D72283: [Matrix] Add __builtin_matrix_insert to Clang (WIP).

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

{icon question-circle color=gray} Unit tests: unknown.

{icon question-circle color=gray} clang-tidy: unknown.

{icon question-circle color=gray} clang-format: unknown.

Build artifacts 
: 
diff.json 
,
 console-log.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72283



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

[PATCH] D72282: [clang-tidy] Add `bugprone-unintended-adl`

[Logan Smith via Phabricator via cfe-commits]

logan-5 created this revision.
logan-5 added reviewers: aaron.ballman, hokein, alexfh.
logan-5 added a project: clang-tools-extra.
Herald added subscribers: cfe-commits, xazax.hun, mgorny.
Herald added a project: clang.

This patch adds `bugprone-unintended-adl` which flags uses of ADL that are not 
on the provided whitelist.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72282

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/UnintendedADLCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/UnintendedADLCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone-unintended-adl.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone-unintended-adl.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-unintended-adl.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-unintended-adl.cpp
@@ -0,0 +1,75 @@
+// RUN: %check_clang_tidy %s bugprone-unintended-adl %t
+
+namespace aspace {
+struct A {};
+void func(const A &);
+} // namespace aspace
+
+namespace bspace {
+void func(int);
+void test() {
+  aspace::A a;
+  func(5);
+  func(a);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: expression calls 'aspace::func' through ADL [bugprone-unintended-adl]
+}
+} // namespace bspace
+
+namespace ops {
+struct Stream {
+} stream;
+Stream &operator<<(Stream &s, int) {
+  return s;
+}
+void smooth_operator(Stream);
+} // namespace ops
+
+void test() {
+  ops::stream << 5;
+  operator<<(ops::stream, 5);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: expression calls 'ops::operator<<' through ADL [bugprone-unintended-adl]
+  smooth_operator(ops::stream);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: expression calls 'ops::smooth_operator' through ADL [bugprone-unintended-adl]
+}
+
+namespace ns {
+struct Swappable {};
+void swap(Swappable &a, Swappable &b); // whitelisted by default
+
+void move(Swappable) {} // non-whitelisted
+template 
+void forward(T) {} // non-whitelisted
+
+struct Swappable2 {};
+} // namespace ns
+
+void move(ns::Swappable2);
+auto ref = [](ns::Swappable2) {};
+
+void test2() {
+  ns::Swappable a, b;
+  using namespace std;
+  swap(a, b);
+  move(a);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: expression calls 'ns::move' through ADL [bugprone-unintended-adl]
+  forward(a);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: expression calls 'ns::forward' through ADL [bugprone-unintended-adl]
+}
+
+template 
+void templateFunction(T t) {
+  swap(t, t);
+
+  move(t);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: unqualified call to 'move' may be resolved through ADL [bugprone-unintended-adl]
+
+  ns::move(t);
+  ::move(t);
+
+  ref(t);
+  ::ref(t);
+
+  t << 5;
+  operator<<(t, 5);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: unqualified call to 'operator<<' may be resolved through ADL [bugprone-unintended-adl]
+}
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone-unintended-adl.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone-unintended-adl.rst
@@ -0,0 +1,39 @@
+.. title:: clang-tidy - bugprone-unintended-adl
+
+bugprone-unintended-adl
+===
+
+Finds usages of ADL (argument-dependent lookup), or potential ADL in the case of templates, that are not on the provided whitelist.
+
+.. code-block:: c++
+
+  namespace aspace {
+  struct A {};
+  void func(const A &);
+  } // namespace aspace
+  
+  namespace bspace {
+  void func(int);
+  void test() {
+aspace::A a;
+func(5);
+func(a); // calls 'aspace::func' through ADL
+  }
+  } // namespace bspace
+
+(Example respectfully borrowed from `Abseil TotW #49 `_.)
+
+ADL can be surprising, and can lead to `subtle bugs `_ without the utmost attention. However, it very is useful for lookup of overloaded operators, and for customization points within libraries (e.g. `swap` in the C++ standard library). As such, this check can be configured to ignore calls to overloaded operators as well as other legitimate uses of ADL specified in a whitelist.
+
+This check does not suggest any fixes.
+
+Options
+---
+
+.. option:: IgnoreOverloadedOperators
+
+   If non-zero, ignores calls to overloaded operators using the operator syntax (e.g. `a + b`), but not the function call syntax (e.g. `operator+(a, b)`). Default is `1`.
+
+.. option:: Whitelist
+
+   Semicolon-separated list of names that the check ignores. Default is `swap`.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -104,6 +104,12 @@
   
   Violating the naming rules above results in undefined b

[PATCH] D72283: [Matrix] Add __builtin_matrix_insert to Clang (WIP).

[Florian Hahn via Phabricator via cfe-commits]

fhahn created this revision.
Herald added a subscriber: tschuett.
Herald added a project: clang.

This patch adds a __builtin_matrix_insert builtin as described in
"Matrix Support in Clang" on cfe-dev [1]. The patch is not intended for
review yet, just to provide an idea how the implementation could look
like.

[1] http://lists.llvm.org/pipermail/cfe-dev/2019-December/064141.html


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72283

Files:
  clang/include/clang/Basic/Builtins.def
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/builtin-matrix.c
  clang/test/CodeGenCXX/builtin-matrix.cpp
  clang/test/SemaCXX/builtin-matrix.cpp

Index: clang/test/SemaCXX/builtin-matrix.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/builtin-matrix.cpp
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 %s -fenable-matrix -pedantic -std=c++11 -verify -triple=x86_64-apple-darwin9
+
+typedef float sx10x10_t __attribute__((matrix_type(10, 10)));
+sx10x10_t a;
+
+struct Foo {
+  char *s;
+};
+
+void insert(sx10x10_t *a, float f) {
+  *a = __builtin_matrix_insert(
+  10, // expected-error {{First argument must be a matrix}}
+  a,  // expected-error {{Row argument must be an unsigned integer}}
+  a,  // expected-error {{Column argument must be an unsigned integer}}
+  10);
+
+  int x = __builtin_matrix_insert(*a, 3u, 5u, 10.0); // expected-error {{cannot initialize a variable of type 'int' with an rvalue of type 'sx10x10_t' (aka 'float __attribute__((matrix_type(10, 10))) ')}}
+
+  // TODO: Should error here (index out of range).
+  *a = __builtin_matrix_insert(*a, -1u, 5u, 10.0);
+
+  // FIXME: Column argument is fine!
+  *a = __builtin_matrix_insert(*a, f, // expected-error {{Row argument must be an unsigned integer}}
+   5u, 10.0); // expected-error {{Column argument must be an unsigned integer}}
+}
Index: clang/test/CodeGenCXX/builtin-matrix.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/builtin-matrix.cpp
@@ -0,0 +1,150 @@
+// RUN: %clang_cc1 -fenable-matrix -triple x86_64-apple-darwin %s -emit-llvm -disable-llvm-passes -o - -std=c++11 | FileCheck %s
+
+typedef double dx5x5_t __attribute__((matrix_type(5, 5)));
+using fx2x3_t = float __attribute__((matrix_type(2, 3)));
+
+void insert_fp(dx5x5_t *a, double d, fx2x3_t *b, float e) {
+  *a = __builtin_matrix_insert(*a, 0u, 1u, d);
+  *b = __builtin_matrix_insert(*b, 1u, 0u, e);
+
+  // CHECK-LABEL: @_Z9insert_fpPDm5_5_ddPDm2_3_ff(
+  // CHECK-NEXT: entry:
+  // CHECK-NEXT:%a.addr = alloca [25 x double]*, align 8
+  // CHECK-NEXT:%d.addr = alloca double, align 8
+  // CHECK-NEXT:%b.addr = alloca [6 x float]*, align 8
+  // CHECK-NEXT:%e.addr = alloca float, align 4
+  // CHECK-NEXT:store [25 x double]* %a, [25 x double]** %a.addr, align 8
+  // CHECK-NEXT:store double %d, double* %d.addr, align 8
+  // CHECK-NEXT:store [6 x float]* %b, [6 x float]** %b.addr, align 8
+  // CHECK-NEXT:store float %e, float* %e.addr, align 4
+  // CHECK-NEXT:%0 = load [25 x double]*, [25 x double]** %a.addr, align 8
+  // CHECK-NEXT:%1 = bitcast [25 x double]* %0 to <25 x double>*
+  // CHECK-NEXT:%2 = load <25 x double>, <25 x double>* %1, align 8
+  // CHECK-NEXT:%3 = load double, double* %d.addr, align 8
+  // CHECK-NEXT:%4 = insertelement <25 x double> %2, double %3, i32 5
+  // CHECK-NEXT:%5 = load [25 x double]*, [25 x double]** %a.addr, align 8
+  // CHECK-NEXT:%6 = bitcast [25 x double]* %5 to <25 x double>*
+  // CHECK-NEXT:store <25 x double> %4, <25 x double>* %6, align 8
+  // CHECK-NEXT:%7 = load [6 x float]*, [6 x float]** %b.addr, align 8
+  // CHECK-NEXT:%8 = bitcast [6 x float]* %7 to <6 x float>*
+  // CHECK-NEXT:%9 = load <6 x float>, <6 x float>* %8, align 4
+  // CHECK-NEXT:%10 = load float, float* %e.addr, align 4
+  // CHECK-NEXT:%11 = insertelement <6 x float> %9, float %10, i32 1
+  // CHECK-NEXT:%12 = load [6 x float]*, [6 x float]** %b.addr, align 8
+  // CHECK-NEXT:%13 = bitcast [6 x float]* %12 to <6 x float>*
+  // CHECK-NEXT:store <6 x float> %11, <6 x float>* %13, align 4
+  // CHECK-NEXT:   ret void
+}
+
+typedef int ix9x3_t __attribute__((matrix_type(9, 3)));
+
+void insert_int(ix9x3_t *a, int i) {
+  *a = __builtin_matrix_insert(*a, 4u, 1u, i);
+
+  // CHECK-LABEL: @_Z10insert_intPDm9_3_ii(
+  // CHECK-NEXT: entry:
+  // CHECK-NEXT:%a.addr = alloca [27 x i32]*, align 8
+  // CHECK-NEXT:%i.addr = alloca i32, align 4
+  // CHECK-NEXT:store [27 x i32]* %a, [27 x i32]** %a.addr, align 8
+  // CHECK-NEXT:store i32 %i, i32* %i.addr, align 4
+  // CHECK-NEXT:%0 = load [27 x i32]*, [27 x i32]** %a.addr, align 8
+  // CHECK-NEXT:%1 = bitcast [27 x i32]* %0 to

[PATCH] D72281: [Matrix] Add matrix type to Clang (WIP).

[Florian Hahn via Phabricator via cfe-commits]

fhahn created this revision.
Herald added a reviewer: martong.
Herald added subscribers: tschuett, arphaman.
Herald added a project: clang.

This patch adds a matrix type to Clang as described in
"Matrix Support in Clang" on cfe-dev [1]. The patch is not intended for
review yet, just to provide an idea how the implementation would look
like.

One aspect in particular I would appreciate feedback on is how to best
ensure matrix type values are aligned the same as pointers to the
element type, while using LLVM's vector type to lower operations.

The main problem is struct layouting, where LLVM's vector type has a
larger alignment than desired.

To work around that fact, the patch uses array types as storage types for
matrix values, but vector types in other contexts. After loading/before
storing, we bitcast between array type and vector type. Alternatively
we could opt for generating packed LLVM structs.

The builtins will be added in separate, follow-on patches.

[1] http://lists.llvm.org/pipermail/cfe-dev/2019-December/064141.html


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72281

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/AST/Type.h
  clang/include/clang/AST/TypeLoc.h
  clang/include/clang/AST/TypeProperties.td
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Basic/TypeNodes.td
  clang/include/clang/Driver/Options.td
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Serialization/TypeBitCodes.def
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ASTStructuralEquivalence.cpp
  clang/lib/AST/ExprConstant.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/MicrosoftMangle.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGDebugInfo.h
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenTypes.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Sema/SemaTemplateDeduction.cpp
  clang/lib/Sema/SemaType.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/CodeGen/matrix-type.c
  clang/test/CodeGenCXX/matrix-type.cpp
  clang/test/SemaCXX/matrix-type.cpp
  clang/tools/libclang/CIndex.cpp

Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -1792,6 +1792,8 @@
 DEFAULT_TYPELOC_IMPL(DependentSizedExtVector, Type)
 DEFAULT_TYPELOC_IMPL(Vector, Type)
 DEFAULT_TYPELOC_IMPL(ExtVector, VectorType)
+DEFAULT_TYPELOC_IMPL(Matrix, Type)
+DEFAULT_TYPELOC_IMPL(DependentSizedMatrix, Type)
 DEFAULT_TYPELOC_IMPL(FunctionProto, FunctionType)
 DEFAULT_TYPELOC_IMPL(FunctionNoProto, FunctionType)
 DEFAULT_TYPELOC_IMPL(Record, TagType)
Index: clang/test/SemaCXX/matrix-type.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/matrix-type.cpp
@@ -0,0 +1,53 @@
+// RUN: %clang_cc1 -fsyntax-only -pedantic -fenable-matrix -std=c++11 -verify -triple x86_64-apple-darwin %s
+
+using matrix_double_t = double __attribute__((matrix_type(6, 6)));
+using matrix_float_t = float __attribute__((matrix_type(6, 6)));
+using matrix_int_t = int __attribute__((matrix_type(6, 6)));
+
+void matrix_var_dimensions(int Rows, unsigned Columns, char C) {
+  using matrix1_t = int __attribute__((matrix_type(Rows, 1)));// expected-error{{matrix_type attribute requires an integer constant}}
+  using matrix2_t = int __attribute__((matrix_type(1, Columns))); // expected-error{{matrix_type attribute requires an integer constant}}
+  using matrix3_t = int __attribute__((matrix_type(C, C)));   // expected-error{{matrix_type attribute requires an integer constant}}
+  using matrix4_t = int __attribute__((matrix_type(-1, 1)));  // expected-error{{vector size too large}}
+  using matrix5_t = int __attribute__((matrix_type(1, -1)));  // expected-error{{vector size too large}}
+  using matrix6_t = int __attribute__((matrix_type(0, 1)));   // expected-error{{zero vector size}}
+  using matrix7_t = int __attribute__((matrix_type(1, 0)));   // expected-error{{zero vector size}}
+  using matrix7_t = int __attribute__((matrix_type(char, 0)));// expected-error{{expected '(' for function-style cast or type construction}}
+}
+
+struct S1 {};
+
+void matrix_unsupported_element_type() {
+  using matrix1_t = char *__attribute__((matrix_type(1, 1))); // expected-error{{invalid matrix element type 'char *'}}
+  using matrix2_t = S1 __attribute__((matrix_type(1, 1)));// expected-error{{invalid matrix element type 

[PATCH] D72053: [RFC] Handling implementation limits

[Mark de Wever via Phabricator via cfe-commits]

Mordante marked 2 inline comments as done.
Mordante added inline comments.



Comment at: clang/docs/ImplementationQuantities.rst:11
+This page lists the limits implemented in the Clang compiler. The available
+resources on the system running Clang may imposse other limits. For example,
+the system may have insufficient memory to compile the translation unit before

tahonermann wrote:
> Typo: imposse -> impose.
Thanks, it will be updated in the next revision of the patch.


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

https://reviews.llvm.org/D72053



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

[PATCH] D69878: Consoldiate internal denormal flushing controls

[Cameron McInally via Phabricator via cfe-commits]

cameron.mcinally added a comment.

Ok, thanks for the clarifications. Looks good to me, but it would be good to 
have experts in OpenCL/Cuda/AMDGPU review the target specific changes.


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

https://reviews.llvm.org/D69878



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

[PATCH] D72268: [ARM,MVE] Support -ve offsets in gather-load intrinsics.

[Simon Tatham via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG4978296cd8e4: [ARM,MVE] Support -ve offsets in gather-load 
intrinsics. (authored by simon_tatham).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72268

Files:
  clang/include/clang/Basic/arm_mve_defs.td
  clang/test/CodeGen/arm-mve-intrinsics/scatter-gather.c
  clang/test/Sema/arm-mve-immediates.c
  clang/utils/TableGen/MveEmitter.cpp
  llvm/test/CodeGen/Thumb2/mve-intrinsics/scatter-gather.ll

Index: llvm/test/CodeGen/Thumb2/mve-intrinsics/scatter-gather.ll
===
--- llvm/test/CodeGen/Thumb2/mve-intrinsics/scatter-gather.ll
+++ llvm/test/CodeGen/Thumb2/mve-intrinsics/scatter-gather.ll
@@ -191,11 +191,11 @@
 define arm_aapcs_vfpcc <2 x i64> @test_vldrdq_gather_base_u64(<2 x i64> %addr) {
 ; CHECK-LABEL: test_vldrdq_gather_base_u64:
 ; CHECK:   @ %bb.0: @ %entry
-; CHECK-NEXT:vldrd.u64 q1, [q0, #336]
+; CHECK-NEXT:vldrd.u64 q1, [q0, #-336]
 ; CHECK-NEXT:vmov q0, q1
 ; CHECK-NEXT:bx lr
 entry:
-  %0 = call <2 x i64> @llvm.arm.mve.vldr.gather.base.v2i64.v2i64(<2 x i64> %addr, i32 336)
+  %0 = call <2 x i64> @llvm.arm.mve.vldr.gather.base.v2i64.v2i64(<2 x i64> %addr, i32 -336)
   ret <2 x i64> %0
 }
 
@@ -221,12 +221,12 @@
 ; CHECK-LABEL: test_vldrdq_gather_base_wb_u64:
 ; CHECK:   @ %bb.0: @ %entry
 ; CHECK-NEXT:vldrw.u32 q0, [r0]
-; CHECK-NEXT:vldrd.u64 q1, [q0, #328]!
+; CHECK-NEXT:vldrd.u64 q1, [q0, #-328]!
 ; CHECK-NEXT:vstrw.32 q1, [r0]
 ; CHECK-NEXT:bx lr
 entry:
   %0 = load <2 x i64>, <2 x i64>* %addr, align 8
-  %1 = call { <2 x i64>, <2 x i64> } @llvm.arm.mve.vldr.gather.base.wb.v2i64.v2i64(<2 x i64> %0, i32 328)
+  %1 = call { <2 x i64>, <2 x i64> } @llvm.arm.mve.vldr.gather.base.wb.v2i64.v2i64(<2 x i64> %0, i32 -328)
   %2 = extractvalue { <2 x i64>, <2 x i64> } %1, 1
   store <2 x i64> %2, <2 x i64>* %addr, align 8
   %3 = extractvalue { <2 x i64>, <2 x i64> } %1, 0
@@ -297,13 +297,13 @@
 ; CHECK:   @ %bb.0: @ %entry
 ; CHECK-NEXT:vmsr p0, r0
 ; CHECK-NEXT:vpst
-; CHECK-NEXT:vldrdt.u64 q1, [q0, #1000]
+; CHECK-NEXT:vldrdt.u64 q1, [q0, #-1000]
 ; CHECK-NEXT:vmov q0, q1
 ; CHECK-NEXT:bx lr
 entry:
   %0 = zext i16 %p to i32
   %1 = call <4 x i1> @llvm.arm.mve.pred.i2v.v4i1(i32 %0)
-  %2 = call <2 x i64> @llvm.arm.mve.vldr.gather.base.predicated.v2i64.v2i64.v4i1(<2 x i64> %addr, i32 1000, <4 x i1> %1)
+  %2 = call <2 x i64> @llvm.arm.mve.vldr.gather.base.predicated.v2i64.v2i64.v4i1(<2 x i64> %addr, i32 -1000, <4 x i1> %1)
   ret <2 x i64> %2
 }
 
@@ -728,12 +728,12 @@
 ; CHECK-LABEL: test_vldrwq_gather_base_wb_f32:
 ; CHECK:   @ %bb.0: @ %entry
 ; CHECK-NEXT:vldrw.u32 q0, [r0]
-; CHECK-NEXT:vldrw.u32 q1, [q0, #64]!
+; CHECK-NEXT:vldrw.u32 q1, [q0, #-64]!
 ; CHECK-NEXT:vstrw.32 q1, [r0]
 ; CHECK-NEXT:bx lr
 entry:
   %0 = load <4 x i32>, <4 x i32>* %addr, align 8
-  %1 = call { <4 x float>, <4 x i32> } @llvm.arm.mve.vldr.gather.base.wb.v4f32.v4i32(<4 x i32> %0, i32 64)
+  %1 = call { <4 x float>, <4 x i32> } @llvm.arm.mve.vldr.gather.base.wb.v4f32.v4i32(<4 x i32> %0, i32 -64)
   %2 = extractvalue { <4 x float>, <4 x i32> } %1, 1
   store <4 x i32> %2, <4 x i32>* %addr, align 8
   %3 = extractvalue { <4 x float>, <4 x i32> } %1, 0
@@ -782,14 +782,14 @@
 ; CHECK-NEXT:vmsr p0, r1
 ; CHECK-NEXT:vldrw.u32 q0, [r0]
 ; CHECK-NEXT:vpst
-; CHECK-NEXT:vldrwt.u32 q1, [q0, #352]!
+; CHECK-NEXT:vldrwt.u32 q1, [q0, #-352]!
 ; CHECK-NEXT:vstrw.32 q1, [r0]
 ; CHECK-NEXT:bx lr
 entry:
   %0 = load <4 x i32>, <4 x i32>* %addr, align 8
   %1 = zext i16 %p to i32
   %2 = call <4 x i1> @llvm.arm.mve.pred.i2v.v4i1(i32 %1)
-  %3 = call { <4 x float>, <4 x i32> } @llvm.arm.mve.vldr.gather.base.wb.predicated.v4f32.v4i32.v4i1(<4 x i32> %0, i32 352, <4 x i1> %2)
+  %3 = call { <4 x float>, <4 x i32> } @llvm.arm.mve.vldr.gather.base.wb.predicated.v4f32.v4i32.v4i1(<4 x i32> %0, i32 -352, <4 x i1> %2)
   %4 = extractvalue { <4 x float>, <4 x i32> } %3, 1
   store <4 x i32> %4, <4 x i32>* %addr, align 8
   %5 = extractvalue { <4 x float>, <4 x i32> } %3, 0
@@ -845,13 +845,13 @@
 ; CHECK:   @ %bb.0: @ %entry
 ; CHECK-NEXT:vmsr p0, r0
 ; CHECK-NEXT:vpst
-; CHECK-NEXT:vldrwt.u32 q1, [q0, #300]
+; CHECK-NEXT:vldrwt.u32 q1, [q0, #-300]
 ; CHECK-NEXT:vmov q0, q1
 ; CHECK-NEXT:bx lr
 entry:
   %0 = zext i16 %p to i32
   %1 = call <4 x i1> @llvm.arm.mve.pred.i2v.v4i1(i32 %0)
-  %2 = call <4 x float> @llvm.arm.mve.vldr.gather.base.predicated.v4f32.v4i32.v4i1(<4 x i32> %addr, i32 300, <4 x i1> %1)
+  %2 = call <4 x float> @llvm.arm.mve.vldr.gather.base.predicated.v4f32.v4i32.v4i1(<4 x i32> %addr, i32 -300, <4 x i1> %1)
   ret <4 x float> %2
 }
 
@@ -1254,10 +1254,10 @@
 define arm_aapcs_vfpcc void @test_vstrdq_scatter_base_u64(<2 x i64> %addr, <2 x i64> 

[clang] 4978296 - [ARM, MVE] Support -ve offsets in gather-load intrinsics.

[Simon Tatham via cfe-commits]

Author: Simon Tatham
Date: 2020-01-06T16:33:07Z
New Revision: 4978296cd8e4d10724cfa41f0308d256c0fd490c

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

LOG: [ARM,MVE] Support -ve offsets in gather-load intrinsics.

Summary:
The ACLE intrinsics with `gather_base` or `scatter_base` in the name
are wrappers on the MVE load/store instructions that take a vector of
base addresses and an immediate offset. The immediate offset can be up
to 127 times the alignment unit, and it can be positive or negative.

At the MC layer, we got that right. But in the Sema error checking for
the wrapping intrinsics, the offset was erroneously constrained to be
positive.

To fix this I've adjusted the `imm_mem7bit` class in the Tablegen that
defines the intrinsics. But that causes integer literals like
`0xfe04` to appear in the autogenerated calls to
`SemaBuiltinConstantArgRange`, which provokes a compiler warning
because that's out of the non-overflowing range of an `int64_t`. So
I've also tweaked `MveEmitter` to emit that as `-0x1fc` instead.

Updated the tests of the Sema checks themselves, and also adjusted a
random sample of the CodeGen tests to actually use negative offsets
and prove they get all the way through code generation without causing
a crash.

Reviewers: dmgreen, miyuki, MarkMurrayARM

Reviewed By: dmgreen

Subscribers: kristof.beyls, cfe-commits, llvm-commits

Tags: #clang, #llvm

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

Added: 


Modified: 
clang/include/clang/Basic/arm_mve_defs.td
clang/test/CodeGen/arm-mve-intrinsics/scatter-gather.c
clang/test/Sema/arm-mve-immediates.c
clang/utils/TableGen/MveEmitter.cpp
llvm/test/CodeGen/Thumb2/mve-intrinsics/scatter-gather.ll

Removed: 




diff  --git a/clang/include/clang/Basic/arm_mve_defs.td 
b/clang/include/clang/Basic/arm_mve_defs.td
index 939d5eb0cd6b..6fba88df34bf 100644
--- a/clang/include/clang/Basic/arm_mve_defs.td
+++ b/clang/include/clang/Basic/arm_mve_defs.td
@@ -345,9 +345,10 @@ def imm_1248 : Immediate> {
 
 // imm_mem7bit is a valid immediate offset for a load/store intrinsic whose
 // memory access size is n bytes (e.g. 1 for vldrb_[whatever], 2 for vldrh,
-// ...). The set of valid immediates for these is {0*n, 1*n, ..., 127*n}.
+// ...). The set of valid immediates for these is {-127*n, ..., -1*n, 0*n, 1*n,
+// ..., 127*n}.
 class imm_mem7bit
-  : Immediate> {
+  : Immediate> {
   let extra = !if(!eq(membytes, 1), ?, "Multiple");
   let extraarg = !cast(membytes);
 }

diff  --git a/clang/test/CodeGen/arm-mve-intrinsics/scatter-gather.c 
b/clang/test/CodeGen/arm-mve-intrinsics/scatter-gather.c
index 8bf2111a9e63..564965acc04d 100644
--- a/clang/test/CodeGen/arm-mve-intrinsics/scatter-gather.c
+++ b/clang/test/CodeGen/arm-mve-intrinsics/scatter-gather.c
@@ -196,12 +196,12 @@ int64x2_t test_vldrdq_gather_base_s64(uint64x2_t addr)
 
 // CHECK-LABEL: @test_vldrdq_gather_base_u64(
 // CHECK-NEXT:  entry:
-// CHECK-NEXT:[[TMP0:%.*]] = call <2 x i64> 
@llvm.arm.mve.vldr.gather.base.v2i64.v2i64(<2 x i64> [[ADDR:%.*]], i32 336)
+// CHECK-NEXT:[[TMP0:%.*]] = call <2 x i64> 
@llvm.arm.mve.vldr.gather.base.v2i64.v2i64(<2 x i64> [[ADDR:%.*]], i32 -336)
 // CHECK-NEXT:ret <2 x i64> [[TMP0]]
 //
 uint64x2_t test_vldrdq_gather_base_u64(uint64x2_t addr)
 {
-return vldrdq_gather_base_u64(addr, 0x150);
+return vldrdq_gather_base_u64(addr, -0x150);
 }
 
 // CHECK-LABEL: @test_vldrdq_gather_base_wb_s64(
@@ -221,7 +221,7 @@ int64x2_t test_vldrdq_gather_base_wb_s64(uint64x2_t *addr)
 // CHECK-LABEL: @test_vldrdq_gather_base_wb_u64(
 // CHECK-NEXT:  entry:
 // CHECK-NEXT:[[TMP0:%.*]] = load <2 x i64>, <2 x i64>* [[ADDR:%.*]], 
align 8
-// CHECK-NEXT:[[TMP1:%.*]] = call { <2 x i64>, <2 x i64> } 
@llvm.arm.mve.vldr.gather.base.wb.v2i64.v2i64(<2 x i64> [[TMP0]], i32 328)
+// CHECK-NEXT:[[TMP1:%.*]] = call { <2 x i64>, <2 x i64> } 
@llvm.arm.mve.vldr.gather.base.wb.v2i64.v2i64(<2 x i64> [[TMP0]], i32 -328)
 // CHECK-NEXT:[[TMP2:%.*]] = extractvalue { <2 x i64>, <2 x i64> } 
[[TMP1]], 1
 // CHECK-NEXT:store <2 x i64> [[TMP2]], <2 x i64>* [[ADDR]], align 8
 // CHECK-NEXT:[[TMP3:%.*]] = extractvalue { <2 x i64>, <2 x i64> } 
[[TMP1]], 0
@@ -229,7 +229,7 @@ int64x2_t test_vldrdq_gather_base_wb_s64(uint64x2_t *addr)
 //
 uint64x2_t test_vldrdq_gather_base_wb_u64(uint64x2_t *addr)
 {
-return vldrdq_gather_base_wb_u64(addr, 0x148);
+return vldrdq_gather_base_wb_u64(addr, -0x148);
 }
 
 // CHECK-LABEL: @test_vldrdq_gather_base_wb_z_s64(
@@ -280,12 +280,12 @@ int64x2_t test_vldrdq_gather_base_z_s64(uint64x2_t addr, 
mve_pred16_t p)
 // CHECK-NEXT:  entry:
 // CHECK-NEXT:[[TMP0:%.*]] = zext i16 [[P:%.*]] to i32
 // CHECK-NEXT:[[TMP1:%.*]] = call <4 x i1> @llvm.ar

[PATCH] D72268: [ARM,MVE] Support -ve offsets in gather-load intrinsics.

[Dave Green via Phabricator via cfe-commits]

dmgreen accepted this revision.
dmgreen 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/D72268/new/

https://reviews.llvm.org/D72268



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

[PATCH] D72270: [ARM,MVE] Fix many signedness errors in MVE intrinsics.

[Dave Green via Phabricator via cfe-commits]

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

OK. LGTM then.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72270



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

[PATCH] D72276: [clang-format] Add IndentCaseBlocks option

[Nicolas Capens via Phabricator via cfe-commits]

capn created this revision.
capn added a reviewer: clang-format.
capn added a project: clang-format.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The documentation for IndentCaseLabels claimed that the "Switch
statement body is always indented one level more than case labels". This
is technically false for the code block immediately following the label.
Its closing bracket aligns with the start of the label.

If the case label are not indented, it leads to a style where the
closing bracket of the block aligns with the closing bracket of the
switch statement, which can be hard to parse.

This change introduces a new option, IndentCaseBlocks, which when true
treats the block as a scope block (which it technically is).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72276

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1211,6 +1211,28 @@
"  }\n"
"}",
Style));
+  Style.IndentCaseLabels = false;
+  Style.IndentCaseBlocks = true;
+  EXPECT_EQ("switch (n)\n"
+"{\n"
+"case 0:\n"
+"  {\n"
+"return false;\n"
+"  }\n"
+"default:\n"
+"  {\n"
+"return true;\n"
+"  }\n"
+"}",
+format("switch (n) {\n"
+   "case 0: {\n"
+   "  return false;\n"
+   "}\n"
+   "default: {\n"
+   "  return true;\n"
+   "}\n"
+   "}",
+   Style));
 }
 
 TEST_F(FormatTest, CaseRanges) {
@@ -12578,6 +12600,7 @@
   CHECK_PARSE_BOOL_FIELD(DerivePointerAlignment, "DerivePointerBinding");
   CHECK_PARSE_BOOL(DisableFormat);
   CHECK_PARSE_BOOL(IndentCaseLabels);
+  CHECK_PARSE_BOOL(IndentCaseBlocks);
   CHECK_PARSE_BOOL(IndentGotoLabels);
   CHECK_PARSE_BOOL(IndentWrappedFunctionNames);
   CHECK_PARSE_BOOL(KeepEmptyLinesAtTheStartOfBlocks);
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1985,7 +1985,8 @@
 --Line->Level;
   if (LeftAlignLabel)
 Line->Level = 0;
-  if (CommentsBeforeNextToken.empty() && FormatTok->Tok.is(tok::l_brace)) {
+  if (!Style.IndentCaseBlocks && CommentsBeforeNextToken.empty() &&
+  FormatTok->Tok.is(tok::l_brace)) {
 CompoundStatementIndenter Indenter(this, Line->Level,
Style.BraceWrapping.AfterCaseLabel,
Style.BraceWrapping.IndentBraces);
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -480,6 +480,7 @@
 IO.mapOptional("IncludeIsMainSourceRegex",
Style.IncludeStyle.IncludeIsMainSourceRegex);
 IO.mapOptional("IndentCaseLabels", Style.IndentCaseLabels);
+IO.mapOptional("IndentCaseBlocks", Style.IndentCaseBlocks);
 IO.mapOptional("IndentGotoLabels", Style.IndentGotoLabels);
 IO.mapOptional("IndentPPDirectives", Style.IndentPPDirectives);
 IO.mapOptional("IndentWidth", Style.IndentWidth);
@@ -782,6 +783,7 @@
   LLVMStyle.IncludeStyle.IncludeIsMainRegex = "(Test)?$";
   LLVMStyle.IncludeStyle.IncludeBlocks = tooling::IncludeStyle::IBS_Preserve;
   LLVMStyle.IndentCaseLabels = false;
+  LLVMStyle.IndentCaseBlocks = false;
   LLVMStyle.IndentGotoLabels = true;
   LLVMStyle.IndentPPDirectives = FormatStyle::PPDIS_None;
   LLVMStyle.IndentWrappedFunctionNames = false;
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -1314,7 +1314,8 @@
   ///
   /// When ``false``, use the same indentation level as for the switch
   /// statement. Switch statement body is always indented one level more than
-  /// case labels.
+  /// case labels (except the first block following the case label, which
+  /// itself indents the code - unless IndentCaseBlocks is enabled).
   /// \code
   ///false: true:
   ///switch (fool) {vs. switch (fool) {
@@ -1327,6 +1328,28 @@
   /// \endcode
   bool IndentCaseLabels;
 
+  /// Indent case label blocks one level from the case label.
+  ///
+  /// When ``false``, the block following the case label uses the same
+  /// indentation level as for the case label, treating the case label the

[PATCH] D72270: [ARM,MVE] Fix many signedness errors in MVE intrinsics.

[Simon Tatham via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/Basic/arm_mve.td:204
 let params = T.Float in {
-  defm vminnmq : VectorVectorArithmetic<"min_predicated">;
-  defm vmaxnmq : VectorVectorArithmetic<"max_predicated">;
+  defm vminnmq : VectorVectorArithmetic<"min_predicated", (? (u32 0))>;
+  defm vmaxnmq : VectorVectorArithmetic<"max_predicated", (? (u32 0))>;

dmgreen wrote:
> What do these 0's mean?
The IR intrinsic `int_arm_mve_min_predicated` is used for both integer min/max 
and floating-point minnm/maxnm. For the integer case it needs a signed/unsigned 
flag parameter; so it has to have that parameter in the floating-point case as 
well, even though there's nothing to distinguish.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72270



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

[PATCH] D72270: [ARM,MVE] Fix many signedness errors in MVE intrinsics.

[Dave Green via Phabricator via cfe-commits]

dmgreen added inline comments.



Comment at: clang/include/clang/Basic/arm_mve.td:204
 let params = T.Float in {
-  defm vminnmq : VectorVectorArithmetic<"min_predicated">;
-  defm vmaxnmq : VectorVectorArithmetic<"max_predicated">;
+  defm vminnmq : VectorVectorArithmetic<"min_predicated", (? (u32 0))>;
+  defm vmaxnmq : VectorVectorArithmetic<"max_predicated", (? (u32 0))>;

What do these 0's mean?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72270



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

[PATCH] D72274: [libTooling] Fix bug in Stencil handling of macro ranges

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel created this revision.
ymandel added a reviewer: gribozavr.
Herald added a project: clang.

Currently, an attempt to rewrite source code inside a macro expansion succeeds, 
but results in empty text, rather than failing with an error.  This patch 
restructures to the code to explicitly validate ranges before attempting to 
edit them.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72274

Files:
  clang/include/clang/Tooling/Transformer/SourceCode.h
  clang/lib/Tooling/Transformer/SourceCode.cpp
  clang/lib/Tooling/Transformer/Stencil.cpp
  clang/unittests/Tooling/SourceCodeTest.cpp
  clang/unittests/Tooling/StencilTest.cpp

Index: clang/unittests/Tooling/StencilTest.cpp
===
--- clang/unittests/Tooling/StencilTest.cpp
+++ clang/unittests/Tooling/StencilTest.cpp
@@ -368,6 +368,21 @@
   testExpr(Id, "3;", run(SimpleFn), "Bound");
 }
 
+TEST_F(StencilTest, CatOfInvalidRangeFails) {
+  StringRef Snippet = R"cpp(
+#define MACRO (3.77)
+  double foo(double d);
+  foo(MACRO);)cpp";
+
+  auto StmtMatch =
+  matchStmt(Snippet, callExpr(callee(functionDecl(hasName("foo"))),
+  argumentCountIs(1),
+  hasArgument(0, expr().bind("arg";
+  ASSERT_TRUE(StmtMatch);
+  Stencil S = cat(node("arg"));
+  EXPECT_THAT_EXPECTED(S->eval(StmtMatch->Result), Failed());
+}
+
 TEST(StencilToStringTest, RawTextOp) {
   auto S = cat("foo bar baz");
   StringRef Expected = R"("foo bar baz")";
Index: clang/unittests/Tooling/SourceCodeTest.cpp
===
--- clang/unittests/Tooling/SourceCodeTest.cpp
+++ clang/unittests/Tooling/SourceCodeTest.cpp
@@ -10,16 +10,20 @@
 #include "TestVisitor.h"
 #include "clang/Basic/Diagnostic.h"
 #include "llvm/Testing/Support/Annotations.h"
+#include "llvm/Testing/Support/Error.h"
 #include "llvm/Testing/Support/SupportHelpers.h"
 #include 
 #include 
 
 using namespace clang;
 
+using llvm::Failed;
+using llvm::Succeeded;
 using llvm::ValueIs;
 using tooling::getExtendedText;
 using tooling::getRangeForEdit;
 using tooling::getText;
+using tooling::validateEditRange;
 
 namespace {
 
@@ -200,4 +204,116 @@
   Visitor.runOver(Code.code());
 }
 
+TEST(SourceCodeTest, EditRangeWithMacroExpansionsIsValid) {
+  // The call expression, whose range we are extracting, includes two macro
+  // expansions.
+  llvm::StringRef Code = R"cpp(
+#define M(a) a * 13
+int foo(int x, int y);
+int a = foo(M(1), M(2));
+)cpp";
+
+  CallsVisitor Visitor;
+
+  Visitor.OnCall = [](CallExpr *CE, ASTContext *Context) {
+auto Range = CharSourceRange::getTokenRange(CE->getSourceRange());
+EXPECT_THAT_ERROR(validateEditRange(Range, Context->getSourceManager()),
+  Succeeded());
+  };
+  Visitor.runOver(Code);
+}
+
+TEST(SourceCodeTest, SpellingRangeOfMacroArgIsValid) {
+  llvm::StringRef Code = R"cpp(
+#define FOO(a) a + 7.0;
+int a = FOO(10);
+)cpp";
+
+  IntLitVisitor Visitor;
+  Visitor.OnIntLit = [](IntegerLiteral *Expr, ASTContext *Context) {
+SourceLocation ArgLoc =
+Context->getSourceManager().getSpellingLoc(Expr->getBeginLoc());
+// The integer literal is a single token.
+auto ArgRange = CharSourceRange::getTokenRange(ArgLoc);
+EXPECT_THAT_ERROR(validateEditRange(ArgRange, Context->getSourceManager()),
+  Succeeded());
+  };
+  Visitor.runOver(Code);
+}
+
+TEST(SourceCodeTest, InvalidEditRangeIsInvalid) {
+  llvm::StringRef Code = "int c = 10;";
+
+  // We use the visitor just to get a valid context.
+  IntLitVisitor Visitor;
+  Visitor.OnIntLit = [](IntegerLiteral *, ASTContext *Context) {
+CharSourceRange Invalid;
+EXPECT_THAT_ERROR(validateEditRange(Invalid, Context->getSourceManager()),
+  Failed());
+  };
+  Visitor.runOver(Code);
+}
+
+TEST(SourceCodeTest, InvertedEditRangeIsInvalid) {
+  llvm::StringRef Code = R"cpp(
+int foo(int x);
+int a = foo(2);
+)cpp";
+
+  CallsVisitor Visitor;
+  Visitor.OnCall = [](CallExpr *Expr, ASTContext *Context) {
+auto InvertedRange = CharSourceRange::getTokenRange(
+SourceRange(Expr->getEndLoc(), Expr->getBeginLoc()));
+EXPECT_THAT_ERROR(
+validateEditRange(InvertedRange, Context->getSourceManager()),
+Failed());
+  };
+  Visitor.runOver(Code);
+}
+
+TEST(SourceCodeTest, MacroArgIsInvalid) {
+  llvm::StringRef Code = R"cpp(
+#define FOO(a) a + 7.0;
+int a = FOO(10);
+)cpp";
+
+  IntLitVisitor Visitor;
+  Visitor.OnIntLit = [](IntegerLiteral *Expr, ASTContext *Context) {
+auto Range = CharSourceRange::getTokenRange(Expr->getSourceRange());
+EXPECT_THAT_ERROR(validateEditRange(Range, Context->getSourceManager()),
+  Failed());
+  };
+  Visitor.runOver(Code);
+}
+
+TEST(SourceCodeTest, EditWholeMacroExpansionIsInvalid) {
+  llvm::StringRef Code = R"cpp(
+#define FOO 10
+int a = FOO;
+)cpp";
+
+  IntLitVisitor Visitor

[PATCH] D72053: [RFC] Handling implementation limits

[Tom Honermann via Phabricator via cfe-commits]

tahonermann added inline comments.



Comment at: clang/docs/ImplementationQuantities.rst:11
+This page lists the limits implemented in the Clang compiler. The available
+resources on the system running Clang may imposse other limits. For example,
+the system may have insufficient memory to compile the translation unit before

Typo: imposse -> impose.


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

https://reviews.llvm.org/D72053



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