Hi,
"Kewen.Lin" <li...@linux.ibm.com> writes:
> Hi Jeff,
>
> on 2024/6/5 16:30, Jiufu Guo wrote:
>> Hi,
>>
>> For PR96866, when printing asm code for modifier "%a", an addressable
>> operand is required. While the constraint "X" allow any kind of
>> operand even which is hard to get the address directly. e.g. extern
>> symbol whose address is in TOC.
>> An error message would be reported to indicate the invalid asm operand.
>>
>> Compare with previous version, changelog and emit message are updated.
>>
>> Bootstrap®test pass on ppc64{,le}.
>> Is this ok for trunk?
>>
>> BR,
>> Jeff(Jiufu Guo)
>>
>> PR target/96866
>>
>> gcc/ChangeLog:
>>
>> * config/rs6000/rs6000.cc (print_operand_address): Emit message for
>> Unsupported operand.
>>
>> gcc/testsuite/ChangeLog:
>>
>> * gcc.target/powerpc/pr96866-1.c: New test.
>> * gcc.target/powerpc/pr96866-2.c: New test.
>>
>> ---
>> gcc/config/rs6000/rs6000.cc | 7 ++++++-
>> gcc/testsuite/gcc.target/powerpc/pr96866-1.c | 18 ++++++++++++++++++
>> gcc/testsuite/gcc.target/powerpc/pr96866-2.c | 13 +++++++++++++
>> 3 files changed, 37 insertions(+), 1 deletion(-)
>> create mode 100644 gcc/testsuite/gcc.target/powerpc/pr96866-1.c
>> create mode 100644 gcc/testsuite/gcc.target/powerpc/pr96866-2.c
>>
>> diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc
>> index 117999613d8..7e7c36a1bad 100644
>> --- a/gcc/config/rs6000/rs6000.cc
>> +++ b/gcc/config/rs6000/rs6000.cc
>> @@ -14664,7 +14664,12 @@ print_operand_address (FILE *file, rtx x)
>> fprintf (file, "@%s(%s)", SMALL_DATA_RELOC,
>> reg_names[SMALL_DATA_REG]);
>> else
>> - gcc_assert (!TARGET_TOC);
>> + {
>> + /* Do not support getting address directly from TOC, emit error.
>> + No more work is needed for !TARGET_TOC. */
>> + if (TARGET_TOC)
>> + output_operand_lossage ("%%a requires an address of memory");
>> + }
>> }
>> else if (GET_CODE (x) == PLUS && REG_P (XEXP (x, 0))
>> && REG_P (XEXP (x, 1)))
>> diff --git a/gcc/testsuite/gcc.target/powerpc/pr96866-1.c
>> b/gcc/testsuite/gcc.target/powerpc/pr96866-1.c
>> new file mode 100644
>> index 00000000000..bcebbd6e310
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/powerpc/pr96866-1.c
>> @@ -0,0 +1,18 @@
>> +/* The "%a" modifier can't get the address of extern symbol directly from
>> TOC
>> + with -fPIC, even the symbol is propgated for "X" constraint under -O2. */
>> +/* { dg-options "-fPIC -O2" } */
>> +
>> +/* It's to verify no ICE here, ignore error messages about invalid 'asm'.
>> */
>> +/* { dg-excess-errors "pr96866-1.c" } */
>
> This seems to XPASS on Power10 with pcrel? This needs ! powerpc_pcrel
> guard if so.
Oh, yeap. Thanks for point out this! %a would accept "X"(&x) with pcrel.
>
>> +
>> +int x[2];
>> +
>> +int __attribute__ ((noipa))
>> +f1 (void)
>> +{
>> + int n;
>> + int *p = x;
>> + *p++;
>> + __asm__ volatile("ld %0, %a1" : "=r"(n) : "X"(p));
>> + return n;
>> +}
>> diff --git a/gcc/testsuite/gcc.target/powerpc/pr96866-2.c
>> b/gcc/testsuite/gcc.target/powerpc/pr96866-2.c
>> new file mode 100644
>> index 00000000000..0577fd6d588
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/powerpc/pr96866-2.c
>> @@ -0,0 +1,13 @@
>> +/* The "%a" modifier can't get the address of extern symbol directly from
>> TOC
>> + with -fPIC. */
>> +/* { dg-options "-fPIC -O2" } */
>> +
>> +/* It's to verify no ICE here, ignore error messages about invalid 'asm'.
>> */
>> +/* { dg-excess-errors "pr96866-2.c" } */
>
> Ditto.
Thanks again.
BR,
Jeff(Jiufu) Guo.
>
> The others look good to me.
>
> BR,
> Kewen
>
>> +
>> +void
>> +f (void)
>> +{
>> + extern int x;
>> + __asm__ volatile("#%a0" ::"X"(&x));
>> +}
