[flang-commits] [PATCH] D154418: [flang][hlfir] avoid useless LHS temporaries inside WHERE

[Jean Perier via Phabricator via flang-commits]

jeanPerier created this revision.
jeanPerier added reviewers: vzakhari, tblah.
jeanPerier added a project: Flang.
Herald added subscribers: sunshaoce, mehdi_amini, jdoerfert.
Herald added a project: All.
jeanPerier requested review of this revision.

The need to save LHS addresses on a stack before doing an assignment
is very limited: it is only really needed for forall and vectore
subscripted LHS where the LHS cannot be computed as a descriptor.

The previous current WHERE codegen was creating address stacks for
LHS element addresses when the LHS evaluation conflicts with the
assignment (may depend on the LHS value). This is not needed
since the computed array designator for the LHS is already "saved"
before the assignment from an SSA point of view.

This patch prevents LHS temporary stack from being created outside
of forall and vector subscripted assignments.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D154418

Files:
  flang/lib/Optimizer/HLFIR/Transforms/ScheduleOrderedAssignments.cpp
  flang/test/HLFIR/order_assignments/where-scheduling.f90


Index: flang/test/HLFIR/order_assignments/where-scheduling.f90
===================================================================
--- flang/test/HLFIR/order_assignments/where-scheduling.f90
+++ flang/test/HLFIR/order_assignments/where-scheduling.f90
@@ -82,6 +82,27 @@
   end forall
 end subroutine
 
+subroutine no_need_to_make_lhs_temp(x, y, i, j)
+  integer :: j, i, x(:, :), y(:, :)
+  call internal
+contains
+subroutine internal
+  ! The internal procedure context currently gives a hard time to
+  ! FIR alias analysis that flags the read of i,j and y as conflicting
+  ! with the write to x. But this is not a reason to create a temporary
+  ! storage for the LHS: the address is anyway fully computed in
+  ! a descriptor (fir.box) before assigning any element of x.
+
+  ! Note that the where mask is also saved while there is no real
+  ! need to: it is addressing x elements in the same order as they
+  ! are being assigned. But this will require more work in the
+  ! conflict analysis to prove that the lowered DAG of `x(:, y(i, j))`
+  ! are the same and that the access to this designator is done in the
+  ! same ordered inside the mask and LHS.
+  where (x(:, y(i, j)) == y(i, j))  x(:, y(i, j)) = 42
+end subroutine
+end subroutine
+
 !CHECK-LABEL: ------------ scheduling where in _QPno_conflict ------------
 !CHECK-NEXT: run 1 evaluate: where/region_assign1
 !CHECK-LABEL: ------------ scheduling where in _QPfake_conflict ------------
@@ -126,3 +147,7 @@
 !CHECK-NEXT: conflict: R/W: <block argument> of type '!fir.box<!fir.array<?x?xf32>>' at index: 0 W:<block argument> of type '!fir.box<!fir.array<?x?xf32>>' at index: 0
 !CHECK-NEXT: run 1 save    : forall/where1/region_assign1/rhs
 !CHECK-NEXT: run 2 evaluate: forall/where1/region_assign1
+!CHECK-LABEL: ------------ scheduling where in _QFno_need_to_make_lhs_tempPinternal ------------
+!CHECK-NEXT: conflict: R/W: %7 = fir.load %6 : !fir.llvm_ptr<!fir.ref<i32>> W:%13 = fir.load %12 : !fir.ref<!fir.box<!fir.array<?x?xi32>>>
+!CHECK-NEXT: run 1 save    : where/mask
+!CHECK-NEXT: run 2 evaluate: where/region_assign1
Index: flang/lib/Optimizer/HLFIR/Transforms/ScheduleOrderedAssignments.cpp
===================================================================
--- flang/lib/Optimizer/HLFIR/Transforms/ScheduleOrderedAssignments.cpp
+++ flang/lib/Optimizer/HLFIR/Transforms/ScheduleOrderedAssignments.cpp
@@ -544,9 +544,15 @@
     scheduler.startIndependentEvaluationGroup();
     scheduler.saveEvaluationIfConflict(assign.getRhsRegion(),
                                        leafRegionsMayOnlyRead);
-    scheduler.saveEvaluationIfConflict(assign.getLhsRegion(),
-                                       leafRegionsMayOnlyRead,
-                                       /*yieldIsImplicitRead=*/false);
+    // There is no point to save the LHS outside of Forall and assignment to a
+    // vector subscripted LHS because the LHS is already fully evaluated and
+    // saved in the resulting SSA address value (that may be a descriptor or
+    // descriptor address).
+    if (mlir::isa<hlfir::ForallOp>(root.getOperation()) ||
+        mlir::isa<hlfir::ElementalAddrOp>(assign.getLhsRegion().back().back()))
+      scheduler.saveEvaluationIfConflict(assign.getLhsRegion(),
+                                         leafRegionsMayOnlyRead,
+                                         /*yieldIsImplicitRead=*/false);
     scheduler.finishIndependentEvaluationGroup();
     scheduler.finishSchedulingAssignment(assign);
   }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D154418.536992.patch
Type: text/x-patch
Size: 3504 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/flang-commits/attachments/20230704/5d6bacc0/attachment-0001.bin>