E2E MaxPool2d Bringup #642
Conversation
LPanosTT
commented
Sep 6, 2024
•
LPanosTT
commented
- Bringup MaxPool2d: TTIR->Pass to add reshapes on input/output -> TTNN -> Flatbuffer -> runtime
- Working for shapes where H, W, and C are tile-dim aligned, and C is a power of 2
LPanosTT
requested review from
jnie-TT,
kmabeeTT,
AleksKnezevic,
sdjordjevicTT,
rpavlovicTT,
nobradovictt,
mrakitaTT,
tapspatel and
nsmithtt
as code owners
| SI32Attr:$input_height, | ||
| SI32Attr:$input_width, | ||
| SI32Attr:$kernel_height, | ||
| SI32Attr:$kernel_width, |
There was a problem hiding this comment.
Same as conv op, we should remove / infer these 4 attributes.
There was a problem hiding this comment.
Btw, all of these can be changed from:
SI32Attr:$stride_height,
To:
"int64_t":$stride_height,
And then tablgen will generate the getters and setters to work with ints instead of IntAttr's, which is easier to work with.
There was a problem hiding this comment.
@nsmithtt Since pooling doesn't have weights there is nothing to infer the kernel size from. Furthermore the TTNN maxpool2d op requires the input be (1, 1, N*H*W, C), so the height width and batch size information is lost by the time we get to runtime, unless they are stored as attributes.
There was a problem hiding this comment.
TTNN maxpool2d op requires the input be (1, 1, NHW, C)
Yes the op should be defined this way in TTNN dialect, but in TTIR we shouldn't define it this way. Agree about the kernel size, but input_height/width should be removed.
LPanosTT
force-pushed
the
lpanos/maxpool2d_bringup
branch
from
16afb37
to
bd8285d
Compare
|
I've made the pass which inserts reshapes onto MaxPool2d generalized as I hope to re use the code to insert reshapes on Conv2d as well. |
LPanosTT
force-pushed
the
lpanos/maxpool2d_bringup
branch
2 times, most recently
from
328f709
to
11a40cd
Compare
| SI32Attr:$input_height, | ||
| SI32Attr:$input_width, | ||
| SI32Attr:$kernel_height, | ||
| SI32Attr:$kernel_width, |
There was a problem hiding this comment.
TTNN maxpool2d op requires the input be (1, 1, NHW, C)
Yes the op should be defined this way in TTNN dialect, but in TTIR we shouldn't define it this way. Agree about the kernel size, but input_height/width should be removed.
| return failure(); | ||
| } | ||
|
|
||
| if (!llvm::isa<MaxPool2dOp>(op)) { |
LPanosTT
force-pushed
the
lpanos/maxpool2d_bringup
branch
2 times, most recently
from
fef525d
to
ef5b219
Compare
LPanosTT
force-pushed
the
lpanos/maxpool2d_bringup
branch
from
a67d176
to
04b5681
Compare
Add reshape insertion pass for maxpool2d Clenup maxpool fix shpes Remove MLIR module dump in TTIRToTTNNPass.cpp, add more checks to verify() methods for TTIR/TTNN MaxPool2dOp Fix verify condition for maxpool2d Generalize reshape-inserting pass with template fix build warnings remove use of auto keyword Rebase fixes Make input height/width optional in TTIR as per nicks suggestion Remove stray prints add emit C pattern for maxpool2d fix typo in error message Remove clang-tidy from cmake file fix
LPanosTT
force-pushed
the
lpanos/maxpool2d_bringup
branch
from
c13b1f4
to
1e20f6b
Compare
Add E2E maxpool2d op Add reshape insertion pass for maxpool2d
