[PATCH] D63382: [InstCombine] fold a shifted zext to a select

Mon Jun 17 17:10:36 PDT 2019

zvi marked an inline comment as done.
zvi added a comment.

In D63382#1547090 <https://reviews.llvm.org/D63382#1547090>, @spatel wrote:

> The only problem that I see is that the backend (DAGCombiner) isn't prepared to generically reverse this pattern yet (x86 might have some code that can be lifted):

`combineSelectOfTwoConstants` seems to be the handler for X86 you mentioned . I like your idea to migrate it to a target-independent combine.

================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineShifts.cpp:668-669
   Constant *C1;
   if (match(Op1, m_Constant(C1))) {
     Constant *C2;
     Value *X;
----------------
spatel wrote:
> zvi wrote:
> > lebedev.ri wrote:
> > > I think you want to add the fold here, no reason to restrict it to identical vectors
> > What is the policy for handling non-splat cases, should they always be handled when possible or are they limited only to certain cases to reduce compile-time?
> There's no official policy AFAIK. If it's easy enough to apply the transform to an arbitrary vector constant, then I'd prefer we do it. 
> 
> There should not be much compile-time overhead because (1) either way, we're checking each element of the vector constant to determine if it's a splat and (2) there probably aren't that many non-splat vector constants to begin with.
This makes sense, thanks.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63382/new/

https://reviews.llvm.org/D63382