Skip to content

Commit a59f712

Browse files
authored
[flang][hlfir] do not consider local temps as conflicting in assignment (#113330)
Last patch required to avoid creating a temporary for the LHS when dealing with `x([a,b]) = y`. The code dealing with "ordered assignments" (where, forall, user and vector subscripted assignments) is saving the evaluated RHS/LHS and masks if they have write effects because this write effects should not be evaluated when they affect entities that may be written to in other contexts after the evaluation and before the re-evaluation. But when dealing with write to storage allocated in the region for the expression being evluated, there is no problem to re-evaluate the write: it has no effect outside of the expression evaluation that owns the allocation. In the case of `x([a,b]) = y`, the temporary is created for the vector subscript. Raising the HLFIR abstraction for simple array constructors may be a good idea, but local temps are created in other contexts, so this fix is more generic.
1 parent 03cef62 commit a59f712

File tree

3 files changed

+58
-8
lines changed

3 files changed

+58
-8
lines changed

flang/lib/Optimizer/HLFIR/Transforms/ScheduleOrderedAssignments.cpp

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -347,12 +347,23 @@ conflict(llvm::ArrayRef<mlir::MemoryEffects::EffectInstance> effectsA,
347347
anyRAWorWAW(effectsB, effectsA, aliasAnalysis);
348348
}
349349

350-
/// Could there be any write effects in "effects"?
350+
/// Could there be any write effects in "effects" affecting memory storages
351+
/// that are not local to the current region.
351352
static bool
352-
anyWrite(llvm::ArrayRef<mlir::MemoryEffects::EffectInstance> effects) {
353+
anyNonLocalWrite(llvm::ArrayRef<mlir::MemoryEffects::EffectInstance> effects,
354+
mlir::Region &region) {
353355
return llvm::any_of(
354-
effects, [](const mlir::MemoryEffects::EffectInstance &effect) {
355-
return mlir::isa<mlir::MemoryEffects::Write>(effect.getEffect());
356+
effects, [&region](const mlir::MemoryEffects::EffectInstance &effect) {
357+
if (mlir::isa<mlir::MemoryEffects::Write>(effect.getEffect())) {
358+
if (mlir::Value v = effect.getValue()) {
359+
v = getStorageSource(v);
360+
if (v.getDefiningOp<fir::AllocaOp>() ||
361+
v.getDefiningOp<fir::AllocMemOp>())
362+
return !region.isAncestor(v.getParentRegion());
363+
}
364+
return true;
365+
}
366+
return false;
356367
});
357368
}
358369

@@ -393,9 +404,13 @@ void Scheduler::saveEvaluationIfConflict(mlir::Region &yieldRegion,
393404
if (entity && hlfir::isFortranVariableType(entity->get().getType()))
394405
effects.emplace_back(mlir::MemoryEffects::Read::get(), entity);
395406
}
396-
if (!leafRegionsMayOnlyRead && anyWrite(effects)) {
397-
// Region with write effect must be executed only once: save it the first
398-
// time it is encountered.
407+
if (!leafRegionsMayOnlyRead && anyNonLocalWrite(effects, yieldRegion)) {
408+
// Region with write effect must be executed only once (unless all writes
409+
// affect storages allocated inside the region): save it the first time it
410+
// is encountered.
411+
LLVM_DEBUG(llvm::dbgs()
412+
<< "saving eval because write effect prevents re-evaluation"
413+
<< "\n";);
399414
saveEvaluation(yieldRegion, effects, /*anyWrite=*/true);
400415
} else if (conflict(effects, assignEffects)) {
401416
// Region that conflicts with the current assignments must be fully
@@ -411,7 +426,8 @@ void Scheduler::saveEvaluationIfConflict(mlir::Region &yieldRegion,
411426
// For example, a WHERE mask might be written by the masked assignment
412427
// evaluations, and it has to be saved in this case:
413428
// where (mask) r = f() ! function f modifies mask
414-
saveEvaluation(yieldRegion, effects, anyWrite(effects));
429+
saveEvaluation(yieldRegion, effects,
430+
anyNonLocalWrite(effects, yieldRegion));
415431
} else {
416432
// Can be executed while doing the assignment.
417433
independentEvaluationEffects.append(effects.begin(), effects.end());
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
// Test local alloca and store inside hlfir.region_assign do not trigger the
2+
// creation of a temporary for the LHS.
3+
4+
// RUN: fir-opt -o - -lower-hlfir-ordered-assignments --debug-only=flang-ordered-assignment -flang-dbg-order-assignment-schedule-only %s 2>&1 | FileCheck %s
5+
// REQUIRES: asserts
6+
7+
func.func @simple(%arg0: !fir.ref<!fir.array<100x10xf32>> , %arg1: !fir.ref<!fir.array<10xi64>> , %arg2: !fir.ref<!fir.array<10xf32>>, %i: i64, %f: f32) {
8+
%c10 = arith.constant 10 : index
9+
%c100 = arith.constant 100 : index
10+
%0 = fir.shape %c100, %c10 : (index, index) -> !fir.shape<2>
11+
%1:2 = hlfir.declare %arg0(%0) {uniq_name = "_QFsimpleEx"} : (!fir.ref<!fir.array<100x10xf32>>, !fir.shape<2>) -> (!fir.ref<!fir.array<100x10xf32>>, !fir.ref<!fir.array<100x10xf32>>)
12+
%2 = fir.shape %c10 : (index) -> !fir.shape<1>
13+
%3:2 = hlfir.declare %arg1(%2) {uniq_name = "y"} : (!fir.ref<!fir.array<10xi64>>, !fir.shape<1>) -> (!fir.ref<!fir.array<10xi64>>, !fir.ref<!fir.array<10xi64>>)
14+
hlfir.region_assign {
15+
hlfir.yield %f : f32
16+
} to {
17+
%local_temp = fir.alloca i64
18+
fir.store %i to %local_temp : !fir.ref<i64>
19+
%icopy = fir.load %local_temp : !fir.ref<i64>
20+
hlfir.elemental_addr %2 : !fir.shape<1> {
21+
^bb0(%arg3: index):
22+
%5 = hlfir.designate %3#0 (%arg3) : (!fir.ref<!fir.array<10xi64>>, index) -> !fir.ref<i64>
23+
%6 = fir.load %5 : !fir.ref<i64>
24+
%7 = hlfir.designate %1#0 (%icopy, %6) : (!fir.ref<!fir.array<100x10xf32>>, i64, i64) -> !fir.ref<f32>
25+
hlfir.yield %7 : !fir.ref<f32>
26+
}
27+
}
28+
return
29+
}
30+
31+
// CHECK: run 1 evaluate: region_assign

flang/test/HLFIR/order_assignments/where-scheduling.f90

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,7 @@ end function f
135135
!CHECK-NEXT: run 2 evaluate: where/region_assign1
136136
!CHECK-LABEL: ------------ scheduling where in _QPonly_once ------------
137137
!CHECK-NEXT: unknown effect: %{{[0-9]+}} = llvm.intr.stacksave : !llvm.ptr
138+
!CHECK-NEXT: saving eval because write effect prevents re-evaluation
138139
!CHECK-NEXT: run 1 save (w): where/mask
139140
!CHECK-NEXT: run 2 evaluate: where/region_assign1
140141
!CHECK-NEXT: run 3 evaluate: where/region_assign2
@@ -180,6 +181,7 @@ end function f
180181
!CHECK-NEXT: conflict: R/W: %{{.*}} = hlfir.declare %{{.*}} {uniq_name = "_QFwhere_construct_unknown_conflictEmask"} : (!fir.box<!fir.array<?x!fir.logical<4>>>, !fir.dscope) -> (!fir.box<!fir.array<?x!fir.logical<4>>>, !fir.box<!fir.array<?x!fir.logical<4>>>) W:<unknown>
181182
!CHECK-NEXT: run 1 save : where/mask
182183
!CHECK-NEXT: unknown effect: %{{.*}} = fir.call @_QPf() fastmath<contract> : () -> f32
184+
!CHECK-NEXT: saving eval because write effect prevents re-evaluation
183185
!CHECK-NEXT: run 2 save (w): where/region_assign1/rhs
184186
!CHECK-NEXT: run 3 evaluate: where/region_assign1
185187
!CHECK-NEXT: ------------ scheduling where in _QPelsewhere_construct_unknown_conflict ------------
@@ -190,5 +192,6 @@ end function f
190192
!CHECK-NEXT: conflict: R/W: %{{.*}} = hlfir.declare %{{.*}} {uniq_name = "_QFelsewhere_construct_unknown_conflictEmask2"} : (!fir.box<!fir.array<?x!fir.logical<4>>>, !fir.dscope) -> (!fir.box<!fir.array<?x!fir.logical<4>>>, !fir.box<!fir.array<?x!fir.logical<4>>>) W:<unknown>
191193
!CHECK-NEXT: run 2 save : where/elsewhere1/mask
192194
!CHECK-NEXT: unknown effect: %{{.*}} = fir.call @_QPf() fastmath<contract> : () -> f32
195+
!CHECK-NEXT: saving eval because write effect prevents re-evaluation
193196
!CHECK-NEXT: run 3 save (w): where/elsewhere1/region_assign1/rhs
194197
!CHECK-NEXT: run 4 evaluate: where/elsewhere1/region_assign1

0 commit comments

Comments
 (0)