This is an archive of the discontinued LLVM Phabricator instance.

[InstSimplify] use computeKnownBits on shift amount operands
ClosedPublic

Authored by spatel on May 3 2016, 10:09 AM.

Details

Summary

Do some simplifications common to all shift instructions based on the amount shifted.

Note that we could generalize the shift-by-zero transform into a shift-by-constant if all of the valid bits in the shift amount are known, but I think that would have to be done in InstCombine rather than here because it would mean we need to create a new shift instruction.

Diff Detail

Event Timeline

spatel updated this revision to Diff 56017.May 3 2016, 10:09 AM
spatel retitled this revision from to [InstSimplify] use computeKnownBits on shift amount operands.
spatel updated this object.
spatel added reviewers: escha, majnemer, hfinkel.
spatel added a subscriber: llvm-commits.
majnemer added inline comments.May 3 2016, 10:52 AM
test/Transforms/InstSimplify/shift-knownbits.ll
142–144

Out of curiosity, isn't the result of the shl equivalent to %a? %b cannot be as large as the bit width, 1.

spatel added inline comments.May 3 2016, 11:14 AM
test/Transforms/InstSimplify/shift-knownbits.ll
142–144

Yes. The BitWidth check for '1' survived an earlier, buggy draft of this patch. Let me remove that; Log2_32_Ceil() gives us '0' for an input of '1', and that should work fine.

spatel updated this revision to Diff 56038.May 3 2016, 11:30 AM

Patch updated:
We don't have to special case an i1 type - the mask calculation works as-is there. Any shift of an i1 will always return the shifted operand because an actual shift would be undefined.

majnemer added inline comments.May 10 2016, 1:23 PM
test/Transforms/InstSimplify/shift-knownbits.ll
116–126

Is this still true? I might have changed this recently...

spatel updated this revision to Diff 56803.May 10 2016, 1:45 PM

Patch updated:
One less FIXME - computeKnownBits got smarter in the last week. Thanks, David!

majnemer accepted this revision.May 10 2016, 1:47 PM
majnemer edited edge metadata.

LGTM

This revision is now accepted and ready to land.May 10 2016, 1:47 PM
This revision was automatically updated to reflect the committed changes.