-
Notifications
You must be signed in to change notification settings - Fork 12.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Clang][OpenMP] Fix the wrong transform of num_teams
claused introduced in #99732
#102716
[Clang][OpenMP] Fix the wrong transform of num_teams
claused introduced in #99732
#102716
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
@llvm/pr-subscribers-clang Author: Shilei Tian (shiltian) ChangesFull diff: https://github.com/llvm/llvm-project/pull/102716.diff 1 Files Affected:
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 8f6f30434af65e..029d6d57319ac0 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -11018,11 +11018,16 @@ TreeTransform<Derived>::TransformOMPAllocateClause(OMPAllocateClause *C) {
template <typename Derived>
OMPClause *
TreeTransform<Derived>::TransformOMPNumTeamsClause(OMPNumTeamsClause *C) {
- ExprResult E = getDerived().TransformExpr(C->getNumTeams().front());
- if (E.isInvalid())
- return nullptr;
+ llvm::SmallVector<Expr *, 3> Vars;
+ Vars.reserve(C->varlist_size());
+ for (auto *VE : C->varlist()) {
+ ExprResult EVar = getDerived().TransformExpr(cast<Expr>(VE));
+ if (EVar.isInvalid())
+ return nullptr;
+ Vars.push_back(EVar.get());
+ }
return getDerived().RebuildOMPNumTeamsClause(
- E.get(), C->getBeginLoc(), C->getLParenLoc(), C->getEndLoc());
+ Vars, C->getBeginLoc(), C->getLParenLoc(), C->getEndLoc());
}
template <typename Derived>
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A test is required
I literally don't know how to test this TBH. I think our current OpenMP tests generally don't cover this at all. |
They do. To test this, you need to use clause in the template and check that template params successfully replaced upon instantiation. |
Aha, good to know. Thanks. Will add tests. |
2850033
to
838e1cc
Compare
838e1cc
to
5f168ad
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LG
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/123/builds/3507 Here is the relevant piece of the build log for the reference:
|
No description provided.