On 3/9/23 8:55 AM, Segher Boessenkool wrote:
> On Thu, Mar 09, 2023 at 05:30:53PM +0800, Kewen.Lin wrote:
>> on 2023/3/9 07:01, Peter Bergner via Gcc-patches wrote:
>>> This patch was tested in both GCC 11 and GCC 10 on powerpc64le-linux and
>>> showed no regressions. Ok for backports?
>
> It isn't truly a backport. You can put it on 11 and 10 at the same time,
> there is no benefit doing it on 11 only first.
Correct. I just meant that they're targeted at the two release branches
and not trunk.
>>> op[nopnds++] = build_pointer_type (void_type_node);
>>> if (d->code == MMA_BUILTIN_DISASSEMBLE_ACC)
>>> - op[nopnds++] = build_pointer_type (vector_quad_type_node);
>>> + op[nopnds++] = build_pointer_type (build_qualified_type
>>> + (vector_quad_type_node,
>>> + TYPE_QUAL_CONST));
>>
>> Nit: Maybe we can build them out of the loop once and then just use the
>> built one in the loop.
>
> Or as globals even. Currently we have X and pointer to X, but no
> pointer to const X (and no const X either, but that isn't so useful).
>
> The generic code doesn't have this either, hrm.
I can have a look at that, but was trying to keep the change as small
as possible. Especially since we're not trying to create code that
will be "easier" to maintain in the future, because this is all changed
in GCC12 with Bill's builtin re-write.
>> Simply testing __builtin_mma_xxmtacc and __builtin_mma_xxmfacc as below:
>>
>> $ cat test.C
>> void foo0(const __vector_quad *acc) {
>> __builtin_mma_xxmtacc(acc);
>> __builtin_mma_xxmfacc(acc);
>> }
>>
>> test.C:2:25: error: invalid conversion from ‘const __vector_quad*’ to
>> ‘__vector_quad*’ [-fpermissive]
>> 2 | __builtin_mma_xxmtacc(acc);
>>
>> test.C:3:25: error: invalid conversion from ‘const __vector_quad*’ to
>> ‘__vector_quad*’ [-fpermissive]
>> 3 | __builtin_mma_xxmfacc(acc);
>>
>> They also suffered the same error on gcc11 branch but not on trunk.
>
> Yeah, there is more to be done here.
Well I'm sure there are non-MMA builtins that have the same issue.
I was just fixing the ones Chip ran into and similar builtins.
I don't think we want to go and make everything work like it does
on trunk, especially when no one has complained about hitting
them.
As for the __builtin_mma_xxm[ft]acc() errors, I'm not sure any actual
code will ever hit this. All realistic examples declare a __vector_quad
var and the pointer passed to the builtin comes from doing &var as the
operand. Clearly we cannot have a "const __vector_quad var;" and
use that in the builtins.
>> Besides, I'm not sure if the existing bif declarations using
>> ptr_vector_pair_type_node
>> and ptr_vector_quad_type_node are all intentional, at least it looks weird
>> to me that
>> we declare const __vector_pair* for this __builtin_vsx_stxvp, which is meant
>> to store 32
>> bytes into the memory provided by the pointer biasing the sizetype offset,
>> but the "const"
>> qualifier seems to tell that this bif doesn't modify the memory pointed by
>> the given pointer.
I'm not a language lawyer and I don't play one on TV. What we're accepting
here, is a pointer with a "const" value that points to non-const memory.
I'll double check the trunk code, but I don't think it allows (and we don't
want it to) using a pointer (const or non-const) that points to a const memory
...at least for the stxvp builtin.
> Since the patch is a strict improvement already, it is okay for 11 and
> 10. But you (Peter) may want to flesh it out a bit first? Or first
> commit only this if that works better for you.
I'll see about making some of the changes above and then I'll report back.
Peter
