[llvm] r214628 - [x86] Largely complete the use of PSHUFB in the new vector shuffle

[Chandler Carruth]

On Thu, Aug 14, 2014 at 3:45 AM, Patrik Hägglund H <
patrik.h.hagglund at ericsson.com> wrote:

> Hi Chandler,
>
> Ping!
>

Sorry, I got caught up in other bug chasing activities and forgot about
this one. I'll try to get to it if you don't sooner....


>
> I don't know anything about how this code works. Anyhow, I took a quick
> look at this. It seems like the call to getTargetShuffleMask (in
> combineX86ShufflesRecursively) returns a Mask with 'undef' (-1) values.
> However, the code following the call seems to assume that no elements are
> 'undef'.
>

That seems... very, very strange. If it isn't a bug in getTargetShuffleMask
(How does it know they are *undef*???) then I think the code following it
just needs to be taught to deal with undef masks.


>
> (I suggest adding pre- or post-conditions (asserts) for functions that
> assumes or returns masks without 'undef' values. Also, I think it would be
> better if getShuffleMask returned a vector of unsigned values, instead of
> signed. Now, I see a lot of "arbitrary" casts between signed and unsigned
> around the handling of mask values, rather than just for assigning/checking
> for 'undef'. However, that change seems rather invasive/tedious. :-( )
>

I generally disagree with this... at least at this stage. I don't think
unsigned vs. signed is going to help anything here.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140814/2aada173/attachment.html>