-
Notifications
You must be signed in to change notification settings - Fork 29
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
iree-codegen-iree-comprehensive-bufferize
genereates memref
s with dynamic offset
#847
Comments
iree-codegen-iree-comprehensive-bufferize
genereates memref.subview
with dynamic sizeiree-codegen-iree-comprehensive-bufferize
genereates memref
s with dynamic offset
I've made a change to duplicate Empty tensor ops here: https://github.com/iree-org/iree/blob/05bbcf1385146d075829cd940a52bf06961614d0/compiler/src/iree/compiler/Codegen/Common/IREEComprehensiveBufferizePass.cpp#L177 Since, we are not using the destination passing style as a preprocessing for distribute-using-for-all we had to make that decision. If your pipeline uses convert-to-destination passing style pass, then it shouldn't make a difference. @MaheshRavishankar, do you think the error might be caused by the change mentioned? |
No, I don't think the error is caused by your change. The reason is like @makslevental mentioned because of
It generates I don't know how to get rid of the dynamic offsets, but if we remove this check for now, then we can proceed without problem. |
Maybe you just need to drop those hints using this pass https://github.com/MaheshRavishankar/iree/blob/6950dc0a5a2e6af2d8ba18e323534df72df984ad/compiler/src/iree/compiler/Codegen/LLVMCPU/Passes.cpp#L822 |
I think Stella's optimization PRs from yesterday solved the problem, my local build with new iree bump works. I'll update the branch later after fixing some other conflicts. |
that's like two wrongs make a right lol. cool. |
Hey maybe this is two rights!! |
Fixed by #845 |
#845 is blocked because at that commit of IREE,
iree-codegen-iree-comprehensive-bufferize
generatesmemref
s with dynamic offsets and we get an error here.@MaheshRavishankar any clue what changed recently that might produce this behavior? Possibly @pashu123 might be able to give a hint (I'm seeing recent changes in git-blame...).
cc @jtuyls @yzhang93 @newling @Abhishek-Varma
Failing snippet follows; what stands out to me as odd/a clue is that
hal.interface.binding.subspan
now has amemref.assume_alignment
with a dynamic offset:The text was updated successfully, but these errors were encountered: