[PATCH] Transform OR of SELECTs to SELECT of ORs

Muhammad Tauqir Ahmad muhammad.t.ahmad at intel.com
Fri Feb 1 07:21:51 PST 2013


In a comment on test32 in test/Transforms/InstCombine/or.ll (which was
failing after I had implemented the transform), it says "Don't turn
this into a vector select until codegen matures to handle them better"
which is why I am not turning this into a vector-select. That comment
was added by Chris Lattner and I took it to mean that any optmization
that converts this into a vector-select should not.

- Muhammad Tauqir


On Fri, Feb 1, 2013 at 10:15 AM, Hal Finkel <hfinkel at anl.gov> wrote:
> ----- 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
>> >>
>>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits



More information about the llvm-commits mailing list