This is an archive of the discontinued LLVM Phabricator instance.

[mlir][sparse] Case coverage fix no errorhandling
ClosedPublic

Authored by NathanielMcVicar on Oct 5 2022, 12:32 PM.

Details

Summary

Restores the fix from D134925 for MSVC without breaking cpu runner.

Diff Detail

Event Timeline

Herald added a project: Restricted Project. · View Herald TranscriptOct 5 2022, 12:32 PM
NathanielMcVicar requested review of this revision.Oct 5 2022, 12:32 PM
This revision was not accepted when it landed; it landed in state Needs Review.Oct 5 2022, 3:35 PM
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
aartbik added inline comments.Oct 5 2022, 3:41 PM
mlir/lib/ExecutionEngine/SparseTensor/File.cpp
111

I believe Wren was already taking care of that in https://reviews.llvm.org/D135186
That would also roll back the bazel change, which is pulling in too much deps right now

Can I kindly request to wait for reviews going forward on this?

mlir/lib/ExecutionEngine/SparseTensor/File.cpp
111

We're happy to wait as long as we don't keep getting broken. @vitalybuka reverted Nate's change from yesterday today putting the windows mlir bot in the red again. We'd love to get to a solution that works for everyone, but if the changes keep getting reverted with no communication befor or after and getting the bot broken, it's really disruptive and we definitely don't want to be in a place where anyone is broken for multiple days again.

aartbik added inline comments.Oct 5 2022, 3:59 PM
mlir/lib/ExecutionEngine/SparseTensor/File.cpp
111

Ah, so this was needed again to fix breakage, I missed that completely? In that case, that makes sense of course, and you did of course the right thing. I really lost track of who is reverting and resubmitting this seemingly similar change ;-) [and earlier had to address a blaze breakage as a result].

So is all green now? In that case, we will proceed with the cosmetic follow ups ....

mlir/lib/ExecutionEngine/SparseTensor/File.cpp
111

Yes, the windows mlir buildbot is green again. I hope our change did not break anyone else either.