Skip to content

Commit 7095a86

Browse files
committed
[flang][hlfir] Fixed where/elsewhere mask saving in case of conflicts.
The assignments inside where/elsewhere may affect variables participating in the mask expression, but execution of the assignments must not affect the established control mask(s) (F'18 10.2.3.2 p. 13). The following example must print all 42's: ``` program test integer c(3) logical :: mask(3) = .true. where (mask) c = f() end where print *, c contains integer function f() mask = .false. f = 42 end function f end program test ``` Reviewed By: tblah Differential Revision: https://reviews.llvm.org/D156959
1 parent 4400018 commit 7095a86

File tree

3 files changed

+93
-7
lines changed

3 files changed

+93
-7
lines changed

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

Lines changed: 44 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,8 @@ class Scheduler {
8181
/// current assignment: the saved value will be used.
8282
void saveEvaluationIfConflict(mlir::Region &yieldRegion,
8383
bool leafRegionsMayOnlyRead,
84-
bool yieldIsImplicitRead = true);
84+
bool yieldIsImplicitRead = true,
85+
bool evaluationsMayConflict = false);
8586

8687
/// Finish evaluating a group of independent regions. The current independent
8788
/// regions effects are added to the "parent" effect list since evaluating the
@@ -117,6 +118,10 @@ class Scheduler {
117118

118119
/// Memory effects of the assignments being lowered.
119120
llvm::SmallVector<mlir::MemoryEffects::EffectInstance> assignEffects;
121+
/// Memory effects of the evaluations implied by the assignments
122+
/// being lowered. They do not include the implicit writes
123+
/// to the LHS of the assignments.
124+
llvm::SmallVector<mlir::MemoryEffects::EffectInstance> assignEvaluateEffects;
120125
/// Memory effects of the unsaved evaluation region that are controlling or
121126
/// masking the current assignments.
122127
llvm::SmallVector<mlir::MemoryEffects::EffectInstance>
@@ -260,6 +265,20 @@ static void gatherAssignEffects(
260265
}
261266
}
262267

268+
/// Gather the effects of evaluations implied by the given assignment.
269+
/// These are the effects of operations from LHS and RHS.
270+
static void gatherAssignEvaluationEffects(
271+
hlfir::RegionAssignOp regionAssign,
272+
bool userDefAssignmentMayOnlyWriteToAssignedVariable,
273+
llvm::SmallVectorImpl<mlir::MemoryEffects::EffectInstance> &assignEffects) {
274+
gatherMemoryEffects(regionAssign.getLhsRegion(),
275+
userDefAssignmentMayOnlyWriteToAssignedVariable,
276+
assignEffects);
277+
gatherMemoryEffects(regionAssign.getRhsRegion(),
278+
userDefAssignmentMayOnlyWriteToAssignedVariable,
279+
assignEffects);
280+
}
281+
263282
//===----------------------------------------------------------------------===//
264283
// Scheduling Implementation : finding conflicting memory effects.
265284
//===----------------------------------------------------------------------===//
@@ -344,11 +363,19 @@ anyWrite(llvm::ArrayRef<mlir::MemoryEffects::EffectInstance> effects) {
344363
void Scheduler::startSchedulingAssignment(hlfir::RegionAssignOp assign,
345364
bool leafRegionsMayOnlyRead) {
346365
gatherAssignEffects(assign, leafRegionsMayOnlyRead, assignEffects);
366+
// Unconditionally collect effects of the evaluations of LHS and RHS
367+
// in case they need to be analyzed for any parent that might be
368+
// affected by conflicts of these evaluations.
369+
// This collection migth be skipped, if there are no such parents,
370+
// but for the time being we run it always.
371+
gatherAssignEvaluationEffects(assign, leafRegionsMayOnlyRead,
372+
assignEvaluateEffects);
347373
}
348374

349375
void Scheduler::saveEvaluationIfConflict(mlir::Region &yieldRegion,
350376
bool leafRegionsMayOnlyRead,
351-
bool yieldIsImplicitRead) {
377+
bool yieldIsImplicitRead,
378+
bool evaluationsMayConflict) {
352379
// If the region evaluation was previously executed and saved, the saved
353380
// value will be used when evaluating the current assignment and this has
354381
// no effects in the current assignment evaluation.
@@ -377,6 +404,14 @@ void Scheduler::saveEvaluationIfConflict(mlir::Region &yieldRegion,
377404
// implies that it never conflicted with a prior assignment, so its value
378405
// should be the same.)
379406
saveEvaluation(yieldRegion, effects, /*anyWrite=*/false);
407+
} else if (evaluationsMayConflict &&
408+
conflict(effects, assignEvaluateEffects)) {
409+
// If evaluations of the assignment may conflict with the yield
410+
// evaluations, we have to save yield evaluation.
411+
// For example, a WHERE mask might be written by the masked assignment
412+
// evaluations, and it has to be saved in this case:
413+
// where (mask) r = f() ! function f modifies mask
414+
saveEvaluation(yieldRegion, effects, anyWrite(effects));
380415
} else {
381416
// Can be executed while doing the assignment.
382417
independentEvaluationEffects.append(effects.begin(), effects.end());
@@ -444,6 +479,7 @@ void Scheduler::finishSchedulingAssignment(hlfir::RegionAssignOp assign) {
444479
schedule.back().memoryEffects.append(assignEffects.begin(),
445480
assignEffects.end());
446481
assignEffects.clear();
482+
assignEvaluateEffects.clear();
447483
parentEvaluationEffects.clear();
448484
independentEvaluationEffects.clear();
449485
savedAnyRegionForCurrentAssignment = false;
@@ -533,9 +569,13 @@ hlfir::buildEvaluationSchedule(hlfir::OrderedAssignmentTreeOpInterface root,
533569
scheduler.startIndependentEvaluationGroup();
534570
llvm::SmallVector<mlir::Region *, 4> yieldRegions;
535571
parent.getLeafRegions(yieldRegions);
572+
// TODO: is this really limited to WHERE/ELSEWHERE?
573+
bool evaluationsMayConflict = mlir::isa<hlfir::WhereOp>(parent) ||
574+
mlir::isa<hlfir::ElseWhereOp>(parent);
536575
for (mlir::Region *yieldRegion : yieldRegions)
537-
scheduler.saveEvaluationIfConflict(*yieldRegion,
538-
leafRegionsMayOnlyRead);
576+
scheduler.saveEvaluationIfConflict(*yieldRegion, leafRegionsMayOnlyRead,
577+
/*yieldIsImplicitRead=*/true,
578+
evaluationsMayConflict);
539579
scheduler.finishIndependentEvaluationGroup();
540580
}
541581
// Look for conflicts between the RHS/LHS evaluation and the assignments.

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ pure real function foo(x, i)
8989
end subroutine
9090
!CHECK-LABEL: ------------ scheduling forall in _QPunknown_function_call ------------
9191
!CHECK-NEXT: unknown effect: {{.*}} fir.call @_QPfoo
92+
!CHECK-NEXT: unknown effect: {{.*}} fir.call @_QPfoo
9293
!CHECK-NEXT: conflict: R/W: <unknown> W:<block argument> of type '!fir.ref<!fir.array<10xf32>>' at index: 0
9394
!CHECK-NEXT: run 1 save : forall/region_assign1/rhs
9495
!CHECK-NEXT: run 2 evaluate: forall/region_assign1
@@ -107,6 +108,7 @@ pure real function foo2(i)
107108
end subroutine
108109
!CHECK-LABEL: ------------ scheduling forall in _QPunknown_function_call2 ------------
109110
!CHECK-NEXT: unknown effect: {{.*}} fir.call @_QPfoo2(
111+
!CHECK-NEXT: unknown effect: {{.*}} fir.call @_QPfoo2(
110112
!CHECK-NEXT: conflict: R/W: <unknown> W:<block argument> of type '!fir.box<!fir.array<?xf32>>' at index: 0
111113
!CHECK-NEXT: run 1 save : forall/region_assign1/rhs
112114
!CHECK-NEXT: run 2 evaluate: forall/region_assign1

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

Lines changed: 47 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,30 @@ subroutine internal
103103
end subroutine
104104
end subroutine
105105

106+
subroutine where_construct_unknown_conflict(x, mask)
107+
real :: x(:)
108+
logical :: mask(:)
109+
interface
110+
real function f()
111+
end function f
112+
end interface
113+
where (mask) x = f()
114+
end subroutine
115+
116+
subroutine elsewhere_construct_unknown_conflict(x, y, mask1, mask2)
117+
real :: x(:), y(:)
118+
logical :: mask1(:), mask2(:)
119+
interface
120+
real function f()
121+
end function f
122+
end interface
123+
where (mask1)
124+
x = 1.0
125+
elsewhere (mask2)
126+
y = f()
127+
end where
128+
end subroutine
129+
106130
!CHECK-LABEL: ------------ scheduling where in _QPno_conflict ------------
107131
!CHECK-NEXT: run 1 evaluate: where/region_assign1
108132
!CHECK-LABEL: ------------ scheduling where in _QPfake_conflict ------------
@@ -115,9 +139,12 @@ subroutine internal
115139
!CHECK-NEXT: run 2 evaluate: where/region_assign1
116140
!CHECK-NEXT: run 3 evaluate: where/region_assign2
117141
!CHECK-LABEL: ------------ scheduling where in _QPrhs_lhs_conflict ------------
118-
!CHECK-NEXT: unknown effect: %2 = hlfir.transpose %0#0 : (!fir.box<!fir.array<?x?xf32>>) -> !hlfir.expr<?x?xf32>
119-
!CHECK-NEXT: run 1 save (w): where/region_assign1/rhs
120-
!CHECK-NEXT: run 2 evaluate: where/region_assign1
142+
!CHECK-NEXT: unknown effect: %{{.*}} = hlfir.transpose %{{.*}} : (!fir.box<!fir.array<?x?xf32>>) -> !hlfir.expr<?x?xf32>
143+
!CHECK-NEXT: conflict: R/W: %6 = hlfir.designate %{{.*}} (%{{.*}}, %{{.*}}) : (!fir.box<!fir.array<?x?xf32>>, index, index) -> !fir.ref<f32> W:<unknown>
144+
!CHECK-NEXT: run 1 save : where/mask
145+
!CHECK-NEXT: unknown effect: %{{.*}} = hlfir.transpose %{{.*}} : (!fir.box<!fir.array<?x?xf32>>) -> !hlfir.expr<?x?xf32>
146+
!CHECK-NEXT: run 2 save (w): where/region_assign1/rhs
147+
!CHECK-NEXT: run 3 evaluate: where/region_assign1
121148
!CHECK-LABEL: ------------ scheduling where in _QPwhere_construct_no_conflict ------------
122149
!CHECK-NEXT: run 1 evaluate: where/region_assign1
123150
!CHECK-NEXT: run 2 evaluate: where/elsewhere1/region_assign1
@@ -151,3 +178,20 @@ subroutine internal
151178
!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>>>
152179
!CHECK-NEXT: run 1 save : where/mask
153180
!CHECK-NEXT: run 2 evaluate: where/region_assign1
181+
!CHECK-NEXT: ------------ scheduling where in _QPwhere_construct_unknown_conflict ------------
182+
!CHECK-NEXT: unknown effect: %{{.*}} = fir.call @_QPf() fastmath<contract> : () -> f32
183+
!CHECK-NEXT: conflict: R/W: %{{.*}} = hlfir.declare %{{.*}} {uniq_name = "_QFwhere_construct_unknown_conflictEmask"} : (!fir.box<!fir.array<?x!fir.logical<4>>>) -> (!fir.box<!fir.array<?x!fir.logical<4>>>, !fir.box<!fir.array<?x!fir.logical<4>>>) W:<unknown>
184+
!CHECK-NEXT: run 1 save : where/mask
185+
!CHECK-NEXT: unknown effect: %{{.*}} = fir.call @_QPf() fastmath<contract> : () -> f32
186+
!CHECK-NEXT: run 2 save (w): where/region_assign1/rhs
187+
!CHECK-NEXT: run 3 evaluate: where/region_assign1
188+
!CHECK-NEXT: ------------ scheduling where in _QPelsewhere_construct_unknown_conflict ------------
189+
!CHECK-NEXT: run 1 evaluate: where/region_assign1
190+
!CHECK-NEXT: unknown effect: %{{.*}} = fir.call @_QPf() fastmath<contract> : () -> f32
191+
!CHECK-NEXT: conflict: R/W: %{{.*}} = hlfir.declare %{{.*}} {uniq_name = "_QFelsewhere_construct_unknown_conflictEmask1"} : (!fir.box<!fir.array<?x!fir.logical<4>>>) -> (!fir.box<!fir.array<?x!fir.logical<4>>>, !fir.box<!fir.array<?x!fir.logical<4>>>) W:<unknown>
192+
!CHECK-NEXT: run 2 save : where/mask
193+
!CHECK-NEXT: conflict: R/W: %{{.*}} = hlfir.declare %{{.*}} {uniq_name = "_QFelsewhere_construct_unknown_conflictEmask2"} : (!fir.box<!fir.array<?x!fir.logical<4>>>) -> (!fir.box<!fir.array<?x!fir.logical<4>>>, !fir.box<!fir.array<?x!fir.logical<4>>>) W:<unknown>
194+
!CHECK-NEXT: run 2 save : where/elsewhere1/mask
195+
!CHECK-NEXT: unknown effect: %{{.*}} = fir.call @_QPf() fastmath<contract> : () -> f32
196+
!CHECK-NEXT: run 3 save (w): where/elsewhere1/region_assign1/rhs
197+
!CHECK-NEXT: run 4 evaluate: where/elsewhere1/region_assign1

0 commit comments

Comments
 (0)