[llvm] r216678 - Do not introduce new shuffle patterns after operation legalization if SHUFFLE_VECTOR

[Hal Finkel]

----- Original Message -----
> From: "Hal Finkel" <hfinkel at anl.gov>
> To: "Owen Anderson" <resistor at mac.com>
> Cc: "Commit Messages and Patches for LLVM" <llvm-commits at cs.uiuc.edu>
> Sent: Thursday, August 28, 2014 1:41:04 PM
> Subject: Re: [llvm] r216678 - Do not introduce new shuffle patterns after	operation legalization if SHUFFLE_VECTOR
> 
> ----- Original Message -----
> > From: "Owen Anderson" <resistor at mac.com>
> > To: "Chandler Carruth" <chandlerc at google.com>
> > Cc: "Commit Messages and Patches for LLVM"
> > <llvm-commits at cs.uiuc.edu>
> > Sent: Thursday, August 28, 2014 1:26:57 PM
> > Subject: Re: [llvm] r216678 - Do not introduce new shuffle patterns
> > after	operation legalization if SHUFFLE_VECTOR
> > 
> > On Aug 28, 2014, at 11:08 AM, Chandler Carruth <
> > chandlerc at google.com
> > > wrote:
> > 
> > On Thu, Aug 28, 2014 at 11:04 AM, Chandler Carruth <
> > chandlerc at google.com > wrote:
> > 
> > On Thu, Aug 28, 2014 at 10:49 AM, Owen Anderson < resistor at mac.com
> > >
> > wrote:
> > 
> > Do not introduce new shuffle patterns after operation legalization
> > if
> > SHUFFLE_VECTOR
> > was marked custom. The target independent DAG combine has no way to
> > know if
> > the shuffles it is introducing are ones that the target could
> > support
> > or not.
> > Test case? This seems somewhat arbitrary otherwise...
> > (To be completely clear, in case this email wasn't, I understand
> > that
> > currently not all backends support a custom lowering of all
> > shuffles.... but the x86 backend actually does support it (for
> > legal
> > types) and so it seems quite likely that someone could accidentally
> > change this back unless we have more checking in the backend to
> > back
> > up the assertion that we can't form an arbitrary shuffle here.
> > Essentially, I'm fine with the patch, but I'm worried that I or
> > someone else could easily re-break this in the future unless we
> > have
> > some test coverage.)
> 
> I don't understand; we have a isShuffleMaskLegal callback in TLI, why
> can't you use that?

To follow up, as I recall we discussed this on IRC and the core problem was that isShuffleMaskLegal was not properly implemented for X86. I believe that I was promised a FIXME that never arrived ;) -- but in any case, I'd like to know if, how that Chandler has finished up his shuffle work, we can properly fix isShuffleMaskLegal and return this code to a better state?

Thanks again,
Hal

> 
>  -Hal
> 
> > 
> > The only testcase I have for this requires a not-in-tree target.
> > 
> > 
> > —Owen
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> > 
> 
> --
> Hal Finkel
> Assistant Computational Scientist
> Leadership Computing Facility
> Argonne National Laboratory
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory