From 626b77db7727e6584834ab571c9c1244e311903e Mon Sep 17 00:00:00 2001 From: Jean Perier Date: Tue, 22 Oct 2024 06:50:27 -0700 Subject: [PATCH] [flang][hlfir] do not consider local temps as conflicting in assignment --- .../Transforms/ScheduleOrderedAssignments.cpp | 32 ++++++++++++++----- .../vector-subscripts-scheduling.fir | 31 ++++++++++++++++++ .../order_assignments/where-scheduling.f90 | 3 ++ 3 files changed, 58 insertions(+), 8 deletions(-) create mode 100644 flang/test/HLFIR/order_assignments/vector-subscripts-scheduling.fir diff --git a/flang/lib/Optimizer/HLFIR/Transforms/ScheduleOrderedAssignments.cpp b/flang/lib/Optimizer/HLFIR/Transforms/ScheduleOrderedAssignments.cpp index efe59b8ef988e6..5971b5b9d76a0e 100644 --- a/flang/lib/Optimizer/HLFIR/Transforms/ScheduleOrderedAssignments.cpp +++ b/flang/lib/Optimizer/HLFIR/Transforms/ScheduleOrderedAssignments.cpp @@ -347,12 +347,23 @@ conflict(llvm::ArrayRef effectsA, anyRAWorWAW(effectsB, effectsA, aliasAnalysis); } -/// Could there be any write effects in "effects"? +/// Could there be any write effects in "effects" affecting memory storages +/// that are not local to the current region. static bool -anyWrite(llvm::ArrayRef effects) { +anyNonLocalWrite(llvm::ArrayRef effects, + mlir::Region ®ion) { return llvm::any_of( - effects, [](const mlir::MemoryEffects::EffectInstance &effect) { - return mlir::isa(effect.getEffect()); + effects, [®ion](const mlir::MemoryEffects::EffectInstance &effect) { + if (mlir::isa(effect.getEffect())) { + if (mlir::Value v = effect.getValue()) { + v = getStorageSource(v); + if (v.getDefiningOp() || + v.getDefiningOp()) + return !region.isAncestor(v.getParentRegion()); + } + return true; + } + return false; }); } @@ -393,9 +404,13 @@ void Scheduler::saveEvaluationIfConflict(mlir::Region &yieldRegion, if (entity && hlfir::isFortranVariableType(entity->get().getType())) effects.emplace_back(mlir::MemoryEffects::Read::get(), entity); } - if (!leafRegionsMayOnlyRead && anyWrite(effects)) { - // Region with write effect must be executed only once: save it the first - // time it is encountered. + if (!leafRegionsMayOnlyRead && anyNonLocalWrite(effects, yieldRegion)) { + // Region with write effect must be executed only once (unless all writes + // affect storages allocated inside the region): save it the first time it + // is encountered. + LLVM_DEBUG(llvm::dbgs() + << "saving eval because write effect prevents re-evaluation" + << "\n";); saveEvaluation(yieldRegion, effects, /*anyWrite=*/true); } else if (conflict(effects, assignEffects)) { // Region that conflicts with the current assignments must be fully @@ -411,7 +426,8 @@ void Scheduler::saveEvaluationIfConflict(mlir::Region &yieldRegion, // For example, a WHERE mask might be written by the masked assignment // evaluations, and it has to be saved in this case: // where (mask) r = f() ! function f modifies mask - saveEvaluation(yieldRegion, effects, anyWrite(effects)); + saveEvaluation(yieldRegion, effects, + anyNonLocalWrite(effects, yieldRegion)); } else { // Can be executed while doing the assignment. independentEvaluationEffects.append(effects.begin(), effects.end()); diff --git a/flang/test/HLFIR/order_assignments/vector-subscripts-scheduling.fir b/flang/test/HLFIR/order_assignments/vector-subscripts-scheduling.fir new file mode 100644 index 00000000000000..b76492efe2a562 --- /dev/null +++ b/flang/test/HLFIR/order_assignments/vector-subscripts-scheduling.fir @@ -0,0 +1,31 @@ +// Test local alloca and store inside hlfir.region_assign do not trigger the +// creation of a temporary for the LHS. + +// RUN: fir-opt -o - -lower-hlfir-ordered-assignments --debug-only=flang-ordered-assignment -flang-dbg-order-assignment-schedule-only %s 2>&1 | FileCheck %s +// REQUIRES: asserts + +func.func @simple(%arg0: !fir.ref> , %arg1: !fir.ref> , %arg2: !fir.ref>, %i: i64, %f: f32) { + %c10 = arith.constant 10 : index + %c100 = arith.constant 100 : index + %0 = fir.shape %c100, %c10 : (index, index) -> !fir.shape<2> + %1:2 = hlfir.declare %arg0(%0) {uniq_name = "_QFsimpleEx"} : (!fir.ref>, !fir.shape<2>) -> (!fir.ref>, !fir.ref>) + %2 = fir.shape %c10 : (index) -> !fir.shape<1> + %3:2 = hlfir.declare %arg1(%2) {uniq_name = "y"} : (!fir.ref>, !fir.shape<1>) -> (!fir.ref>, !fir.ref>) + hlfir.region_assign { + hlfir.yield %f : f32 + } to { + %local_temp = fir.alloca i64 + fir.store %i to %local_temp : !fir.ref + %icopy = fir.load %local_temp : !fir.ref + hlfir.elemental_addr %2 : !fir.shape<1> { + ^bb0(%arg3: index): + %5 = hlfir.designate %3#0 (%arg3) : (!fir.ref>, index) -> !fir.ref + %6 = fir.load %5 : !fir.ref + %7 = hlfir.designate %1#0 (%icopy, %6) : (!fir.ref>, i64, i64) -> !fir.ref + hlfir.yield %7 : !fir.ref + } + } + return +} + +// CHECK: run 1 evaluate: region_assign diff --git a/flang/test/HLFIR/order_assignments/where-scheduling.f90 b/flang/test/HLFIR/order_assignments/where-scheduling.f90 index ab87ae92de5799..3010476d4a188f 100644 --- a/flang/test/HLFIR/order_assignments/where-scheduling.f90 +++ b/flang/test/HLFIR/order_assignments/where-scheduling.f90 @@ -135,6 +135,7 @@ end function f !CHECK-NEXT: run 2 evaluate: where/region_assign1 !CHECK-LABEL: ------------ scheduling where in _QPonly_once ------------ !CHECK-NEXT: unknown effect: %{{[0-9]+}} = llvm.intr.stacksave : !llvm.ptr +!CHECK-NEXT: saving eval because write effect prevents re-evaluation !CHECK-NEXT: run 1 save (w): where/mask !CHECK-NEXT: run 2 evaluate: where/region_assign1 !CHECK-NEXT: run 3 evaluate: where/region_assign2 @@ -180,6 +181,7 @@ end function f !CHECK-NEXT: conflict: R/W: %{{.*}} = hlfir.declare %{{.*}} {uniq_name = "_QFwhere_construct_unknown_conflictEmask"} : (!fir.box>>, !fir.dscope) -> (!fir.box>>, !fir.box>>) W: !CHECK-NEXT: run 1 save : where/mask !CHECK-NEXT: unknown effect: %{{.*}} = fir.call @_QPf() fastmath : () -> f32 +!CHECK-NEXT: saving eval because write effect prevents re-evaluation !CHECK-NEXT: run 2 save (w): where/region_assign1/rhs !CHECK-NEXT: run 3 evaluate: where/region_assign1 !CHECK-NEXT: ------------ scheduling where in _QPelsewhere_construct_unknown_conflict ------------ @@ -190,5 +192,6 @@ end function f !CHECK-NEXT: conflict: R/W: %{{.*}} = hlfir.declare %{{.*}} {uniq_name = "_QFelsewhere_construct_unknown_conflictEmask2"} : (!fir.box>>, !fir.dscope) -> (!fir.box>>, !fir.box>>) W: !CHECK-NEXT: run 2 save : where/elsewhere1/mask !CHECK-NEXT: unknown effect: %{{.*}} = fir.call @_QPf() fastmath : () -> f32 +!CHECK-NEXT: saving eval because write effect prevents re-evaluation !CHECK-NEXT: run 3 save (w): where/elsewhere1/region_assign1/rhs !CHECK-NEXT: run 4 evaluate: where/elsewhere1/region_assign1