Re: [RFC][PATCH][MIPS] Patch to enable LRA for MIPS backend

[Richard Sandiford <rdsandiford at googlemail dot com>]

Richard Sandiford <rdsandiford@googlemail.com> writes:
> Robert Suchanek <Robert.Suchanek@imgtec.com> writes:
>> I'm not particularly happy with this either. This was an attempt to fix an ICE for 
>> the following RTL (gcc.dg/torture/asm-subreg-1.c compiled with -mips32r2 -mips16 -O1):
>>
>> (insn 9 8 0 2 (asm_operands/v ("") ("") 0 [
>>             (mem/v/j/c:HI (lo_sum:SI (const:SI (unspec:SI [
>>                                 (const_int 0 [0])
>>                             ] UNSPEC_GP))
>>                     (symbol_ref:SI ("_const_32") [flags 0x6]  <var_decl 0x7f50acd17558 _const_32>)) [0 _const_32+0 S2 A32])]
>>  [(asm_input:HI ("X") (null):0)]
>>  [] asm-subreg-1.c:13) asm-subreg-1.c:13 -1 (nil))
>>
>> Any suggestions to handle this case?
>
> Thanks for the pointer.  I think this shows a more fundamental problem
> with the handling of "X" constraints.  With something like:
>
> void
> foo (int **x, int y, int z)
> {
>   int *ptr = *x + y * z / 11;
>   __asm__ __volatile__ ("" : : "X" (*ptr));
> }
>
> the entire expression gets treated as a MEM address, which neither
> reload nor LRA can handle.  And with something like that, it isn't
> obvious what class all the registers in the address should have.
> With a sufficiently-complicated expression you could run out of registers.
>
> So perhaps we should limit the propagation to things that
> decompose_mem_address can handle.

Even that might be too loose, since invalid scales will need to be reloaded
as a multiplication or shift, and there's no guarantee that the target
can do that without clobbering the flags.  So maybe we should do something
like the patch below.

Alternatively we could stick to the decompose_mem_address-based check
above and teach LRA to keep invalid addresses for 'X'.  The problem then
is that we might ICE while printing the operand.

Thanks,
Richard


gcc/
	* recog.c (asm_operand_ok): Tighten MEM validity for 'X'.

gcc/testsuite/
	* gcc.dg/torture/asm-x-constraint-1.c: New test.

Index: gcc/recog.c
===================================================================
--- gcc/recog.c	2014-04-10 21:18:02.778009424 +0100
+++ gcc/recog.c	2014-04-10 21:18:02.996011570 +0100
@@ -1840,7 +1840,11 @@ asm_operand_ok (rtx op, const char *cons
 	  break;
 
 	case 'X':
-	  result = 1;
+	  /* Still enforce memory requirements for non-constant addresses,
+	     since we can't reload MEMs with completely arbitrary addresses.  */
+	  result = (!MEM_P (op)
+		    || CONSTANT_P (XEXP (op, 0))
+		    || memory_operand (op, VOIDmode));
 	  break;
 
 	case 'g':
Index: gcc/testsuite/gcc.dg/torture/asm-x-constraint-1.c
===================================================================
--- /dev/null	2014-04-10 19:40:00.640011981 +0100
+++ gcc/testsuite/gcc.dg/torture/asm-x-constraint-1.c	2014-04-10 21:19:05.405623027 +0100
@@ -0,0 +1,6 @@
+void
+foo (int **x, int y, int z)
+{
+  int *ptr = *x + y * z / 11;
+  __asm__ __volatile__ ("foo %0" : : "X" (*ptr));
+}