patch to fix PR57559 for s390

patch to fix PR57559 for s390

[Vladimir Makarov]

[-- Attachment #1: Type: text/plain, Size: 851 bytes --]

The following patch fixes

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57559

The problem was in systematic discouraging reload symbol address in 
memory.  The patch actually reverts a fix for a x86/x86-64 PR for 
-fpie.  The patch implements another solution for this problem (see 
changes for lra_emit_add).

The patch was successfully bootstrapped and tested on x86/x86-64 and s390.

Committed as rev. 199859.

2013-06-08  Vladimir Makarov  <vmakarov@redhat.com>

         PR rtl-optimization/57559
         * lra-constraints.c (process_alt_operands): Don't discourage
         memory with known offset for offsetable memory constraint.
         * lra.c (lra_emit_add): Exchange y and z for 2-op add insn.

2013-06-08  Vladimir Makarov  <vmakarov@redhat.com>

         PR rtl-optimization/57559
         * gcc.target/s390/pr57559.c : New    test.


[-- Attachment #2: pr57559.patch --]
[-- Type: text/plain, Size: 2261 bytes --]

Index: lra-constraints.c
===================================================================
--- lra-constraints.c	(revision 199762)
+++ lra-constraints.c	(working copy)
@@ -1997,15 +1997,8 @@ process_alt_operands (int only_alternati
 			      (op, this_alternative) == NO_REGS))))
 		reject += LRA_MAX_REJECT;
 
-	      if (MEM_P (op) && offmemok)
-		{
-		  /* If we know offset and this non-offsetable memory,
-		     something wrong with this memory and it is better
-		     to try other memory possibilities.  */
-		  if (MEM_OFFSET_KNOWN_P (op))
-		    reject += LRA_MAX_REJECT;
-		}
-	      else if (! (const_to_mem && constmemok))
+	      if (! (MEM_P (op) && offmemok)
+		  && ! (const_to_mem && constmemok))
 		{
 		  /* We prefer to reload pseudos over reloading other
 		     things, since such reloads may be able to be
Index: lra.c
===================================================================
--- lra.c	(revision 199753)
+++ lra.c	(working copy)
@@ -306,11 +306,11 @@ lra_emit_add (rtx x, rtx y, rtx z)
 	  || (disp != NULL_RTX && ! CONSTANT_P (disp))
 	  || (scale != NULL_RTX && ! CONSTANT_P (scale)))
 	{
-	  /* Its is not an address generation.	Probably we have no 3 op
+	  /* It is not an address generation.	Probably we have no 3 op
 	     add.  Last chance is to use 2-op add insn.	 */
 	  lra_assert (x != y && x != z);
-	  emit_move_insn (x, z);
-	  insn = gen_add2_insn (x, y);
+	  emit_move_insn (x, y);
+	  insn = gen_add2_insn (x, z);
 	  emit_insn (insn);
 	}
       else
Index: testsuite/gcc.target/s390/pr57559.c
===================================================================
--- testsuite/gcc.target/s390/pr57559.c	(revision 0)
+++ testsuite/gcc.target/s390/pr57559.c	(working copy)
@@ -0,0 +1,24 @@
+/* PR rtl-optimization/57559  */
+
+/* { dg-do compile } */
+/* { dg-options "-march=z10 -m64 -mzarch  -O1" } */
+
+typedef int int32_t;
+typedef unsigned char uint8_t;
+typedef unsigned int uint32_t;
+struct _IO_marker
+{
+};
+static const int32_t mfcone = 1;
+static const uint8_t *mfctop = (const uint8_t *) &mfcone;
+int32_t
+decContextTestEndian (uint8_t quiet)
+{
+  int32_t res = 0;
+  uint32_t dle = (uint32_t) 0;
+  if (*(int *) 10 != 0)
+    {
+      res = (int32_t) * mfctop - dle;
+    }
+  return res;
+}

Re: patch to fix PR57559 for s390

[Richard Sandiford]

Vladimir Makarov <vmakarov@redhat.com> writes:
> Index: lra.c
> ===================================================================
> --- lra.c	(revision 199753)
> +++ lra.c	(working copy)
> @@ -306,11 +306,11 @@ lra_emit_add (rtx x, rtx y, rtx z)
>  	  || (disp != NULL_RTX && ! CONSTANT_P (disp))
>  	  || (scale != NULL_RTX && ! CONSTANT_P (scale)))
>  	{
> -	  /* Its is not an address generation.	Probably we have no 3 op
> +	  /* It is not an address generation.	Probably we have no 3 op
>  	     add.  Last chance is to use 2-op add insn.	 */
>  	  lra_assert (x != y && x != z);
> -	  emit_move_insn (x, z);
> -	  insn = gen_add2_insn (x, y);
> +	  emit_move_insn (x, y);
> +	  insn = gen_add2_insn (x, z);
>  	  emit_insn (insn);
>  	}
>        else

Could you add a comment to lra_emit_add saying why it has to be this
way round (move y, add z)?

Thanks,
Richard

Re: patch to fix PR57559 for s390

[Richard Sandiford]

Richard Sandiford <rdsandiford@googlemail.com> writes:
> Vladimir Makarov <vmakarov@redhat.com> writes:
>> Index: lra.c
>> ===================================================================
>> --- lra.c	(revision 199753)
>> +++ lra.c	(working copy)
>> @@ -306,11 +306,11 @@ lra_emit_add (rtx x, rtx y, rtx z)
>>  	  || (disp != NULL_RTX && ! CONSTANT_P (disp))
>>  	  || (scale != NULL_RTX && ! CONSTANT_P (scale)))
>>  	{
>> -	  /* Its is not an address generation.	Probably we have no 3 op
>> +	  /* It is not an address generation.	Probably we have no 3 op
>>  	     add.  Last chance is to use 2-op add insn.	 */
>>  	  lra_assert (x != y && x != z);
>> -	  emit_move_insn (x, z);
>> -	  insn = gen_add2_insn (x, y);
>> +	  emit_move_insn (x, y);
>> +	  insn = gen_add2_insn (x, z);
>>  	  emit_insn (insn);
>>  	}
>>        else
>
> Could you add a comment to lra_emit_add saying why it has to be this
> way round (move y, add z)?

Ping.

Re: patch to fix PR57559 for s390

[Vladimir Makarov]

On 13-06-19 2:31 PM, Richard Sandiford wrote:
> Richard Sandiford <rdsandiford@googlemail.com> writes:
>> Vladimir Makarov <vmakarov@redhat.com> writes:
>>> Index: lra.c
>>> ===================================================================
>>> --- lra.c	(revision 199753)
>>> +++ lra.c	(working copy)
>>> @@ -306,11 +306,11 @@ lra_emit_add (rtx x, rtx y, rtx z)
>>>   	  || (disp != NULL_RTX && ! CONSTANT_P (disp))
>>>   	  || (scale != NULL_RTX && ! CONSTANT_P (scale)))
>>>   	{
>>> -	  /* Its is not an address generation.	Probably we have no 3 op
>>> +	  /* It is not an address generation.	Probably we have no 3 op
>>>   	     add.  Last chance is to use 2-op add insn.	 */
>>>   	  lra_assert (x != y && x != z);
>>> -	  emit_move_insn (x, z);
>>> -	  insn = gen_add2_insn (x, y);
>>> +	  emit_move_insn (x, y);
>>> +	  insn = gen_add2_insn (x, z);
>>>   	  emit_insn (insn);
>>>   	}
>>>         else
>> Could you add a comment to lra_emit_add saying why it has to be this
>> way round (move y, add z)?
> Ping.
I am going to add a comment when I submit my next patch (it will happen 
today or tomorrow).  The reason is simple as address segment is stored 
in y not in z and generation of addition of address segment to pseudo 
can fail (that is what happens for the PR).


Thanks, Richard.

Re: patch to fix PR57559 for s390

[Richard Sandiford]

Vladimir Makarov <vmakarov@redhat.com> writes:
> On 13-06-19 2:31 PM, Richard Sandiford wrote:
>> Richard Sandiford <rdsandiford@googlemail.com> writes:
>>> Vladimir Makarov <vmakarov@redhat.com> writes:
>>>> Index: lra.c
>>>> ===================================================================
>>>> --- lra.c	(revision 199753)
>>>> +++ lra.c	(working copy)
>>>> @@ -306,11 +306,11 @@ lra_emit_add (rtx x, rtx y, rtx z)
>>>>   	  || (disp != NULL_RTX && ! CONSTANT_P (disp))
>>>>   	  || (scale != NULL_RTX && ! CONSTANT_P (scale)))
>>>>   	{
>>>> -	  /* Its is not an address generation.	Probably we have no 3 op
>>>> +	  /* It is not an address generation.	Probably we have no 3 op
>>>>   	     add.  Last chance is to use 2-op add insn.	 */
>>>>   	  lra_assert (x != y && x != z);
>>>> -	  emit_move_insn (x, z);
>>>> -	  insn = gen_add2_insn (x, y);
>>>> +	  emit_move_insn (x, y);
>>>> +	  insn = gen_add2_insn (x, z);
>>>>   	  emit_insn (insn);
>>>>   	}
>>>>         else
>>> Could you add a comment to lra_emit_add saying why it has to be this
>>> way round (move y, add z)?
>> Ping.
> I am going to add a comment when I submit my next patch (it will happen 
> today or tomorrow).

Thanks.

> The reason is simple as address segment is stored in y not in z and
> generation of addition of address segment to pseudo can fail (that is
> what happens for the PR).

Do you mean address segment in the x86 sense of "segment"?  I was just
a bit confused because the current comment says "It is not an address
generation", whereas it sounds like addresses are involved somewhere.

I suppose the commutation rules are that Y should be "no less complicated"
than Z, so maybe it wins from that point of view too.

Richard