ABataev added inline comments.

================
Comment at: clang/include/clang/AST/StmtOpenMP.h:4781-4784
+/// This represents the '#pragma omp tile' loop transformation directive.
+class OMPTileDirective final
+    : public OMPLoopDirective,
+      private llvm::TrailingObjects<OMPTileDirective, OMPClause *, Stmt *> {
----------------
Meinersbur wrote:
> ABataev wrote:
> > Not sure that this is a good idea to treat this directive as the executable 
> > directive. To me, it looks like kind of `AttributedStmt`. Maybe better to 
> > introduce some kind of a new base node for this and similar constructs, 
> > which does not own the loop but is its kind of attribute-like entity?
> > Also, can we have something like:
> > ```
> > #pragma omp simd
> > #pragma omp tile ...
> > for(...) ;
> > ```
> > Thoughts?
> While not executed at runtime, syntactically it is parsed like a executable 
> (loop-associated) directive. IMHO it does 'own' the loop, but produces 
> another one for to be owned(/associated) by a different directive, as in your 
> tile/simd example, which should already work. Allowing this was the 
> motivation to do the transformation on the AST-level for now.
I'm not saying that we should separate parsing of this directive from others, 
it is just better to treat this directive as a little bit different node. 
Currently, it introduces too many changes in the base classes. Better to create 
a new base class, that does not relies on `CapturedStmt` as the base, and 
derive `OMPExecutableDirective` and this directive and other similar (+ maybe, 
`OMPSimdDirective`) from this new base class.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76342



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

Reply via email to