Re: [PATCH] rs6000: Teach rs6000_opaque_type_invalid_use_p about gcall [PR108348]

["Kewen.Lin" <linkw@linux.ibm.com>]

Hi Segher,

Thanks for the review comments!

on 2023/1/16 16:49, Segher Boessenkool wrote:
> Hi!
> 
> On Mon, Jan 16, 2023 at 04:33:36PM +0800, Kewen.Lin wrote:
>> PR108348 shows one special case that MMA opaque types are
>> used in function arguments and treated as pass by reference,
>> it results in one copying from argument to a temp variable,
>> since this copying happens before rs6000_function_arg check,
>> it can cause ICE without MMA support then.  This patch is to
>> teach function rs6000_opaque_type_invalid_use_p to check if
>> any function argument in a gcall stmt has the invalid use of
>> MMA opaque types.
>>
>> btw, I checked the handling on return value, it doesn't have
>> this kind of issue as its checking and error emission is quite
>> early, so this doesn't handle function return value.
>>
>> Bootstrapped and regtested on powerpc64-linux-gnu P8 and
>> powerpc64le-linux-gnu P9 and P10.
>>
>> I'm going to push this soon if no objections.
> 
> Looks okay.  Some testcase stuff though:
> 
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/powerpc/pr108348-1.c
>> @@ -0,0 +1,23 @@
>> +/* { dg-require-effective-target powerpc_p9modulo_ok } */
> 
> Please use a saner selector?  If one doesn't exist yet, make a new one?
> Something that just says "p9", not "modulo".

The has_arch_pwr9 looks not suitable here as it doesn't check
the assembler behavior?  Do you have some instruction in mind
for being used as the tested instruction mnemonic like modsw?

Maybe later we can deprecate -mmodulo like we get rid of
-mdirect-move, then we can rename p9modulo_ok to p9_ok here?  :)

BR,
Kewen