Skip to content

Commit

Permalink
[Flang][OpenMP] Use fir.if instead of scf.if in lastprivate lowering
Browse files Browse the repository at this point in the history 
For finding the last iteration of a loop, or the last section an
if condition is generated. Using scf::if can cause some lowering
issues since the code contained inside it can have branches. Using
fir::if instead ensures that the fir::if is lowered into branches
along with any code contained inside that can generate branches.

Fixes #62458.

Reviewed By: NimishMishra

Differential Revision: https://reviews.llvm.org/D149547
  • Loading branch information
@kiranchandramohan
kiranchandramohan committed May 2, 2023
1 parent 52882de commit 39e8e59
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 16 deletions.
13 changes: 6 additions & 7 deletions flang/lib/Lower/OpenMP.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
#include "flang/Parser/parse-tree.h"
#include "flang/Semantics/tools.h"
#include "mlir/Dialect/OpenMP/OpenMPDialect.h"
#include "mlir/Dialect/SCF/IR/SCF.h"
#include "llvm/Frontend/OpenMP/OMPConstants.h"

using namespace mlir;
Expand Down Expand Up @@ -222,20 +221,20 @@ void DataSharingProcessor::insertLastPrivateCompare(mlir::Operation *op) {
// omp.section {
// fir.allocate for `private`/`firstprivate`
// <More operations here>
// scf.if %true {
// fir.if %true {
// ^%lpv_update_blk
// }
// }
// }
//
// To keep code consistency while handling privatization
// through this control flow, add a `scf.if` operation
// through this control flow, add a `fir.if` operation
// that always evaluates to true, in order to create
// a dedicated sub-region in `omp.section` where
// lastprivate FIR can reside. Later canonicalizations
// will optimize away this operation.

mlir::scf::IfOp ifOp = firOpBuilder.create<mlir::scf::IfOp>(
auto ifOp = firOpBuilder.create<fir::IfOp>(
op->getLoc(),
firOpBuilder.createIntegerConstant(
op->getLoc(), firOpBuilder.getIntegerType(1), 0x1),
Expand Down Expand Up @@ -280,7 +279,7 @@ void DataSharingProcessor::insertLastPrivateCompare(mlir::Operation *op) {
// omp.wsloop { ...
// ... store
// store ===> %cmp = llvm.icmp "eq" %iv %ub
// omp.yield scf.if %cmp {
// omp.yield fir.if %cmp {
// } ^%lpv_update_blk:
// }
// omp.yield
Expand All @@ -295,8 +294,8 @@ void DataSharingProcessor::insertLastPrivateCompare(mlir::Operation *op) {
op->getRegion(0).front().getArguments()[0],
mlir::dyn_cast<mlir::omp::WsLoopOp>(op).getUpperBound()[0]);
}
mlir::scf::IfOp ifOp = firOpBuilder.create<mlir::scf::IfOp>(
op->getLoc(), cmpOp, /*else*/ false);
auto ifOp =
firOpBuilder.create<fir::IfOp>(op->getLoc(), cmpOp, /*else*/ false);
firOpBuilder.setInsertionPointToStart(&ifOp.getThenRegion().front());
lastPrivIP = firOpBuilder.saveInsertionPoint();
} else {
Expand Down
12 changes: 6 additions & 6 deletions flang/test/Lower/OpenMP/parallel-lastprivate-clause-scalar.f90
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@

! Testing last iteration check
!CHECK-NEXT: %[[IV_CMP:.*]] = arith.cmpi eq, %[[INDX_WS]]
!CHECK: scf.if %[[IV_CMP]] {
!CHECK: fir.if %[[IV_CMP]] {

! Testing lastprivate val update
!CHECK-DAG: %[[CVT:.*]] = fir.convert %[[ARG1_REF]] : (!fir.ref<!fir.char<1,5>>) -> !fir.ref<i8>
Expand Down Expand Up @@ -53,7 +53,7 @@ subroutine lastprivate_character(arg1)

! Testing last iteration check
!CHECK-DAG: %[[IV_CMP:.*]] = arith.cmpi eq, %[[INDX_WS]]
!CHECK-DAG: scf.if %[[IV_CMP]] {
!CHECK-DAG: fir.if %[[IV_CMP]] {

! Testing lastprivate val update
!CHECK-NEXT: %[[CLONE_LD:.*]] = fir.load %[[CLONE]] : !fir.ref<i32>
Expand Down Expand Up @@ -82,7 +82,7 @@ subroutine lastprivate_int(arg1)

! Testing last iteration check
!CHECK: %[[IV_CMP1:.*]] = arith.cmpi eq, %[[INDX_WS]]
!CHECK-NEXT: scf.if %[[IV_CMP1]] {
!CHECK-NEXT: fir.if %[[IV_CMP1]] {
! Testing lastprivate val update
!CHECK-DAG: %[[CLONE_LD1:.*]] = fir.load %[[CLONE1]] : !fir.ref<i32>
!CHECK-DAG: fir.store %[[CLONE_LD1]] to %[[ARG1]] : !fir.ref<i32>
Expand Down Expand Up @@ -113,7 +113,7 @@ subroutine mult_lastprivate_int(arg1, arg2)

!Testing last iteration check
!CHECK: %[[IV_CMP1:.*]] = arith.cmpi eq, %[[INDX_WS]]
!CHECK-NEXT: scf.if %[[IV_CMP1]] {
!CHECK-NEXT: fir.if %[[IV_CMP1]] {
!Testing lastprivate val update
!CHECK-DAG: %[[CLONE_LD2:.*]] = fir.load %[[CLONE2]] : !fir.ref<i32>
!CHECK-DAG: fir.store %[[CLONE_LD2]] to %[[ARG2]] : !fir.ref<i32>
Expand Down Expand Up @@ -149,7 +149,7 @@ subroutine mult_lastprivate_int2(arg1, arg2)

! Testing last iteration check
!CHECK: %[[IV_CMP1:.*]] = arith.cmpi eq, %[[INDX_WS]]
!CHECK-NEXT: scf.if %[[IV_CMP1]] {
!CHECK-NEXT: fir.if %[[IV_CMP1]] {
! Testing lastprivate val update
!CHECK-NEXT: %[[CLONE_LD:.*]] = fir.load %[[CLONE2]] : !fir.ref<i32>
!CHECK-NEXT: fir.store %[[CLONE_LD]] to %[[ARG2]] : !fir.ref<i32>
Expand Down Expand Up @@ -180,7 +180,7 @@ subroutine firstpriv_lastpriv_int(arg1, arg2)
!CHECK: omp.wsloop for (%[[INDX_WS:.*]]) : {{.*}} {
! Testing last iteration check
!CHECK: %[[IV_CMP1:.*]] = arith.cmpi eq, %[[INDX_WS]]
!CHECK-NEXT: scf.if %[[IV_CMP1]] {
!CHECK-NEXT: fir.if %[[IV_CMP1]] {
! Testing lastprivate val update
!CHECK-NEXT: %[[CLONE_LD:.*]] = fir.load %[[CLONE1]] : !fir.ref<i32>
!CHECK-NEXT: fir.store %[[CLONE_LD]] to %[[ARG1]] : !fir.ref<i32>
Expand Down
6 changes: 3 additions & 3 deletions flang/test/Lower/OpenMP/sections.f90
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ subroutine lastprivate()
!CHECK: %[[const:.*]] = arith.constant 1 : i32
!CHECK: %[[result:.*]] = arith.addi %[[temp]], %[[const]] : i32
!CHECK: fir.store %[[result]] to %[[PRIVATE_X]] : !fir.ref<i32>
!CHECK: scf.if %[[true]] {
!CHECK: fir.if %[[true]] {
!CHECK: %[[temp:.*]] = fir.load %[[PRIVATE_X]] : !fir.ref<i32>
!CHECK: fir.store %[[temp]] to %[[X]] : !fir.ref<i32>
!CHECK: }
Expand Down Expand Up @@ -168,7 +168,7 @@ subroutine lastprivate()
!CHECK: %[[const:.*]] = arith.constant 1 : i32
!CHECK: %[[result:.*]] = arith.addi %[[temp]], %[[const]] : i32
!CHECK: fir.store %[[result]] to %[[PRIVATE_X]] : !fir.ref<i32>
!CHECK: scf.if %true {
!CHECK: fir.if %true {
!CHECK: %[[temp:.*]] = fir.load %[[PRIVATE_X]] : !fir.ref<i32>
!CHECK: fir.store %[[temp]] to %[[X]] : !fir.ref<i32>
!CHECK: }
Expand Down Expand Up @@ -205,7 +205,7 @@ subroutine lastprivate()
!CHECK: %[[const:.*]] = arith.constant 1 : i32
!CHECK: %[[result:.*]] = arith.addi %[[temp]], %[[const]] : i32
!CHECK: fir.store %[[result]] to %[[PRIVATE_X]] : !fir.ref<i32>
!CHECK: scf.if %true {
!CHECK: fir.if %true {
!CHECK: %[[temp:.*]] = fir.load %[[PRIVATE_X]] : !fir.ref<i32>
!CHECK: fir.store %[[temp]] to %[[X]] : !fir.ref<i32>
!CHECK: omp.barrier
Expand Down

0 comments on commit 39e8e59

Please sign in to comment.