[MLIR][OpenMP] Address review comments

Author: kiranchandramohan

mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td

@@ -443,13 +443,11 @@ def WsLoopOp : OpenMP_Op<"wsloop", [AttrSizedOperandSegments,
     accumulator variables in `reduction_vars` and symbols referring to reduction
     declarations in the `reductions` attribute. Each reduction is identified
     by the accumulator it uses and accumulators must not be repeated in the same
-    reduction. The `omp.reduction` operation accepts the accumulator and a
-    partial value which is considered to be produced by the current loop
-    iteration for the given reduction. If multiple values are produced for the
-    same accumulator, i.e. there are multiple `omp.reduction`s, the last value
-    is taken. The reduction declaration specifies how to combine the values from
-    each iteration into the final value, which is available in the accumulator
-    after the loop completes.
+    reduction. A private variable corresponding to the accumulator is used in
+    place of the accumulator inside the body of the worksharing-loop. The
+    reduction declaration specifies how to combine the values from each
+    iteration into the final value, which is available in the accumulator after
+    the loop completes.
 
     The optional `schedule_val` attribute specifies the loop schedule for this
     loop, determining how the loop is distributed across the parallel threads.

mlir/lib/Conversion/SCFToOpenMP/SCFToOpenMP.cpp

@@ -400,19 +400,20 @@ struct ParallelOpLowering : public OpRewritePattern<scf::ParallelOp> {
     // Replace the reduction operations contained in this loop. Must be done
     // here rather than in a separate pattern to have access to the list of
     // reduction variables.
-    unsigned int reductionIndex = 0;
-    for (auto [x, y] :
-         llvm::zip_equal(reductionVariables, reduce.getOperands())) {
+    for (auto [x, y, rD] : llvm::zip_equal(
+             reductionVariables, reduce.getOperands(), ompReductionDecls)) {
       OpBuilder::InsertionGuard guard(rewriter);
       rewriter.setInsertionPoint(reduce);
-      Region &redRegion =
-          ompReductionDecls[reductionIndex].getReductionRegion();
+      Region &redRegion = rD.getReductionRegion();
+      // The SCF dialect by definition contains only structured operations
+      // and hence the SCF reduction region will contain a single block.
+      // The ompReductionDecls region is a copy of the SCF reduction region
+      // and hence has the same property.
       assert(redRegion.hasOneBlock() &&
              "expect reduction region to have one block");
       Value pvtRedVar = parallelOp.getRegion().addArgument(x.getType(), loc);
-      Value pvtRedVal = rewriter.create<LLVM::LoadOp>(
-          reduce.getLoc(), ompReductionDecls[reductionIndex].getType(),
-          pvtRedVar);
+      Value pvtRedVal = rewriter.create<LLVM::LoadOp>(reduce.getLoc(),
+                                                      rD.getType(), pvtRedVar);
       // Make a copy of the reduction combiner region in the body
       mlir::OpBuilder builder(rewriter.getContext());
       builder.setInsertionPoint(reduce);
@@ -432,7 +433,6 @@ struct ParallelOpLowering : public OpRewritePattern<scf::ParallelOp> {
           break;
         }
       }
-      reductionIndex++;
     }
     rewriter.eraseOp(reduce);