Patchwork Fix PR53908

login
register
mail settings
Submitter Bernd Schmidt
Date July 11, 2012, 11:24 a.m.
Message ID <4FFD627E.8070207@codesourcery.com>
Download mbox | patch
Permalink /patch/170429/
State New
Headers show

Comments

Bernd Schmidt - July 11, 2012, 11:24 a.m.
We're moving a load across a call since we don't recognize calls as
memory-clobbering.

Bootstrapping and testing now on 4.7 x86_64-linux, ok everywhere?


Bernd
Steven Bosscher - July 11, 2012, 6:49 p.m.
On Wed, Jul 11, 2012 at 1:24 PM, Bernd Schmidt <bernds@codesourcery.com> wrote:
> We're moving a load across a call since we don't recognize calls as
> memory-clobbering.
>
> Bootstrapping and testing now on 4.7 x86_64-linux, ok everywhere?

Maybe:
+      if (CALL_P (insn)
+         && ! RTL_CONST_OR_PURE_CALL_P (insn))

?

Ciao!
Steven
Richard Sandiford - July 12, 2012, 7:18 p.m.
Steven Bosscher <stevenb.gcc@gmail.com> writes:
> On Wed, Jul 11, 2012 at 1:24 PM, Bernd Schmidt <bernds@codesourcery.com> wrote>> We're moving a load across a call since we don't recognize calls as
>> memory-clobbering.
>>
>> Bootstrapping and testing now on 4.7 x86_64-linux, ok everywhere?
>
> Maybe:
> +      if (CALL_P (insn)
> +         && ! RTL_CONST_OR_PURE_CALL_P (insn))

AIUI pure functions can read memory, and const functions can read
arguments that the ABI says must be passed on the stack.  So it sounds
like it should be something like:

      if (CALL_P (insn))
	{
	  if (RTL_CONST_OR_PURE_CALL_P (insn))
	    /* Pure functions can read from memory.  Const functions can
	       read from arguments that the ABI has forced onto the stack.
	       Neither sort of read can be volatile.  */
	    memrefs_in_across |= MEMREF_NORMAL;
	  else
	    {
	      memrefs_in_across |= MEMREF_VOLATILE;
	      mem_sets_in_across |= MEMREF_VOLATILE;
	    }
	}

OK with that change if you agree.

Richard
Steven Bosscher - July 13, 2012, 7:21 a.m.
On Fri, Jul 13, 2012 at 8:47 AM, Hans-Peter Nilsson
<hans-peter.nilsson@axis.com> wrote:
>> From: Richard Sandiford <rsandifo@nildram.co.uk>
>> Date: Thu, 12 Jul 2012 21:18:54 +0200
>
>>       if (CALL_P (insn))
>>       {
>>         if (RTL_CONST_OR_PURE_CALL_P (insn))
>>           /* Pure functions can read from memory.  Const functions can
>>              read from arguments that the ABI has forced onto the stack.
>>              Neither sort of read can be volatile.  */
>>           memrefs_in_across |= MEMREF_NORMAL;
>>         else
>>           {
>>             memrefs_in_across |= MEMREF_VOLATILE;
>>             mem_sets_in_across |= MEMREF_VOLATILE;
>>           }
>>       }
>>
>> OK with that change if you agree.
>
> (Steven or Bernd, please ACK/NAK, for quick resolution.)

I can't ACK, but I think Richard S.'s fix is correct.

Ciao!
Steven
Richard Guenther - July 13, 2012, 7:37 a.m.
On Fri, Jul 13, 2012 at 9:21 AM, Steven Bosscher <stevenb.gcc@gmail.com> wrote:
> On Fri, Jul 13, 2012 at 8:47 AM, Hans-Peter Nilsson
> <hans-peter.nilsson@axis.com> wrote:
>>> From: Richard Sandiford <rsandifo@nildram.co.uk>
>>> Date: Thu, 12 Jul 2012 21:18:54 +0200
>>
>>>       if (CALL_P (insn))
>>>       {
>>>         if (RTL_CONST_OR_PURE_CALL_P (insn))
>>>           /* Pure functions can read from memory.  Const functions can
>>>              read from arguments that the ABI has forced onto the stack.
>>>              Neither sort of read can be volatile.  */
>>>           memrefs_in_across |= MEMREF_NORMAL;
>>>         else
>>>           {
>>>             memrefs_in_across |= MEMREF_VOLATILE;
>>>             mem_sets_in_across |= MEMREF_VOLATILE;
>>>           }
>>>       }
>>>
>>> OK with that change if you agree.
>>
>> (Steven or Bernd, please ACK/NAK, for quick resolution.)
>
> I can't ACK, but I think Richard S.'s fix is correct.

I agree, so, ok for trunk if it passes bootstrap & testing.

Thanks,
Richard.

> Ciao!
> Steven
Richard Guenther - July 13, 2012, 8:08 a.m.
On Fri, Jul 13, 2012 at 9:59 AM, Hans-Peter Nilsson
<hans-peter.nilsson@axis.com> wrote:
>> From: Richard Guenther <richard.guenther@gmail.com>
>> Date: Fri, 13 Jul 2012 09:37:13 +0200
>
>> On Fri, Jul 13, 2012 at 9:21 AM, Steven Bosscher <stevenb.gcc@gmail.com> wrote:
>> > On Fri, Jul 13, 2012 at 8:47 AM, Hans-Peter Nilsson
>> > <hans-peter.nilsson@axis.com> wrote:
>> >>> From: Richard Sandiford <rsandifo@nildram.co.uk>
>> >>> Date: Thu, 12 Jul 2012 21:18:54 +0200
>> >>> OK with that change if you agree.
>> >>
>> >> (Steven or Bernd, please ACK/NAK, for quick resolution.)
>> >
>> > I can't ACK, but I think Richard S.'s fix is correct.
>
> (There was a delegation from a maintainer so yes.)
>
>> I agree, so, ok for trunk if it passes bootstrap & testing.
>
> Ok for 4.7 too?

Of course.

Richard.

> brgds, H-P

Patch

	PR rtl-optimization/53908
	* df-problems.c (can_move_insns_across): Calls can clobber memory.

Index: gcc/df-problems.c
===================================================================
--- gcc/df-problems.c	(revision 189425)
+++ gcc/df-problems.c	(working copy)
@@ -3961,6 +3961,11 @@  can_move_insns_across (rtx from, rtx to,
 
   for (insn = across_to; ; insn = next)
     {
+      if (CALL_P (insn))
+	{
+	  memrefs_in_across |= MEMREF_VOLATILE;
+	  mem_sets_in_across |= MEMREF_VOLATILE;
+	}
       if (NONDEBUG_INSN_P (insn))
 	{
 	  memrefs_in_across |= for_each_rtx (&PATTERN (insn), find_memory,