Page MenuHomePhabricator

[InstCombine] ror/rol(X, RotAmt) == C --> X == rol/ror(C, RotAmt) (PR51567)
ClosedPublic

Authored by xbolva00 on Sep 4 2021, 4:31 PM.

Details

Summary
----------------------------------------
define i1 @src(i32 %0) {
%1:
  %2 = fshl i32 %0, i32 %0, i32 25
  %3 = icmp eq i32 %2, 5
  ret i1 %3
}
=>
define i1 @tgt(i32 %0) {
%1:
  %2 = icmp eq i32 %0, 640
  ret i1 %2
}
Transformation seems to be correct!

https://alive2.llvm.org/ce/z/GdY8Jm

Solves PR51567

Diff Detail

Event Timeline

xbolva00 created this revision.Sep 4 2021, 4:31 PM
xbolva00 requested review of this revision.Sep 4 2021, 4:31 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 4 2021, 4:31 PM
xbolva00 updated this revision to Diff 370772.Sep 4 2021, 4:34 PM
xbolva00 added inline comments.
llvm/test/Transforms/InstCombine/icmp-rotate.ll
149

Enough tests?

spatel added inline comments.Sep 6 2021, 8:55 AM
llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
3238

Calling this "RotC" or "RotAmtC" makes it more obvious that we are matching a constant.

llvm/test/Transforms/InstCombine/icmp-rotate.ll
114

The over-shifting isn't relevant - please reduce to a valid amount (for example 28 -> 4) in this test and others.
But don't use "4" with a bitwidth of 8 either because that makes the test ambiguous about whether it is rotating in the wrong direction.

149

We're missing a fshl with ne. Also at least one extra-use test should be here.
Please pre-commit baseline tests.

xbolva00 updated this revision to Diff 370962.Sep 6 2021, 12:52 PM

Addressed review comments

xbolva00 marked 3 inline comments as done.Sep 6 2021, 12:52 PM
spatel accepted this revision.Sep 7 2021, 5:45 AM

LGTM

This revision is now accepted and ready to land.Sep 7 2021, 5:45 AM