This is an archive of the discontinued LLVM Phabricator instance.

[InstCombine] (ShiftValC >> Y) >s -1/<s 0 --> Y != 0/==0
ClosedPublic

Authored by Chenbing.Zheng on Jul 13 2022, 8:37 PM.

Details

Summary

We can do
(ShiftValC >> Y) >s -1 --> Y != 0,
(ShiftValC >> Y) <s 0 --> Y == 0,
with ShiftValC < 0

https://alive2.llvm.org/ce/z/-PRHfD
https://alive2.llvm.org/ce/z/bWfp-s

Diff Detail

Event Timeline

Chenbing.Zheng created this revision.Jul 13 2022, 8:37 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 13 2022, 8:37 PM
Herald added a subscriber: hiraditya. · View Herald Transcript
Chenbing.Zheng requested review of this revision.Jul 13 2022, 8:37 PM
spatel added a comment.EditedJul 14 2022, 7:27 AM

This will not solve the problem in general. We need to add a fold for lshr values. We should be able to optimize something like this:
https://godbolt.org/z/7Mhdd8dfa
https://alive2.llvm.org/ce/z/3sB6au

Can we generalize or adapt this recent patch?
d4f39d8333324bdcf2056

Chenbing.Zheng retitled this revision from [InstCombine] add a constraint for x <u minSigned --> x >s -1 to [InstCombine] (ShiftValC >> Y) >s -1 --> Y != 0 with ShiftValC < 0.
Chenbing.Zheng edited the summary of this revision. (Show Details)

address comment,
add fold (ShiftValC >> Y) >s -1 --> Y != 0 with ShiftValC < 0
https://alive2.llvm.org/ce/z/TTnvgs

spatel added inline comments.Jul 18 2022, 5:23 AM
llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
2242

We should also handle the related pattern that is checking if the shifted value is negative:
https://alive2.llvm.org/ce/z/bWfp-s

Using InstCombiner::isSignBitCheck() can reduce the code needed to match these patterns. Look around this file for examples of usage.

llvm/test/Transforms/InstCombine/icmp-shr.ll
1195

typo: "Negative"

You may pre-commit tests in "NFC" patches. You can push that kind of patch without pre-commit review on Phabricator. It would be good to test at least 2 more code patterns:

  1. Use a vector type with "splat" constants.
  2. Add an extra use of the shift.
spatel edited the summary of this revision. (Show Details)Jul 18 2022, 5:23 AM
Chenbing.Zheng retitled this revision from [InstCombine] (ShiftValC >> Y) >s -1 --> Y != 0 with ShiftValC < 0 to [InstCombine] (ShiftValC >> Y) >s -1/<s 0 --> Y != 0/==0 .
Chenbing.Zheng edited the summary of this revision. (Show Details)

address comments

spatel accepted this revision.Jul 20 2022, 6:26 AM

LGTM

llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
2246

We are inside of the "InstCombiner" namespace, so there is no reason to specify "InstCombiner::" explicitly.

This revision is now accepted and ready to land.Jul 20 2022, 6:26 AM
spatel edited the summary of this revision. (Show Details)Jul 20 2022, 6:27 AM
This revision was landed with ongoing or failed builds.Jul 20 2022, 7:16 PM
This revision was automatically updated to reflect the committed changes.