[Mlir-commits] [flang] [mlir] [MLIR][OpenMP][Flang] Normalize clause arguments names (PR #99505)

Michael Kruse llvmlistbot at llvm.org
Fri Jul 19 06:11:04 PDT 2024


================
@@ -32,37 +32,37 @@ namespace omp {
 
 struct AlignedClauseOps {
   llvm::SmallVector<Value> alignedVars;
-  llvm::SmallVector<Attribute> alignmentAttrs;
+  llvm::SmallVector<Attribute> alignments;
 };
 
 struct AllocateClauseOps {
-  llvm::SmallVector<Value> allocatorVars, allocateVars;
+  llvm::SmallVector<Value> allocateVars, allocatorVars;
 };
 
 struct CancelDirectiveNameClauseOps {
-  ClauseCancellationConstructTypeAttr cancelDirectiveNameAttr;
+  ClauseCancellationConstructTypeAttr cancelDirective;
 };
 
 struct CollapseClauseOps {
-  llvm::SmallVector<Value> loopLBVar, loopUBVar, loopStepVar;
+  llvm::SmallVector<Value> collapseLowerBound, collapseUpperBound, collapseStep;
----------------
Meinersbur wrote:

The are inside the `CollapseClauseOps` struct, I guess this is where the `collapse` prefix comes from? But these are the bounds before the collapse, so I find the name slightly confusing. In the spec these are the "affected loops", and only how many are affected is determined by the `collapse` clause. But there are also loop-nest-associated directives that don't take a `collapse` clause. (E.g. `tile` whole number of affected loops is determines by the `sizes` clause).

Also: Shouldn't the names be plural as there are multiple LowerBounds, UpperBounds, etc?

Proposal: Just `lowerBounds`, `upperBounds`, `steps`.

https://github.com/llvm/llvm-project/pull/99505


More information about the Mlir-commits mailing list