[flang-commits] [PATCH] D144567: [flang][hlfir] Simplify hlfir.assign default codegen for arrays

[Jean Perier via Phabricator via flang-commits]

jeanPerier created this revision.
jeanPerier added a reviewer: PeteSteinfeld.
jeanPerier added a project: Flang.
Herald added subscribers: mehdi_amini, jdoerfert.
Herald added a project: All.
jeanPerier requested review of this revision.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D144567

Files:
  flang/lib/Optimizer/HLFIR/Transforms/ConvertToFIR.cpp
  flang/test/HLFIR/assign-codegen.fir


Index: flang/test/HLFIR/assign-codegen.fir
===================================================================
--- flang/test/HLFIR/assign-codegen.fir
+++ flang/test/HLFIR/assign-codegen.fir
@@ -132,7 +132,6 @@
 // CHECK-SAME:    %[[VAL_0:.*]]: !fir.box<!fir.array<?xi32>>,
 // CHECK-SAME:    %[[VAL_1:.*]]: !fir.ref<!fir.array<100xi32>>) {
 // CHECK:  %[[VAL_2:.*]] = fir.alloca !fir.box<!fir.array<?xi32>>
-// CHECK:  %[[VAL_3:.*]] = fir.alloca !fir.box<!fir.heap<!fir.array<100xi32>>>
 // CHECK:  %[[VAL_4:.*]] = arith.constant 100 : index
 // CHECK:  %[[VAL_5:.*]] = fir.declare %[[VAL_0]] {uniq_name = "x"} : (!fir.box<!fir.array<?xi32>>) -> !fir.box<!fir.array<?xi32>>
 // CHECK:  %[[VAL_6:.*]] = fir.rebox %[[VAL_5]] : (!fir.box<!fir.array<?xi32>>) -> !fir.box<!fir.array<?xi32>>
@@ -140,21 +139,10 @@
 // CHECK:  %[[VAL_8:.*]] = fir.declare %[[VAL_1]](%[[VAL_7]]) {uniq_name = "y"} : (!fir.ref<!fir.array<100xi32>>, !fir.shape<1>) -> !fir.ref<!fir.array<100xi32>>
 // CHECK:  %[[VAL_9:.*]] = fir.shape %[[VAL_4]] : (index) -> !fir.shape<1>
 // CHECK:  %[[VAL_10:.*]] = fir.embox %[[VAL_8]](%[[VAL_9]]) : (!fir.ref<!fir.array<100xi32>>, !fir.shape<1>) -> !fir.box<!fir.array<100xi32>>
-// CHECK:  %[[VAL_11:.*]] = fir.zero_bits !fir.heap<!fir.array<100xi32>>
-// 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.array<100xi32>>, !fir.shape<1>) -> !fir.box<!fir.heap<!fir.array<100xi32>>>
-// CHECK:  fir.store %[[VAL_14]] to %[[VAL_3]] : !fir.ref<!fir.box<!fir.heap<!fir.array<100xi32>>>>
-// CHECK:  %[[VAL_18:.*]] = fir.convert %[[VAL_3]] : (!fir.ref<!fir.box<!fir.heap<!fir.array<100xi32>>>>) -> !fir.ref<!fir.box<none>>
-// CHECK:  %[[VAL_19:.*]] = fir.convert %[[VAL_10]] : (!fir.box<!fir.array<100xi32>>) -> !fir.box<none>
-// CHECK:  %[[VAL_21:.*]] = fir.call @_FortranAAssign(%[[VAL_18]], %[[VAL_19]], %{{.*}}, %{{.*}}) : (!fir.ref<!fir.box<none>>, !fir.box<none>, !fir.ref<i8>, i32) -> none
-// CHECK:  %[[VAL_22:.*]] = fir.load %[[VAL_3]] : !fir.ref<!fir.box<!fir.heap<!fir.array<100xi32>>>>
 // CHECK:  fir.store %[[VAL_5]] to %[[VAL_2]] : !fir.ref<!fir.box<!fir.array<?xi32>>>
 // CHECK:  %[[VAL_26:.*]] = fir.convert %[[VAL_2]] : (!fir.ref<!fir.box<!fir.array<?xi32>>>) -> !fir.ref<!fir.box<none>>
-// CHECK:  %[[VAL_27:.*]] = fir.convert %[[VAL_22]] : (!fir.box<!fir.heap<!fir.array<100xi32>>>) -> !fir.box<none>
+// CHECK:  %[[VAL_27:.*]] = fir.convert %[[VAL_10]] : (!fir.box<!fir.array<100xi32>>) -> !fir.box<none>
 // CHECK:  %[[VAL_29:.*]] = fir.call @_FortranAAssign(%[[VAL_26]], %[[VAL_27]], %{{.*}}, %{{.*}}) : (!fir.ref<!fir.box<none>>, !fir.box<none>, !fir.ref<i8>, i32) -> none
-// CHECK:  %[[VAL_30:.*]] = fir.box_addr %[[VAL_22]] : (!fir.box<!fir.heap<!fir.array<100xi32>>>) -> !fir.heap<!fir.array<100xi32>>
-// CHECK:  fir.freemem %[[VAL_30]] : !fir.heap<!fir.array<100xi32>>
 
 
 func.func @test_scalar_to_array(%lhs: !fir.box<!fir.array<?xi32>>, %rhs: i32) {
Index: flang/lib/Optimizer/HLFIR/Transforms/ConvertToFIR.cpp
===================================================================
--- flang/lib/Optimizer/HLFIR/Transforms/ConvertToFIR.cpp
+++ flang/lib/Optimizer/HLFIR/Transforms/ConvertToFIR.cpp
@@ -64,8 +64,6 @@
 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<hlfir::AssignOp> {
 public:
   explicit AssignOpConversion(mlir::MLIRContext *ctx) : OpRewritePattern{ctx} {}
@@ -92,6 +90,10 @@
            "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 +112,6 @@
       // 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 +119,6 @@
       // type and that the mutableBox will not be modified.
       builder.create<fir::StoreOp>(loc, to, toMutableBox);
       fir::runtime::genAssign(builder, loc, toMutableBox, from);
-      if (cleanUpTemp) {
-        mlir::Value addr = builder.create<fir::BoxAddrOp>(loc, from);
-        builder.create<fir::FreeMemOp>(loc, addr);
-      }
     } else {
       // Assume overlap does not matter for scalar (dealt with memmove for
       // characters).


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D144567.499529.patch
Type: text/x-patch
Size: 5206 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/flang-commits/attachments/20230222/3bf83fcf/attachment-0001.bin>