[Flang][OpenMP] Fix conversion to LLVM IR for Fortran code with OpenMP SIMD directive and nested loop

[domada]

Summary

Flang cannot convert to LLVM IR the following Fortran code:

subroutine simdloop_with_nested_loop
integer :: i, j
integer :: a(10)
 !$OMP SIMD
  do i=1, 10
   do j=1, 10
     a(i) = i
    end do
  end do
  !$OMP END SIMD
end subroutine

Command:
bin/bbc simdloop_with_nested_loop.f90 -fopenmp -emit-fir -o - | bin/tco
Fails with the error message:
loc("<stdin>":16:10): error: failed to legalize operation 'cf.br'

This patch enables compilation of the test case with SIMD directive and nested loop.

[peixin]

flang/test/Lower/OpenMP/simd.f90

4

This is not expected. You should add the test in flang/test/Fir/convert-to-llvm-openmp-and-fir.fir.

mlir/lib/Conversion/OpenMPToLLVM/OpenMPToLLVM.cpp

126

SectionsOp is already there. Why do you add it repeatedly?

[domada]

Scope of changes:

1. moved test from flang/test/Lower/OpenMP/simd.f90 to flang/test/Fir/convert-to-llvm-openmp-and-fir.fir
2. removed duplicated addition of SectionsOp in populateOpenMPToLLVMConversionPatterns function

[domada]

mlir/lib/Conversion/OpenMPToLLVM/OpenMPToLLVM.cpp

126

Copy-paste error.

[peixin]

LGTM. Please enable the CI passed before landing this patch. It seems the build bot stuck.

[kiranchandramohan]

Changes to MLIR code should ideally be tested with MLIR tree dialect tests. The test for this code should have been in https://github.com/llvm/llvm-project/blob/main/mlir/test/Conversion/OpenMPToLLVM/convert-to-llvmir.mlir.

flang/test/Lower/OpenMP/simd.f90

 ! Tests for 2.9.3.1 Simd
 
 ! RUN: bbc -fopenmp -emit-fir %s -o - | FileCheck %s
+! RUN: bbc -fopenmp -emit-fir %s -o - | tco | FileCheck %s --check-prefixes="LLVMIR"
 
 !CHECK-LABEL: func @_QPsimdloop()
 subroutine simdloop
 integer :: i
   !$OMP SIMD
   ! CHECK: %[[LB:.*]] = arith.constant 1 : i32
   ! CHECK-NEXT: %[[UB:.*]] = arith.constant 9 : i32
   ! CHECK-NEXT: %[[STEP:.*]] = arith.constant 1 : i32
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
+
+! LLVMIR-LABEL:  define void @_QPsimdloop_with_nested_loop()
+subroutine simdloop_with_nested_loop
+integer :: i, j
+integer :: a(10)
+! check if nested loop is handled correctly
+! LLVMIR-LABEL: omp_loop.preheader
+! LLVMIR-LABEL: omp_loop.header
+! LLVMIR-LABEL: omp_loop.cond
+! LLVMIR-LABEL: omp_loop.body
+! LLVMIR-LABEL: omp.simdloop.region3
+! LLVMIR-LABEL: omp.simdloop.region2
+! LLVMIR-LABEL: omp.simdloop.region1
+! LLVMIR-LABEL: omp.simdloop.region
+! LLVMIR-LABEL: omp.region.cont
+! LLVMIR-LABEL: omp_loop.inc
+! LLVMIR-LABEL: omp_loop.exit
+! LLVMIR-LABEL: omp_loop.after
+  !$OMP SIMD
+  do i=1, 10
+  do j=1, 10
+    a(i) = i
+  end do
+  end do
+  !$OMP END SIMD
+end subroutine

mlir/lib/Conversion/OpenMPToLLVM/OpenMPToLLVM.cpp

matchAndRewrite(omp::ReductionOp curOp, OpAdaptor adaptor,
     return success();
   }
 };
 } // namespace
 
 void mlir::configureOpenMPToLLVMConversionLegality(
     ConversionTarget &target, LLVMTypeConverter &typeConverter) {
   target.addDynamicallyLegalOp<mlir::omp::CriticalOp, mlir::omp::ParallelOp,
-                               mlir::omp::WsLoopOp, mlir::omp::MasterOp,
-                               mlir::omp::SectionsOp, mlir::omp::SingleOp>(
-      [&](Operation *op) {
-        return typeConverter.isLegal(&op->getRegion(0)) &&
-               typeConverter.isLegal(op->getOperandTypes()) &&
-               typeConverter.isLegal(op->getResultTypes());
-      });
+                               mlir::omp::WsLoopOp, mlir::omp::SimdLoopOp,
+                               mlir::omp::MasterOp, mlir::omp::SectionsOp,
+                               mlir::omp::SingleOp>([&](Operation *op) {
   target
       .addDynamicallyLegalOp<mlir::omp::AtomicReadOp, mlir::omp::AtomicWriteOp,
                              mlir::omp::FlushOp, mlir::omp::ThreadprivateOp>(
           [&](Operation *op) {
             return typeConverter.isLegal(op->getOperandTypes()) &&
                    typeConverter.isLegal(op->getResultTypes());
           });
   target.addDynamicallyLegalOp<mlir::omp::ReductionOp>([&](Operation *op) {
     return typeConverter.isLegal(op->getOperandTypes());
   });
 }
 
 void mlir::populateOpenMPToLLVMConversionPatterns(LLVMTypeConverter &converter,
                                                   RewritePatternSet &patterns) {
   patterns.add<
       ReductionOpConversion, RegionOpConversion<omp::CriticalOp>,
       RegionOpConversion<omp::MasterOp>, ReductionOpConversion,
       RegionOpConversion<omp::MasterOp>, RegionOpConversion<omp::ParallelOp>,
       RegionOpConversion<omp::WsLoopOp>, RegionOpConversion<omp::SectionsOp>,
+      RegionOpConversion<omp::SimdLoopOp>, RegionOpConversion<omp::SectionsOp>,
       RegionOpConversion<omp::SingleOp>,
       RegionLessOpWithVarOperandsConversion<omp::AtomicReadOp>,
       RegionLessOpWithVarOperandsConversion<omp::AtomicWriteOp>,
       RegionLessOpWithVarOperandsConversion<omp::FlushOp>,
       RegionLessOpWithVarOperandsConversion<omp::ThreadprivateOp>>(converter);
 }
 
 namespace {