This is an archive of the discontinued LLVM Phabricator instance.

[InstCombine] dropRedundantMaskingOfLeftShiftInput(): truncation (PR42563)
ClosedPublic

Authored by lebedev.ri on Oct 17 2019, 11:14 AM.

Details

Summary

That fold keeps growing and growing :(
I think this may be one of the last pieces for it.

Since D67677/D67725, the fold knowns the general form
of the pattern - where some masking is needed:
https://rise4fun.com/Alive/F5R
https://rise4fun.com/Alive/gslRa

But there is one more huge piece missing - if you are extracting some bits,
it is not impossible that the origin is wider than the extraction,
i.e. there may be a truncation. And we don't deal with that yet.

But we can, and the generalization remains fully identical:
https://rise4fun.com/Alive/Uar
https://rise4fun.com/Alive/5SW

After a preparatory cleanup i think the diff looks rather clean.
One missing piece is that in some patterns (especially pat. b),
-1 only needs to be -1 in final type, but that is for later..

https://bugs.llvm.org/show_bug.cgi?id=42563

Diff Detail

Event Timeline

lebedev.ri created this revision.Oct 17 2019, 11:14 AM

Drop redundant shadowing variable.

Trim diff even more one last time.

spatel accepted this revision.Nov 4 2019, 6:29 AM

LGTM

llvm/lib/Transforms/InstCombine/InstCombineShifts.cpp
196

This match construct is unusual enough (not inside an 'if' statement and not testing the return value some other way) that I'd put a comment on it like:

// Peek through an optional zext of the shift amount.
This revision is now accepted and ready to land.Nov 4 2019, 6:29 AM
lebedev.ri marked an inline comment as done.Nov 5 2019, 1:39 AM

LGTM

Thank you for the review.

This revision was automatically updated to reflect the committed changes.