This is an archive of the discontinued LLVM Phabricator instance.

[flang][hlfir] Implement the scheduling part of hlfir.forall codegen
ClosedPublic

Authored by jeanPerier on May 12 2023, 9:22 AM.

Details

Summary

The lowering of hlfir.forall to loops (and later hlfir.where) requires
doing a data dependency analysis to avoid creating temporary storage for
every control/mask/rhs/lhs expressions.

The added code implements a data dependency analysis for the hlfir
ordered assignment trees (it is not specific to Forall since these nodes
includes Where, user defined assignments, and assignment to vector
subscripted entities, but the added code is only plugged and tested
with hlfir.forall in this patch).

This data dependency analysis returns a "schedule", which is a list of
runs containing actions. Each runs will result in a single loop nest
evaluating all its action "at the same time" inside the loop body.
Actions may either evaluating an assignment, or saving some expression
evaluation (the value yielded inside the ordered assignment hlfir operations)
in a temporary storage before doing the assignment that requires this
expression value but may "conflict" with it.

A "conflict" is a read in an expression E to a variable that is, or may
be (analysis is conservative), written by an assignment that depends on
E.

The analysis is based on MLIR SideEffectInterface and fir AliasAnalysis
which makes it generic.

For now, the codegen that will apply the schedule and rewrite the
hlfir.forall into a set of loops is not implemented, but the scheduling
is tested on its own (from Fortran, because it allows testing many cases
in very readable fashions).

The current scheduling has limitations, for instance
"forall(i=1, 10) x(i)=2*x(i)" does not require saving the RHS values for
all "i" before doing the assignments since the RHS does not depend
on values computed during previous iterations. Any user call will also
trigger a conservative assumption that there is a conflict. Finally,
a lot of operations are missing memory effect interfaces (especially
in HLFIR). This patch adds a few so that it can be tested, but more
will be added in later patches.

Diff Detail

Event Timeline

jeanPerier created this revision.May 12 2023, 9:22 AM
jeanPerier requested review of this revision.May 12 2023, 9:22 AM
jeanPerier added inline comments.May 12 2023, 9:25 AM
flang/include/flang/Optimizer/Dialect/FIROps.td
261

Note: the read effect is already on the operand "[MemRead]>:$memref", adding [MemoryEffects<[MemRead]>] actually adds an unknown read effect on top of that. Same for StoreOp.

Posting intermediate minor remarks. Still looking through the code...

flang/include/flang/Optimizer/HLFIR/HLFIROps.td
173

Is it true for dynamically sized derived types that the DesignateOp does not have memory effects? Does the call generated during FieldIndexOp code-gen access any memory?

flang/lib/Optimizer/HLFIR/Transforms/LowerHLFIROrderedAssignments.cpp
38

The description string looks off.

42

typo

50

typo

flang/lib/Optimizer/HLFIR/Transforms/ScheduleOrderAssignments.cpp
1 ↗(On Diff #521685)

nit: maybe shrink the comment to fit the line limit.

50 ↗(On Diff #521685)
66 ↗(On Diff #521685)
70 ↗(On Diff #521685)

It is not doing anything, is this WIP?

77 ↗(On Diff #521685)
98 ↗(On Diff #521685)
104 ↗(On Diff #521685)

typo: is evaluating is

111 ↗(On Diff #521685)
199 ↗(On Diff #521685)
flang/lib/Optimizer/HLFIR/Transforms/ScheduleOrderAssignments.h
1 ↗(On Diff #521685)

nit: -*- C++ -*- missing.

1 ↗(On Diff #521685)
14 ↗(On Diff #521685)
15 ↗(On Diff #521685)
32 ↗(On Diff #521685)

typo

68 ↗(On Diff #521685)

typo

76 ↗(On Diff #521685)

nit: line alignment

84 ↗(On Diff #521685)

typo

90 ↗(On Diff #521685)

typo

97 ↗(On Diff #521685)

nit

Looks great! Thank you, Jean!

flang/lib/Optimizer/HLFIR/Transforms/ScheduleOrderAssignments.cpp
257 ↗(On Diff #521685)
345 ↗(On Diff #521685)
416 ↗(On Diff #521685)
442 ↗(On Diff #521685)
480 ↗(On Diff #521685)
528 ↗(On Diff #521685)
This revision is now accepted and ready to land.May 15 2023, 2:16 PM
jeanPerier marked 28 inline comments as done.May 16 2023, 1:35 AM

Thanks a lot for the review and comments!

flang/include/flang/Optimizer/HLFIR/HLFIROps.td
173

I do not have a good answer here because it is still not 100% clear to me how this will be dealt with. The runtime approach lays out dynamically sized component in "automatic components" that are special allocatable components.

So if this is not abstracted in FIR/HLFIR, the memory effects will happen via a fir.load of the addresses component fir.box (like for allocatable components).

The FieldIndexOp codegen was forcasting an approach where the data would be directly laid out in the structure with no descriptor indirection.

Right now, it is safe to say it has no memory effects (it may however crash if the incoming fir.box is an absent optional, that is why I did not add Pure: it is not speculatable with optional input for now).

We will likely need to revisit this depending on how PDTs are implemented (we can implement the MemoryEffectInterface to have finer control about when an operation has side effects and when it does not based on the arguments).

flang/lib/Optimizer/HLFIR/Transforms/LowerHLFIROrderedAssignments.cpp
38

Thanks

flang/lib/Optimizer/HLFIR/Transforms/ScheduleOrderAssignments.cpp
70 ↗(On Diff #521685)

There is no much to do here, I added it because there is something to do after visiting the group, and I found it weird to not mark the beginning.

I added an assert to make it useful.

flang/lib/Optimizer/HLFIR/Transforms/ScheduleOrderAssignments.h
1 ↗(On Diff #521685)

Actually, I need to rename the files instead to be consistent. Thanks for the catch.

jeanPerier marked 3 inline comments as done.
  • Rename ScheduleOrderAssignment to ScheduleOrderedAssignment to be consistent with the operation and other file names.
  • Fix numerous typos in comments, thanks Slava.
  • Add an assert in startIndependentEvaluationGroup.
  • Rebase.
clementval accepted this revision.May 16 2023, 8:45 AM

Still LGTM. Precommit CI is broken because of a cmake version requirement bump.

vzakhari accepted this revision.May 16 2023, 9:03 AM

Thank you for the update, Jean!

flang/include/flang/Optimizer/HLFIR/HLFIROps.td
173

Thank you for the explanation! I am okay with having NoMemoryEffect for the time being.

tblah added a comment.May 16 2023, 2:06 PM

This was part 1 of the review (just typos). I'll finish tomorrow.

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

typo

235

typo

tblah accepted this revision.May 17 2023, 2:54 AM

LGTM. Nice work on this!

jeanPerier marked 2 inline comments as done.May 17 2023, 5:25 AM

Thanks for the review. Typos fixed in the merge commit.

This revision was landed with ongoing or failed builds.May 17 2023, 5:27 AM
This revision was automatically updated to reflect the committed changes.