Add signed int32 data type support end to end #2272
Conversation
jnie-TT
requested review from
tapspatel,
nsmithtt,
kmabeeTT,
AleksKnezevic,
pilkicTT,
sdjordjevicTT,
svuckovicTT,
mtopalovicTT,
jserbedzijaTT,
azecevicTT and
mrakitaTT
as code owners
jnie-TT
force-pushed
the
jnie/signed_int32_support
branch
from
cad58e4 to
db818b2
Compare
jnie-TT
requested review from
odjuricicTT,
tt-mpantic,
vprajapati-tt and
vcanicTT
as code owners
|
I manually uplifted tt-xla to this branch and now I'm getting during compilation on models which used to compile successfully but fail in runtime due to lack of int32 buffer support. Minimal repro .mlir : https://gist.github.com/sgligorijevicTT/35394562e16a3a0958fbfed4839cc2a7 |
|
@sgligorijevicTT @jnie-TT Seems that we are missing folder on to layout op |
|
@sgligorijevicTT @mtopalovicTT yeah we've seen this issue with the workaround pass, where redundant to_layout ops aren't removed. This change adds a couple of workarounds so it adds up. Issue tracking this here: #2102, I'll hold off from merging this until it's resolved. Marking this blocked by #2102 |
|
Small update - We merged tt-metal uplift to tt-mlir just now (#2256) without waiting for this to be merged (pending on #2102 resolution from @sdjordjevicTT I believe) by instead using a ~localized tt-mlir runtime op hack to convert uint32 to int32 for binary ops. |
jnie-TT
force-pushed
the
jnie/signed_int32_support
branch
from
db818b2 to
3893bb2
Compare
|
Ran https://gist.github.com/sgligorijevicTT/35394562e16a3a0958fbfed4839cc2a7 after rebasing on top of #2292, redundant to_layout op issue is fixed |
Great! 😊 |
### Problem description We don't support signed integers in tt-mlir, and we've been shifting to uint32 when encountering signed/signless integers. With newest metal uplift, binary ops will assert on uint32 (they used to run with uint32 but produce garbage data), therefore we need to add int32 support in our stack. ### What's changed * Added int32 data type end to end * Convert signed and signless integers to `TT_Int32` * Added workarounds for full op and reshape op to typecast to uint32 if input/output datatypes are int32. ### Checklist - [X] New/Existing tests provide coverage for changes - [x] Remove TODO 2272 workaround in ops to force INT32 FYI @brataTT @kmabeeTT please try the uplift on top of my branch and see if the asserts go away. If needed we might want to merge this branch before the 24 hour limit to support the uplift.
Problem description
We don't support signed integers in tt-mlir, and we've been shifting to uint32 when encountering signed/signless integers. With newest metal uplift, binary ops will assert on uint32 (they used to run with uint32 but produce garbage data), therefore we need to add int32 support in our stack.
What's changed
TT_Int32Checklist
FYI @brataTT @kmabeeTT please try the uplift on top of my branch and see if the asserts go away. If needed we might want to merge this branch before the 24 hour limit to support the uplift.