[AMD][TDM] Verify descriptor and smem encodings#10372
Conversation
This PR adds check that tensor descriptor and smem encodings are equal.
| if (failed(verifyResult)) | ||
| return verifyResult; | ||
|
|
||
| if (tensorDescTy.getSharedLayout() && |
There was a problem hiding this comment.
Nit: Do we need explicit checking this first? Won't it automatically be checked at L653 if null attribute?
There was a problem hiding this comment.
This additional condition skips the check if tensor descriptor do not have shared layout. I've added it because we have number of lit tests that do not care about both encodings and I did not want to change them. They have something like this: amdg.async_tdm_copy_local_to_global ...: !ttg.memdesc<128x64xf16, #shared> -> !tt.tensordesc<128x64xf16>.
I think this combination(one layout exists, second does not) is also illegal, but it will not go unnoticed in real applications. There are checks in other places: in gluon layout must be one of few particular classes, llvm lowering will simply crash.
My intent is to avoid confusing situations with two different encodings, because I've spend an hour debugging non-existent bug because of error in lit test yesterday.
There was a problem hiding this comment.
I did not make this check strict just because I did not want to make lit tests too verbose, but if you think it worth it I can change this.
alefimov-amd
force-pushed
the
fix_layout_tdm_copy
branch
from
fa5dd9d to
dba132a
Compare
alefimov-amd
force-pushed
the
fix_layout_tdm_copy
branch
from
dba132a to
e782a85
Compare
|
I've made this check strict and adjusted lit tests, but removed checks from scatter/gather from this PR. I am not sure about some details, need to discuss operation design with the author first. |
alefimov-amd
force-pushed
the
fix_layout_tdm_copy
branch
from
9586233 to
0b6562e
Compare
alefimov-amd
force-pushed
the
fix_layout_tdm_copy
branch
from
0b6562e to
2f365c4
Compare
|
@antiagainst @zhanglx13 @ptillet |
3 participants
This PR adds check that tensor descriptor and smem encodings are equal.