L1 interleaved policy #1117
L1 interleaved policy #1117
Conversation
fbajraktariTT
force-pushed
the
l1_interleaved_policy
branch
2 times, most recently
from
5c18e7d to
608a531
Compare
fbajraktariTT
force-pushed
the
l1_interleaved_policy
branch
from
93b8890 to
0194e80
Compare
fbajraktariTT
requested review from
sdjordjevicTT,
svuckovicTT,
mtopalovicTT,
rpavlovicTT,
nobradovictt,
jserbedzijaTT,
nsmithtt and
mrakitaTT
as code owners
fbajraktariTT
force-pushed
the
l1_interleaved_policy
branch
from
0194e80 to
e5bd01d
Compare
|
|
||
| // Check if currentOp is valid l1 interleaved op. | ||
| // | ||
| if (legalLayouts.lookup(currentOp).size() > 0) { |
There was a problem hiding this comment.
You don't even check if adjacent OPs in chain are connected?
There was a problem hiding this comment.
That is true, because we decided that is ok for V0 to put all ops with possible l1 interleaved layout into a single L1ChainConfig. Also, I added a comment that it will be fixed in V1.
| // Resolve l1 chain configs. | ||
| // | ||
| for (auto &l1ChainConfig : *l1ChainConfigs) { | ||
| l1ChainConfig.build(); |
There was a problem hiding this comment.
Shouldn't build happen when chain is fully populated. Using it here like this is pointless?
There was a problem hiding this comment.
Also here, for V0 we only have one L1ChainConfig and therefore build and resolve should happen at the same time. This for loop is only a placeholder for V1 and future versions of this policy.
|
|
||
| // Check if currentOp is valid l1 interleaved op. | ||
| // | ||
| if (legalLayouts.lookup(currentOp).size() > 0) { |
There was a problem hiding this comment.
What if we don't have a valid l1 interleaved config?
There was a problem hiding this comment.
In that case, legalLayouts for that specific op will not be overridden by l1 interleaved layout and therefore keep the legalLayouts from LegalGridAnalysis.
| // This is V0 implementation of L1 interleaved policy. In the current | ||
| // implementation we have a single L1ChainCofig per FuncOp. This implies | ||
| // that in case of DRAM spil we will have a disconnected chain of L1 ops. | ||
| // This will be fixed in V1. |
There was a problem hiding this comment.
I don't like this proposed staging, disconnected chain is not a chain and this policy is just random then.
There was a problem hiding this comment.
This has already been fixed in V1 version of this policy (#1132).
|
Hey @fbajraktariTT, can you please add a description on what the purpose of this PR is? Additionally, can you attach tracking issue? It's a little hidden, but after you create a PR, you can click on the development section (right-side bar) to attach issues. |
fbajraktariTT
force-pushed
the
l1_interleaved_policy
branch
from
c8010b7 to
e303453
Compare
|
Looks good to me. Let's add two tests. One with a simple fork join and one with a huge tensor size that won't fit into L1. |
fbajraktariTT
force-pushed
the
l1_interleaved_policy
branch
from
0ba6873 to
902a363
Compare
Refactor sharding into mem layout analysis. All l1 interleaved policy for mem layout analysis Add mnist test Add option in both optimizer pass and ttnn-ttir-backedn-pipeline to specify memory layout analysis policy type
…ass and TTIRToTTNNBackendPipeline
fbajraktariTT
force-pushed
the
l1_interleaved_policy
branch
from
f46bee4 to
11f0ea7
Compare
The goal of this PR is to add new MemoryLayoutAnalysis policy. Apart from DFShardingPolicy, the idea is to implement L1Interleaved policy that will try to build l1 interleaved chains instead of l1 sharding chains.