Index: flang/lib/Optimizer/HLFIR/Transforms/ScheduleOrderedAssignments.cpp =================================================================== --- flang/lib/Optimizer/HLFIR/Transforms/ScheduleOrderedAssignments.cpp +++ flang/lib/Optimizer/HLFIR/Transforms/ScheduleOrderedAssignments.cpp @@ -81,7 +81,8 @@ /// current assignment: the saved value will be used. void saveEvaluationIfConflict(mlir::Region &yieldRegion, bool leafRegionsMayOnlyRead, - bool yieldIsImplicitRead = true); + bool yieldIsImplicitRead = true, + bool evaluationsMayConflict = false); /// Finish evaluating a group of independent regions. The current independent /// regions effects are added to the "parent" effect list since evaluating the @@ -117,6 +118,10 @@ /// Memory effects of the assignments being lowered. llvm::SmallVector assignEffects; + /// Memory effects of the evaluations implied by the assignments + /// being lowered. They do not include the implicit writes + /// to the LHS of the assignments. + llvm::SmallVector assignEvaluateEffects; /// Memory effects of the unsaved evaluation region that are controlling or /// masking the current assignments. llvm::SmallVector @@ -260,6 +265,20 @@ } } +/// Gather the effects of evaluations implied by the given assignment. +/// These are the effects of operations from LHS and RHS. +static void gatherAssignEvaluationEffects( + hlfir::RegionAssignOp regionAssign, + bool userDefAssignmentMayOnlyWriteToAssignedVariable, + llvm::SmallVectorImpl &assignEffects) { + gatherMemoryEffects(regionAssign.getLhsRegion(), + userDefAssignmentMayOnlyWriteToAssignedVariable, + assignEffects); + gatherMemoryEffects(regionAssign.getRhsRegion(), + userDefAssignmentMayOnlyWriteToAssignedVariable, + assignEffects); +} + //===----------------------------------------------------------------------===// // Scheduling Implementation : finding conflicting memory effects. //===----------------------------------------------------------------------===// @@ -344,11 +363,19 @@ void Scheduler::startSchedulingAssignment(hlfir::RegionAssignOp assign, bool leafRegionsMayOnlyRead) { gatherAssignEffects(assign, leafRegionsMayOnlyRead, assignEffects); + // Unconditionally collect effects of the evaluations of LHS and RHS + // in case they need to be analyzed for any parent that might be + // affected by conflicts of these evaluations. + // This collection migth be skipped, if there are no such parents, + // but for the time being we run it always. + gatherAssignEvaluationEffects(assign, leafRegionsMayOnlyRead, + assignEvaluateEffects); } void Scheduler::saveEvaluationIfConflict(mlir::Region &yieldRegion, bool leafRegionsMayOnlyRead, - bool yieldIsImplicitRead) { + bool yieldIsImplicitRead, + bool evaluationsMayConflict) { // If the region evaluation was previously executed and saved, the saved // value will be used when evaluating the current assignment and this has // no effects in the current assignment evaluation. @@ -377,6 +404,14 @@ // implies that it never conflicted with a prior assignment, so its value // should be the same.) saveEvaluation(yieldRegion, effects, /*anyWrite=*/false); + } else if (evaluationsMayConflict && + conflict(effects, assignEvaluateEffects)) { + // If evaluations of the assignment may conflict with the yield + // evaluations, we have to save yield evaluation. + // 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)); } else { // Can be executed while doing the assignment. independentEvaluationEffects.append(effects.begin(), effects.end()); @@ -444,6 +479,7 @@ schedule.back().memoryEffects.append(assignEffects.begin(), assignEffects.end()); assignEffects.clear(); + assignEvaluateEffects.clear(); parentEvaluationEffects.clear(); independentEvaluationEffects.clear(); savedAnyRegionForCurrentAssignment = false; @@ -533,9 +569,13 @@ scheduler.startIndependentEvaluationGroup(); llvm::SmallVector yieldRegions; parent.getLeafRegions(yieldRegions); + // TODO: is this really limited to WHERE/ELSEWHERE? + bool evaluationsMayConflict = mlir::isa(parent) || + mlir::isa(parent); for (mlir::Region *yieldRegion : yieldRegions) - scheduler.saveEvaluationIfConflict(*yieldRegion, - leafRegionsMayOnlyRead); + scheduler.saveEvaluationIfConflict(*yieldRegion, leafRegionsMayOnlyRead, + /*yieldIsImplicitRead=*/true, + evaluationsMayConflict); scheduler.finishIndependentEvaluationGroup(); } // Look for conflicts between the RHS/LHS evaluation and the assignments. Index: flang/test/HLFIR/order_assignments/forall-scheduling.f90 =================================================================== --- flang/test/HLFIR/order_assignments/forall-scheduling.f90 +++ flang/test/HLFIR/order_assignments/forall-scheduling.f90 @@ -89,6 +89,7 @@ end subroutine !CHECK-LABEL: ------------ scheduling forall in _QPunknown_function_call ------------ !CHECK-NEXT: unknown effect: {{.*}} fir.call @_QPfoo +!CHECK-NEXT: unknown effect: {{.*}} fir.call @_QPfoo !CHECK-NEXT: conflict: R/W: W: of type '!fir.ref>' at index: 0 !CHECK-NEXT: run 1 save : forall/region_assign1/rhs !CHECK-NEXT: run 2 evaluate: forall/region_assign1 @@ -107,6 +108,7 @@ end subroutine !CHECK-LABEL: ------------ scheduling forall in _QPunknown_function_call2 ------------ !CHECK-NEXT: unknown effect: {{.*}} fir.call @_QPfoo2( +!CHECK-NEXT: unknown effect: {{.*}} fir.call @_QPfoo2( !CHECK-NEXT: conflict: R/W: W: of type '!fir.box>' at index: 0 !CHECK-NEXT: run 1 save : forall/region_assign1/rhs !CHECK-NEXT: run 2 evaluate: forall/region_assign1 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 @@ -103,6 +103,30 @@ end subroutine end subroutine +subroutine where_construct_unknown_conflict(x, mask) + real :: x(:) + logical :: mask(:) + interface + real function f() + end function f + end interface + where (mask) x = f() +end subroutine + +subroutine elsewhere_construct_unknown_conflict(x, y, mask1, mask2) + real :: x(:), y(:) + logical :: mask1(:), mask2(:) + interface + real function f() + end function f + end interface + where (mask1) + x = 1.0 + elsewhere (mask2) + y = f() + end where +end subroutine + !CHECK-LABEL: ------------ scheduling where in _QPno_conflict ------------ !CHECK-NEXT: run 1 evaluate: where/region_assign1 !CHECK-LABEL: ------------ scheduling where in _QPfake_conflict ------------ @@ -115,9 +139,12 @@ !CHECK-NEXT: run 2 evaluate: where/region_assign1 !CHECK-NEXT: run 3 evaluate: where/region_assign2 !CHECK-LABEL: ------------ scheduling where in _QPrhs_lhs_conflict ------------ -!CHECK-NEXT: unknown effect: %2 = hlfir.transpose %0#0 : (!fir.box>) -> !hlfir.expr -!CHECK-NEXT: run 1 save (w): where/region_assign1/rhs -!CHECK-NEXT: run 2 evaluate: where/region_assign1 +!CHECK-NEXT: unknown effect: %{{.*}} = hlfir.transpose %{{.*}} : (!fir.box>) -> !hlfir.expr +!CHECK-NEXT: conflict: R/W: %6 = hlfir.designate %{{.*}} (%{{.*}}, %{{.*}}) : (!fir.box>, index, index) -> !fir.ref W: +!CHECK-NEXT: run 1 save : where/mask +!CHECK-NEXT: unknown effect: %{{.*}} = hlfir.transpose %{{.*}} : (!fir.box>) -> !hlfir.expr +!CHECK-NEXT: run 2 save (w): where/region_assign1/rhs +!CHECK-NEXT: run 3 evaluate: where/region_assign1 !CHECK-LABEL: ------------ scheduling where in _QPwhere_construct_no_conflict ------------ !CHECK-NEXT: run 1 evaluate: where/region_assign1 !CHECK-NEXT: run 2 evaluate: where/elsewhere1/region_assign1 @@ -151,3 +178,20 @@ !CHECK-NEXT: conflict: R/W: %7 = fir.load %6 : !fir.llvm_ptr> W:%13 = fir.load %12 : !fir.ref>> !CHECK-NEXT: run 1 save : where/mask !CHECK-NEXT: run 2 evaluate: where/region_assign1 +!CHECK-NEXT: ------------ scheduling where in _QPwhere_construct_unknown_conflict ------------ +!CHECK-NEXT: unknown effect: %{{.*}} = fir.call @_QPf() fastmath : () -> f32 +!CHECK-NEXT: conflict: R/W: %{{.*}} = hlfir.declare %{{.*}} {uniq_name = "_QFwhere_construct_unknown_conflictEmask"} : (!fir.box>>) -> (!fir.box>>, !fir.box>>) W: +!CHECK-NEXT: run 1 save : where/mask +!CHECK-NEXT: unknown effect: %{{.*}} = fir.call @_QPf() fastmath : () -> f32 +!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 ------------ +!CHECK-NEXT: run 1 evaluate: where/region_assign1 +!CHECK-NEXT: unknown effect: %{{.*}} = fir.call @_QPf() fastmath : () -> f32 +!CHECK-NEXT: conflict: R/W: %{{.*}} = hlfir.declare %{{.*}} {uniq_name = "_QFelsewhere_construct_unknown_conflictEmask1"} : (!fir.box>>) -> (!fir.box>>, !fir.box>>) W: +!CHECK-NEXT: run 2 save : where/mask +!CHECK-NEXT: conflict: R/W: %{{.*}} = hlfir.declare %{{.*}} {uniq_name = "_QFelsewhere_construct_unknown_conflictEmask2"} : (!fir.box>>) -> (!fir.box>>, !fir.box>>) W: +!CHECK-NEXT: run 2 save : where/elsewhere1/mask +!CHECK-NEXT: unknown effect: %{{.*}} = fir.call @_QPf() fastmath : () -> f32 +!CHECK-NEXT: run 3 save (w): where/elsewhere1/region_assign1/rhs +!CHECK-NEXT: run 4 evaluate: where/elsewhere1/region_assign1