on 2024/8/1 01:52, Carl Love wrote:
> Kewen:
>
> On 7/31/24 2:12 AM, Kewen.Lin wrote:
>> Hi Carl,
>>
>> on 2024/7/27 06:56, Carl Love wrote:
>>> GCC maintainers:
>>>
>>> Per a report from a user, the existing vec_test_lsbb_all_ones and,
>>> vec_test_lsbb_all_zeros built-ins are not documented in the GCC
>>> documentation file.
>>>
>>> The following patch adds missing documentation for the
>>> vec_test_lsbb_all_ones and, vec_test_lsbb_all_zeros built-ins.
>>>
>>> Please let me know if the patch is acceptable for mainline. Thanks.
>>>
>>> Carl
>>>
>>> -------------------------------------------------------------------
>>> rs6000, document built-ins vec_test_lsbb_all_ones and
>>> vec_test_lsbb_all_zeros
>>>
>>> Add documentation for the Power 10 built-ins vec_test_lsbb_all_ones
>>> and vec_test_lsbb_all_zeros. The vec_test_lsbb_all_ones built-in
>>> returns 1 if the least significant bit in each byte is a 1, returns
>>> 0 otherwise. Similarly, vec_test_lsbb_all_zeros returns a 1 if
>>> the least significant bit in each byte is a zero and 0 otherwise.
>>>
>>> The test cases for the built-ins are in files:
>>> gcc/testsuite/gcc.target/powerpc/lsbb.c
>>> gcc/testsuite/gcc.target/powerpc/lsbb-runnable.c
>>>
>>>
>>> gcc/ChangeLog:
>>> * doc/extend.texi (vec_test_lsbb_all_ones,
>>> vec_test_lsbb_all_zeros): Add documentation for the
>>> existing built-ins.
>>> ---
>>> gcc/doc/extend.texi | 15 +++++++++++++++
>>> 1 file changed, 15 insertions(+)
>>>
>>> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
>>> index 83ff168faf6..96e41c9a905 100644
>>> --- a/gcc/doc/extend.texi
>>> +++ b/gcc/doc/extend.texi
>>> @@ -23240,6 +23240,21 @@ signed long long will sign extend the rightmost
>>> byte of each doubleword.
>>> The following additional built-in functions are also available for the
>>> PowerPC family of processors, starting with ISA 3.1
>>> (@option{-mcpu=power10}):
>>>
>>> +@smallexample
>>> +@exdent int vec_test_lsbb_all_ones (vector char);
>> I think we need to specify "unsigned" char explicitly since we don't actually
>> allow vector "signed" char as the below testing shows:
>>
>> int foo11 (vector signed char va)
>> {
>> return vec_test_lsbb_all_ones (va);
>> }
>>
>> <source>:17:3: error: invalid parameter combination for AltiVec intrinsic
>> '__builtin_vec_xvtlsbb_all_ones'
>> 17 | return vec_test_lsbb_all_ones (va);
>>
>>
>> Now we make these two bifs as overload, but there is only one instance
>> respectively,
> Yes, I noticed that the built-ins were defined as overloaded but only had one
> definition. Did seem odd to me.
>
>> either is with "vector unsigned char" as argument type, but the
>> corresponding instance
>> prototype in builtin table is with "vector signed char". It's inconsistent
>> and weird,
>> I think we can just update the prototype in builtin table with "vector
>> unsigned char"
>> and remove the entries in overload table. It can be a follow up patch.
>
> I didn't notice that it was signed in the instance prototype but unsigned in
> the overloaded definition. That is definitely inconsistent.
>
> That said, should we just go ahead and support both signed and unsigned
> argument versions of the all ones and all zeros built-ins?
Good question, I thought about that but found openxl only supports the unsigned
version
so I felt it's probably better to keep consistent with it. But I'm fine for
either, if
we decide to extend it to cover both signed and unsigned, we should notify
openxl team
to extend it as well.
openxl doc links:
https://www.ibm.com/docs/en/openxl-c-and-cpp-aix/17.1.2?topic=functions-vec-test-lsbb-all-ones
https://www.ibm.com/docs/en/openxl-c-and-cpp-aix/17.1.2?topic=functions-vec-test-lsbb-all-zeros
BR,
Kewen
>
> For example
>
> [VEC_TEST_LSBB_ALL_ONES, vec_test_lsbb_all_ones,
> __builtin_vec_xvtlsbb_all_ones]
> signed int __builtin_vec_xvtlsbb_all_ones (vsc);
> XVTLSBB_ONES LSBB_ALL_ONES_VSC
> signed int __builtin_vec_xvtlsbb_all_ones (vuc);
> XVTLSBB_ONES LSBB_ALL_ONES_VUC
>
> I tried this with the testcase, I borrowed from you and extended:
>
> int foo11 (vector char va) <-
> compiles fine
> {
> return vec_test_lsbb_all_ones (va);
> }
>
> int sfoo11 (vector signed char sva) <- currently fails to compile without
> change to overloaded def, compiles with
> { additional overloaded definition.
> return vec_test_lsbb_all_ones (sva);
> }
>
> int ufoo11 (vector unsigned char uva) <- compiles fine
> {
> return vec_test_lsbb_all_ones (uva);
> }
>
> I did a quick test to see that the testcase does compile. We would need to
> add testcases to lsbb.c and lsbb-runnable.c and then update
> the documentation to say both are supported.
>
> Thoughts on expanding the scope of the patch from just documentation to
> adding additional overloaded cases and updating the documentation?
>
>>
>>> +@end smallexample
>>> +@findex vec_test_lsbb_all_ones
>>> +
>>> +The builtin @code{vec_test_lsbb_all_ones} returns 1 if the least
>>> significant
>>> +bit in each byte is a one. It returns a zero otherwise.
>> May be better to use the wording "equal to 1" referred from ISA and "returns
>> 0"
>> matches the preceding "returns 1", like:
>>
>> “... in each byte is equal to 1. It returns 0 otherwise.”
>
> Changed.
>>> +
>>> +@smallexample
>>> +@exdent int vec_test_lsbb_all_zeros (vector char);
>>> +@end smallexample
>>> +@findex vec_test_lsbb_all_zeros
>>> +
>>> +The builtin @code{vec_test_lsbb_all_zeros} returns 1 if the least
>>> significant
>>> +bit in each byte is a zero. It returns a zero otherwise.
>> Likewise, "... in each byte is equal to 0. It returns 0 otherwise."
>
> Changed.
>
> Carl
>
