[PATCH] D70111: [DWARF5]Addition of alignment field in the typedef for dwarf5
djtodoro added inline comments. Comment at: llvm/docs/ReleaseNotes.rst:169 +* C DebugInfo API ``LLVMDIBuilderCreateTypedef`` is updated to include an extra +argument ``AlignInBits``, to facilitate / propagate specified Alignment information SouraVX wrote: > djtodoro wrote: > > djtodoro wrote: > > > It looks like this breaks the llvm-sphinx-docs build. > > Fixed with the 5762648. > > > > Please take a look into the http://lab.llvm.org:8011/console when > > committing changes. There you can find info about build bots failures, if > > any. > Thank You @djtodoro , had a brief look at 5762648. Seems good. > BTW are sphinx bots still failing due to this ? The builder is back to passing again. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70111/new/ https://reviews.llvm.org/D70111 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D70111: [DWARF5]Addition of alignment field in the typedef for dwarf5
SouraVX added inline comments. Comment at: llvm/docs/ReleaseNotes.rst:169 +* C DebugInfo API ``LLVMDIBuilderCreateTypedef`` is updated to include an extra +argument ``AlignInBits``, to facilitate / propagate specified Alignment information djtodoro wrote: > djtodoro wrote: > > It looks like this breaks the llvm-sphinx-docs build. > Fixed with the 5762648. > > Please take a look into the http://lab.llvm.org:8011/console when committing > changes. There you can find info about build bots failures, if any. Thank You @djtodoro , had a brief look at 5762648. Seems good. BTW are sphinx bots still failing due to this ? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70111/new/ https://reviews.llvm.org/D70111 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D71884: [OpenMP] Fix formatting of OpenMP error message.
jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. I would have just stripped the last chars of ` Out.str()` but if this passes all the tests I'm fine with it. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71884/new/ https://reviews.llvm.org/D71884 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D71884: [OpenMP] Fix formatting of OpenMP error message.
tianqing created this revision.
Herald added subscribers: cfe-commits, guansong.
Herald added a reviewer: jdoerfert.
Herald added a project: clang.
tianqing added a reviewer: ABataev.
`getListOfPossibleValues()` formatted incorrectly when there is only one value,
emitting something like `expected 'conditional' or in OpenMP clause
'lastprivate'`.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D71884
Files:
clang/lib/Sema/SemaOpenMP.cpp
clang/test/OpenMP/for_lastprivate_messages.cpp
clang/test/OpenMP/for_simd_lastprivate_messages.cpp
clang/test/OpenMP/parallel_for_lastprivate_messages.cpp
clang/test/OpenMP/parallel_for_simd_lastprivate_messages.cpp
clang/test/OpenMP/parallel_sections_lastprivate_messages.cpp
clang/test/OpenMP/sections_lastprivate_messages.cpp
clang/test/OpenMP/simd_lastprivate_messages.cpp
clang/test/OpenMP/target_parallel_for_lastprivate_messages.cpp
clang/test/OpenMP/target_parallel_for_simd_lastprivate_messages.cpp
clang/test/OpenMP/target_simd_lastprivate_messages.cpp
Index: clang/test/OpenMP/target_simd_lastprivate_messages.cpp
===
--- clang/test/OpenMP/target_simd_lastprivate_messages.cpp
+++ clang/test/OpenMP/target_simd_lastprivate_messages.cpp
@@ -107,6 +107,9 @@
#pragma omp target simd lastprivate(conditional: s,argc) lastprivate(conditional: // omp45-error 2 {{use of undeclared identifier 'conditional'}} omp50-error {{expected expression}} expected-error {{expected ')'}} expected-note {{to match this '('}} omp50-error {{expected list item of scalar type in 'lastprivate' clause with 'conditional' modifier}}
for (int k = 0; k < argc; ++k)
++k;
+#pragma omp target simd lastprivate(foo:argc) // omp50-error {{expected 'conditional' in OpenMP clause 'lastprivate'}} omp45-error {{expected ',' or ')' in 'lastprivate' clause}} omp45-error {{expected ')'}} omp45-error {{expected variable name}} omp45-note {{to match this '('}}
+ for (int k = 0; k < argc; ++k)
+++k;
#pragma omp target simd lastprivate(S1) // expected-error {{'S1' does not refer to a value}}
for (int k = 0; k < argc; ++k)
++k;
Index: clang/test/OpenMP/target_parallel_for_simd_lastprivate_messages.cpp
===
--- clang/test/OpenMP/target_parallel_for_simd_lastprivate_messages.cpp
+++ clang/test/OpenMP/target_parallel_for_simd_lastprivate_messages.cpp
@@ -107,6 +107,9 @@
#pragma omp target parallel for simd lastprivate(conditional: argc,s) lastprivate(conditional: // omp50-error {{expected expression}} omp45-error 2 {{use of undeclared identifier 'conditional'}} expected-error {{expected ')'}} expected-note {{to match this '('}} omp45-error 2 {{calling a private constructor of class 'S6'}} omp50-error {{expected list item of scalar type in 'lastprivate' clause with 'conditional' modifier}}
for (int k = 0; k < argc; ++k)
++k;
+#pragma omp target parallel for simd lastprivate(foo:argc) // omp50-error {{expected 'conditional' in OpenMP clause 'lastprivate'}} omp45-error {{expected ',' or ')' in 'lastprivate' clause}} omp45-error {{expected ')'}} omp45-error {{expected variable name}} omp45-note {{to match this '('}}
+ for (int k = 0; k < argc; ++k)
+++k;
#pragma omp target parallel for simd lastprivate(S1) // expected-error {{'S1' does not refer to a value}}
for (int k = 0; k < argc; ++k)
++k;
Index: clang/test/OpenMP/target_parallel_for_lastprivate_messages.cpp
===
--- clang/test/OpenMP/target_parallel_for_lastprivate_messages.cpp
+++ clang/test/OpenMP/target_parallel_for_lastprivate_messages.cpp
@@ -107,6 +107,9 @@
#pragma omp target parallel for lastprivate(conditional: s,argc) lastprivate(conditional: // omp50-error {{expected expression}} omp45-error 2 {{use of undeclared identifier 'conditional'}} expected-error {{expected ')'}} expected-note {{to match this '('}} omp50-error {{expected list item of scalar type in 'lastprivate' clause with 'conditional' modifier}}
for (int k = 0; k < argc; ++k)
++k;
+#pragma omp target parallel for lastprivate(foo:argc) // omp50-error {{expected 'conditional' in OpenMP clause 'lastprivate'}} omp45-error {{expected ',' or ')' in 'lastprivate' clause}} omp45-error {{expected ')'}} omp45-error {{expected variable name}} omp45-note {{to match this '('}}
+ for (int k = 0; k < argc; ++k)
+++k;
#pragma omp target parallel for lastprivate(S1) // expected-error {{'S1' does not refer to a value}}
for (int k = 0; k < argc; ++k)
++k;
Index: clang/test/OpenMP/simd_lastprivate_messages.cpp
===
--- clang/test/OpenMP/simd_lastprivate_messages.cpp
+++ clang/test/OpenMP/simd_lastprivate_messages.cpp
@@ -96,6 +96,9 @@
#pragma omp simd lastprivate(conditional: argc,g) lastprivate(conditional: // omp50-error {{expected expression}} omp45-error 2
[PATCH] D71846: [ASTMatchers] Fix for https://bugs.llvm.org/show_bug.cgi?id=44364
njames93 updated this revision to Diff 235301.
njames93 edited the summary of this revision.
njames93 added a comment.
fixed a spelling mistake in unit tests
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D71846/new/
https://reviews.llvm.org/D71846
Files:
clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return.cpp
clang/docs/LibASTMatchersReference.html
clang/include/clang/ASTMatchers/ASTMatchers.h
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
@@ -1071,6 +1071,35 @@
LanguageMode::Cxx17OrLater));
}
+TEST(hasInitStatement, MatchesSelectionInitializers) {
+ EXPECT_TRUE(matches("void baz() { if (int i = 1; i > 0) {} }",
+ ifStmt(hasInitStatement(anything())),
+ LanguageMode::Cxx17OrLater));
+ EXPECT_TRUE(notMatches("void baz() { if (int i = 1) {} }",
+ ifStmt(hasInitStatement(anything();
+ EXPECT_TRUE(notMatches("void baz() { if (1 > 0) {} }",
+ ifStmt(hasInitStatement(anything();
+ EXPECT_TRUE(matches(
+ "void baz(int i) { switch (int j = i; j) { default: break; } }",
+ switchStmt(hasInitStatement(anything())), LanguageMode::Cxx17OrLater));
+ EXPECT_TRUE(notMatches("void baz(int i) { switch (i) { default: break; } }",
+ switchStmt(hasInitStatement(anything();
+}
+
+TEST(hasInitStatement, MatchesRangeForInitializers) {
+ EXPECT_TRUE(matches("void baz() {"
+ "int items[] = {};"
+ "for (auto = items; auto : arr) {}"
+ "}",
+ cxxForRangeStmt(hasInitStatement(anything())),
+ LanguageMode::Cxx2aOrLater));
+ EXPECT_TRUE(notMatches("void baz() {"
+ "int items[] = {};"
+ "for (auto : items) {}"
+ "}",
+ cxxForRangeStmt(hasInitStatement(anything();
+}
+
TEST(TemplateArgumentCountIs, Matches) {
EXPECT_TRUE(
matches("template struct C {}; C c;",
Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -279,6 +279,7 @@
REGISTER_MATCHER(hasIndex);
REGISTER_MATCHER(hasInit);
REGISTER_MATCHER(hasInitializer);
+ REGISTER_MATCHER(hasInitStatement);
REGISTER_MATCHER(hasKeywordSelector);
REGISTER_MATCHER(hasLHS);
REGISTER_MATCHER(hasLocalQualifiers);
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -4297,6 +4297,35 @@
return Node.isConstexpr();
}
+/// Matches selection statements with initializer.
+///
+/// Given:
+/// \code
+/// void foo() {
+///if (int i = foobar(); i > 0) {}
+///switch (int i = foobar(); i) {}
+///for (auto& a = get_range(); auto& x : a) {}
+/// }
+/// void bar() {
+///if (foobar() > 0) {}
+///switch (foobar()) {}
+///for (auto& x : get_range()) {}
+/// }
+/// \endcode
+/// ifStmt(hasInitStatement(anything()))
+/// matches the if statement in foo but not in bar.
+/// switchStmt(hasInitStatement(anything()))
+/// matches the switch statement in foo but not in bar.
+/// cxxForRangeStmt(hasInitStatement(anything()))
+/// matches the range for statement in foo but not in bar.
+AST_POLYMORPHIC_MATCHER_P(hasInitStatement,
+ AST_POLYMORPHIC_SUPPORTED_TYPES(IfStmt, SwitchStmt,
+ CXXForRangeStmt),
+ internal::Matcher, InnerMatcher) {
+ const Stmt *const Init = Node.getInit();
+ return (Init != nullptr && InnerMatcher.matches(*Init, Finder, Builder));
+}
+
/// Matches the condition expression of an if statement, for loop,
/// switch statement or conditional operator.
///
Index: clang/docs/LibASTMatchersReference.html
===
--- clang/docs/LibASTMatchersReference.html
+++ clang/docs/LibASTMatchersReference.html
@@ -3009,6 +3009,23 @@
+Matcherhttps://clang.llvm.org/doxygen/classclang_1_1Expr.html;>ExprnullPointerConstant
+Matches expressions that resolve to a null pointer constant, such as
+GNU's __null, C++11's nullptr, or C's NULL macro.
+
+Given:
+ void *v1 = NULL;
+ void *v2 = nullptr;
+ void *v3 = __null; // GNU extension
+ char *cp = (char *)0;
+ int *ip = 0;
+
[PATCH] D69868: Allow "callbr" to return non-void values
xbolva00 added subscribers: aaron.ballman, xbolva00.
xbolva00 added inline comments.
Comment at: llvm/lib/CodeGen/MachineBasicBlock.cpp:1116
+if (auto *cbr = dyn_cast(getBasicBlock()->getTerminator()))
+ if (cbr->getDefaultDest() != bb)
+for (unsigned i = 0, e = cbr->getNumIndirectDests(); i != e; ++i)
void wrote:
> nickdesaulniers wrote:
> > void wrote:
> > > rnk wrote:
> > > > Is it possible for a BB to be both an indirect successor and a
> > > > fallthrough successor? I suppose that could be the case with the Linux
> > > > macro that gets the current PC.
> > > >
> > > > In any case, it's probably safe to remove this condition, and then we
> > > > don't have to worry.
> > > It is possible. I have a testcase for it in this patch. :-)
> > But you don't have a test case for what @rnk is asking about. (Unsure if it
> > would be necessary).
> >
> > I think @rnk is getting at this case from C:
> >
> > ```
> > void foo(void) {
> > asm goto ("#NICK":: "r"(&) :: hello);
> > hello:
> > return;
> > }
> > ```
> > Clang will emit this as:
> > ```
> > define dso_local void @foo() #0 {
> > entry:
> > callbr void asm sideeffect "#NICK", "r,X,~{dirflag},~{fpsr},~{flags}"(i8*
> > blockaddress(@foo, %hello), i8* blockaddress(@foo, %hello)) #1
> > to label %asm.fallthrough [label %hello], !srcloc !2
> >
> > asm.fallthrough: ; preds = %entry
> > br label %hello
> >
> > hello:; preds =
> > %asm.fallthrough, %entry
> > ret void
> > }
> > ```
> > ie. the `blockaddress` is passed twice, once as the address of a label (GNU
> > C extension), once as the indirect destination of the `asm goto`.
> >
> > So to @rnk 's question:
> > > Is it possible for a BB to be both an indirect successor and a
> > > fallthrough successor?
> > It is valid LLVM IR to have a BB be both; Clang today (or with
> > https://reviews.llvm.org/D69876) will not emit such formation (but could).
> >
> > ie. imagine the above:
> >
> > > callbr void asm sideeffect "#NICK",
> > > "r,X,~{dirflag},~{fpsr},~{flags}"(i8* blockaddress(@foo, %hello), i8*
> > > blockaddress(@foo, %hello)) #1
> > > to label %asm.fallthrough [label %hello], !srcloc !2
> >
> > to instead be:
> >
> > > callbr void asm sideeffect "#NICK",
> > > "r,X,~{dirflag},~{fpsr},~{flags}"(i8* blockaddress(@foo, %hello), i8*
> > > blockaddress(@foo, %asm.fallthrough)) #1
> > > to label %asm.fallthrough [label %asm.fallthrough], !srcloc !2
> >
> > I still don't understand @rnk 's point about
> > > In any case, it's probably safe to remove this condition, and then we
> > > don't have to worry.
> > though.
> I meant that I had a testcase that required the conditional he suggested I
> remove. Sorry for the confusion.
>
> You're right that we can generate the clang code where the fallthrough is the
> same as the indirect. I didn't mean to imply that it wasn't the case. I can
> add a testcase. I asked a question on the "cfe-dev" mailing list to determine
> how I could identify the "fallthrough" block from the CFG. So far no one has
> responded. Until I can determine that, I won't be able to handle that
> properly with this conditional.
Maybe @aaron.ballman can answer your question related to “fallthrough” block..
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D69868/new/
https://reviews.llvm.org/D69868
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D71846: [ASTMatchers] Fix for https://bugs.llvm.org/show_bug.cgi?id=44364
njames93 updated this revision to Diff 235300.
njames93 marked an inline comment as done.
njames93 added a comment.
Renamed 'hasInitStorage' to 'hasInitStatement'.
Added support for range for loops in 'hasInitStatement'.
'hasInitStatement' now has a match predicate on the init statement.
ReadabilityElseAfterReturn check now checks the init statement is a decl
statement before disregarding it.
Added test cases for check and the matcher.
Regenerated the docs for the AST Matchers.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D71846/new/
https://reviews.llvm.org/D71846
Files:
clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return.cpp
clang/docs/LibASTMatchersReference.html
clang/include/clang/ASTMatchers/ASTMatchers.h
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
@@ -1071,6 +1071,35 @@
LanguageMode::Cxx17OrLater));
}
+TEST(hasInitStorage, MatchesSelectionInitializers) {
+ EXPECT_TRUE(matches("void baz() { if (int i = 1; i > 0) {} }",
+ ifStmt(hasInitStatement(anything())),
+ LanguageMode::Cxx17OrLater));
+ EXPECT_TRUE(notMatches("void baz() { if (int i = 1) {} }",
+ ifStmt(hasInitStatement(anything();
+ EXPECT_TRUE(notMatches("void baz() { if (1 > 0) {} }",
+ ifStmt(hasInitStatement(anything();
+ EXPECT_TRUE(matches(
+ "void baz(int i) { switch (int j = i; j) { default: break; } }",
+ switchStmt(hasInitStatement(anything())), LanguageMode::Cxx17OrLater));
+ EXPECT_TRUE(notMatches("void baz(int i) { switch (i) { default: break; } }",
+ switchStmt(hasInitStatement(anything();
+}
+
+TEST(hasInitStorage, MatchesRangeForInitializers) {
+ EXPECT_TRUE(matches("void baz() {"
+ "int items[] = {};"
+ "for (auto = items; auto : arr) {}"
+ "}",
+ cxxForRangeStmt(hasInitStatement(anything())),
+ LanguageMode::Cxx2aOrLater));
+ EXPECT_TRUE(notMatches("void baz() {"
+ "int items[] = {};"
+ "for (auto : items) {}"
+ "}",
+ cxxForRangeStmt(hasInitStatement(anything();
+}
+
TEST(TemplateArgumentCountIs, Matches) {
EXPECT_TRUE(
matches("template struct C {}; C c;",
Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -279,6 +279,7 @@
REGISTER_MATCHER(hasIndex);
REGISTER_MATCHER(hasInit);
REGISTER_MATCHER(hasInitializer);
+ REGISTER_MATCHER(hasInitStatement);
REGISTER_MATCHER(hasKeywordSelector);
REGISTER_MATCHER(hasLHS);
REGISTER_MATCHER(hasLocalQualifiers);
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -4297,6 +4297,35 @@
return Node.isConstexpr();
}
+/// Matches selection statements with initializer.
+///
+/// Given:
+/// \code
+/// void foo() {
+///if (int i = foobar(); i > 0) {}
+///switch (int i = foobar(); i) {}
+///for (auto& a = get_range(); auto& x : a) {}
+/// }
+/// void bar() {
+///if (foobar() > 0) {}
+///switch (foobar()) {}
+///for (auto& x : get_range()) {}
+/// }
+/// \endcode
+/// ifStmt(hasInitStatement(anything()))
+/// matches the if statement in foo but not in bar.
+/// switchStmt(hasInitStatement(anything()))
+/// matches the switch statement in foo but not in bar.
+/// cxxForRangeStmt(hasInitStatement(anything()))
+/// matches the range for statement in foo but not in bar.
+AST_POLYMORPHIC_MATCHER_P(hasInitStatement,
+ AST_POLYMORPHIC_SUPPORTED_TYPES(IfStmt, SwitchStmt,
+ CXXForRangeStmt),
+ internal::Matcher, InnerMatcher) {
+ const Stmt *const Init = Node.getInit();
+ return (Init != nullptr && InnerMatcher.matches(*Init, Finder, Builder));
+}
+
/// Matches the condition expression of an if statement, for loop,
/// switch statement or conditional operator.
///
Index: clang/docs/LibASTMatchersReference.html
===
--- clang/docs/LibASTMatchersReference.html
+++ clang/docs/LibASTMatchersReference.html
@@ -3009,6 +3009,23 @@
[PATCH] D70258: [OpenMP][IR-Builder] Introduce the finalization stack
This revision was automatically updated to reflect the committed changes.
Closed by commit rGf9c3c5da19ab: [OpenMP][IR-Builder] Introduce the
finalization stack (authored by jdoerfert).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D70258/new/
https://reviews.llvm.org/D70258
Files:
clang/lib/CodeGen/CGOpenMPRuntime.cpp
llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
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
@@ -104,7 +104,15 @@
BasicBlock *CBB = BasicBlock::Create(Ctx, "", F);
new UnreachableInst(Ctx, CBB);
- OMPBuilder.setCancellationBlock(CBB);
+ auto FiniCB = [CBB](llvm::OpenMPIRBuilder::InsertPointTy IP) {
+assert(IP.getBlock()->end() == IP.getPoint() &&
+ "Clang CG should cause non-terminated block!");
+BranchInst::Create(CBB, IP.getBlock());
+ };
+ // Emulate an outer parallel.
+ llvm::OpenMPIRBuilder::FinalizationInfo FI(
+ {FiniCB, OMPD_parallel, /* HasCancel */ true});
+ OMPBuilder.pushFinalizationCB(std::move(FI));
IRBuilder<> Builder(BB);
@@ -113,7 +121,7 @@
Builder.restoreIP(NewIP);
EXPECT_FALSE(M->global_empty());
EXPECT_EQ(M->size(), 3U);
- EXPECT_EQ(F->size(), 3U);
+ EXPECT_EQ(F->size(), 4U);
EXPECT_EQ(BB->size(), 4U);
CallInst *GTID = dyn_cast(>front());
@@ -133,10 +141,15 @@
Instruction *BarrierBBTI = Barrier->getParent()->getTerminator();
EXPECT_EQ(BarrierBBTI->getNumSuccessors(), 2U);
EXPECT_EQ(BarrierBBTI->getSuccessor(0), NewIP.getBlock());
- EXPECT_EQ(BarrierBBTI->getSuccessor(1), CBB);
+ EXPECT_EQ(BarrierBBTI->getSuccessor(1)->getTerminator()->getNumSuccessors(),
+1U);
+ EXPECT_EQ(BarrierBBTI->getSuccessor(1)->getTerminator()->getSuccessor(0),
+CBB);
EXPECT_EQ(cast(Barrier)->getArgOperand(1), GTID);
+ OMPBuilder.popFinalizationCB();
+
Builder.CreateUnreachable();
EXPECT_FALSE(verifyModule(*M));
}
Index: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -205,7 +205,8 @@
// If we are in a cancellable parallel region, barriers are cancellation
// points.
// TODO: Check why we would force simple calls or to ignore the cancel flag.
- bool UseCancelBarrier = !ForceSimpleCall && CancellationBlock;
+ bool UseCancelBarrier =
+ !ForceSimpleCall && isLastFinalizationInfoCancellable(OMPD_parallel);
Value *Result = Builder.CreateCall(
getOrCreateRuntimeFunction(UseCancelBarrier ? OMPRTL___kmpc_cancel_barrier
@@ -217,19 +218,22 @@
BasicBlock *BB = Builder.GetInsertBlock();
BasicBlock *NonCancellationBlock = BasicBlock::Create(
BB->getContext(), BB->getName() + ".cont", BB->getParent());
+BasicBlock *CancellationBlock = BasicBlock::Create(
+BB->getContext(), BB->getName() + ".cncl", BB->getParent());
// Jump to them based on the return value.
Value *Cmp = Builder.CreateIsNull(Result);
Builder.CreateCondBr(Cmp, NonCancellationBlock, CancellationBlock,
/* TODO weight */ nullptr, nullptr);
-Builder.SetInsertPoint(NonCancellationBlock);
-assert(CancellationBlock->getParent() == BB->getParent() &&
- "Unexpected cancellation block parent!");
+// From the cancellation block we finalize all variables and go to the
+// post finalization block that is known to the FiniCB callback.
+Builder.SetInsertPoint(CancellationBlock);
+auto = FinalizationStack.back();
+FI.FiniCB(Builder.saveIP());
-// TODO: This is a workaround for now, we always reset the cancellation
-// block until we manage it ourselves here.
-CancellationBlock = nullptr;
+// The continuation block is where code generation continues.
+Builder.SetInsertPoint(NonCancellationBlock);
}
return Builder.saveIP();
Index: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
===
--- llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
+++ llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
@@ -37,12 +37,44 @@
/// Add attributes known for \p FnID to \p Fn.
void addAttributes(omp::RuntimeFunction FnID, Function );
- /// Set the cancellation block to \p CBB.
- void setCancellationBlock(BasicBlock *CBB) { CancellationBlock = CBB; }
-
/// Type used throughout for insertion points.
using InsertPointTy = IRBuilder<>::InsertPoint;
+ /// Callback type for variable finalization (think destructors).
+ ///
+ /// \param CodeGenIP is the insertion point at which the finalization code
+ /// should be placed.
+ ///
+
[clang] f9c3c5d - [OpenMP][IR-Builder] Introduce the finalization stack
Author: Johannes Doerfert
Date: 2019-12-25T16:57:08-06:00
New Revision: f9c3c5da19ab6d8cbbd4611fbb24e97fe7a85078
URL:
https://github.com/llvm/llvm-project/commit/f9c3c5da19ab6d8cbbd4611fbb24e97fe7a85078
DIFF:
https://github.com/llvm/llvm-project/commit/f9c3c5da19ab6d8cbbd4611fbb24e97fe7a85078.diff
LOG: [OpenMP][IR-Builder] Introduce the finalization stack
As a permanent and generic solution to the problem of variable
finalization (destructors, lastprivate, ...), this patch introduces the
finalization stack. The objects on the stack describe (1) the
(structured) regions the OpenMP-IR-Builder is currently constructing,
(2) if these are cancellable, and (3) the callback that will perform the
finalization (=cleanup) when necessary.
As the finalization can be necessary multiple times, at different source
locations, the callback takes the position at which code is currently
generated. This position will also encode the destination of the "region
exit" block *iff* the finalization call was issues for a region
generated by the OpenMPIRBuilder. For regions generated through the old
Clang OpenMP code geneneration, the "region exit" is determined by Clang
inside the finalization call instead (see getOMPCancelDestination).
As a first user, the parallel + cancel barrier interaction is changed.
In contrast to the temporary solution before, the barrier generation in
Clang does not need to be aware of the "CancelDestination" block.
Instead, the finalization callback is and, as described above, later
even that one does not need to be.
D70109 will be updated to use this scheme.
Reviewed By: ABataev
Differential Revision: https://reviews.llvm.org/D70258
Added:
Modified:
clang/lib/CodeGen/CGOpenMPRuntime.cpp
llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
Removed:
diff --git a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
index 2a53e9993533..c5859a52f3f3 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -1441,6 +1441,50 @@ CGOpenMPRuntime::getUserDefinedReduction(const
OMPDeclareReductionDecl *D) {
return UDRMap.lookup(D);
}
+// Temporary RAII solution to perform a push/pop stack event on the OpenMP IR
+// Builder if one is present.
+struct PushAndPopStackRAII {
+ PushAndPopStackRAII(llvm::OpenMPIRBuilder *OMPBuilder, CodeGenFunction ,
+ bool HasCancel)
+ : OMPBuilder(OMPBuilder) {
+if (!OMPBuilder)
+ return;
+
+// The following callback is the crucial part of clangs cleanup process.
+//
+// NOTE:
+// Once the OpenMPIRBuilder is used to create parallel regions (and
+// similar), the cancellation destination (Dest below) is determined via
+// IP. That means if we have variables to finalize we split the block at
IP,
+// use the new block (=BB) as destination to build a JumpDest (via
+// getJumpDestInCurrentScope(BB)) which then is fed to
+// EmitBranchThroughCleanup. Furthermore, there will not be the need
+// to push & pop an FinalizationInfo object.
+// The FiniCB will still be needed but at the point where the
+// OpenMPIRBuilder is asked to construct a parallel (or similar) construct.
+auto FiniCB = [](llvm::OpenMPIRBuilder::InsertPointTy IP) {
+ assert(IP.getBlock()->end() == IP.getPoint() &&
+ "Clang CG should cause non-terminated block!");
+ CGBuilderTy::InsertPointGuard IPG(CGF.Builder);
+ CGF.Builder.restoreIP(IP);
+ CodeGenFunction::JumpDest Dest =
+ CGF.getOMPCancelDestination(OMPD_parallel);
+ CGF.EmitBranchThroughCleanup(Dest);
+};
+
+// TODO: Remove this once we emit parallel regions through the
+// OpenMPIRBuilder as it can do this setup internally.
+llvm::OpenMPIRBuilder::FinalizationInfo FI(
+{FiniCB, OMPD_parallel, HasCancel});
+OMPBuilder->pushFinalizationCB(std::move(FI));
+ }
+ ~PushAndPopStackRAII() {
+if (OMPBuilder)
+ OMPBuilder->popFinalizationCB();
+ }
+ llvm::OpenMPIRBuilder *OMPBuilder;
+};
+
static llvm::Function *emitParallelOrTeamsOutlinedFunction(
CodeGenModule , const OMPExecutableDirective , const CapturedStmt
*CS,
const VarDecl *ThreadIDVar, OpenMPDirectiveKind InnermostKind,
@@ -1465,6 +1509,11 @@ static llvm::Function
*emitParallelOrTeamsOutlinedFunction(
else if (const auto *OPFD =
dyn_cast())
HasCancel = OPFD->hasCancel();
+
+ // TODO: Temporarily inform the OpenMPIRBuilder, if any, about the new
+ // parallel region to make cancellation barriers work properly.
+ llvm::OpenMPIRBuilder *OMPBuilder = CGM.getOpenMPIRBuilder();
+ PushAndPopStackRAII PSR(OMPBuilder, CGF, HasCancel);
CGOpenMPOutlinedRegionInfo CGInfo(*CS, ThreadIDVar, CodeGen, InnermostKind,
[PATCH] D71880: [clangd] Implement Decl canonicalization rules for rename
merge_guards_bot added a comment.
{icon check-circle color=green} Unit tests: pass. 61107 tests passed, 0 failed
and 728 were skipped.
{icon times-circle color=red} clang-tidy: fail. Please fix clang-tidy findings
.
{icon check-circle color=green} clang-format: pass.
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/D71880/new/
https://reviews.llvm.org/D71880
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D71880: [clangd] Implement Decl canonicalization rules for rename
kbobyrev planned changes to this revision. kbobyrev added a comment. This is a WIP draft. Few items to address before pushing for a review - More tests: renaming is a complex feature and I want to make sure there are no regressions/corner-cases I did not handle. It might make sense to adopt a bunch of tests from Clang-Rename and also add a number of new tests. - Detailed documentation: explanation of AST navigation used here, description of limitations (virtual functions "incorrect" handling, notes on performance, etc). - Better naming The logical change after this patch might be changing Clang-Rename to rely on this API and completely removing USR-based approach, because it is no longer maintained and is unlikely to evolve to match the expectations of existing users, but this might require some substantial infrastructure changes and I'm not planning to do that soon. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71880/new/ https://reviews.llvm.org/D71880 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D71880: [clangd] Implement Decl canonicalization rules for rename
kbobyrev created this revision.
Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous,
MaskRay, ilya-biryukov.
Herald added a project: clang.
This patch introduces new canonicalization rules which are used for AST-based
rename in Clangd. By comparing two canonical declarations of inspected nodes,
Clangd determines whether both of them belong to the same entity user would
like to rename. Such functionality is relatively concise compared to the
Clang-Rename API that is used right now. It also helps to overcome the
limitations that Clang-Rename originally had and helps to eliminate several
classes of bugs.
Clangd AST-based rename currently relies on Clangd-Rename functionality, which
has a better test coverage and hence provides some correctness guarantees. This
pattch decouples two rename features and therefore introduces new challenges to
the Clangd maintenance. To prevent regressions and improve stability of Clangd
rename feature, there are new unittests that come with this change.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D71880
Files:
clang-tools-extra/clangd/refactor/Rename.cpp
clang-tools-extra/clangd/unittests/RenameTests.cpp
Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -88,10 +88,87 @@
}
TEST(RenameTest, WithinFileRename) {
- // rename is runnning on all "^" points, and "[[]]" ranges point to the
- // identifier that is being renamed.
+ // For each "^" this test moves cursor to its location and applies renaming
+ // while checking that all identifiers enclosed in [[]] ranges are handled
+ // correctly.
+ // TODO(kirillbobyrev): Add more tests.
llvm::StringRef Tests[] = {
- // Function.
+ // Templated static method instantiation.
+ R"cpp(
+template
+class Foo {
+public:
+ static T [[f^oo]]() {}
+}
+
+void bar() {
+ Foo::[[f^oo]]();
+}
+ )cpp",
+
+ // Templated method instantiation.
+ R"cpp(
+template
+class Foo {
+public:
+ T [[f^oo]]() {}
+}
+
+void bar() {
+ Foo().[[f^oo]]();
+}
+ )cpp",
+
+ // Class template (partial) specialization forward declarations.
+ R"cpp(
+template
+class [[Foo^]];
+
+template
+class [[Foo^]] {};
+
+template
+class [[Foo^]];
+ )cpp",
+
+ // Class template (full) specialization forward declaration.
+ R"cpp(
+template
+class [[Foo^]];
+
+template
+class [[Foo^]] {};
+ )cpp",
+
+ // Function template specialization forward declaration.
+ R"cpp(
+template
+U [[foo^]]();
+
+template
+U [[foo^]]() {};
+ )cpp",
+
+ // Function template specialization forward declaration.
+ R"cpp(
+template
+U [[foo^]]() {};
+
+template
+U [[foo^]]();
+ )cpp",
+
+ // Function template specialization forward declaration without function
+ // definition.
+ R"cpp(
+template
+U [[foo^]]();
+
+template
+U [[foo^]]();
+ )cpp",
+
+ // Simple recursive function.
R"cpp(
void [[foo^]]() {
[[fo^o]]();
@@ -116,15 +193,16 @@
}
)cpp",
- // Rename class, including constructor/destructor.
+ // Class, its constructor and destructor.
R"cpp(
class [[F^oo]] {
+public:
[[F^oo]]();
- ~[[Foo]]();
+ ~[[Fo^o]]();
void foo(int x);
};
-[[Foo]]::[[Fo^o]]() {}
-void [[Foo]]::foo(int x) {}
+[[Fo^o]]::[[Fo^o]]() {}
+void [[Fo^o]]::foo(int x) {}
)cpp",
// Class in template argument.
@@ -175,9 +253,9 @@
class [[F^oo]] {};
void test() {
- [[Foo]] x;
- [[Foo]] y;
- [[Foo]] z;
+ [[F^oo]] x;
+ [[Fo^o]] y;
+ [[Foo^]] z;
}
)cpp",
@@ -303,7 +381,7 @@
void qoo() {
[[foo]]();
- boo([[foo]]());
+ boo([[fo^o]]());
M1();
boo(M1());
M2([[foo]]());
@@ -396,7 +474,7 @@
}
)cpp",
- // template class in template argument list.
+ // Template class in template argument list.
R"cpp(
template
class [[Fo^o]] {};
Index: clang-tools-extra/clangd/refactor/Rename.cpp
===
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Rename.cpp
@@ -209,6 +209,57 @@
llvm::inconvertibleErrorCode());
}
+const Decl *getRenameRootDecl(const ClassTemplateSpecializationDecl *D) {
+ return
[PATCH] D71846: [ASTMatchers] Fix for https://bugs.llvm.org/show_bug.cgi?id=44364
njames93 updated this revision to Diff 235278.
njames93 added a comment.
I added test cases for the matchers and the check, wasn't able to add the
CXXRangeFor to the matcher as that class uses a different dialect for the
initializer statement handling. I also couldn't generate the new documentation
as the script always fails on my machine, even without the changes i have made,
resulting in the ASTMatcher html being empty
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D71846/new/
https://reviews.llvm.org/D71846
Files:
clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return.cpp
clang/include/clang/ASTMatchers/ASTMatchers.h
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
@@ -1071,6 +1071,20 @@
LanguageMode::Cxx17OrLater));
}
+TEST(hasInitStorage, MatchesSelectionInitializers) {
+ EXPECT_TRUE(matches("void baz() { if (int i = 1; i > 0) {} }",
+ ifStmt(hasInitStorage()), LanguageMode::Cxx17OrLater));
+ EXPECT_TRUE(
+ notMatches("void baz() { if (int i = 1) {} }", ifStmt(hasInitStorage(;
+ EXPECT_TRUE(
+ notMatches("void baz() { if (1 > 0) {} }", ifStmt(hasInitStorage(;
+ EXPECT_TRUE(
+ matches("void baz(int i) { switch (int j = i; j) { default: break; } }",
+ switchStmt(hasInitStorage()), LanguageMode::Cxx17OrLater));
+ EXPECT_TRUE(notMatches("void baz(int i) { switch (i) { default: break; } }",
+ switchStmt(hasInitStorage(;
+}
+
TEST(TemplateArgumentCountIs, Matches) {
EXPECT_TRUE(
matches("template struct C {}; C c;",
Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -279,6 +279,7 @@
REGISTER_MATCHER(hasIndex);
REGISTER_MATCHER(hasInit);
REGISTER_MATCHER(hasInitializer);
+ REGISTER_MATCHER(hasInitStorage);
REGISTER_MATCHER(hasKeywordSelector);
REGISTER_MATCHER(hasLHS);
REGISTER_MATCHER(hasLocalQualifiers);
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -4297,6 +4297,22 @@
return Node.isConstexpr();
}
+/// Matches selection statements with initializer.
+///
+/// Given:
+/// \code
+/// if (int foo = bar(); bar > 0) {}
+/// switch (int baz = bar(); baz) {}
+/// \endcode
+/// ifStmt(hasInitStorage())
+/// matches the declaration of foo.
+/// switchStmt(hasInitStorage())
+/// matches the declaration of baz.
+AST_POLYMORPHIC_MATCHER(hasInitStorage,
+AST_POLYMORPHIC_SUPPORTED_TYPES(IfStmt, SwitchStmt)) {
+ return Node.hasInitStorage();
+}
+
/// Matches the condition expression of an if statement, for loop,
/// switch statement or conditional operator.
///
Index: clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s readability-else-after-return %t -- -- -fexceptions
+// RUN: %check_clang_tidy %s readability-else-after-return %t -- -- -fexceptions -std=c++17
namespace std {
struct string {
@@ -117,3 +117,12 @@
return x;
}
}
+
+int *init_and_condition() {
+ if (int *x = g(); x != nullptr) {
+return x;
+ } else {
+h();
+return x;
+ }
+}
Index: clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
@@ -26,12 +26,14 @@
compoundStmt(forEach(
ifStmt(unless(isConstexpr()),
// FIXME: Explore alternatives for the
- // `if (T x = ...) {... return; } else { }`
- // pattern:
+ // `if (T x = ...) {... return; } else { }` and
+ // 'if (T x = ...; cond) {... return; } else { use }'
+ // patterns:
// * warn, but don't fix;
// * fix by pulling out the variable declaration out of
// the condition.
unless(hasConditionVariableStatement(anything())),
+
[PATCH] D70876: [clang-tidy] Add spuriously-wake-up-functions check
sylvestre.ledru added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/list.rst:75 `bugprone-sizeof-expression `_, , "high" + `bugprone-spuriously-wake-up-functions `_, , "" `bugprone-string-constructor `_, "Yes", "high" Could you try to evaluate the severity? :) thanks Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70876/new/ https://reviews.llvm.org/D70876 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
