[Optimizer] Greedy solution for join nodes in L1 Interleaved policy #1162
Conversation
|
PR title does not correspond to changes made. |
fbajraktariTT
force-pushed
the
v2_interleaved_policy
branch
4 times, most recently
from
5a24709 to
daed82a
Compare
fbajraktariTT
force-pushed
the
v2_interleaved_policy
branch
2 times, most recently
from
e6f54a9 to
4d1c249
Compare
fbajraktariTT
force-pushed
the
v2_interleaved_policy
branch
3 times, most recently
from
2036257 to
0cb6250
Compare
fbajraktariTT
requested review from
sdjordjevicTT,
svuckovicTT,
rpavlovicTT,
jserbedzijaTT,
jnie-TT,
nsmithtt and
mrakitaTT
as code owners
fbajraktariTT
force-pushed
the
v2_interleaved_policy
branch
from
0cb6250 to
485bfa0
Compare
There was a problem hiding this comment.
Left a few comments, mostly about readability. As @nobradovic mentioned, the PR title should try to explain what the change is in a way so that people not familiar with the work can uderstand. Maybe something like: "[Optimizer] Solve join nodes in L1 interleaved policy"
| } | ||
| l1ChainConfigs->back().build(); | ||
| l1ChainConfigs->back().resolve(); | ||
| std::unordered_set<Edge> memReconfigEdges; |
There was a problem hiding this comment.
Do we see a scenario in future where in L1Interleaved policy we will need reconfig edge?
There was a problem hiding this comment.
It depends on the op and there may be a scenario where we need reconfigEdges. For example, if we greedily decide that the output of an op's operand should be in DRAM and there is enough available L1 memory this operand then we can insert a ToLayout op on that edge so we can gain on the performance. I assume this may be true depending on the op.
There was a problem hiding this comment.
Yes but in this scenario it would not be coming from policy like for example in DFSharding, but only as an external override, because if policy found out that reconfigEdge is needed it could directly change OP output instead. Just thinking if it should be completely removed from L1Interleaved policy as it is currently only used as an empty dummy struct to satisfy API.
fbajraktariTT
changed the title
fbajraktariTT
force-pushed
the
v2_interleaved_policy
branch
from
4f43072 to
c9adb16
Compare
|
Change has no description. |
fbajraktariTT
force-pushed
the
v2_interleaved_policy
branch
3 times, most recently
from
5eed6db to
d559286
Compare
fbajraktariTT
force-pushed
the
v2_interleaved_policy
branch
from
5b7f37d to
74ce175
Compare
fbajraktariTT
force-pushed
the
v2_interleaved_policy
branch
from
74ce175 to
062a978
Compare
This PR introduces new MemoryLayoutAnalysis policy as an alternative to the DFSharding policy with the goal of greedily solving join nodes. This means that if an op is a join node (node with multiple operands, e.g. Add, Matmul,...) we should consider placing multiple op's operands in the L1 memory and limit ourselves to only one operand being placed in L1 memory as is it the case with DFSharding. For all the combination of operands, we pick the one that maximizes L1 usage (by maximising L1 usage we try to have as few DRAM spills as possible) and minimizes required L1 usage (minimal amount of L1 memory neccesseary in order to reach desired memory configuration). Furthermore, it should be emphasized that this may not lead to globally optimal solution because we pick the optimal config at moment of analysing the op.