Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Merged
merged 1 commit into from
Sep 18, 2023
Merged

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

merged 1 commit into from
Sep 18, 2023

Conversation

kiranchandramohan
Copy link
Contributor

Fixes #65570

@kiranchandramohan kiranchandramohan requested a review from a team as a code owner September 12, 2023 13:17
@llvmbot llvmbot added mlir flang Flang issues not falling into any other category mlir:openmp flang:openmp labels Sep 12, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Sep 12, 2023

@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
Copy link
Collaborator

llvmbot commented Sep 12, 2023

@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
Copy link
Collaborator

llvmbot commented Sep 12, 2023

@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
NimishMishra approved these changes Sep 18, 2023
View reviewed changes
Copy link
Contributor

@NimishMishra NimishMishra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@agozillon
Copy link
Contributor

also LGTM!

@kiranchandramohan kiranchandramohan merged commit e2733a6 into llvm:main Sep 18, 2023
1 of 2 checks passed
ZijunZhaoCCK pushed a commit to ZijunZhaoCCK/llvm-project that referenced this pull request Sep 19, 2023
@kiranchandramohan @ZijunZhaoCCK
[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
@kiranchandramohan @zahiraam
[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
@kiranchandramohan @zahiraam
[Flang][OpenMP] Add trivial conversion pattern for omp.ordered_region (…
317032d 
…llvm#66085)

Fixes llvm#65570
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@NimishMishra NimishMishra NimishMishra approved these changes

@skatrak skatrak Awaiting requested review from skatrak

@agozillon agozillon Awaiting requested review from agozillon

Assignees
No one assigned
Labels
flang:openmp flang Flang issues not falling into any other category mlir:openmp mlir
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Flang][OpenMP] Compilation error of ordered construct not in DO loops
4 participants
@kiranchandramohan @llvmbot @agozillon @NimishMishra