Integrate llvm-project@ad947503831a [ours a60d6603fbf8] #23130
Conversation
Integrate torch-mlir@ac33bab4f3 to pick up their integrate Reverts carried forward: * Local revert of llvm/llvm-project#169614 due to llvm/llvm-project#172932. New reverts: * Revert https://github.com/llvm/llvm-project/174945 until torch-mlir adapts to the change Local changes: * https://github.com/llvm/llvm-project/172595 disables a load->extract folder on to_buffer ops when the buffer isn't declared read_only. IREE would only mark to_buffer operations on tensor constants as read_only when the constant only has one use, for reasons I'm not clear on and that I couldn't find. That restriction was leading to some of the winnograd tests failing because they couldn't bufferize. So, we drop the one use restriction in order to make everything pass again. (Claude was used to find this one.)
krzysz00
requested review from
MaheshRavishankar,
Max191,
hanhanW and
qedawkins
as code owners
Cant find this PR. Is this the right PR? |
|
Why is there a torch-mlir bump here as well? |
|
@MaheshRavishankar I formed the URL wrong, the PR causing the local change is llvm/llvm-project#172595 There's a torch-mlir bump because I did one just in case they'd already picked up the fix for the commit I ended up reverting, and I noticed that one of their recent commits was an integrate, so I figured I should keep it |
|
Given the dispatch count regressions, I suspect there's something interesting going on where we aren't setting the read_only flag as aggressively as we should've been |
…izes constants fixes our dispatch count problem
|
Ok no there's something up here and I'll need to dig into it. It we're lucky, torch-nlir will let me drop an integrate in to meantime |
|
@krzysz00 Maybe roll up torch-mlir in a separate smaller integrate while we fix our own issues? |
|
I could probably do a smaller integrate if I specifically target the revert of the commit causing the RDNA3 issue with yesterday's abandoned integrate |
krzysz00
changed the title
|
Update: those dispatch count regressions are actually in the Torch integrate |
|
I didn't know about this integrate yesterday. I'm restarting the failed jobs after rebooting the machines. I believe they should pass. |
|
The only tests that failed are the ones that were disabled in #23190 . As this branch was targetting an older base reference, then those tests are not disabled in this CI. To disable them in this CI, one would need to update the PR. Since we also have #23199 and a discussion on whether they should be skipped #23190 (comment) , I'll wait until that discussion happens. I'll prepare a list of reasons of failure for these tests. |
|
I believe this is good to merge since the failing tests predate this integrate. |
Reverts carried forward: * Local revert of llvm/llvm-project#169614 due to llvm/llvm-project#172932. New reverts: * Revert llvm/llvm-project#174945 until torch-mlir adapts to the change Local changes: * llvm/llvm-project#172595 disables a load->extract folder on to_buffer ops when the buffer isn't declared read_only. IREE would only mark to_buffer operations on tensor constants as read_only when the constant only has one use, for reasons I'm not clear on and that I couldn't find. That restriction was leading to some of the winnograd tests failing because they couldn't bufferize. So, we drop the one use restriction in order to make everything pass again. (Claude was used to find this one.) Signed-off-by: Keshav Vinayak Jha <[email protected]>
Reverts carried forward:
New reverts:
RegionSuccessordesign / API llvm/llvm-project#174945 until torch-mlir adapts to the changeLocal changes: