On Wed, Aug 17, 2011 at 3:30 PM, Artem Shinkarov
<[email protected]> wrote:
> Hi
>
> Several comments before the new version of the patch.
> 1) x != x
> I am happy to adjust constant_boolean_node, but look at the code
> around line 9074 in fold-const.c, you will see that x <op> x
> elimination, even with adjusted constant_boolean_node, will look about
> the same as my code. Because I need to check the parameters (!FLOAT_P,
> HONOR_NANS) on TREE_TYPE (arg0) not arg0, and I need to construct
> constant_boolean_node (-1), not 1 in case of true.
> But I will change constant_boolean_node to accept vector types.
Hm, that should be handled transparently if you look at the defines
of FLOAT_TYPE_P and the HONOR_* macros.
>
> 2) comparison vs vcond
> v = v1 < v2;
> v = v1 < v2 ? {-1,...} : {0,...};
>
> are not the same.
> 16,25c16,22
> < movdqa .LC1(%rip), %xmm1
> < pshufd $225, %xmm1, %xmm1
> < pshufd $39, %xmm0, %xmm0
> < movss %xmm2, %xmm1
> < pshufd $225, %xmm1, %xmm1
> < pcmpgtd %xmm1, %xmm0
> < pcmpeqd %xmm1, %xmm1
> < pcmpeqd %xmm1, %xmm0
> < pand %xmm1, %xmm0
> < movdqa %xmm0, -24(%rsp)
> ---
>> pshufd $39, %xmm0, %xmm1
>> movdqa .LC1(%rip), %xmm0
>> pshufd $225, %xmm0, %xmm0
>> movss %xmm2, %xmm0
>> pshufd $225, %xmm0, %xmm0
>> pcmpgtd %xmm0, %xmm1
>> movdqa %xmm1, -24(%rsp)
>
> So I would keep the hook, it could be removed at any time when the
> standard expansion will start to work fine.
Which one is which? I'd really like to make this patch simpler at first,
and removing that hook is an obvious thing that _should_ be possible,
even optimally (by fixing the targets).
> 3) mask ? vec0 : vec1
> So no, I don't think we need to convert {3, 4, -1, 5} to {0,0,-1,0}
> (that would surprise my anyway, I'd have expected {-1,-1,-1,-1} ;)).
>
> Does OpenCL somehow support you here?
>
> OpenCL says that vector operation mask ? vec0 : vec1 is the same as
> select (vec0, vec1, mask). The semantics of select operation is the
> following:
>
> gentype select (gentype a, gentype b, igentype c)
> For each component of a vector type,
> result[i] = if MSB of c[i] is set ? b[i] : a[i].
>
> I am not sure what they really understand using the term MSB. As far
> as I know MSB is Most Significant Bit, so does it mean that in case of
> 3-bit integer 100 would trigger true but 011 would be still false...
Yes, MSB is Most Significant Bit - that's a somewhat odd definition ;)
> My reading would be that if all bits set, then take the first element,
> otherwise the second.
>
> It is also confusing when a ? vec0 : vec1, and a != 0 ? vec0 vec1
> produce different results. So I would stick to all bits set being true
> scenario.
For the middle-end part definitely. Thus I'd simply leave the mask alone.
> 4) Backend stuff. Ok, we could always fall back to reject the cases
> when cond and operands have different type, and then fix the backend.
>
> Adjustments are coming.
>
>
> Artem.
>
