[v1] LoongArch: Fixed a compilation failure with '%c' in inline assembly [PR107731].

Message ID 20221123064934.1560808-1-chenglulu@loongson.cn
State Accepted
Headers
Series [v1] LoongArch: Fixed a compilation failure with '%c' in inline assembly [PR107731]. |

Checks

Context Check Description
snail/gcc-patch-check success Github commit url

Commit Message

chenglulu Nov. 23, 2022, 6:49 a.m. UTC
  gcc/ChangeLog:

	* config/loongarch/loongarch.cc (loongarch_classify_address):
	Add precessint for CONST_INT.
	(loongarch_print_operand): Increase the processing of '%c'.

gcc/testsuite/ChangeLog:

	* gcc.target/loongarch/tst-asm-const.c: Moved to...
	* gcc.target/loongarch/pr107731.c: ...here.
---
 gcc/config/loongarch/loongarch.cc                  | 14 ++++++++++++++
 .../loongarch/{tst-asm-const.c => pr107731.c}      |  6 +++---
 2 files changed, 17 insertions(+), 3 deletions(-)
 rename gcc/testsuite/gcc.target/loongarch/{tst-asm-const.c => pr107731.c} (78%)
  

Comments

Xi Ruoyao Nov. 23, 2022, 8:59 a.m. UTC | #1
On Wed, 2022-11-23 at 14:49 +0800, Lulu Cheng wrote:
>     'A' Print a _DB suffix if the memory model requires a release.
>     'b' Print the address of a memory operand, without offset.
> +   'c'  print an integer.

Nit:
      'c' Print an integer.

to match the format of other entries.

>     'C' Print the integer branch condition for comparison OP.
>     'd' Print CONST_INT OP in decimal.
>     'F' Print the FPU branch condition for comparison OP.

And I'd consider this a new feature and delay it to GCC 14: we never
claimed we supported 'c' and it has not worked since the day one we
merged LoongArch port.  Is there any emergency reason to support 'c' in
GCC 13?
  
chenglulu Nov. 23, 2022, 9:14 a.m. UTC | #2
在 2022/11/23 16:59, Xi Ruoyao 写道:
> On Wed, 2022-11-23 at 14:49 +0800, Lulu Cheng wrote:
>>      'A' Print a _DB suffix if the memory model requires a release.
>>      'b' Print the address of a memory operand, without offset.
>> +   'c'  print an integer.
> Nit:
>        'c' Print an integer.
>
> to match the format of other entries.
>
>>      'C' Print the integer branch condition for comparison OP.
>>      'd' Print CONST_INT OP in decimal.
>>      'F' Print the FPU branch condition for comparison OP.
> And I'd consider this a new feature and delay it to GCC 14: we never
> claimed we supported 'c' and it has not worked since the day one we
> merged LoongArch port.  Is there any emergency reason to support 'c' in
> GCC 13?
>
I don't think this is a new feature.

There is a description of '%c' in section 17.5 of gccint.pdf, which I 
understand is a public descriptor,

but right now loongarch doesn't support it.🙁
  
Xi Ruoyao Nov. 23, 2022, 9:25 a.m. UTC | #3
On Wed, 2022-11-23 at 17:14 +0800, chenglulu wrote:
> 
> 在 2022/11/23 16:59, Xi Ruoyao 写道:
> > On Wed, 2022-11-23 at 14:49 +0800, Lulu Cheng wrote:
> > >      'A' Print a _DB suffix if the memory model requires a
> > > release.
> > >      'b' Print the address of a memory operand, without offset.
> > > +   'c'  print an integer.
> > Nit:
> >        'c' Print an integer.
> > 
> > to match the format of other entries.
> > 
> > >      'C' Print the integer branch condition for comparison OP.
> > >      'd' Print CONST_INT OP in decimal.
> > >      'F' Print the FPU branch condition for comparison OP.
> > And I'd consider this a new feature and delay it to GCC 14: we never
> > claimed we supported 'c' and it has not worked since the day one we
> > merged LoongArch port.  Is there any emergency reason to support 'c'
> > in
> > GCC 13?
> > 
> I don't think this is a new feature.
> 
> There is a description of '%c' in section 17.5 of gccint.pdf, which I 
> understand is a public descriptor,
> 
> but right now loongarch doesn't support it.🙁

I'm not sure if gccint is designed for normal users to read, but since
we lack a documentation about those descriptors in GCC user manual, I
guess many users will indeed use gccint as a reference ...

Ok to me.  But regarding the test case I suggest to keep those "%a"
tests there (so we won't inadvertently cause a regression in case some
user code already uses it for printing an integer).  Unless we
deliberately want to stop people from using "%a" for this purpose.
  
chenglulu Nov. 23, 2022, 9:30 a.m. UTC | #4
在 2022/11/23 17:25, Xi Ruoyao 写道:
> On Wed, 2022-11-23 at 17:14 +0800, chenglulu wrote:
>> 在 2022/11/23 16:59, Xi Ruoyao 写道:
>>> On Wed, 2022-11-23 at 14:49 +0800, Lulu Cheng wrote:
>>>>       'A' Print a _DB suffix if the memory model requires a
>>>> release.
>>>>       'b' Print the address of a memory operand, without offset.
>>>> +   'c'  print an integer.
>>> Nit:
>>>         'c' Print an integer.
>>>
>>> to match the format of other entries.
>>>
>>>>       'C' Print the integer branch condition for comparison OP.
>>>>       'd' Print CONST_INT OP in decimal.
>>>>       'F' Print the FPU branch condition for comparison OP.
>>> And I'd consider this a new feature and delay it to GCC 14: we never
>>> claimed we supported 'c' and it has not worked since the day one we
>>> merged LoongArch port.  Is there any emergency reason to support 'c'
>>> in
>>> GCC 13?
>>>
>> I don't think this is a new feature.
>>
>> There is a description of '%c' in section 17.5 of gccint.pdf, which I
>> understand is a public descriptor,
>>
>> but right now loongarch doesn't support it.🙁
> I'm not sure if gccint is designed for normal users to read, but since
> we lack a documentation about those descriptors in GCC user manual, I
> guess many users will indeed use gccint as a reference ...
>
> Ok to me.  But regarding the test case I suggest to keep those "%a"
> tests there (so we won't inadvertently cause a regression in case some
> user code already uses it for printing an integer).  Unless we
> deliberately want to stop people from using "%a" for this purpose.

Ok thanks! I'll be making changes to the patch.

I think I'll have to go and supplement the documentation.
  

Patch

diff --git a/gcc/config/loongarch/loongarch.cc b/gcc/config/loongarch/loongarch.cc
index 8d5d8d965dd..7f02b0bab23 100644
--- a/gcc/config/loongarch/loongarch.cc
+++ b/gcc/config/loongarch/loongarch.cc
@@ -2029,6 +2029,11 @@  loongarch_classify_address (struct loongarch_address_info *info, rtx x,
       return (loongarch_valid_base_register_p (info->reg, mode, strict_p)
 	      && loongarch_valid_lo_sum_p (info->symbol_type, mode,
 					   info->offset));
+    case CONST_INT:
+      /* Small-integer addresses don't occur very often, but they
+	 are legitimate if $r0 is a valid base register.  */
+      info->type = ADDRESS_CONST_INT;
+      return IMM12_OPERAND (INTVAL (x));
 
     default:
       return false;
@@ -4889,6 +4894,7 @@  loongarch_print_operand_reloc (FILE *file, rtx op, bool hi64_part,
 
    'A'	Print a _DB suffix if the memory model requires a release.
    'b'	Print the address of a memory operand, without offset.
+   'c'  print an integer.
    'C'	Print the integer branch condition for comparison OP.
    'd'	Print CONST_INT OP in decimal.
    'F'	Print the FPU branch condition for comparison OP.
@@ -4935,6 +4941,14 @@  loongarch_print_operand (FILE *file, rtx op, int letter)
        fputs ("_db", file);
       break;
 
+    case 'c':
+      if (CONST_INT_P (op))
+	fprintf (file, HOST_WIDE_INT_PRINT_DEC, INTVAL (op));
+      else
+	output_operand_lossage ("unsupported operand for code '%c'", letter);
+
+      break;
+
     case 'C':
       loongarch_print_int_branch_condition (file, code, letter);
       break;
diff --git a/gcc/testsuite/gcc.target/loongarch/tst-asm-const.c b/gcc/testsuite/gcc.target/loongarch/pr107731.c
similarity index 78%
rename from gcc/testsuite/gcc.target/loongarch/tst-asm-const.c
rename to gcc/testsuite/gcc.target/loongarch/pr107731.c
index 2e04b99e301..80d84c48c6e 100644
--- a/gcc/testsuite/gcc.target/loongarch/tst-asm-const.c
+++ b/gcc/testsuite/gcc.target/loongarch/pr107731.c
@@ -1,13 +1,13 @@ 
-/* Test asm const. */
 /* { dg-do compile } */
 /* { dg-final { scan-assembler-times "foo:.*\\.long 1061109567.*\\.long 52" 1 } } */
+
 int foo ()
 {
   __asm__ volatile (
           "foo:"
           "\n\t"
-	  ".long %a0\n\t"
-	  ".long %a1\n\t"
+	  ".long %c0\n\t"
+	  ".long %c1\n\t"
 	  :
 	  :"i"(0x3f3f3f3f), "i"(52)
 	  :