From: Jean Perier Date: Thu, 23 Feb 2023 08:10:09 +0000 (+0100) Subject: [flang][hlfir] Simplify hlfir.assign default codegen for arrays X-Git-Tag: upstream/17.0.6~16705 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=3064d6b7b062cca348fd5528f3a77bff7df7e5f9;p=platform%2Fupstream%2Fllvm.git [flang][hlfir] Simplify hlfir.assign default codegen for arrays The previous code was always emitting two genAssign calls to create a temporary copy of the RHS if it could overlap with the LHS. This inline temporary creation is not needed anymore after: https://github.com/llvm/llvm-project/commit/755535b5eb5f6d60e9cc347cecd9e057231b92bb that updated the assignment runtime to detect overlap and make a temporary copy in the runtime directly. Note that optimized inlined assignment will still have to do the alias analysis to skip the copy when added later. Differential Revision: https://reviews.llvm.org/D144567 --- diff --git a/flang/lib/Optimizer/HLFIR/Transforms/ConvertToFIR.cpp b/flang/lib/Optimizer/HLFIR/Transforms/ConvertToFIR.cpp index 51cca5c..181bc2f 100644 --- a/flang/lib/Optimizer/HLFIR/Transforms/ConvertToFIR.cpp +++ b/flang/lib/Optimizer/HLFIR/Transforms/ConvertToFIR.cpp @@ -54,18 +54,9 @@ static mlir::Value genAllocatableTempFromSourceBox(mlir::Location loc, return copy; } -static std::pair -genTempFromSourceBox(mlir::Location loc, fir::FirOpBuilder &builder, - mlir::Value sourceBox) { - return {genAllocatableTempFromSourceBox(loc, builder, sourceBox), - /*cleanUpTemp=*/true}; -} - namespace { /// May \p lhs alias with \p rhs? /// TODO: implement HLFIR alias analysis. -static bool mayAlias(hlfir::Entity lhs, hlfir::Entity rhs) { return true; } - class AssignOpConversion : public mlir::OpRewritePattern { public: explicit AssignOpConversion(mlir::MLIRContext *ctx) : OpRewritePattern{ctx} {} @@ -92,6 +83,10 @@ public: "variable to fir::ExtendedValue must not require cleanup"); if (lhs.isArray()) { + // There may be overlap between lhs and rhs. The runtime is able to detect + // and to make a copy of the rhs before modifying the lhs if needed. + // The code below relies on this and does not do any compile time alias + // analysis. const bool rhsIsValue = fir::isa_trivial(fir::getBase(rhsExv).getType()); if (rhsIsValue) { // createBox can only be called for fir::ExtendedValue that are @@ -110,9 +105,6 @@ public: // inline array assignment when profitable. auto to = fir::getBase(builder.createBox(loc, lhsExv)); auto from = fir::getBase(builder.createBox(loc, rhsExv)); - bool cleanUpTemp = false; - if (!rhsIsValue && mayAlias(rhs, lhs)) - std::tie(from, cleanUpTemp) = genTempFromSourceBox(loc, builder, from); auto toMutableBox = builder.createTemporary(loc, to.getType()); // As per 10.2.1.2 point 1 (1) polymorphic variables must be allocatable. @@ -120,10 +112,6 @@ public: // type and that the mutableBox will not be modified. builder.create(loc, to, toMutableBox); fir::runtime::genAssign(builder, loc, toMutableBox, from); - if (cleanUpTemp) { - mlir::Value addr = builder.create(loc, from); - builder.create(loc, addr); - } } else { // Assume overlap does not matter for scalar (dealt with memmove for // characters). diff --git a/flang/test/HLFIR/assign-codegen.fir b/flang/test/HLFIR/assign-codegen.fir index 91ca33d..5736ca9 100644 --- a/flang/test/HLFIR/assign-codegen.fir +++ b/flang/test/HLFIR/assign-codegen.fir @@ -132,7 +132,6 @@ func.func @array(%arg0: !fir.box>, %arg1: !fir.ref>, // CHECK-SAME: %[[VAL_1:.*]]: !fir.ref>) { // CHECK: %[[VAL_2:.*]] = fir.alloca !fir.box> -// CHECK: %[[VAL_3:.*]] = fir.alloca !fir.box>> // CHECK: %[[VAL_4:.*]] = arith.constant 100 : index // CHECK: %[[VAL_5:.*]] = fir.declare %[[VAL_0]] {uniq_name = "x"} : (!fir.box>) -> !fir.box> // CHECK: %[[VAL_6:.*]] = fir.rebox %[[VAL_5]] : (!fir.box>) -> !fir.box> @@ -140,21 +139,10 @@ func.func @array(%arg0: !fir.box>, %arg1: !fir.ref>, !fir.shape<1>) -> !fir.ref> // CHECK: %[[VAL_9:.*]] = fir.shape %[[VAL_4]] : (index) -> !fir.shape<1> // CHECK: %[[VAL_10:.*]] = fir.embox %[[VAL_8]](%[[VAL_9]]) : (!fir.ref>, !fir.shape<1>) -> !fir.box> -// CHECK: %[[VAL_11:.*]] = fir.zero_bits !fir.heap> -// CHECK: %[[VAL_12:.*]] = arith.constant 0 : index -// CHECK: %[[VAL_13:.*]] = fir.shape %[[VAL_12]] : (index) -> !fir.shape<1> -// CHECK: %[[VAL_14:.*]] = fir.embox %[[VAL_11]](%[[VAL_13]]) : (!fir.heap>, !fir.shape<1>) -> !fir.box>> -// CHECK: fir.store %[[VAL_14]] to %[[VAL_3]] : !fir.ref>>> -// CHECK: %[[VAL_18:.*]] = fir.convert %[[VAL_3]] : (!fir.ref>>>) -> !fir.ref> -// CHECK: %[[VAL_19:.*]] = fir.convert %[[VAL_10]] : (!fir.box>) -> !fir.box -// CHECK: %[[VAL_21:.*]] = fir.call @_FortranAAssign(%[[VAL_18]], %[[VAL_19]], %{{.*}}, %{{.*}}) : (!fir.ref>, !fir.box, !fir.ref, i32) -> none -// CHECK: %[[VAL_22:.*]] = fir.load %[[VAL_3]] : !fir.ref>>> // CHECK: fir.store %[[VAL_5]] to %[[VAL_2]] : !fir.ref>> // CHECK: %[[VAL_26:.*]] = fir.convert %[[VAL_2]] : (!fir.ref>>) -> !fir.ref> -// CHECK: %[[VAL_27:.*]] = fir.convert %[[VAL_22]] : (!fir.box>>) -> !fir.box +// CHECK: %[[VAL_27:.*]] = fir.convert %[[VAL_10]] : (!fir.box>) -> !fir.box // CHECK: %[[VAL_29:.*]] = fir.call @_FortranAAssign(%[[VAL_26]], %[[VAL_27]], %{{.*}}, %{{.*}}) : (!fir.ref>, !fir.box, !fir.ref, i32) -> none -// CHECK: %[[VAL_30:.*]] = fir.box_addr %[[VAL_22]] : (!fir.box>>) -> !fir.heap> -// CHECK: fir.freemem %[[VAL_30]] : !fir.heap> func.func @test_scalar_to_array(%lhs: !fir.box>, %rhs: i32) {