[NFC][OpenMP] Update simd loop collapse support description

[domada]

Summary

Simdloop collapse clause is supported in the same way as colllapse clause for worksharing loops.

[peixin]

nit: add NFC to the title.

flang/test/Lower/OpenMP/simd.f90

123

This is not one good test case for SIMD directive. Can you replace it with the one that can be vectorized?

[kiranchandramohan]

As far as possible we should try to have Flang and MLIR OpenMP dialect changes in separate patches.

I would recommend splitting this into two simple patches.

1. Testing the lowering of OpenMP dialect simdloop collapse to LLVM IR. There is an existing test case, simdloop_multi in the translation tests. You need to strengthen it minimally to check the occurrence of collapse. And adding the collapse information to the OpenMP dialect op description.

The relevant code for this came in the patch https://reviews.llvm.org/D118065.

2. Test the lowering from source to FIR+OpenMP simdloop with collapse. Add the relevant patch that enabled lowering of the collapse in simdloop. I think the patches that you have provided are not the right ones.

flang/test/Lower/OpenMP/simd.f90

109

All the above checks can either be with the same CHECK line or with the CHECK-SAME directive.

[domada]

Add only OpenMP specific changes. Update simd loop MLIR description and strengthen translation test.

[domada]

In D131674#3722172, @kiranchandramohan wrote:

As far as possible we should try to have Flang and MLIR OpenMP dialect changes in separate patches.

I would recommend splitting this into two simple patches.

1. Testing the lowering of OpenMP dialect simdloop collapse to LLVM IR. There is an existing test case, simdloop_multi in the translation tests. You need to strengthen it minimally to check the occurrence of collapse. And adding the collapse information to the OpenMP dialect op description.

The relevant code for this came in the patch https://reviews.llvm.org/D118065.

Done. I modified this review so that it contains only OpenMP specific changes.

2. Test the lowering from source to FIR+OpenMP simdloop with collapse. Add the relevant patch that enabled lowering of the collapse in simdloop. I think the patches that you have provided are not the right ones.

Will be done as the separate patch.

[domada]

@peixin
Thank you for your review. I will apply your remarks in the separate patch which will be Flang specific.

[kiranchandramohan]

Thanks @domada, this looks better. I have a comment about the way the test is written.

mlir/test/Target/LLVMIR/openmp-llvm.mlir

719–722 ↗

(On Diff #452946)

As you know checking for names is not always the best. Can you instead do something similar to the way the collapse test is written in D105706?

[domada]

Updated test: Do not rely on hardcoded labels. Check if collapsed loop bound is calculated.

[kiranchandramohan]

LGTM. Thanks for making the changes.

[domada]

Follow up review: https://reviews.llvm.org/D132023 . This review contains Fortran test with simd clause.

flang/test/Lower/OpenMP/simd.f90

109

Done

123

@peixin done: please look at https://reviews.llvm.org/D132023

flang/test/Lower/OpenMP/simd.f90

integer, parameter :: simdlen = 2;
   do i = 1, n
     ! CHECK: fir.store %[[I]] to %[[LOCAL:.*]] : !fir.ref<i32>
     ! CHECK: %[[LD:.*]] = fir.load %[[LOCAL]] : !fir.ref<i32>
     ! CHECK: fir.call @_FortranAioOutputInteger32({{.*}}, %[[LD]]) : (!fir.ref<i8>, i32) -> i1
     print*, i
   end do
   !$OMP END SIMD
 end subroutine
+
+!CHECK-LABEL: func @_QPsimdloop_with_collapse_clause
+!CHECK: %[[VAL_0:.*]] = fir.alloca i32 {adapt.valuebyref}
+!CHECK: %[[VAL_1:.*]] = fir.alloca i32 {adapt.valuebyref}
+!CHECK: %[[LOWER_I:.*]] = arith.constant 1 : i32
+!CHECK: %[[UPPER_I:.*]] = fir.load %[[PARAM_ARG:.*]] : !fir.ref<i32>
+!CHECK: %[[STEP_I:.*]] = arith.constant 1 : i32
+!CHECK: %[[LOWER_J:.*]] = arith.constant 1 : i32
+!CHECK: %[[UPPER_J:.*]] = fir.load %[[PARAM_ARG:.*]] : !fir.ref<i32>
+!CHECK: %[[STEP_J:.*]] = arith.constant 1 : i32
+! CHECK-LABEL:   omp.simdloop   for  (
+! CHECK:                         %[[ARG_0:.*]], %[[ARG_1:.*]]) : i32 = (
+! CHECK:                         %[[LOWER_I]],
+! CHECK:                         %[[LOWER_J]]) to (
+! CHECK:                         %[[UPPER_I]],
+! CHECK:                         %[[UPPER_J]]) inclusive step (
+! CHECK:                         %[[STEP_I]],
+! CHECK:                         %[[STEP_J]]) {
+! CHECK:           fir.store %[[ARG_0]] to %[[VAL_1]] : !fir.ref<i32>
+! CHECK:           fir.store %[[ARG_1]] to %[[VAL_0]] : !fir.ref<i32>
+! CHECK:           fir.call @_QPfoo(%[[VAL_1]]) : (!fir.ref<i32>) -> ()
+! CHECK:           fir.call @_QPbar(%[[VAL_0]]) : (!fir.ref<i32>) -> ()
+! CHECK:           omp.yield
+! CHECK:         }
+! CHECK:         return
+
+subroutine simdloop_with_collapse_clause(n)
+integer :: i, j, n
+  !$OMP SIMD collapse(2)
+  do i = 1, n
+    do j = 1, n
+      call foo(i)
+      call bar(j)
+    end do
+  end do
+  !$OMP END SIMD
+end subroutine

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

let description = [{
     be executed concurrently using SIMD instructions).. The lower and upper 
     bounds specify a half-open range: the range includes the lower bound but 
     does not include the upper bound. If the `inclusive` attribute is specified
     then the upper bound is also included.
 
     The body region can contain any number of blocks. The region is terminated
     by "omp.yield" instruction without operands.
 
+    Collapsed loops are represented by the simd-loop having a list of indices,
+    bounds and steps where the size of the list is equal to the collapse value.
+
     When an if clause is present and evaluates to false, the preferred number of
     iterations to be executed concurrently is one, regardless of whether
     a simdlen clause is specified.
 
     When a simdlen clause is present, the preferred number of iterations to be
     executed concurrently is the value provided to the simdlen clause.
     ```
     omp.simdloop <clauses>