Kewen:
On 5/21/24 20:05, Kewen.Lin wrote:
> Hi Carl,
>
> on 2024/5/22 08:13, Carl Love wrote:
>> Kewen:
<snip>
>>> Why did you place this in a section for ISA 3.1 (Power10)? It doesn't
>>> really
>>> require this support. The used instance VSEL_1TI and VSEL_1TI_UNS are
>>> placed
>>> in altivec stanza, so it looks that we should put it under the section
>>> "PowerPC AltiVec Built-in Functions on ISA 2.05". And since it's an
>>> extension
>>> of @code{vec_sel} documented in the PVIPR, I prefer to just mention it's "an
>>> extension of the @code{vec_sel} built-in documented in the PVIPR" and
>>> omitting
>>> the description to avoid possible slightly different wording.
>>
>> Honestly, at this point in time I don't remember why I put it there. It has
>> been too long since I created the patch. That said, the test case requires
>> Power 10 do to the comparison check using built-in vec_all_eq but that is
>> another issue.
>> The built-in generates the xxsel instruction that is an ISA 2.06
>> instruction. So, I would say it should to into the ISA 2.06 section. I
>> moved it to the ISA 2.06 section.
>
> But the underlying implementation is:
>
> const vsq __builtin_altivec_vsel_1ti (vsq, vsq, vuq);
> VSEL_1TI vector_select_v1ti {}
>
> const vuq __builtin_altivec_vsel_1ti_uns (vuq, vuq, vuq);
> VSEL_1TI_UNS vector_select_v1ti_uns {}
>
> , it's under altivec stanza and can result with insn vsel (so not xxsel),
> vsel is ISA 2.03, so I think ISA 2.05 better matches the implementation.
OK, moved to ISA 2.05
>
<snip>
>>
>> Sounds like there was some issue that you noticed on
>> r14-10011-g6e62ede7aaccc6. The new version of
>> print_i128 should be functionally equivalent but perhaps is "safer"?
>
> Thanks for checking! Looking into this more closely, I realized you didn't
> apply the previously
> adopted way for printing (the way used in
> gcc.target/powerpc/builtins-6-p9-runnable.c), sorry for
> the false alarm! So your supposed print_i128 is fine to me.
OK, no problem. Will go with the original print_i128 function.
Carl
