[PATCH] D94973: [clang][OpenMP] Use OpenMPIRBuilder for workshare loops.
Michael Kruse via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Feb 15 13:16:50 PST 2021
Meinersbur added inline comments.
================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:5215
+ Expr *NegIncAmount =
+ AssertSuccess(Actions.BuildUnaryOp(nullptr, {}, UO_Minus, NewStep));
+ Expr *BackwardDist = AssertSuccess(
----------------
Meinersbur wrote:
> jdenny wrote:
> > It looks like the AST nodes `NewStart`, `NewStop`, and `NewStep` are shared by multiple parent nodes here. Does that ever happen anywhere else in a Clang AST?
> Yes. For instance, when instantiating a template, nodes form the TemplatedDecl are reused unless they need to change. Therefore multiple template instantiation can also share entire AST subtrees. Why rebuild them if they are identical?
>
> However, I also found the following description of TreeTransform::AlwaysRebuild():
> ```
> /// We must always rebuild all AST nodes when performing variadic template
> /// pack expansion, in order to avoid violating the AST invariant that each
> /// statement node appears at most once in its containing declaration.
> ```
> It was added by @rsmith in rG2aa81a718e64fb4ca651ea12ab7afaeffc11e463 to fix llvm.org/PR17800 . The assertion doesn't seem to exist in the current code base.
>
> Does this apply here?
I think the invariant exists because codegen has a map from ast expression nodes to its temporary memory location. This means that reusing the same AST expression node indeed should not be done.
I think the assertion that failed in llvm.org/PR17800 is now here:
https://github.com/llvm/llvm-project/blob/7ba2e1c6011eeb1b91ce3f5d8fa7187b7518e77a/clang/lib/AST/ExprConstant.cpp#L1872
Assertion message was changed in this commit:
https://github.com/llvm/llvm-project/commit/f7f2e4261a98b2da519d58e7f6794b013cda7a4b
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D94973/new/
https://reviews.llvm.org/D94973
More information about the cfe-commits
mailing list