From b501503ca0bc5562e5ef4eb736ab718fd29c7bc3 Mon Sep 17 00:00:00 2001 From: kiranchandramohan Date: Tue, 31 May 2022 09:25:00 +0000 Subject: [PATCH] [Flang][OpenMP] Fix for unstructured regions in OpenMP constructs - 2 The following changes are made for OpenMP operations with unstructured region, 1. For combined constructs the outer operation is considered a structured region and the inner one as the unstructured. 2. Added a condition to ensure that we create new blocks only once for nested unstructured OpenMP constructs. Tests are added for checking the structure of the CFG. Note: This is part of upstreaming from the fir-dev branch of https://github.com/flang-compiler/f18-llvm-project. Code originally reviewed at https://github.com/flang-compiler/f18-llvm-project/pull/1394. Reviewed By: vdonaldson, shraiysh, peixin Differential Revision: https://reviews.llvm.org/D126375 --- flang/lib/Lower/OpenMP.cpp | 15 +- flang/test/Lower/OpenMP/omp-unstructured.f90 | 200 +++++++++++++++++++++++++++ 2 files changed, 209 insertions(+), 6 deletions(-) diff --git a/flang/lib/Lower/OpenMP.cpp b/flang/lib/Lower/OpenMP.cpp index 04cd39a..85a54eb 100644 --- a/flang/lib/Lower/OpenMP.cpp +++ b/flang/lib/Lower/OpenMP.cpp @@ -156,7 +156,7 @@ void createEmptyRegionBlocks( "expected terminator op"); } } - if (eval.hasNestedEvaluations()) + if (!eval.isDirective() && eval.hasNestedEvaluations()) createEmptyRegionBlocks(firOpBuilder, eval.getNestedEvaluations()); } } @@ -179,7 +179,7 @@ createBodyOfOp(Op &op, Fortran::lower::AbstractConverter &converter, const Fortran::parser::OmpClauseList *clauses = nullptr, const SmallVector &args = {}, bool outerCombined = false) { - auto &firOpBuilder = converter.getFirOpBuilder(); + fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder(); // If an argument for the region is provided then create the block with that // argument. Also update the symbol's address with the mlir argument value. // e.g. For loops the argument is the induction variable. And all further @@ -205,13 +205,15 @@ createBodyOfOp(Op &op, Fortran::lower::AbstractConverter &converter, } else { firOpBuilder.createBlock(&op.getRegion()); } - auto &block = op.getRegion().back(); - firOpBuilder.setInsertionPointToStart(&block); + mlir::Block &block = op.getRegion().back(); + firOpBuilder.setInsertionPointToEnd(&block); - if (eval.lowerAsUnstructured()) + // If it is an unstructured region and is not the outer region of a combined + // construct, create empty blocks for all evaluations. + if (eval.lowerAsUnstructured() && !outerCombined) createEmptyRegionBlocks(firOpBuilder, eval.getNestedEvaluations()); - // Ensure the block is well-formed by inserting terminators. + // Insert the terminator. if constexpr (std::is_same_v) { mlir::ValueRange results; firOpBuilder.create(loc, results); @@ -221,6 +223,7 @@ createBodyOfOp(Op &op, Fortran::lower::AbstractConverter &converter, // Reset the insertion point to the start of the first block. firOpBuilder.setInsertionPointToStart(&block); + // Handle privatization. Do not privatize if this is the outer operation. if (clauses && !outerCombined) privatizeVars(converter, *clauses); diff --git a/flang/test/Lower/OpenMP/omp-unstructured.f90 b/flang/test/Lower/OpenMP/omp-unstructured.f90 index 68f944b..7272d02 100644 --- a/flang/test/Lower/OpenMP/omp-unstructured.f90 +++ b/flang/test/Lower/OpenMP/omp-unstructured.f90 @@ -105,9 +105,209 @@ subroutine ss3(n) ! nested unstructured OpenMP constructs !$omp end parallel end +! CHECK-LABEL: func @_QPss4{{.*}} { +! CHECK: omp.parallel { +! CHECK: omp.wsloop for (%[[ARG:.*]]) : {{.*}} { +! CHECK: cond_br %{{.*}}, ^bb1, ^bb2 +! CHECK: ^bb1: +! CHECK: @_FortranAioBeginExternalListOutput +! CHECK: @_FortranAioOutputInteger32(%{{.*}}, %[[ARG]]) +! CHECK: br ^bb2 +! CHECK: ^bb2: +! CHECK-NEXT: omp.yield +! CHECK-NEXT: } +! CHECK: omp.terminator +! CHECK-NEXT:} +subroutine ss4(n) ! CYCLE in OpenMP wsloop constructs + !$omp parallel + do i = 1, 3 + !$omp do + do j = 1, 3 + if (j .eq. n) cycle + print*, 'ss4', j + enddo + !$omp end do + enddo + !$omp end parallel +end + +! CHECK-LABEL: func @_QPss5() { +! CHECK: omp.parallel { +! CHECK: omp.wsloop {{.*}} { +! CHECK: br ^[[BB1:.*]] +! CHECK: ^[[BB1]]: +! CHECK: cond_br %{{.*}}, ^[[BB2:.*]], ^[[BB4:.*]] +! CHECK: ^[[BB2]]: +! CHECK: cond_br %{{.*}}, ^[[BB4]], ^[[BB3:.*]] +! CHECK: ^[[BB3]]: +! CHECK: br ^[[BB1]] +! CHECK: ^[[BB4]]: +! CHECK: omp.yield +! CHECK: } +! CHECK: omp.terminator +! CHECK: } +subroutine ss5() ! EXIT inside OpenMP wsloop (inside parallel) + integer :: x + !$omp parallel private(x) + !$omp do + do j = 1, 3 + x = j * i + do k = 1, 3 + if (k .eq. n) exit + x = k + x = x + k + enddo + x = j - 222 + enddo + !$omp end do + !$omp end parallel +end + +! CHECK-LABEL: func @_QPss6() { +! CHECK: omp.parallel { +! CHECK: br ^[[BB1_OUTER:.*]] +! CHECK: ^[[BB1_OUTER]]: +! CHECK: cond_br %{{.*}}, ^[[BB2_OUTER:.*]], ^[[BB3_OUTER:.*]] +! CHECK: ^[[BB2_OUTER]]: +! CHECK: omp.wsloop {{.*}} { +! CHECK: br ^[[BB1:.*]] +! CHECK: ^[[BB1]]: +! CHECK: cond_br %{{.*}}, ^[[BB2:.*]], ^[[BB4:.*]] +! CHECK: ^[[BB2]]: +! CHECK: cond_br %{{.*}}, ^[[BB4]], ^[[BB3:.*]] +! CHECK: ^[[BB3]]: +! CHECK: br ^[[BB1]] +! CHECK: ^[[BB4]]: +! CHECK: omp.yield +! CHECK: } +! CHECK: br ^[[BB1_OUTER]] +! CHECK: ^[[BB3_OUTER]]: +! CHECK: omp.terminator +! CHECK: } +subroutine ss6() ! EXIT inside OpenMP wsloop in a do loop (inside parallel) + integer :: x + !$omp parallel private(x) + do i = 1, 3 + !$omp do + do j = 1, 3 + x = j * i + do k = 1, 3 + if (k .eq. n) exit + x = k + x = x + k + enddo + x = j - 222 + enddo + !$omp end do + enddo + !$omp end parallel +end + +! CHECK-LABEL: func @_QPss7() { +! CHECK: br ^[[BB1_OUTER:.*]] +! CHECK: ^[[BB1_OUTER]]: +! CHECK: cond_br %{{.*}}, ^[[BB2_OUTER:.*]], ^[[BB3_OUTER:.*]] +! CHECK-NEXT: ^[[BB2_OUTER:.*]]: +! CHECK: omp.parallel { +! CHECK: omp.wsloop {{.*}} { +! CHECK: br ^[[BB1:.*]] +! CHECK-NEXT: ^[[BB1]]: +! CHECK: cond_br %{{.*}}, ^[[BB2:.*]], ^[[BB4:.*]] +! CHECK-NEXT: ^[[BB2]]: +! CHECK: cond_br %{{.*}}, ^[[BB4]], ^[[BB3:.*]] +! CHECK-NEXT: ^[[BB3]]: +! CHECK: br ^bb1 +! CHECK-NEXT: ^[[BB4]]: +! CHECK: omp.yield +! CHECK: } +! CHECK: omp.terminator +! CHECK: } +! CHECK: br ^[[BB1_OUTER]] +! CHECK-NEXT: ^[[BB3_OUTER]]: +! CHECK-NEXT: return +subroutine ss7() ! EXIT inside OpenMP parallel do (inside do loop) + integer :: x + do i = 1, 3 + !$omp parallel do private(x) + do j = 1, 3 + x = j * i + do k = 1, 3 + if (k .eq. n) exit + x = k + x = x + k + enddo + enddo + !$omp end parallel do + enddo +end + +! CHECK-LABEL: func @_QPss8() { +! CHECK: omp.parallel { +! CHECK: omp.wsloop {{.*}} { +! CHECK: br ^[[BB1:.*]] +! CHECK: ^[[BB1]]: +! CHECK: cond_br %{{.*}}, ^[[BB2:.*]], ^[[BB4:.*]] +! CHECK: ^[[BB2]]: +! CHECK: cond_br %{{.*}}, ^[[BB4]], ^[[BB3:.*]] +! CHECK: ^[[BB3]]: +! CHECK: br ^[[BB1]] +! CHECK: ^[[BB4]]: +! CHECK: omp.yield +! CHECK: } +! CHECK: omp.terminator +! CHECK: } +subroutine ss8() ! EXIT inside OpenMP parallel do + integer :: x + !$omp parallel do private(x) + do j = 1, 3 + x = j * i + do k = 1, 3 + if (k .eq. n) exit + x = k + x = x + k + enddo + enddo + !$omp end parallel do +end + +! CHECK-LABEL: func @_QPss9() { +! CHECK: omp.parallel { +! CHECK-NEXT: omp.parallel { +! CHECK: br ^[[BB1:.*]] +! CHECK: ^[[BB1]]: +! CHECK: cond_br %{{.*}}, ^[[BB2:.*]], ^[[BB4:.*]] +! CHECK-NEXT: ^[[BB2]]: +! CHECK: cond_br %{{.*}}, ^[[BB4]], ^[[BB3:.*]] +! CHECK-NEXT: ^[[BB3]]: +! CHECK: br ^[[BB1]] +! CHECK-NEXT: ^[[BB4]]: +! CHECK: omp.terminator +! CHECK-NEXT: } +! CHECK: omp.terminator +! CHECK-NEXT } +! CHECK: } +subroutine ss9() ! EXIT inside OpenMP parallel (inside parallel) + integer :: x + !$omp parallel + !$omp parallel private(x) + do k = 1, 3 + if (k .eq. n) exit + x = k + x = x + k + end do + !$omp end parallel + !$omp end parallel +end + ! CHECK-LABEL: func @_QQmain program p call ss1(2) call ss2(2) call ss3(2) + call ss4(2) + call ss5() + call ss6() + call ss7() + call ss8() + call ss9() end -- 2.7.4