[PATCH] Transform OR of SELECTs to SELECT of ORs

[Hal Finkel]

----- Original Message -----
> From: "Muhammad Tauqir Ahmad" <muhammad.t.ahmad at intel.com>
> To: "Hal Finkel" <hfinkel at anl.gov>
> Cc: reviews+D362+public+3c58d351ec48fa07 at llvm-reviews.chandlerc.com, llvm-commits at cs.uiuc.edu
> Sent: Friday, February 1, 2013 9:08:36 AM
> Subject: Re: [PATCH] Transform OR of SELECTs to SELECT of ORs
> 
> I submitted a patch for
> PR14664(http://llvm.org/bugs/show_bug.cgi?id=14664) which reversed
> another optmization (more specifically, the tests in
> logical-select.ll) so I am submitting this patch so that cases like
> the ones in logical-select.ll can be handled again through these two
> optmizations (the one in the patch for PR14664 + this patch).
> 
> Does that answer your question as to why I am submitting this patch?

I don't think so. I'd specifically like to better understand the rationale for disabling the transformation for vector selects?

Thanks again,
Hal

> 
> Thanks.
> - Muhammad Tauqir
> 
> 
> On Fri, Feb 1, 2013 at 9:59 AM, Hal Finkel <hfinkel at anl.gov> wrote:
> > ----- Original Message -----
> >> From: "Muhammad Tauqir" <muhammadtauqir16 at gmail.com>
> >> To: muhammadtauqir16 at gmail.com
> >> Cc: llvm-commits at cs.uiuc.edu
> >> Sent: Friday, February 1, 2013 7:59:28 AM
> >> Subject: [PATCH] Transform OR of SELECTs to SELECT of ORs
> >>
> >> More descriptively, the transform is:
> >> (or (bool?A:B),(bool?C:D)) --> (bool?(or A,C):(or B,D))
> >>
> >> By the time the OR is visited, both the SELECTs have been visited
> >> and
> >> not optimized and the OR itself hasn't been transformed so we do
> >> this transform in the hopes that the new ORs will be optimized.
> >>
> >> The transform is explicitly disabled for vector-selects until
> >> "codegen matures to handle them better".
> >
> > Can you be more specific about what needs to be improved, and on
> > what targets?
> >
> > Thanks again,
> > Hal
> >
> >>
> >> http://llvm-reviews.chandlerc.com/D362
> >>
> >> Files:
> >>   lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
> >>   test/Transforms/InstCombine/logical-select.ll
> >>
> >> Index: lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
> >> ===================================================================
> >> --- lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
> >> +++ lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
> >> @@ -2071,6 +2071,22 @@
> >>      return BinaryOperator::CreateOr(Inner, C1);
> >>    }
> >>
> >> +  // Change (or (bool?A:B),(bool?C:D)) --> (bool?(or A,C):(or
> >> B,D))
> >> +  // Since this OR statement hasn't been optimized further yet,
> >> we
> >> hope
> >> +  // that this transformation will allow the new ORs to be
> >> optimized.
> >> +  {
> >> +    Value *X = 0;
> >> +    // FIXME: Disable vector-select until codegen matures to
> >> handle
> >> them better.
> >> +    // See test32 in test/Transforms/InstCombine/or.ll
> >> +    if (!I.getType()->isVectorTy())
> >> +      if (match(Op0, m_Select(m_Value(X), m_Value(A),
> >> m_Value(B)))
> >> &&
> >> +          match(Op1, m_Select(m_Value(X), m_Value(C),
> >> m_Value(D))))
> >> {
> >> +        Value *orTrue = Builder->CreateOr(A, C);
> >> +        Value *orFalse = Builder->CreateOr(B, D);
> >> +        return SelectInst::Create(X, orTrue, orFalse);
> >> +      }
> >> +  }
> >> +
> >>    return Changed ? &I : 0;
> >>  }
> >>
> >> Index: test/Transforms/InstCombine/logical-select.ll
> >> ===================================================================
> >> --- test/Transforms/InstCombine/logical-select.ll
> >> +++ test/Transforms/InstCombine/logical-select.ll
> >> @@ -10,10 +10,8 @@
> >>    %j = or i32 %g, %i
> >>    ret i32 %j
> >>  ; CHECK: %e = icmp slt i32 %a, %b
> >> -; CHECK-NEXT: %g = select i1 %e, i32 %c, i32 0
> >> -; CHECK-NEXT: %i = select i1 %e, i32 0, i32 %d
> >> -; CHECK-NEXT: %j = or i32 %g, %i
> >> -; CHECK-NEXT: ret i32 %j
> >> +; CHECK-NEXT: [[result:%.*]] = select i1 %e, i32 %c, i32 %d
> >> +; CHECK-NEXT: ret i32 [[result]]
> >>  }
> >>  define i32 @bar(i32 %a, i32 %b, i32 %c, i32 %d) nounwind {
> >>    %e = icmp slt i32 %a, %b
> >> @@ -24,10 +22,8 @@
> >>    %j = or i32 %i, %g
> >>    ret i32 %j
> >>  ; CHECK: %e = icmp slt i32 %a, %b
> >> -; CHECK-NEXT: %g = select i1 %e, i32 %c, i32 0
> >> -; CHECK-NEXT: %i = select i1 %e, i32 0, i32 %d
> >> -; CHECK-NEXT: %j = or i32 %i, %g
> >> -; CHECK-NEXT: ret i32 %j
> >> +; CHECK-NEXT: [[result:%.*]] = select i1 %e, i32 %c, i32 %d
> >> +; CHECK-NEXT: ret i32 [[result]]
> >>  }
> >>
> >>  define i32 @goo(i32 %a, i32 %b, i32 %c, i32 %d) nounwind {
> >> @@ -40,10 +36,8 @@
> >>    %3 = or i32 %1, %2
> >>    ret i32 %3
> >>  ; CHECK: %0 = icmp slt i32 %a, %b
> >> -; CHECK-NEXT: %1 = select i1 %0, i32 %c, i32 0
> >> -; CHECK-NEXT: %2 = select i1 %0, i32 0, i32 %d
> >> -; CHECK-NEXT: %3 = or i32 %1, %2
> >> -; CHECK-NEXT: ret i32 %3
> >> +; CHECK-NEXT: [[result:%.*]] = select i1 %0, i32 %c, i32 %d
> >> +; CHECK-NEXT: ret i32 [[result]]
> >>  }
> >>  define i32 @poo(i32 %a, i32 %b, i32 %c, i32 %d) nounwind {
> >>  entry:
> >> @@ -55,10 +49,8 @@
> >>    %3 = or i32 %1, %2
> >>    ret i32 %3
> >>  ; CHECK: %0 = icmp slt i32 %a, %b
> >> -; CHECK-NEXT: %1 = select i1 %0, i32 %c, i32 0
> >> -; CHECK-NEXT: %2 = select i1 %0, i32 0, i32 %d
> >> -; CHECK-NEXT: %3 = or i32 %1, %2
> >> -; CHECK-NEXT: ret i32 %3
> >> +; CHECK-NEXT: [[result:%.*]] = select i1 %0, i32 %c, i32 %d
> >> +; CHECK-NEXT: ret i32 [[result]]
> >>  }
> >>
> >>  define i32 @par(i32 %a, i32 %b, i32 %c, i32 %d) nounwind {
> >> @@ -71,8 +63,6 @@
> >>    %3 = or i32 %1, %2
> >>    ret i32 %3
> >>  ; CHECK: %0 = icmp slt i32 %a, %b
> >> -; CHECK-NEXT: %1 = select i1 %0, i32 %c, i32 0
> >> -; CHECK-NEXT: %2 = select i1 %0, i32 0, i32 %d
> >> -; CHECK-NEXT: %3 = or i32 %1, %2
> >> -; CHECK-NEXT: ret i32 %3
> >> +; CHECK-NEXT: [[result:%.*]] = select i1 %0, i32 %c, i32 %d
> >> +; CHECK-NEXT: ret i32 [[result]]
> >>  }
> >>
> >> _______________________________________________
> >> llvm-commits mailing list
> >> llvm-commits at cs.uiuc.edu
> >> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> >>
>