Meinersbur added inline comments.
================
Comment at: clang/lib/CodeGen/CodeGenFunction.h:292
+ /// nest would extend.
+ SmallVector<llvm::CanonicalLoopInfo *, 4> OMPLoopNestStack;
+
----------------
jdenny wrote:
> Unless I missed something, the only accesses to `OMPLoopNestStack` are
> `push_back`, `clear`, `back`, and `size`. It's never popped. That is, every
> push appears to mean that all existing elements will never be accessed again.
> So why is it a stack? Is that purely for the sake of the assertion that
> calls `size`?
When eventually supporting loop nests, we will need not only the outermost
loop, but also inner ones.
================
Comment at: clang/lib/Sema/TreeTransform.h:8321
+TreeTransform<Derived>::TransformOMPCanonicalLoop(OMPCanonicalLoop *L) {
+ // The OMPCanonicalLoop will be recreated when transforming the
loop-associted
+ // directive.
----------------
jdenny wrote:
> I'm used to seeing `TransformX` call `RebuildX` call `ActOnX`. Why not do
> that for `X=OMPCanonicalLoop`? Does `TransformOMPExecutableDirective` really
> need a special case for `OMPCanonicalLoop`?
The intended behaviour is:
1. Transform the child loop
2. Return the child loop as representing the OMPCanonicalLoop (i.e.
OMPCanonicalLoop wrapper is removed).
3. Parent loop-associated directive (e.g. workshare) is processed.
4. `ActOnOpenMPCanonicalLoop` adds new OMPCanonicalLoop wrapper for loop nest.
This guarantees maximum flexibility on what of the loop can be changed, such as
* Change lower bound, upper bound, step
* Convert between CXXForRangeStmt and ForStmt
* Change the associated depth (e.g. different value for `collapse` clause)
* Remove the directive and no OMPCanonicalLoop remain
This also saves adding many lines of code handling transforming each member of
OMPCanonicalLoop separately.
================
Comment at: clang/tools/libclang/CIndex.cpp:2839
+ VisitStmt(L);
+ EnqueueChildren(L);
+}
----------------
jdenny wrote:
> Doesn't `VisitStmt(L)` already call `EnqueueChildren(L)`?
It does indeed.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D94973/new/
https://reviews.llvm.org/D94973
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
