[llvm] r303387 - [InstCombine] add more tests for xor-of-icmps; NFC

Sanjay Patel via llvm-commits llvm-commits at lists.llvm.org
Thu May 18 16:20:00 PDT 2017


No, I haven't looked at VRP yet, so I don't know why it's not handled
there. I was assuming that since InstSimplify/InstCombine already have a
large chunk of folds for the and/or variants, it would make sense to also
handle xor there.

I have a patch in progress that would handle these cases using the existing
code (truth tables!). Let me post that for review and see what you think
about it.

On Thu, May 18, 2017 at 5:11 PM, Davide Italiano <davide at freebsd.org> wrote:

> On Thu, May 18, 2017 at 4:10 PM, Davide Italiano <davide at freebsd.org>
> wrote:
> > On Thu, May 18, 2017 at 3:47 PM, Sanjay Patel via llvm-commits
> > <llvm-commits at lists.llvm.org> wrote:
> >> Author: spatel
> >> Date: Thu May 18 17:47:57 2017
> >> New Revision: 303387
> >>
> >> URL: http://llvm.org/viewvc/llvm-project?rev=303387&view=rev
> >> Log:
> >> [InstCombine] add more tests for xor-of-icmps; NFC
> >>
> >> Modified:
> >>     llvm/trunk/test/Transforms/InstCombine/set.ll
> >>
> >> Modified: llvm/trunk/test/Transforms/InstCombine/set.ll
> >> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/
> Transforms/InstCombine/set.ll?rev=303387&r1=303386&r2=303387&view=diff
> >> ============================================================
> ==================
> >> --- llvm/trunk/test/Transforms/InstCombine/set.ll (original)
> >> +++ llvm/trunk/test/Transforms/InstCombine/set.ll Thu May 18 17:47:57
> 2017
> >> @@ -175,6 +175,36 @@ define i1 @xor_of_icmps(i64 %a) {
> >>    ret i1 %xor
> >>  }
> >>
> >> +; FIXME: This is also equivalent to the previous test.
> >> +
> >> +define i1 @xor_of_icmps_commute(i64 %a) {
> >> +; CHECK-LABEL: @xor_of_icmps_commute(
> >> +; CHECK-NEXT:    [[B:%.*]] = icmp sgt i64 %a, 0
> >> +; CHECK-NEXT:    [[C:%.*]] = icmp eq i64 %a, 1
> >> +; CHECK-NEXT:    [[XOR:%.*]] = xor i1 [[B]], [[C]]
> >> +; CHECK-NEXT:    ret i1 [[XOR]]
> >> +;
> >> +  %b = icmp sgt i64 %a, 0
> >> +  %c = icmp eq i64 %a, 1
> >> +  %xor = xor i1 %b, %c
> >> +  ret i1 %xor
> >> +}
> >> +
> >> +; FIXME: This is (a != 5).
> >> +
> >> +define i1 @xor_of_icmps_folds_more(i64 %a) {
> >> +; CHECK-LABEL: @xor_of_icmps_folds_more(
> >> +; CHECK-NEXT:    [[B:%.*]] = icmp sgt i64 %a, 4
> >> +; CHECK-NEXT:    [[C:%.*]] = icmp slt i64 %a, 6
> >> +; CHECK-NEXT:    [[XOR:%.*]] = xor i1 [[B]], [[C]]
> >> +; CHECK-NEXT:    ret i1 [[XOR]]
> >> +;
> >
> > You can start from having these in `-instcombine`, but these (and
> > other tests) seems like a VRP problem to me. I haven't studied this
> > carefully, but I wonder why we don't catch these. Have you tried to
> > evaluate why our VRP pass doesn't handle these?
> >
> > --
> > Davide
>
> Not necessarily complaining about having these as combiners, but I'd
> first like to understand why we don't catch these somewhere else :)
>
> --
> Davide
>
> "There are no solved problems; there are only problems that are more
> or less solved" -- Henri Poincare
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170518/e84521ac/attachment.html>


More information about the llvm-commits mailing list