[PATCH] D111124: [Clang][OpenMP] Allow loop-transformations with template parameters.

2021-10-05 Thread Michael Kruse via Phabricator via cfe-commits 
Meinersbur created this revision.
Meinersbur added reviewers: ABataev, jdenny.
Meinersbur added projects: OpenMP, clang.
Herald added subscribers: zzheng, guansong, yaxunl.
Meinersbur requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added a subscriber: sstefan1.

Clang would reject

  #pragma omp for
  #pragma omp tile sizes(P)
  for (int i = 0; i < 128; ++i) {}

where P is a template parameter, but the loop itself is not template-dependent. 
Because P context-dependent, the TransformedStmt cannot be generated and 
therefore nullptr (until the template is instantiated). The OMPForDirective 
would still expect the a loop is the dependent context and trigger an error.

Fix by introducing a NumGeneratedLoops field to OMPLoopTransformation. This is 
used to distinguish the case where no TransformedStmt will be generated at all 
(e.g. `#pragma omp unroll full`) and template instantiation is needed. In the 
later case, delay resolving the iteration space like when the for-loop itself 
is template-dependent until the template instatiation.

A more radical solution would always delay the iteration space analysis until 
template instantiation, but would also break many test cases.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D24

Files:
  clang/include/clang/AST/StmtOpenMP.h
  clang/lib/AST/StmtOpenMP.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/test/OpenMP/tile_ast_print.cpp
  clang/test/OpenMP/unroll_ast_print.cpp

Index: clang/test/OpenMP/unroll_ast_print.cpp
===
--- clang/test/OpenMP/unroll_ast_print.cpp
+++ clang/test/OpenMP/unroll_ast_print.cpp
@@ -124,4 +124,26 @@
   unroll_templated();
 }
 
+
+// PRINT-LABEL: template  void unroll_templated_factor(int start, int stop, int step) {
+// DUMP-LABEL:  FunctionTemplateDecl {{.*}} unroll_templated_factor
+template 
+void unroll_templated_factor(int start, int stop, int step) {
+  // PRINT: #pragma omp unroll partial(Factor)
+  // DUMP:  OMPUnrollDirective
+  // DUMP-NEXT: OMPPartialClause
+  // DUMP-NEXT:   DeclRefExpr {{.*}} 'Factor' 'int'
+  #pragma omp unroll partial(Factor)
+// PRINT-NEXT: for (int i = start; i < stop; i += step)
+// DUMP-NEXT:  ForStmt
+for (int i = start; i < stop; i += step)
+  // PRINT-NEXT: body(i);
+  // DUMP:  CallExpr
+  body(i);
+}
+void unroll_template_factor() {
+  unroll_templated_factor<4>(0, 42, 2);
+}
+
+
 #endif
Index: clang/test/OpenMP/tile_ast_print.cpp
===
--- clang/test/OpenMP/tile_ast_print.cpp
+++ clang/test/OpenMP/tile_ast_print.cpp
@@ -162,4 +162,25 @@
 }
 
 
+// PRINT-LABEL: template  void foo7(int start, int stop, int step) {
+// DUMP-LABEL: FunctionTemplateDecl {{.*}} foo7
+template 
+void foo7(int start, int stop, int step) {
+  // PRINT: #pragma omp tile sizes(Tile)
+  // DUMP:  OMPTileDirective
+  // DUMP-NEXT:   OMPSizesClause
+  // DUMP-NEXT: DeclRefExpr {{.*}} 'Tile' 'int'
+  #pragma omp tile sizes(Tile)
+// PRINT-NEXT:  for (int i = start; i < stop; i += step)
+// DUMP-NEXT: ForStmt
+for (int i = start; i < stop; i += step)
+  // PRINT-NEXT: body(i);
+  // DUMP:  CallExpr
+  body(i);
+}
+void tfoo7() {
+  foo7<5>(0, 42, 2);
+}
+
+
 #endif
Index: clang/lib/Serialization/ASTWriterStmt.cpp
===
--- clang/lib/Serialization/ASTWriterStmt.cpp
+++ clang/lib/Serialization/ASTWriterStmt.cpp
@@ -2227,6 +2227,7 @@
 void ASTStmtWriter::VisitOMPLoopTransformationDirective(
 OMPLoopTransformationDirective *D) {
   VisitOMPLoopBasedDirective(D);
+  Record.writeUInt32(D->getNumGeneratedLoops());
 }
 
 void ASTStmtWriter::VisitOMPTileDirective(OMPTileDirective *D) {
Index: clang/lib/Serialization/ASTReaderStmt.cpp
===
--- clang/lib/Serialization/ASTReaderStmt.cpp
+++ clang/lib/Serialization/ASTReaderStmt.cpp
@@ -2327,6 +2327,7 @@
 void ASTStmtReader::VisitOMPLoopTransformationDirective(
 OMPLoopTransformationDirective *D) {
   VisitOMPLoopBasedDirective(D);
+  D->setNumGeneratedLoops(Record.readUInt32());
 }
 
 void ASTStmtReader::VisitOMPTileDirective(OMPTileDirective *D) {
Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -12919,10 +12919,12 @@
   Body, OriginalInits))
 return StmtError();
 
+  unsigned NumGeneratedLoops = PartialClause ? 1 : 0;
+
   // Delay unrolling to when template is completely instantiated.
   if (CurContext->isDependentContext())
 return OMPUnrollDirective::Create(Context, StartLoc, EndLoc, Clauses, AStmt,
-  nullptr, nullptr);
+

[PATCH] D111124: [Clang][OpenMP] Allow loop-transformations with template parameters.

2021-10-05 Thread Alexey Bataev via Phabricator via cfe-commits 
ABataev accepted this revision.
ABataev added a comment.
This revision is now accepted and ready to land.

LG


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D24

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

[PATCH] D111124: [Clang][OpenMP] Allow loop-transformations with template parameters.

2021-10-06 Thread Michael Kruse via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG2130117f92e5: [Clang][OpenMP] Allow loop-transformations 
with template parameters. (authored by Meinersbur).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D24

Files:
  clang/include/clang/AST/StmtOpenMP.h
  clang/lib/AST/StmtOpenMP.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/test/OpenMP/tile_ast_print.cpp
  clang/test/OpenMP/unroll_ast_print.cpp

Index: clang/test/OpenMP/unroll_ast_print.cpp
===
--- clang/test/OpenMP/unroll_ast_print.cpp
+++ clang/test/OpenMP/unroll_ast_print.cpp
@@ -124,4 +124,26 @@
   unroll_templated();
 }
 
+
+// PRINT-LABEL: template  void unroll_templated_factor(int start, int stop, int step) {
+// DUMP-LABEL:  FunctionTemplateDecl {{.*}} unroll_templated_factor
+template 
+void unroll_templated_factor(int start, int stop, int step) {
+  // PRINT: #pragma omp unroll partial(Factor)
+  // DUMP:  OMPUnrollDirective
+  // DUMP-NEXT: OMPPartialClause
+  // DUMP-NEXT:   DeclRefExpr {{.*}} 'Factor' 'int'
+  #pragma omp unroll partial(Factor)
+// PRINT-NEXT: for (int i = start; i < stop; i += step)
+// DUMP-NEXT:  ForStmt
+for (int i = start; i < stop; i += step)
+  // PRINT-NEXT: body(i);
+  // DUMP:  CallExpr
+  body(i);
+}
+void unroll_template_factor() {
+  unroll_templated_factor<4>(0, 42, 2);
+}
+
+
 #endif
Index: clang/test/OpenMP/tile_ast_print.cpp
===
--- clang/test/OpenMP/tile_ast_print.cpp
+++ clang/test/OpenMP/tile_ast_print.cpp
@@ -162,4 +162,25 @@
 }
 
 
+// PRINT-LABEL: template  void foo7(int start, int stop, int step) {
+// DUMP-LABEL: FunctionTemplateDecl {{.*}} foo7
+template 
+void foo7(int start, int stop, int step) {
+  // PRINT: #pragma omp tile sizes(Tile)
+  // DUMP:  OMPTileDirective
+  // DUMP-NEXT:   OMPSizesClause
+  // DUMP-NEXT: DeclRefExpr {{.*}} 'Tile' 'int'
+  #pragma omp tile sizes(Tile)
+// PRINT-NEXT:  for (int i = start; i < stop; i += step)
+// DUMP-NEXT: ForStmt
+for (int i = start; i < stop; i += step)
+  // PRINT-NEXT: body(i);
+  // DUMP:  CallExpr
+  body(i);
+}
+void tfoo7() {
+  foo7<5>(0, 42, 2);
+}
+
+
 #endif
Index: clang/lib/Serialization/ASTWriterStmt.cpp
===
--- clang/lib/Serialization/ASTWriterStmt.cpp
+++ clang/lib/Serialization/ASTWriterStmt.cpp
@@ -2226,6 +2226,7 @@
 void ASTStmtWriter::VisitOMPLoopTransformationDirective(
 OMPLoopTransformationDirective *D) {
   VisitOMPLoopBasedDirective(D);
+  Record.writeUInt32(D->getNumGeneratedLoops());
 }
 
 void ASTStmtWriter::VisitOMPTileDirective(OMPTileDirective *D) {
Index: clang/lib/Serialization/ASTReaderStmt.cpp
===
--- clang/lib/Serialization/ASTReaderStmt.cpp
+++ clang/lib/Serialization/ASTReaderStmt.cpp
@@ -2327,6 +2327,7 @@
 void ASTStmtReader::VisitOMPLoopTransformationDirective(
 OMPLoopTransformationDirective *D) {
   VisitOMPLoopBasedDirective(D);
+  D->setNumGeneratedLoops(Record.readUInt32());
 }
 
 void ASTStmtReader::VisitOMPTileDirective(OMPTileDirective *D) {
Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -12919,10 +12919,12 @@
   Body, OriginalInits))
 return StmtError();
 
+  unsigned NumGeneratedLoops = PartialClause ? 1 : 0;
+
   // Delay unrolling to when template is completely instantiated.
   if (CurContext->isDependentContext())
 return OMPUnrollDirective::Create(Context, StartLoc, EndLoc, Clauses, AStmt,
-  nullptr, nullptr);
+  NumGeneratedLoops, nullptr, nullptr);
 
   OMPLoopBasedDirective::HelperExprs &LoopHelper = LoopHelpers.front();
 
@@ -12941,9 +12943,9 @@
   // The generated loop may only be passed to other loop-associated directive
   // when a partial clause is specified. Without the requirement it is
   // sufficient to generate loop unroll metadata at code-generation.
-  if (!PartialClause)
+  if (NumGeneratedLoops == 0)
 return OMPUnrollDirective::Create(Context, StartLoc, EndLoc, Clauses, AStmt,
-  nullptr, nullptr);
+  NumGeneratedLoops, nullptr, nullptr);
 
   // Otherwise, we need to provide a de-sugared/transformed AST that can be
   // associated with another loop directive.
@@ -13164,7 +13166,8 @@
   LoopHelper.Init->getBeginLoc(), LoopHelper.Inc->getEndLoc());
 
   return OMPUnrollDirective::Create(Context, StartLoc, EndLoc, Clauses,