From 23fbe525ce0645341610b751184882fea264c99e Mon Sep 17 00:00:00 2001 From: Jean Perier Date: Wed, 28 Jun 2023 08:27:16 +0200 Subject: [PATCH] [flang] do not merge block after lowering Lowering relies on dead code generation / unreachable block deletion to delete some code that is potentially invalid. However, calling mlir::simplifyRegion also merges block, which may promote SSA values to block arguments. Not all FIR types are intended to be block arguments. The added test shows an example where block merging led to fir.shape<> being block arguments (and a failure later in codegen). Reviewed By: tblah, clementval, vdonaldson Differential Revision: https://reviews.llvm.org/D153858 --- flang/lib/Lower/Bridge.cpp | 8 +++- flang/test/Lower/HLFIR/no-block-merging.f90 | 59 +++++++++++++++++++++++++++++ flang/test/Lower/block.f90 | 36 +++++++++--------- flang/test/Lower/entry-statement.f90 | 12 +++--- 4 files changed, 92 insertions(+), 23 deletions(-) create mode 100644 flang/test/Lower/HLFIR/no-block-merging.f90 diff --git a/flang/lib/Lower/Bridge.cpp b/flang/lib/Lower/Bridge.cpp index 23a58eb..bc0c13e 100644 --- a/flang/lib/Lower/Bridge.cpp +++ b/flang/lib/Lower/Bridge.cpp @@ -4156,6 +4156,12 @@ private: startBlock(newBlock); } + void eraseDeadCodeAndBlocks(mlir::RewriterBase &rewriter, + llvm::MutableArrayRef regions) { + (void)mlir::eraseUnreachableBlocks(rewriter, regions); + (void)mlir::runRegionDCE(rewriter, regions); + } + /// Finish translation of a function. void endNewFunction(Fortran::lower::pft::FunctionLikeUnit &funit) { setCurrentPosition(Fortran::lower::pft::stmtSourceLoc(funit.endStmt)); @@ -4174,7 +4180,7 @@ private: // Eliminate dead code as a prerequisite to calling other IR passes. // FIXME: This simplification should happen in a normal pass, not here. mlir::IRRewriter rewriter(*builder); - (void)mlir::simplifyRegions(rewriter, {builder->getRegion()}); + (void)eraseDeadCodeAndBlocks(rewriter, {builder->getRegion()}); delete builder; builder = nullptr; hostAssocTuple = mlir::Value{}; diff --git a/flang/test/Lower/HLFIR/no-block-merging.f90 b/flang/test/Lower/HLFIR/no-block-merging.f90 new file mode 100644 index 0000000..7d0eb83 --- /dev/null +++ b/flang/test/Lower/HLFIR/no-block-merging.f90 @@ -0,0 +1,59 @@ +! Test that dead code elimination after lowering does not merge +! blocks. This test was added because block merging on the +! code below created a block with a fir.shape<> block argument +! which FIR codegen is not intended to support. +! RUN: bbc -emit-hlfir -o - %s | FileCheck %s + +subroutine test(res, selector, x1, x2, x3, x4, x5, x6, x7, x8) + integer :: res + character(*) :: selector + real(8), dimension(7) :: x1, x2, x3, x4, x5, x6, x7, x8 + interface + integer function foo(x) + real(8) :: x(:) + end function + end interface + select case (selector) + case ("01") + res = foo(x1) + res = foo(x1) + case ("02") + res = foo(x2) + res = foo(x2) + case ("03") + res = foo(x3) + res = foo(x3) + case ("04") + res = foo(x4) + res = foo(x4) + case ("05") + res = foo(x5) + res = foo(x5) + case ("06") + res = foo(x6) + res = foo(x6) + case ("07") + res = foo(x7) + res = foo(x7) + case ("08") + res = foo(x8) + res = foo(x8) + case default + end select +end subroutine +! CHECK: fir.call @_QPfoo +! CHECK: fir.call @_QPfoo +! CHECK: fir.call @_QPfoo +! CHECK: fir.call @_QPfoo +! CHECK: fir.call @_QPfoo +! CHECK: fir.call @_QPfoo +! CHECK: fir.call @_QPfoo +! CHECK: fir.call @_QPfoo +! CHECK: fir.call @_QPfoo +! CHECK: fir.call @_QPfoo +! CHECK: fir.call @_QPfoo +! CHECK: fir.call @_QPfoo +! CHECK: fir.call @_QPfoo +! CHECK: fir.call @_QPfoo +! CHECK: fir.call @_QPfoo +! CHECK: fir.call @_QPfoo diff --git a/flang/test/Lower/block.f90 b/flang/test/Lower/block.f90 index 520af06..5a73a43 100644 --- a/flang/test/Lower/block.f90 +++ b/flang/test/Lower/block.f90 @@ -9,8 +9,8 @@ program bb ! block stack management and exits ! CHECK: %[[V_3:[0-9]+]] = fir.call @llvm.stacksave() ! CHECK: fir.store %{{.*}} to %[[V_1]] : !fir.ref ! CHECK: br ^bb1 - ! CHECK: ^bb1: // 2 preds: ^bb0, ^bb15 - ! CHECK: cond_br %{{.*}}, ^bb2, ^bb16 + ! CHECK: ^bb1: // 2 preds: ^bb0, ^bb16 + ! CHECK: cond_br %{{.*}}, ^bb2, ^bb17 ! CHECK: ^bb2: // pred: ^bb1 ! CHECK: %[[V_11:[0-9]+]] = fir.call @llvm.stacksave() ! CHECK: fir.store %{{.*}} to %[[V_1]] : !fir.ref @@ -21,44 +21,46 @@ program bb ! block stack management and exits ! CHECK: fir.store %{{.*}} to %[[V_1]] : !fir.ref ! CHECK: cond_br %{{.*}}, ^bb5, ^bb6 ! CHECK: ^bb5: // pred: ^bb4 - ! CHECK: br ^bb7 + ! CHECK: br ^bb14 ! CHECK: ^bb6: // pred: ^bb4 ! CHECK: fir.store %{{.*}} to %[[V_1]] : !fir.ref ! CHECK: cond_br %{{.*}}, ^bb7, ^bb8 - ! CHECK: ^bb7: // 3 preds: ^bb5, ^bb6, ^bb12 + ! CHECK: ^bb7: // pred: ^bb6 ! CHECK: fir.call @llvm.stackrestore(%[[V_11]]) - ! CHECK: br ^bb14 + ! CHECK: br ^bb15 ! CHECK: ^bb8: // pred: ^bb6 ! CHECK: fir.store %{{.*}} to %[[V_1]] : !fir.ref ! CHECK: cond_br %{{.*}}, ^bb9, ^bb10 ! CHECK: ^bb9: // pred: ^bb8 ! CHECK: fir.call @llvm.stackrestore(%[[V_11]]) - ! CHECK: br ^bb15 + ! CHECK: br ^bb16 ! CHECK: ^bb10: // 2 preds: ^bb3, ^bb8 ! CHECK: fir.store %{{.*}} to %[[V_1]] : !fir.ref ! CHECK: cond_br %{{.*}}, ^bb11, ^bb12 ! CHECK: ^bb11: // pred: ^bb10 ! CHECK: fir.call @llvm.stackrestore(%[[V_11]]) - ! CHECK: br ^bb17 + ! CHECK: br ^bb18 ! CHECK: ^bb12: // pred: ^bb10 ! CHECK: fir.store %{{.*}} to %[[V_1]] : !fir.ref - ! CHECK: cond_br %{{.*}}, ^bb13, ^bb7 + ! CHECK: cond_br %{{.*}}, ^bb13, ^bb14 ! CHECK: ^bb13: // pred: ^bb12 ! CHECK: fir.call @llvm.stackrestore(%[[V_11]]) ! CHECK: fir.call @llvm.stackrestore(%[[V_3]]) - ! CHECK: br ^bb18 - ! CHECK: ^bb14: // pred: ^bb7 - ! CHECK: fir.store %{{.*}} to %[[V_1]] : !fir.ref + ! CHECK: br ^bb19 + ! CHECK: ^bb14: // 2 preds: ^bb5, ^bb12 + ! CHECK: fir.call @llvm.stackrestore(%[[V_11]]) ! CHECK: br ^bb15 - ! CHECK: ^bb15: // 2 preds: ^bb9, ^bb14 + ! CHECK: ^bb15: // 2 preds: ^bb7, ^bb14 + ! CHECK: br ^bb16 + ! CHECK: ^bb16: // 2 preds: ^bb9, ^bb15 ! CHECK: br ^bb1 - ! CHECK: ^bb16: // pred: ^bb1 + ! CHECK: ^bb17: // pred: ^bb1 ! CHECK: fir.store %{{.*}} to %[[V_1]] : !fir.ref - ! CHECK: br ^bb17 - ! CHECK: ^bb17: // 2 preds: ^bb11, ^bb16 + ! CHECK: cf.br ^bb18 + ! CHECK: ^bb18: // 2 preds: ^bb11, ^bb17 ! CHECK: fir.call @llvm.stackrestore(%[[V_3]]) - ! CHECK: br ^bb18 - ! CHECK: ^bb18: // 2 preds: ^bb13, ^bb17 + ! CHECK: br ^bb19 + ! CHECK: ^bb19: // 2 preds: ^bb13, ^bb18 ! CHECK: return block i = i + 1 ! 1 increment diff --git a/flang/test/Lower/entry-statement.f90 b/flang/test/Lower/entry-statement.f90 index 83491bd..6a857c8 100644 --- a/flang/test/Lower/entry-statement.f90 +++ b/flang/test/Lower/entry-statement.f90 @@ -280,14 +280,16 @@ function f1(n1) result(res1) ! CHECK: %[[V_17:[0-9]+]] = fir.load %arg2 : !fir.ref ! CHECK: %[[V_18:[0-9]+]] = arith.cmpi eq, %[[V_17]], %c1{{.*}}_i32_4 : i32 ! CHECK: cond_br %[[V_18]], ^bb2, ^bb3 - ! CHECK: ^bb2: // 2 preds: ^bb1, ^bb3 - ! CHECK: br ^bb5 + ! CHECK: ^bb2: // pred: ^bb1 + ! CHECK: br ^bb6 ! CHECK: ^bb3: // pred: ^bb1 ! CHECK: fir.call @_QFf1Ps2(%[[V_2]]) {{.*}}: (!fir.ref, !fir.boxchar<1>>>) -> () ! CHECK: %[[V_19:[0-9]+]] = fir.load %[[V_1]] : !fir.ref ! CHECK: %[[V_20:[0-9]+]] = arith.cmpi eq, %[[V_19]], %c2{{.*}}_i32 : i32 - ! CHECK: cond_br %[[V_20]], ^bb2, ^bb4 + ! CHECK: cond_br %[[V_20]], ^bb4, ^bb5 ! CHECK: ^bb4: // pred: ^bb3 + ! CHECK: cf.br ^bb6 + ! CHECK: ^bb5: // pred: ^bb3 ! CHECK: %[[V_21:[0-9]+]] = fir.address_of(@_QQcl.4320432043) : !fir.ref> ! CHECK: %[[V_22:[0-9]+]] = arith.cmpi slt, %c5{{.*}}, %c5{{.*}} : index ! CHECK: %[[V_23:[0-9]+]] = arith.select %[[V_22]], %c5{{.*}}, %c5{{.*}} : index @@ -305,8 +307,8 @@ function f1(n1) result(res1) ! CHECK: fir.store %[[V_30]] to %[[V_33]] : !fir.ref> ! CHECK: } ! CHECK: fir.call @_QFf1Ps3(%[[V_2]]) {{.*}}: (!fir.ref, !fir.boxchar<1>>>) -> () - ! CHECK: br ^bb5 - ! CHECK: ^bb5: // 2 preds: ^bb2, ^bb4 + ! CHECK: br ^bb6 + ! CHECK: ^bb6: // 3 preds: ^bb2, ^bb4, ^bb5 ! CHECK: %[[V_31:[0-9]+]] = fir.emboxchar %[[V_0]], %c5{{.*}} : (!fir.ref>, index) -> !fir.boxchar<1> ! CHECK: return %[[V_31]] : !fir.boxchar<1> ! CHECK: } -- 2.7.4