[Flang][OpenMP] Add trivial conversion pattern for omp.ordered_region #66085

kiranchandramohan · 2023-09-12T13:17:38Z

Fixes #65570

llvmbot · 2023-09-12T13:18:53Z

@llvm/pr-subscribers-mlir-openmp

Changes

Fixes #65570

Full diff: https://github.com/llvm/llvm-project/pull/66085.diff

3 Files Affected:

(modified) flang/test/Fir/convert-to-llvm-openmp-and-fir.fir (+30)
(modified) mlir/lib/Conversion/OpenMPToLLVM/OpenMPToLLVM.cpp (+5-4)
(modified) mlir/test/Conversion/OpenMPToLLVM/convert-to-llvmir.mlir (+31)

diff --git a/flang/test/Fir/convert-to-llvm-openmp-and-fir.fir b/flang/test/Fir/convert-to-llvm-openmp-and-fir.fir
index 06fc1d0edbe2e62..40f46c9ef19ad4e 100644
--- a/flang/test/Fir/convert-to-llvm-openmp-and-fir.fir
+++ b/flang/test/Fir/convert-to-llvm-openmp-and-fir.fir
@@ -654,4 +654,34 @@ func.func @_QPs(%arg0: !fir.ref> {fir.bindc_name = "x"}) {
   %0 = fir.alloca !fir.complex<4> {bindc_name = "v", uniq_name = "_QFsEv"}
   omp.atomic.read %0 = %arg0   : !fir.ref>, !fir.complex<4>
   return
+}
+
+// -----
+
+// CHECK: llvm.func @sub_
+func.func @sub_() {
+  %c0 = arith.constant 0 : index
+  %c1 = arith.constant 1 : index
+  %0 = fir.alloca i32 {bindc_name = "i", uniq_name = "_QFsubEi"}
+// CHECK: omp.ordered_region
+  omp.ordered_region {
+    %1 = fir.convert %c1 : (index) -> i32
+    cf.br ^bb1(%1, %c1 : i32, index)
+  ^bb1(%2: i32, %3: index):  // 2 preds: ^bb0, ^bb2
+    %4 = arith.cmpi sgt, %3, %c0 : index
+    cf.cond_br %4, ^bb2, ^bb3
+  ^bb2:  // pred: ^bb1
+    fir.store %2 to %0 : !fir.ref
+    %5 = fir.load %0 : !fir.ref
+// CHECK: llvm.add
+    %6 = arith.addi %5, %1 : i32
+// CHECK: llvm.sub
+    %7 = arith.subi %3, %c1 : index
+    cf.br ^bb1(%6, %7 : i32, index)
+  ^bb3:  // pred: ^bb1
+    fir.store %2 to %0 : !fir.ref
+// CHECK: omp.terminator
+    omp.terminator
+  }
+  return
 } 
diff --git a/mlir/lib/Conversion/OpenMPToLLVM/OpenMPToLLVM.cpp b/mlir/lib/Conversion/OpenMPToLLVM/OpenMPToLLVM.cpp
index d06b7033257196a..adcbbc3f0abb249 100644
--- a/mlir/lib/Conversion/OpenMPToLLVM/OpenMPToLLVM.cpp
+++ b/mlir/lib/Conversion/OpenMPToLLVM/OpenMPToLLVM.cpp
@@ -199,10 +199,10 @@ void mlir::configureOpenMPToLLVMConversionLegality(
     ConversionTarget &target, LLVMTypeConverter &typeConverter) {
   target.addDynamicallyLegalOp<
       mlir::omp::AtomicUpdateOp, mlir::omp::CriticalOp, mlir::omp::TargetOp,
-      mlir::omp::DataOp, mlir::omp::ParallelOp, mlir::omp::WsLoopOp,
-      mlir::omp::SimdLoopOp, mlir::omp::MasterOp, mlir::omp::SectionOp,
-      mlir::omp::SectionsOp, mlir::omp::SingleOp, mlir::omp::TaskGroupOp,
-      mlir::omp::TaskOp>([&](Operation *op) {
+      mlir::omp::DataOp, mlir::omp::OrderedRegionOp, mlir::omp::ParallelOp,
+      mlir::omp::WsLoopOp, mlir::omp::SimdLoopOp, mlir::omp::MasterOp,
+      mlir::omp::SectionOp, mlir::omp::SectionsOp, mlir::omp::SingleOp,
+      mlir::omp::TaskGroupOp, mlir::omp::TaskOp>([&](Operation *op) {
     return typeConverter.isLegal(&op->getRegion(0)) &&
            typeConverter.isLegal(op->getOperandTypes()) &&
            typeConverter.isLegal(op->getResultTypes());
@@ -234,6 +234,7 @@ void mlir::populateOpenMPToLLVMConversionPatterns(LLVMTypeConverter &converter,
       AtomicReadOpConversion, ReductionOpConversion,
       ReductionDeclareOpConversion, RegionOpConversion,
       RegionOpConversion, ReductionOpConversion,
+      RegionOpConversion,
       RegionOpConversion, RegionOpConversion,
       RegionOpConversion, RegionOpConversion,
       RegionOpConversion, RegionOpConversion,
diff --git a/mlir/test/Conversion/OpenMPToLLVM/convert-to-llvmir.mlir b/mlir/test/Conversion/OpenMPToLLVM/convert-to-llvmir.mlir
index b83b122f75e4b08..fedbcd401d44c94 100644
--- a/mlir/test/Conversion/OpenMPToLLVM/convert-to-llvmir.mlir
+++ b/mlir/test/Conversion/OpenMPToLLVM/convert-to-llvmir.mlir
@@ -384,3 +384,34 @@ llvm.func @_QQmain() {
 llvm.func @_QFPdo_work(%arg0: !llvm.ptr {fir.bindc_name = "i"}) {
   llvm.return
 }
+
+// -----
+
+// CHECK-LABEL:  @sub_
+llvm.func @sub_() {
+  %0 = llvm.mlir.constant(0 : index) : i64
+  %1 = llvm.mlir.constant(1 : index) : i64
+  %2 = llvm.mlir.constant(1 : i64) : i64
+  %3 = llvm.alloca %2 x i32 {bindc_name = "i", in_type = i32, operandSegmentSizes = array, uniq_name = "_QFsubEi"} : (i64) -> !llvm.ptr
+// CHECK: omp.ordered_region
+  omp.ordered_region {
+    %4 = llvm.trunc %1 : i64 to i32
+    llvm.br ^bb1(%4, %1 : i32, i64)
+  ^bb1(%5: i32, %6: i64):  // 2 preds: ^bb0, ^bb2
+    %7 = llvm.icmp "sgt" %6, %0 : i64
+    llvm.cond_br %7, ^bb2, ^bb3
+  ^bb2:  // pred: ^bb1
+    llvm.store %5, %3 : !llvm.ptr
+    %8 = llvm.load %3 : !llvm.ptr
+// CHECK: llvm.add
+    %9 = arith.addi %8, %4 : i32
+// CHECK: llvm.sub
+    %10 = arith.subi %6, %1 : i64
+    llvm.br ^bb1(%9, %10 : i32, i64)
+  ^bb3:  // pred: ^bb1
+    llvm.store %5, %3 : !llvm.ptr
+// CHECK: omp.terminator
+    omp.terminator
+  }
+  llvm.return
+}

llvmbot · 2023-09-12T13:18:57Z

@llvm/pr-subscribers-flang-openmp

Changes

Fixes #65570

Full diff: https://github.com/llvm/llvm-project/pull/66085.diff

3 Files Affected:

(modified) flang/test/Fir/convert-to-llvm-openmp-and-fir.fir (+30)
(modified) mlir/lib/Conversion/OpenMPToLLVM/OpenMPToLLVM.cpp (+5-4)
(modified) mlir/test/Conversion/OpenMPToLLVM/convert-to-llvmir.mlir (+31)

diff --git a/flang/test/Fir/convert-to-llvm-openmp-and-fir.fir b/flang/test/Fir/convert-to-llvm-openmp-and-fir.fir
index 06fc1d0edbe2e62..40f46c9ef19ad4e 100644
--- a/flang/test/Fir/convert-to-llvm-openmp-and-fir.fir
+++ b/flang/test/Fir/convert-to-llvm-openmp-and-fir.fir
@@ -654,4 +654,34 @@ func.func @_QPs(%arg0: !fir.ref> {fir.bindc_name = "x"}) {
   %0 = fir.alloca !fir.complex<4> {bindc_name = "v", uniq_name = "_QFsEv"}
   omp.atomic.read %0 = %arg0   : !fir.ref>, !fir.complex<4>
   return
+}
+
+// -----
+
+// CHECK: llvm.func @sub_
+func.func @sub_() {
+  %c0 = arith.constant 0 : index
+  %c1 = arith.constant 1 : index
+  %0 = fir.alloca i32 {bindc_name = "i", uniq_name = "_QFsubEi"}
+// CHECK: omp.ordered_region
+  omp.ordered_region {
+    %1 = fir.convert %c1 : (index) -> i32
+    cf.br ^bb1(%1, %c1 : i32, index)
+  ^bb1(%2: i32, %3: index):  // 2 preds: ^bb0, ^bb2
+    %4 = arith.cmpi sgt, %3, %c0 : index
+    cf.cond_br %4, ^bb2, ^bb3
+  ^bb2:  // pred: ^bb1
+    fir.store %2 to %0 : !fir.ref
+    %5 = fir.load %0 : !fir.ref
+// CHECK: llvm.add
+    %6 = arith.addi %5, %1 : i32
+// CHECK: llvm.sub
+    %7 = arith.subi %3, %c1 : index
+    cf.br ^bb1(%6, %7 : i32, index)
+  ^bb3:  // pred: ^bb1
+    fir.store %2 to %0 : !fir.ref
+// CHECK: omp.terminator
+    omp.terminator
+  }
+  return
 } 
diff --git a/mlir/lib/Conversion/OpenMPToLLVM/OpenMPToLLVM.cpp b/mlir/lib/Conversion/OpenMPToLLVM/OpenMPToLLVM.cpp
index d06b7033257196a..adcbbc3f0abb249 100644
--- a/mlir/lib/Conversion/OpenMPToLLVM/OpenMPToLLVM.cpp
+++ b/mlir/lib/Conversion/OpenMPToLLVM/OpenMPToLLVM.cpp
@@ -199,10 +199,10 @@ void mlir::configureOpenMPToLLVMConversionLegality(
     ConversionTarget &target, LLVMTypeConverter &typeConverter) {
   target.addDynamicallyLegalOp<
       mlir::omp::AtomicUpdateOp, mlir::omp::CriticalOp, mlir::omp::TargetOp,
-      mlir::omp::DataOp, mlir::omp::ParallelOp, mlir::omp::WsLoopOp,
-      mlir::omp::SimdLoopOp, mlir::omp::MasterOp, mlir::omp::SectionOp,
-      mlir::omp::SectionsOp, mlir::omp::SingleOp, mlir::omp::TaskGroupOp,
-      mlir::omp::TaskOp>([&](Operation *op) {
+      mlir::omp::DataOp, mlir::omp::OrderedRegionOp, mlir::omp::ParallelOp,
+      mlir::omp::WsLoopOp, mlir::omp::SimdLoopOp, mlir::omp::MasterOp,
+      mlir::omp::SectionOp, mlir::omp::SectionsOp, mlir::omp::SingleOp,
+      mlir::omp::TaskGroupOp, mlir::omp::TaskOp>([&](Operation *op) {
     return typeConverter.isLegal(&op->getRegion(0)) &&
            typeConverter.isLegal(op->getOperandTypes()) &&
            typeConverter.isLegal(op->getResultTypes());
@@ -234,6 +234,7 @@ void mlir::populateOpenMPToLLVMConversionPatterns(LLVMTypeConverter &converter,
       AtomicReadOpConversion, ReductionOpConversion,
       ReductionDeclareOpConversion, RegionOpConversion,
       RegionOpConversion, ReductionOpConversion,
+      RegionOpConversion,
       RegionOpConversion, RegionOpConversion,
       RegionOpConversion, RegionOpConversion,
       RegionOpConversion, RegionOpConversion,
diff --git a/mlir/test/Conversion/OpenMPToLLVM/convert-to-llvmir.mlir b/mlir/test/Conversion/OpenMPToLLVM/convert-to-llvmir.mlir
index b83b122f75e4b08..fedbcd401d44c94 100644
--- a/mlir/test/Conversion/OpenMPToLLVM/convert-to-llvmir.mlir
+++ b/mlir/test/Conversion/OpenMPToLLVM/convert-to-llvmir.mlir
@@ -384,3 +384,34 @@ llvm.func @_QQmain() {
 llvm.func @_QFPdo_work(%arg0: !llvm.ptr {fir.bindc_name = "i"}) {
   llvm.return
 }
+
+// -----
+
+// CHECK-LABEL:  @sub_
+llvm.func @sub_() {
+  %0 = llvm.mlir.constant(0 : index) : i64
+  %1 = llvm.mlir.constant(1 : index) : i64
+  %2 = llvm.mlir.constant(1 : i64) : i64
+  %3 = llvm.alloca %2 x i32 {bindc_name = "i", in_type = i32, operandSegmentSizes = array, uniq_name = "_QFsubEi"} : (i64) -> !llvm.ptr
+// CHECK: omp.ordered_region
+  omp.ordered_region {
+    %4 = llvm.trunc %1 : i64 to i32
+    llvm.br ^bb1(%4, %1 : i32, i64)
+  ^bb1(%5: i32, %6: i64):  // 2 preds: ^bb0, ^bb2
+    %7 = llvm.icmp "sgt" %6, %0 : i64
+    llvm.cond_br %7, ^bb2, ^bb3
+  ^bb2:  // pred: ^bb1
+    llvm.store %5, %3 : !llvm.ptr
+    %8 = llvm.load %3 : !llvm.ptr
+// CHECK: llvm.add
+    %9 = arith.addi %8, %4 : i32
+// CHECK: llvm.sub
+    %10 = arith.subi %6, %1 : i64
+    llvm.br ^bb1(%9, %10 : i32, i64)
+  ^bb3:  // pred: ^bb1
+    llvm.store %5, %3 : !llvm.ptr
+// CHECK: omp.terminator
+    omp.terminator
+  }
+  llvm.return
+}

llvmbot · 2023-09-12T13:18:59Z

@llvm/pr-subscribers-mlir

Changes

Fixes #65570

Full diff: https://github.com/llvm/llvm-project/pull/66085.diff

3 Files Affected:

(modified) flang/test/Fir/convert-to-llvm-openmp-and-fir.fir (+30)
(modified) mlir/lib/Conversion/OpenMPToLLVM/OpenMPToLLVM.cpp (+5-4)
(modified) mlir/test/Conversion/OpenMPToLLVM/convert-to-llvmir.mlir (+31)

diff --git a/flang/test/Fir/convert-to-llvm-openmp-and-fir.fir b/flang/test/Fir/convert-to-llvm-openmp-and-fir.fir
index 06fc1d0edbe2e62..40f46c9ef19ad4e 100644
--- a/flang/test/Fir/convert-to-llvm-openmp-and-fir.fir
+++ b/flang/test/Fir/convert-to-llvm-openmp-and-fir.fir
@@ -654,4 +654,34 @@ func.func @_QPs(%arg0: !fir.ref> {fir.bindc_name = "x"}) {
   %0 = fir.alloca !fir.complex<4> {bindc_name = "v", uniq_name = "_QFsEv"}
   omp.atomic.read %0 = %arg0   : !fir.ref>, !fir.complex<4>
   return
+}
+
+// -----
+
+// CHECK: llvm.func @sub_
+func.func @sub_() {
+  %c0 = arith.constant 0 : index
+  %c1 = arith.constant 1 : index
+  %0 = fir.alloca i32 {bindc_name = "i", uniq_name = "_QFsubEi"}
+// CHECK: omp.ordered_region
+  omp.ordered_region {
+    %1 = fir.convert %c1 : (index) -> i32
+    cf.br ^bb1(%1, %c1 : i32, index)
+  ^bb1(%2: i32, %3: index):  // 2 preds: ^bb0, ^bb2
+    %4 = arith.cmpi sgt, %3, %c0 : index
+    cf.cond_br %4, ^bb2, ^bb3
+  ^bb2:  // pred: ^bb1
+    fir.store %2 to %0 : !fir.ref
+    %5 = fir.load %0 : !fir.ref
+// CHECK: llvm.add
+    %6 = arith.addi %5, %1 : i32
+// CHECK: llvm.sub
+    %7 = arith.subi %3, %c1 : index
+    cf.br ^bb1(%6, %7 : i32, index)
+  ^bb3:  // pred: ^bb1
+    fir.store %2 to %0 : !fir.ref
+// CHECK: omp.terminator
+    omp.terminator
+  }
+  return
 } 
diff --git a/mlir/lib/Conversion/OpenMPToLLVM/OpenMPToLLVM.cpp b/mlir/lib/Conversion/OpenMPToLLVM/OpenMPToLLVM.cpp
index d06b7033257196a..adcbbc3f0abb249 100644
--- a/mlir/lib/Conversion/OpenMPToLLVM/OpenMPToLLVM.cpp
+++ b/mlir/lib/Conversion/OpenMPToLLVM/OpenMPToLLVM.cpp
@@ -199,10 +199,10 @@ void mlir::configureOpenMPToLLVMConversionLegality(
     ConversionTarget &target, LLVMTypeConverter &typeConverter) {
   target.addDynamicallyLegalOp<
       mlir::omp::AtomicUpdateOp, mlir::omp::CriticalOp, mlir::omp::TargetOp,
-      mlir::omp::DataOp, mlir::omp::ParallelOp, mlir::omp::WsLoopOp,
-      mlir::omp::SimdLoopOp, mlir::omp::MasterOp, mlir::omp::SectionOp,
-      mlir::omp::SectionsOp, mlir::omp::SingleOp, mlir::omp::TaskGroupOp,
-      mlir::omp::TaskOp>([&](Operation *op) {
+      mlir::omp::DataOp, mlir::omp::OrderedRegionOp, mlir::omp::ParallelOp,
+      mlir::omp::WsLoopOp, mlir::omp::SimdLoopOp, mlir::omp::MasterOp,
+      mlir::omp::SectionOp, mlir::omp::SectionsOp, mlir::omp::SingleOp,
+      mlir::omp::TaskGroupOp, mlir::omp::TaskOp>([&](Operation *op) {
     return typeConverter.isLegal(&op->getRegion(0)) &&
            typeConverter.isLegal(op->getOperandTypes()) &&
            typeConverter.isLegal(op->getResultTypes());
@@ -234,6 +234,7 @@ void mlir::populateOpenMPToLLVMConversionPatterns(LLVMTypeConverter &converter,
       AtomicReadOpConversion, ReductionOpConversion,
       ReductionDeclareOpConversion, RegionOpConversion,
       RegionOpConversion, ReductionOpConversion,
+      RegionOpConversion,
       RegionOpConversion, RegionOpConversion,
       RegionOpConversion, RegionOpConversion,
       RegionOpConversion, RegionOpConversion,
diff --git a/mlir/test/Conversion/OpenMPToLLVM/convert-to-llvmir.mlir b/mlir/test/Conversion/OpenMPToLLVM/convert-to-llvmir.mlir
index b83b122f75e4b08..fedbcd401d44c94 100644
--- a/mlir/test/Conversion/OpenMPToLLVM/convert-to-llvmir.mlir
+++ b/mlir/test/Conversion/OpenMPToLLVM/convert-to-llvmir.mlir
@@ -384,3 +384,34 @@ llvm.func @_QQmain() {
 llvm.func @_QFPdo_work(%arg0: !llvm.ptr {fir.bindc_name = "i"}) {
   llvm.return
 }
+
+// -----
+
+// CHECK-LABEL:  @sub_
+llvm.func @sub_() {
+  %0 = llvm.mlir.constant(0 : index) : i64
+  %1 = llvm.mlir.constant(1 : index) : i64
+  %2 = llvm.mlir.constant(1 : i64) : i64
+  %3 = llvm.alloca %2 x i32 {bindc_name = "i", in_type = i32, operandSegmentSizes = array, uniq_name = "_QFsubEi"} : (i64) -> !llvm.ptr
+// CHECK: omp.ordered_region
+  omp.ordered_region {
+    %4 = llvm.trunc %1 : i64 to i32
+    llvm.br ^bb1(%4, %1 : i32, i64)
+  ^bb1(%5: i32, %6: i64):  // 2 preds: ^bb0, ^bb2
+    %7 = llvm.icmp "sgt" %6, %0 : i64
+    llvm.cond_br %7, ^bb2, ^bb3
+  ^bb2:  // pred: ^bb1
+    llvm.store %5, %3 : !llvm.ptr
+    %8 = llvm.load %3 : !llvm.ptr
+// CHECK: llvm.add
+    %9 = arith.addi %8, %4 : i32
+// CHECK: llvm.sub
+    %10 = arith.subi %6, %1 : i64
+    llvm.br ^bb1(%9, %10 : i32, i64)
+  ^bb3:  // pred: ^bb1
+    llvm.store %5, %3 : !llvm.ptr
+// CHECK: omp.terminator
+    omp.terminator
+  }
+  llvm.return
+}

NimishMishra

LGTM

agozillon · 2023-09-18T12:09:39Z

also LGTM!

Fixes llvm#65570

…llvm#66085) Fixes llvm#65570

kiranchandramohan requested a review from a team as a code owner September 12, 2023 13:17

llvmbot added mlir flang Flang issues not falling into any other category mlir:openmp flang:openmp labels Sep 12, 2023

kiranchandramohan requested review from skatrak, agozillon and NimishMishra September 13, 2023 16:42

NimishMishra approved these changes Sep 18, 2023

View reviewed changes

[Flang][OpenMP] Add trivial conversion pattern for omp.ordered_region

5f9174b

Fixes llvm#65570

kiranchandramohan force-pushed the ordered_region branch from 2711eeb to 5f9174b Compare September 18, 2023 14:55

kiranchandramohan merged commit e2733a6 into llvm:main Sep 18, 2023

ZijunZhaoCCK pushed a commit to ZijunZhaoCCK/llvm-project that referenced this pull request Sep 19, 2023

[Flang][OpenMP] Add trivial conversion pattern for omp.ordered_region (…

8efc72a

…llvm#66085) Fixes llvm#65570

zahiraam pushed a commit to tahonermann/llvm-project that referenced this pull request Oct 24, 2023

[Flang][OpenMP] Add trivial conversion pattern for omp.ordered_region (…

11c5f2c

…llvm#66085) Fixes llvm#65570

zahiraam pushed a commit to tahonermann/llvm-project that referenced this pull request Oct 24, 2023

[Flang][OpenMP] Add trivial conversion pattern for omp.ordered_region (…

317032d

…llvm#66085) Fixes llvm#65570

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Flang][OpenMP] Add trivial conversion pattern for omp.ordered_region #66085

[Flang][OpenMP] Add trivial conversion pattern for omp.ordered_region #66085

kiranchandramohan commented Sep 12, 2023

llvmbot commented Sep 12, 2023

Fixes #65570

llvmbot commented Sep 12, 2023

Fixes #65570

llvmbot commented Sep 12, 2023

Fixes #65570

NimishMishra left a comment

agozillon commented Sep 18, 2023

[Flang][OpenMP] Add trivial conversion pattern for omp.ordered_region #66085

[Flang][OpenMP] Add trivial conversion pattern for omp.ordered_region #66085

Conversation

kiranchandramohan commented Sep 12, 2023

llvmbot commented Sep 12, 2023

Fixes #65570

llvmbot commented Sep 12, 2023

Fixes #65570

llvmbot commented Sep 12, 2023

Fixes #65570

NimishMishra left a comment

Choose a reason for hiding this comment

agozillon commented Sep 18, 2023