[flang-commits] [flang] e52a6d7 - [flang][hlfir] avoid useless LHS temporaries inside WHERE

[Jean Perier via flang-commits]

Author: Jean Perier
Date: 2023-07-05T14:26:41+02:00
New Revision: e52a6d7784adbcb98b17af7943c69c854aa7b10f

URL: https://github.com/llvm/llvm-project/commit/e52a6d7784adbcb98b17af7943c69c854aa7b10f
DIFF: https://github.com/llvm/llvm-project/commit/e52a6d7784adbcb98b17af7943c69c854aa7b10f.diff

LOG: [flang][hlfir] avoid useless LHS temporaries inside WHERE

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.

Differential Revision: https://reviews.llvm.org/D154418

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/flang/lib/Optimizer/HLFIR/Transforms/ScheduleOrderedAssignments.cpp b/flang/lib/Optimizer/HLFIR/Transforms/ScheduleOrderedAssignments.cpp
index d53543aea1adfb..f4c96a6522266e 100644
--- a/flang/lib/Optimizer/HLFIR/Transforms/ScheduleOrderedAssignments.cpp
+++ b/flang/lib/Optimizer/HLFIR/Transforms/ScheduleOrderedAssignments.cpp
@@ -544,9 +544,15 @@ hlfir::buildEvaluationSchedule(hlfir::OrderedAssignmentTreeOpInterface root,
     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);
   }

diff  --git a/flang/test/HLFIR/order_assignments/where-scheduling.f90 b/flang/test/HLFIR/order_assignments/where-scheduling.f90
index 89a3cadbd69719..bdccf2e4aff039 100644
--- a/flang/test/HLFIR/order_assignments/where-scheduling.f90
+++ b/flang/test/HLFIR/order_assignments/where-scheduling.f90
@@ -82,6 +82,27 @@ subroutine where_in_forall_conflict(x)
   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 @@ subroutine where_in_forall_conflict(x)
 !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