Page MenuHomePhabricator

Clean up usages of asserting vector getters in Type
ClosedPublic

Authored by ctetreau on Apr 1 2020, 4:39 PM.

Details

Summary

Remove usages of asserting vector getters in Type in preparation for the
VectorType refactor. The existence of these functions complicates the
refactor while adding little value.

Diff Detail

Event Timeline

ctetreau created this revision.Apr 1 2020, 4:39 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 1 2020, 4:39 PM
ctetreau updated this revision to Diff 254577.Apr 2 2020, 11:33 AM

Update to mention that this is NFC

ctetreau updated this revision to Diff 254862.Apr 3 2020, 11:15 AM

Rebase, catch some stragglers

ctetreau updated this revision to Diff 254863.Apr 3 2020, 11:20 AM

Fix null dereference

efriedma added inline comments.Apr 3 2020, 1:19 PM
llvm/lib/Transforms/Vectorize/VectorCombine.cpp
276

SrcTy must be a vector here: it's the result type of a shufflevector.

ctetreau updated this revision to Diff 254920.Apr 3 2020, 2:37 PM

address code review comments

sdesmalen added inline comments.Apr 10 2020, 6:54 AM
llvm/lib/Transforms/Vectorize/VectorCombine.cpp
281–282

Now that SrcTy is of type VectorType because of the cast<>, this assert can be removed.

ctetreau marked an inline comment as done.Apr 10 2020, 9:00 AM
ctetreau added inline comments.
llvm/lib/Transforms/Vectorize/VectorCombine.cpp
281–282

I agree and will fix it, but for the record: whenever this happens, I did it so that the assert message wouldn't change.

It might be nice if cast() had an overload that took a message that was used for the assert, but that's a problem for another day. Something like:

auto *t = cast<Foo>(bar, "This is guaranteed to be a Foo becasue it's the return value of a BarInst");
ctetreau updated this revision to Diff 256585.Apr 10 2020, 9:04 AM

address code review issues

ctetreau marked an inline comment as done.Apr 10 2020, 9:07 AM
ctetreau added inline comments.
llvm/lib/Transforms/Vectorize/VectorCombine.cpp
281–282

Obviously in this case, this assert will never fire. But at one point it was a dyn_cast.

This revision is now accepted and ready to land.Apr 10 2020, 10:43 AM
This revision was automatically updated to reflect the committed changes.