[PATCH] D138749: [clang] Compare constraints before diagnosing mismatched ref qualifiers (GH58962)

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson created this revision.
Herald added a project: All.
royjacobson requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

As noticed in GH58962, we should only diagnose illegal overloads of member 
functions
when the ref qualifiers don't match if the trailing constraints are the same.

The fix is to move the existing constraints check earlier in Sema::IsOverload.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138749

Files:
  clang/lib/Sema/SemaOverload.cpp
  clang/test/CXX/over/over.load/p2-0x.cpp

Index: clang/test/CXX/over/over.load/p2-0x.cpp
===
--- clang/test/CXX/over/over.load/p2-0x.cpp
+++ clang/test/CXX/over/over.load/p2-0x.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify %s
 
 // Member function declarations with the same name and the same
 // parameter-type-list as well as mem- ber function template
@@ -22,3 +23,31 @@
   void k(); // expected-note{{previous declaration}}
   void k() &&; // expected-error{{cannot overload a member function with ref-qualifier '&&' with a member function without a ref-qualifier}}
 };
+
+
+#if __cplusplus >= 202002L
+namespace GH58962 {
+
+template
+__add_rvalue_reference(T) declval();
+
+template
+struct type
+{
+void func() requires (R == 0);
+void func() & requires (R == 1);
+void func() && requires (R == 2);
+};
+
+template
+concept test = requires { declval().func(); };
+
+static_assert(test&>);
+static_assert(test&&>);
+static_assert(test&>);
+static_assert(not test&&>);
+static_assert(not test&>);
+static_assert(test&&>);
+
+}
+#endif
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -1316,6 +1316,24 @@
 (!SameTemplateParameterList || !SameReturnType))
   return true;
   }
+
+  if (ConsiderRequiresClauses) {
+Expr *NewRC = New->getTrailingRequiresClause(),
+ *OldRC = Old->getTrailingRequiresClause();
+if ((NewRC != nullptr) != (OldRC != nullptr))
+  // RC are most certainly different - these are overloads.
+  return true;
+
+if (NewRC) {
+  llvm::FoldingSetNodeID NewID, OldID;
+  NewRC->Profile(NewID, Context, /*Canonical=*/true);
+  OldRC->Profile(OldID, Context, /*Canonical=*/true);
+  if (NewID != OldID)
+// RCs are not equivalent - these are overloads.
+return true;
+}
+  }
+
   // If the function is a class member, its signature includes the
   // cv-qualifiers (if any) and ref-qualifier (if any) on the function itself.
   //
@@ -1332,14 +1350,15 @@
   if (!UseMemberUsingDeclRules &&
   (OldMethod->getRefQualifier() == RQ_None ||
NewMethod->getRefQualifier() == RQ_None)) {
-// C++0x [over.load]p2:
-//   - Member function declarations with the same name and the same
-// parameter-type-list as well as member function template
-// declarations with the same name, the same parameter-type-list, and
-// the same template parameter lists cannot be overloaded if any of
-// them, but not all, have a ref-qualifier (8.3.5).
+// C++20 [over.load]p2:
+//   - Member function declarations with the same name, the same
+// parameter-type-list, and the same trailing requires-clause (if
+// any), as well as member function template declarations with the
+// same name, the same parameter-type-list, the same trailing
+// requires-clause (if any), and the same template-head, cannot be
+// overloaded if any of them, but not all, have a ref-qualifier.
 Diag(NewMethod->getLocation(), diag::err_ref_qualifier_overload)
-  << NewMethod->getRefQualifier() << OldMethod->getRefQualifier();
+<< NewMethod->getRefQualifier() << OldMethod->getRefQualifier();
 Diag(OldMethod->getLocation(), diag::note_previous_declaration);
   }
   return true;
@@ -1403,23 +1422,6 @@
 }
   }
 
-  if (ConsiderRequiresClauses) {
-Expr *NewRC = New->getTrailingRequiresClause(),
- *OldRC = Old->getTrailingRequiresClause();
-if ((NewRC != nullptr) != (OldRC != nullptr))
-  // RC are most certainly different - these are overloads.
-  return true;
-
-if (NewRC) {
-  llvm::FoldingSetNodeID NewID, OldID;
-  NewRC->Profile(NewID, Context, /*Canonical=*/true);
-  OldRC->Profile(OldID, Context, /*Canonical=*/true);
-  if (NewID != OldID)
-// RCs are not equivalent - these are overloads.
-return true;
-}
-  }
-
   // The signatures match; this is not an overload.
   return false;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org

[clang] e2138ec - [clang] Use std::size (NFC)

[Kazu Hirata via cfe-commits]

Author: Kazu Hirata
Date: 2022-11-26T13:58:48-08:00
New Revision: e2138ecc72a99cb04fc7d02e57fadcc2b7c85ad9

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

LOG: [clang] Use std::size (NFC)

std::size, introduced in C++17, allows us to directly obtain the
number of elements of an array.

Added: 


Modified: 
clang/unittests/Tooling/CompilationDatabaseTest.cpp
clang/unittests/libclang/LibclangTest.cpp
clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp

Removed: 




diff  --git a/clang/unittests/Tooling/CompilationDatabaseTest.cpp 
b/clang/unittests/Tooling/CompilationDatabaseTest.cpp
index fb8c10df38e85..ee91af7a631fe 100644
--- a/clang/unittests/Tooling/CompilationDatabaseTest.cpp
+++ b/clang/unittests/Tooling/CompilationDatabaseTest.cpp
@@ -642,7 +642,7 @@ TEST(ParseFixedCompilationDatabase, 
ReturnsEmptyCommandLine) {
 
 TEST(ParseFixedCompilationDatabase, HandlesPositionalArgs) {
   const char *Argv[] = {"1", "2", "--", "-c", "somefile.cpp", "-DDEF3"};
-  int Argc = sizeof(Argv) / sizeof(char*);
+  int Argc = std::size(Argv);
   std::string ErrorMsg;
   std::unique_ptr Database =
   FixedCompilationDatabase::loadFromCommandLine(Argc, Argv, ErrorMsg);
@@ -677,7 +677,7 @@ TEST(ParseFixedCompilationDatabase, 
HandlesPositionalArgsSyntaxOnly) {
 
 TEST(ParseFixedCompilationDatabase, HandlesArgv0) {
   const char *Argv[] = {"1", "2", "--", "mytool", "somefile.cpp"};
-  int Argc = sizeof(Argv) / sizeof(char*);
+  int Argc = std::size(Argv);
   std::string ErrorMsg;
   std::unique_ptr Database =
   FixedCompilationDatabase::loadFromCommandLine(Argc, Argv, ErrorMsg);

diff  --git a/clang/unittests/libclang/LibclangTest.cpp 
b/clang/unittests/libclang/LibclangTest.cpp
index a2a17646a871b..0845476f31dea 100644
--- a/clang/unittests/libclang/LibclangTest.cpp
+++ b/clang/unittests/libclang/LibclangTest.cpp
@@ -517,7 +517,7 @@ TEST_F(LibclangReparseTest, ReparseWithModule) {
   std::string ModulesCache = std::string("-fmodules-cache-path=") + TestDir;
   const char *Args[] = { "-fmodules", ModulesCache.c_str(),
  "-I", TestDir.c_str() };
-  int NumArgs = sizeof(Args) / sizeof(Args[0]);
+  int NumArgs = std::size(Args);
   ClangTU = clang_parseTranslationUnit(Index, MName.c_str(), Args, NumArgs,
nullptr, 0, TUFlags);
   EXPECT_EQ(1U, clang_getNumDiagnostics(ClangTU));
@@ -557,7 +557,7 @@ TEST_F(LibclangReparseTest, 
clang_parseTranslationUnit2FullArgv) {
 
   EXPECT_EQ(CXError_Success,
 clang_parseTranslationUnit2FullArgv(Index, Filename.c_str(), Argv,
-sizeof(Argv) / sizeof(Argv[0]),
+std::size(Argv),
 nullptr, 0, TUFlags, 
));
   EXPECT_EQ(0U, clang_getNumDiagnostics(ClangTU));
   DisplayDiagnostics();
@@ -706,7 +706,7 @@ TEST_F(LibclangSerializationTest, 
TokenKindsAreCorrectAfterLoading) {
   const char *Argv[] = {"-xc++-header", "-std=c++11"};
 
   ClangTU = clang_parseTranslationUnit(Index, HeaderName.c_str(), Argv,
-   sizeof(Argv) / sizeof(Argv[0]), nullptr,
+   std::size(Argv), nullptr,
0, TUFlags);
 
   auto CheckTokenKinds = [=]() {

diff  --git a/clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp 
b/clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp
index cddbd7b17c5f6..da5d1fdc2eae3 100644
--- a/clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp
+++ b/clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp
@@ -598,7 +598,7 @@ static unsigned short EncodeVersions(unsigned int 
MinVersion,
   }
 
   unsigned VersionIDs[] = {100, 110, 120, 200, 300};
-  for (unsigned I = 0; I < sizeof(VersionIDs) / sizeof(VersionIDs[0]); I++) {
+  for (unsigned I = 0; I < std::size(VersionIDs); I++) {
 if (VersionIDs[I] >= MinVersion && VersionIDs[I] < MaxVersion) {
   Encoded |= 1 << I;
 }



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

[PATCH] D119138: [clang-format] Further improve support for requires expressions

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

In D119138#3951850 , @klimek wrote:

> I changed it in 49aca00d63e14df8bc68fc4329e6cbc9c9805eb8 
> .
>
> "We" is the people working on clang-format :) I hope that we have a common 
> goal of making clang-format as easy to maintain as we can.
>
> FWIW, I once had the same opinion as you about best doing all parsing as 
> early as possible, but djasper convinced me that the split was a good idea, 
> and in the end, I think it turns out to be significantly less brittle to do 
> more complex annotation in TokenAnnotator. E.g. we now have a lookahead limit 
> of 50, which seems rather arbitrary, while in TokenAnnotator we could simply 
> limit lookahead towards the current UnwrappedLine. Similarly, in 
> TokenAnnotator, we already have all the parens connected, so we could simply 
> look from requires l_paren to the corresponding r_paren and whether the next 
> token is an l_brace. If I can find a bit of time I'll take an attempt at 
> implementing it.

Your commit is in my view a an example of making that maintaining a bit harder, 
it didn't went through review, had you not posted it here I'd never seen it. 
LLVM receives to many commits to scan them for changes in clang-format. And as 
someone who isn't that long involved in clang-format I think there is an 
overview really missing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119138

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

[clang] 25f01d5 - Revert "[SROA] `isVectorPromotionViable()`: memory intrinsics operate on vectors of bytes (take 2)"

[Roman Lebedev via cfe-commits]

Author: Roman Lebedev
Date: 2022-11-27T00:00:06+03:00
New Revision: 25f01d593ce296078f57e872778b77d074ae5888

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

LOG: Revert "[SROA] `isVectorPromotionViable()`: memory intrinsics operate on 
vectors of bytes (take 2)"

TableGen is still getting miscompiled on PPC buildbots.
Sent a mail with request for help.

This reverts commit 3c4d2a03968ccf5889bacffe02d6fa2443b0260f.

Added: 


Modified: 
clang/test/CodeGenOpenCL/amdgpu-nullptr.cl
llvm/lib/Transforms/Scalar/SROA.cpp
llvm/test/CodeGen/AMDGPU/v1024.ll
llvm/test/DebugInfo/X86/sroasplit-1.ll
llvm/test/DebugInfo/X86/sroasplit-4.ll
llvm/test/Transforms/PhaseOrdering/instcombine-sroa-inttoptr.ll
llvm/test/Transforms/SROA/address-spaces.ll
llvm/test/Transforms/SROA/alignment.ll
llvm/test/Transforms/SROA/alloca-address-space.ll
llvm/test/Transforms/SROA/basictest.ll
llvm/test/Transforms/SROA/pointer-offset-size.ll
llvm/test/Transforms/SROA/slice-width.ll
llvm/test/Transforms/SROA/tbaa-struct.ll
llvm/test/Transforms/SROA/tbaa-struct2.ll
llvm/test/Transforms/SROA/vector-promotion.ll

Removed: 




diff  --git a/clang/test/CodeGenOpenCL/amdgpu-nullptr.cl 
b/clang/test/CodeGenOpenCL/amdgpu-nullptr.cl
index 859e81f08d6bd..65f6f2e7d8c24 100644
--- a/clang/test/CodeGenOpenCL/amdgpu-nullptr.cl
+++ b/clang/test/CodeGenOpenCL/amdgpu-nullptr.cl
@@ -515,17 +515,13 @@ typedef struct {
   private char *p;
 } StructTy3;
 
-// CHECK-LABEL: @test_memset_private(
-// CHECK-NEXT:  entry:
-// CHECK-NEXT:[[TMP0:%.*]] = bitcast [[STRUCT_STRUCTTY3:%.*]] 
addrspace(5)* [[PTR:%.*]] to i8 addrspace(5)*
-// CHECK-NEXT:[[S3_SROA_0_SROA_0_0_S3_SROA_0_0__SROA_CAST2_SROA_CAST:%.*]] 
= bitcast [[STRUCT_STRUCTTY3]] addrspace(5)* [[PTR]] to <32 x i8> addrspace(5)*
-// CHECK-NEXT:store <32 x i8> zeroinitializer, <32 x i8> addrspace(5)* 
[[S3_SROA_0_SROA_0_0_S3_SROA_0_0__SROA_CAST2_SROA_CAST]], align 8, !tbaa.struct 
!9
-// CHECK-NEXT:[[S3_SROA_4_0__SROA_IDX6:%.*]] = getelementptr inbounds 
[[STRUCT_STRUCTTY3]], [[STRUCT_STRUCTTY3]] addrspace(5)* [[PTR]], i32 0, i32 4
-// CHECK-NEXT:store i8 addrspace(5)* addrspacecast (i8* null to i8 
addrspace(5)*), i8 addrspace(5)* addrspace(5)* [[S3_SROA_4_0__SROA_IDX6]], 
align 8, !tbaa.struct !12
-// CHECK-NEXT:[[S3_SROA_5_0__SROA_IDX:%.*]] = getelementptr inbounds i8, 
i8 addrspace(5)* [[TMP0]], i32 36
-// CHECK-NEXT:[[S3_SROA_5_0__SROA_CAST8:%.*]] = bitcast i8 addrspace(5)* 
[[S3_SROA_5_0__SROA_IDX]] to i32 addrspace(5)*
-// CHECK-NEXT:store i32 0, i32 addrspace(5)* [[S3_SROA_5_0__SROA_CAST8]], 
align 4, !tbaa.struct !13
-// CHECK-NEXT:ret void
+// CHECK-LABEL: test_memset_private
+// CHECK: call void @llvm.memset.p5i8.i64(i8 addrspace(5)* noundef align 8 
{{.*}}, i8 0, i64 32, i1 false)
+// CHECK: [[GEP:%.*]] = getelementptr inbounds %struct.StructTy3, 
%struct.StructTy3 addrspace(5)* %ptr, i32 0, i32 4
+// CHECK: store i8 addrspace(5)* addrspacecast (i8* null to i8 addrspace(5)*), 
i8 addrspace(5)* addrspace(5)* [[GEP]]
+// CHECK: [[GEP1:%.*]] = getelementptr inbounds i8, i8 addrspace(5)* {{.*}}, 
i32 36
+// CHECK: [[GEP1_CAST:%.*]] = bitcast i8 addrspace(5)* [[GEP1]] to i32 
addrspace(5)*
+// CHECK: store i32 0, i32 addrspace(5)* [[GEP1_CAST]], align 4
 void test_memset_private(private StructTy3 *ptr) {
   StructTy3 S3 = {0, 0, 0, 0, 0};
   *ptr = S3;

diff  --git a/llvm/lib/Transforms/Scalar/SROA.cpp 
b/llvm/lib/Transforms/Scalar/SROA.cpp
index a2d7dc234333f..6dcdd630b6bae 100644
--- a/llvm/lib/Transforms/Scalar/SROA.cpp
+++ b/llvm/lib/Transforms/Scalar/SROA.cpp
@@ -1806,10 +1806,8 @@ static bool isVectorPromotionViableForSlice(Partition 
, const Slice ,
   ? Ty->getElementType()
   : FixedVectorType::get(Ty->getElementType(), 
NumElements);
 
-  Type *SplitIntTy = nullptr;
-  if (uint64_t Bitwidth = NumElements * ElementSize * 8;
-  Bitwidth <= IntegerType::MAX_INT_BITS)
-SplitIntTy = Type::getIntNTy(Ty->getContext(), Bitwidth);
+  Type *SplitIntTy =
+  Type::getIntNTy(Ty->getContext(), NumElements * ElementSize * 8);
 
   Use *U = S.getUse();
 
@@ -1828,8 +1826,7 @@ static bool isVectorPromotionViableForSlice(Partition , 
const Slice ,
 // Disable vector promotion when there are loads or stores of an FCA.
 if (LTy->isStructTy())
   return false;
-if (SplitIntTy &&
-(P.beginOffset() > S.beginOffset() || P.endOffset() < S.endOffset())) {
+if (P.beginOffset() > S.beginOffset() || P.endOffset() < S.endOffset()) {
   assert(LTy->isIntegerTy());
   LTy = SplitIntTy;
 }
@@ -1842,8 +1839,7 @@ static bool isVectorPromotionViableForSlice(Partition , 
const Slice ,
 // Disable vector promotion when 

[clang] 3c4d2a0 - [SROA] `isVectorPromotionViable()`: memory intrinsics operate on vectors of bytes (take 2)

[Roman Lebedev via cfe-commits]

Author: Roman Lebedev
Date: 2022-11-26T23:19:15+03:00
New Revision: 3c4d2a03968ccf5889bacffe02d6fa2443b0260f

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

LOG: [SROA] `isVectorPromotionViable()`: memory intrinsics operate on vectors 
of bytes (take 2)

This is a recommit of cf624b23bc5d5a6161706d1663def49380ff816a,
which was reverted in 5cfc22cafe3f2465e0bb324f8daba82ffcabd0df,
because the cut-off on the number of vector elements was not low enough,
and it triggered both SDAG SDNode operand number assertions,
and caused compile time explosions in some cases.

Let's try with something really *REALLY* conservative first,
just to get somewhere, and try to bump it (to 64/128) later.

FIXME: should this respect TTI reg width * num vec regs?

Original commit message:

Now, there's a big caveat here - these bytes
are abstract bytes, not the i8 we have in LLVM,
so strictly speaking this is not exactly legal,
see e.g. https://github.com/AliveToolkit/alive2/issues/860
^ the "bytes" "could" have been a pointer,
and loading it as an integer inserts an implicit ptrtoint.

But at the same time,
InstCombine's `InstCombinerImpl::SimplifyAnyMemTransfer()`
would expand a memtransfer of 1/2/4/8 bytes
into integer-typed load+store,
so this isn't exactly a new problem.

Note that in memory, poison is byte-wise,
so we really can't widen elements,
but SROA seems to be inconsistent here.

Fixes #59116.

Added: 


Modified: 
clang/test/CodeGenOpenCL/amdgpu-nullptr.cl
llvm/lib/Transforms/Scalar/SROA.cpp
llvm/test/CodeGen/AMDGPU/v1024.ll
llvm/test/DebugInfo/X86/sroasplit-1.ll
llvm/test/DebugInfo/X86/sroasplit-4.ll
llvm/test/Transforms/PhaseOrdering/instcombine-sroa-inttoptr.ll
llvm/test/Transforms/SROA/address-spaces.ll
llvm/test/Transforms/SROA/alignment.ll
llvm/test/Transforms/SROA/alloca-address-space.ll
llvm/test/Transforms/SROA/basictest.ll
llvm/test/Transforms/SROA/pointer-offset-size.ll
llvm/test/Transforms/SROA/slice-width.ll
llvm/test/Transforms/SROA/tbaa-struct.ll
llvm/test/Transforms/SROA/tbaa-struct2.ll
llvm/test/Transforms/SROA/vector-promotion.ll

Removed: 




diff  --git a/clang/test/CodeGenOpenCL/amdgpu-nullptr.cl 
b/clang/test/CodeGenOpenCL/amdgpu-nullptr.cl
index 65f6f2e7d8c24..859e81f08d6bd 100644
--- a/clang/test/CodeGenOpenCL/amdgpu-nullptr.cl
+++ b/clang/test/CodeGenOpenCL/amdgpu-nullptr.cl
@@ -515,13 +515,17 @@ typedef struct {
   private char *p;
 } StructTy3;
 
-// CHECK-LABEL: test_memset_private
-// CHECK: call void @llvm.memset.p5i8.i64(i8 addrspace(5)* noundef align 8 
{{.*}}, i8 0, i64 32, i1 false)
-// CHECK: [[GEP:%.*]] = getelementptr inbounds %struct.StructTy3, 
%struct.StructTy3 addrspace(5)* %ptr, i32 0, i32 4
-// CHECK: store i8 addrspace(5)* addrspacecast (i8* null to i8 addrspace(5)*), 
i8 addrspace(5)* addrspace(5)* [[GEP]]
-// CHECK: [[GEP1:%.*]] = getelementptr inbounds i8, i8 addrspace(5)* {{.*}}, 
i32 36
-// CHECK: [[GEP1_CAST:%.*]] = bitcast i8 addrspace(5)* [[GEP1]] to i32 
addrspace(5)*
-// CHECK: store i32 0, i32 addrspace(5)* [[GEP1_CAST]], align 4
+// CHECK-LABEL: @test_memset_private(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[TMP0:%.*]] = bitcast [[STRUCT_STRUCTTY3:%.*]] 
addrspace(5)* [[PTR:%.*]] to i8 addrspace(5)*
+// CHECK-NEXT:[[S3_SROA_0_SROA_0_0_S3_SROA_0_0__SROA_CAST2_SROA_CAST:%.*]] 
= bitcast [[STRUCT_STRUCTTY3]] addrspace(5)* [[PTR]] to <32 x i8> addrspace(5)*
+// CHECK-NEXT:store <32 x i8> zeroinitializer, <32 x i8> addrspace(5)* 
[[S3_SROA_0_SROA_0_0_S3_SROA_0_0__SROA_CAST2_SROA_CAST]], align 8, !tbaa.struct 
!9
+// CHECK-NEXT:[[S3_SROA_4_0__SROA_IDX6:%.*]] = getelementptr inbounds 
[[STRUCT_STRUCTTY3]], [[STRUCT_STRUCTTY3]] addrspace(5)* [[PTR]], i32 0, i32 4
+// CHECK-NEXT:store i8 addrspace(5)* addrspacecast (i8* null to i8 
addrspace(5)*), i8 addrspace(5)* addrspace(5)* [[S3_SROA_4_0__SROA_IDX6]], 
align 8, !tbaa.struct !12
+// CHECK-NEXT:[[S3_SROA_5_0__SROA_IDX:%.*]] = getelementptr inbounds i8, 
i8 addrspace(5)* [[TMP0]], i32 36
+// CHECK-NEXT:[[S3_SROA_5_0__SROA_CAST8:%.*]] = bitcast i8 addrspace(5)* 
[[S3_SROA_5_0__SROA_IDX]] to i32 addrspace(5)*
+// CHECK-NEXT:store i32 0, i32 addrspace(5)* [[S3_SROA_5_0__SROA_CAST8]], 
align 4, !tbaa.struct !13
+// CHECK-NEXT:ret void
 void test_memset_private(private StructTy3 *ptr) {
   StructTy3 S3 = {0, 0, 0, 0, 0};
   *ptr = S3;

diff  --git a/llvm/lib/Transforms/Scalar/SROA.cpp 
b/llvm/lib/Transforms/Scalar/SROA.cpp
index 6dcdd630b6bae..a2d7dc234333f 100644
--- a/llvm/lib/Transforms/Scalar/SROA.cpp
+++ b/llvm/lib/Transforms/Scalar/SROA.cpp
@@ -1806,8 +1806,10 @@ static bool isVectorPromotionViableForSlice(Partition 
, const Slice ,
   ? Ty->getElementType()
 

[PATCH] D119138: [clang-format] Further improve support for requires expressions

[Manuel Klimek via Phabricator via cfe-commits]

klimek added a comment.

I changed it in 49aca00d63e14df8bc68fc4329e6cbc9c9805eb8 
.

"We" is the people working on clang-format :) I hope that we have a common goal 
of making clang-format as easy to maintain as we can.

FWIW, I once had the same opinion as you about best doing all parsing as early 
as possible, but djasper convinced me that the split was a good idea, and in 
the end, I think it turns out to be significantly less brittle to do more 
complex annotation in TokenAnnotator. E.g. we now have a lookahead limit of 50, 
which seems rather arbitrary, while in TokenAnnotator we could simply limit 
lookahead towards the current UnwrappedLine. Similarly, in TokenAnnotator, we 
already have all the parens connected, so we could simply look from requires 
l_paren to the corresponding r_paren and whether the next token is an l_brace. 
If I can find a bit of time I'll take an attempt at implementing it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119138

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

[clang] 49aca00 - [NFC] Remove peekNextToken(int).

[Manuel Klimek via cfe-commits]

Author: Manuel Klimek
Date: 2022-11-26T18:23:42Z
New Revision: 49aca00d63e14df8bc68fc4329e6cbc9c9805eb8

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

LOG: [NFC] Remove peekNextToken(int).

Arbitrary lookahead restricts the implementation of our TokenSource,
specifically getting in the way of changes to handle macros better.

Instead, use getNextToken to parse lookahead linearly, and
getPosition/setPosition to unwind our lookahead.

Added: 


Modified: 
clang/lib/Format/UnwrappedLineParser.cpp

Removed: 




diff  --git a/clang/lib/Format/UnwrappedLineParser.cpp 
b/clang/lib/Format/UnwrappedLineParser.cpp
index 3bc7d2454c93a..e6bb9c4333b98 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -43,12 +43,6 @@ class FormatTokenSource {
   // getNextToken().
   virtual FormatToken *peekNextToken() = 0;
 
-  // Returns the token that would be returned after the next N calls to
-  // getNextToken(). N needs to be greater than zero, and small enough that
-  // there are still tokens. Check for tok::eof with N-1 before calling it with
-  // N.
-  virtual FormatToken *peekNextToken(int N) = 0;
-
   // Returns whether we are at the end of the file.
   // This can be 
diff erent from whether getNextToken() returned an eof token
   // when the FormatTokenSource is a view on a part of the token stream.
@@ -181,13 +175,6 @@ class ScopedMacroState : public FormatTokenSource {
 return PreviousTokenSource->peekNextToken();
   }
 
-  FormatToken *peekNextToken(int N) override {
-assert(N > 0);
-if (eof())
-  return 
-return PreviousTokenSource->peekNextToken(N);
-  }
-
   bool isEOF() override { return PreviousTokenSource->isEOF(); }
 
   unsigned getPosition() override { return PreviousTokenSource->getPosition(); 
}
@@ -310,16 +297,6 @@ class IndexedTokenSource : public FormatTokenSource {
 return Tokens[Next];
   }
 
-  FormatToken *peekNextToken(int N) override {
-assert(N > 0);
-int Next = Position + N;
-LLVM_DEBUG({
-  llvm::dbgs() << "Peeking (+" << (N - 1) << ") ";
-  dbgToken(Next);
-});
-return Tokens[Next];
-  }
-
   bool isEOF() override { return Tokens[Position]->is(tok::eof); }
 
   unsigned getPosition() override {
@@ -3389,37 +3366,41 @@ bool 
clang::format::UnwrappedLineParser::parseRequires() {
   // So we want basically to check for TYPE NAME, but TYPE can contain all 
kinds
   // of stuff: typename, const, *, &, &&, ::, identifiers.
 
-  int NextTokenOffset = 1;
-  auto NextToken = Tokens->peekNextToken(NextTokenOffset);
-  auto PeekNext = [, , this] {
-++NextTokenOffset;
-NextToken = Tokens->peekNextToken(NextTokenOffset);
+  unsigned StoredPosition = Tokens->getPosition();
+  FormatToken *NextToken = Tokens->getNextToken();
+  int Lookahead = 0;
+  auto PeekNext = [, , this] {
+++Lookahead;
+NextToken = Tokens->getNextToken();
   };
 
   bool FoundType = false;
   bool LastWasColonColon = false;
   int OpenAngles = 0;
 
-  for (; NextTokenOffset < 50; PeekNext()) {
+  for (; Lookahead < 50; PeekNext()) {
 switch (NextToken->Tok.getKind()) {
 case tok::kw_volatile:
 case tok::kw_const:
 case tok::comma:
+  FormatTok = Tokens->setPosition(StoredPosition);
   parseRequiresExpression(RequiresToken);
   return false;
 case tok::r_paren:
 case tok::pipepipe:
+  FormatTok = Tokens->setPosition(StoredPosition);
   parseRequiresClause(RequiresToken);
   return true;
 case tok::eof:
   // Break out of the loop.
-  NextTokenOffset = 50;
+  Lookahead = 50;
   break;
 case tok::coloncolon:
   LastWasColonColon = true;
   break;
 case tok::identifier:
   if (FoundType && !LastWasColonColon && OpenAngles == 0) {
+FormatTok = Tokens->setPosition(StoredPosition);
 parseRequiresExpression(RequiresToken);
 return false;
   }
@@ -3434,14 +3415,15 @@ bool 
clang::format::UnwrappedLineParser::parseRequires() {
   break;
 default:
   if (NextToken->isSimpleTypeSpecifier()) {
+FormatTok = Tokens->setPosition(StoredPosition);
 parseRequiresExpression(RequiresToken);
 return false;
   }
   break;
 }
   }
-
   // This seems to be a complicated expression, just assume it's a clause.
+  FormatTok = Tokens->setPosition(StoredPosition);
   parseRequiresClause(RequiresToken);
   return true;
 }



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

[PATCH] D133633: [CMake] Add ClangBootstrap configuration

[Amir Ayupov via Phabricator via cfe-commits]

Amir added inline comments.



Comment at: clang/cmake/modules/ClangBootstrap.cmake:11
+macro(clang_bootstrap_add name)
+  cmake_parse_arguments(ARG "" "LINKER;AR;RANLIB;OBJCOPY;STRIP"
+"DEPENDS;TABLEGEN;CMAKE_ARGS;BUILD_TOOL_ARGS"

thevinster wrote:
> Were you planning to also use the single arguments list such as `ARG_LINKER` 
> in the `CMAKE_ARGS`? Without it, I have to supply an override to 
> `CLANG_BOLT_INSTRUMENT_EXTRA_CMAKE_FLAGS` so I can avoid using the gnu linker.
Yes, I added those in the first version of the diff and just forgot to remove 
them. But as Peter mentioned:
> I don't think we need a dedicated keyword for each tool, I'd just pass these 
> through CMAKE_ARGS.
I'm neutral about adding ARG_LINKER or setting it through EXTRA_CMAKE_FLAGS, 
but I think explicit overrides for each tool are a bit too verbose. 
Do you think having ARG_LINKER and passing the rest as EXTRA_CMAKE_FLAGS is a 
good tradeoff? cc @phosek 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133633

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

[PATCH] D138746: [clang-format] Add .inc extension to git-clang-format

[Alexandre Ganea via Phabricator via cfe-commits]

aganea created this revision.
aganea added a reviewer: clang-format.
Herald added a project: All.
aganea requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The .inc extension isn't picked up for formatting when doing `git clang-format` 
on the LLVM repo. I'm not sure if we want this at large or more specific just 
for the LLVM codebase.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138746

Files:
  clang/tools/clang-format/git-clang-format


Index: clang/tools/clang-format/git-clang-format
===
--- clang/tools/clang-format/git-clang-format
+++ clang/tools/clang-format/git-clang-format
@@ -88,7 +88,7 @@
   'c', 'h',  # C
   'm',  # ObjC
   'mm',  # ObjC++
-  'cc', 'cp', 'cpp', 'c++', 'cxx', 'hh', 'hpp', 'hxx',  # C++
+  'cc', 'cp', 'cpp', 'c++', 'cxx', 'hh', 'hpp', 'hxx', 'inc',  # C++
   'ccm', 'cppm', 'cxxm', 'c++m',  # C++ Modules
   'cu', 'cuh',  # CUDA
   # Other languages that clang-format supports


Index: clang/tools/clang-format/git-clang-format
===
--- clang/tools/clang-format/git-clang-format
+++ clang/tools/clang-format/git-clang-format
@@ -88,7 +88,7 @@
   'c', 'h',  # C
   'm',  # ObjC
   'mm',  # ObjC++
-  'cc', 'cp', 'cpp', 'c++', 'cxx', 'hh', 'hpp', 'hxx',  # C++
+  'cc', 'cp', 'cpp', 'c++', 'cxx', 'hh', 'hpp', 'hxx', 'inc',  # C++
   'ccm', 'cppm', 'cxxm', 'c++m',  # C++ Modules
   'cu', 'cuh',  # CUDA
   # Other languages that clang-format supports
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D119138: [clang-format] Further improve support for requires expressions

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

In D119138#3951749 , @klimek wrote:

> Generally, why do we need to have that much information? I.e. why do we need 
> to know the exact type of the "requires" keyword?
> I do understand we need to know the brace type, but that seems like it would 
> be easier to figure out in the TokenAnnotator (where we already parsed 
> UnwrappedLines).
> Do we ever parse UnwrappedLines differently depending on requires 
> clauses/expressions?
> If not, we should really do the annotation in TokenAnnotator, where we 
> already have nice parsing bounds from the parsed UnwrappedLines.

Who is //we//, I'm not part of that //we// and haven't heard of some macro 
improvements. And I don't see how that feature is harming //you//, but be my 
guest in changing that. If you look into the history of this change I had a 
heuristic approach which would only look behind to differentiate.

I don't know if that can be solved in the `TokenAnnotator`, but //you// and I 
have different opinions about that. I'd put more annotating in the 
`UnwrappedLineParser`, annotate it as soon as we can.

I'll happily review any changes proposed, but I will not rework this piece of 
code, unless I can see a big flaw in it (which I can't right now).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119138

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

[PATCH] D119138: [clang-format] Further improve support for requires expressions

[Manuel Klimek via Phabricator via cfe-commits]

klimek added a comment.

Generally, why do we need to have that much information? I.e. why do we need to 
know the exact type of the "requires" keyword?
I do understand we need to know the brace type, but that seems like it would be 
easier to figure out in the TokenAnnotator (where we already parsed 
UnwrappedLines).
Do we ever parse UnwrappedLines differently depending on requires 
clauses/expressions?
If not, we should really do the annotation in TokenAnnotator, where we already 
have nice parsing bounds from the parsed UnwrappedLines.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119138

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