[PATCH] Transform OR of SELECTs to SELECT of ORs

Hal Finkel hfinkel at anl.gov
Fri Feb 1 07:34:41 PST 2013 

----- 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:21:51 AM
> Subject: Re: [PATCH] Transform OR of SELECTs to SELECT of ORs
> 
> 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.

It looks like that comment is now three years old:
r95058 | lattner | 2010-02-01 20:43:51 -0600 (Mon, 01 Feb 2010) | 4 lines

don't turn (A & (C0?-1:0)) | (B & ~(C0?-1:0)) ->  C0 ? A : B
for vectors.  Codegen is generating awful code or segfaulting
in various cases (e.g. PR6204).

The specific bug report mentioned has been marked as resolved, but a duplicate of PR1784. PR1784 is still open, although it is not clear to me whether or not the problems with the in-memory representation of i1 vectors affects this transformation. I know that work on vector selects has been done over the past year at least.

 -Hal

> 
> - 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