[patch] PR 33653: don't optimize volatile mem accesses

[patch] PR 33653: don't optimize volatile mem accesses

[Michael Matz]

Hi,

dce.c and dse.c use the wrong predicates to check if an insn is 
undeletable because of volatile accesses.  They use volatile_insn_p(), 
whereas they should use volatile_refs_p() (like the old flow code did).

I've started testing.  Okay for trunk?  How can I best check for the 
existence of the memory access in the testsuite?

The testcase looks like so:

void f (volatile char *p)
{
  char c = p[0];
}

Is it possible in our testsuite to check that RTL doesn't optimize away 
the volatile read?


Ciao,
Michael.
--
	* dce.c (deletable_insn_p_1): Use volatile_refs_p().
	* dse.c (scan_insn): Same.

Index: dce.c
===================================================================
--- dce.c	(revision 129000)
+++ dce.c	(working copy)
@@ -78,7 +78,7 @@ deletable_insn_p_1 (rtx body)
       return false;
 
     default:
-      if (volatile_insn_p (body))
+      if (volatile_refs_p (body))
 	return false;
 
       if (flag_non_call_exceptions && may_trap_p (body))
Index: dse.c
===================================================================
--- dse.c	(revision 129000)
+++ dse.c	(working copy)
@@ -1997,7 +1997,7 @@ scan_insn (bb_info_t bb_info, rtx insn)
   /* Assuming that there are sets in these insns, we cannot delete
      them.  */
   if ((GET_CODE (PATTERN (insn)) == CLOBBER)
-      || volatile_insn_p (PATTERN (insn))
+      || volatile_refs_p (PATTERN (insn))
       || (flag_non_call_exceptions && may_trap_p (PATTERN (insn)))
       || (RTX_FRAME_RELATED_P (insn))
       || find_reg_note (insn, REG_FRAME_RELATED_EXPR, NULL_RTX))

Re: [patch] PR 33653: don't optimize volatile mem accesses

[Eric Botcazou]

> dce.c and dse.c use the wrong predicates to check if an insn is
> undeletable because of volatile accesses.  They use volatile_insn_p(),
> whereas they should use volatile_refs_p() (like the old flow code did).
>
> I've started testing.  Okay for trunk?

Obvious I'd think, but nevertheless OK. :-)

> Is it possible in our testsuite to check that RTL doesn't optimize away
> the volatile read?

By testing that we have a non-null frame at -O or above?

-- 
Eric Botcazou

Re: [patch] PR 33653: don't optimize volatile mem accesses

[Paolo Bonzini]

> Is it possible in our testsuite to check that RTL doesn't optimize away 
> the volatile read?

You could make it p[5678] and scan RTL dumps for 5678?

Paolo

Re: [patch] PR 33653: don't optimize volatile mem accesses

[Michael Matz]

Hi,

On Thu, 4 Oct 2007, Paolo Bonzini wrote:

> > Is it possible in our testsuite to check that RTL doesn't optimize 
> > away the volatile read?
> 
> You could make it p[5678] and scan RTL dumps for 5678?

I like that idea.  But it doesn't seem to work for machines which split 
immediates.  I now think (after trying with signals, which would require 
limiting the number of test platforms) that it's enough to simply check 
for "mem/v" in any late dump.  I chose "shorten" as it seems to be the 
latest one which is run on all platforms.

So, I'll use the below as gcc.dg/pr33653.c.


Ciao,
Michael.

/* { dg-do compile } */
/* { dg-options "-O2 -fdump-rtl-shorten" } */

void f (volatile char *p)
{
  char c = p[0];
}

/* { dg-final { scan-rtl-dump "mem/v" "shorten" } } */
/* { dg-final { cleanup-rtl-dump "shorten" } } */

Re: [patch] PR 33653: don't optimize volatile mem accesses

[Paolo Bonzini]

>>> Is it possible in our testsuite to check that RTL doesn't optimize 
>>> away the volatile read?
>> You could make it p[5678] and scan RTL dumps for 5678?
> 
> I like that idea.  But it doesn't seem to work for machines which split 
> immediates.  I now think (after trying with signals, which would require 
> limiting the number of test platforms) that it's enough to simply check 
> for "mem/v" in any late dump.  I chose "shorten" as it seems to be the 
> latest one which is run on all platforms.

Yeah, that should also work.  I would have thought that the 5678 would 
be still present in some REG_EQUAL note.

Paolo

Re: [patch] PR 33653: don't optimize volatile mem accesses

[Michael Matz]

Hi,

On Thu, 4 Oct 2007, Paolo Bonzini wrote:

> 
> > > > Is it possible in our testsuite to check that RTL doesn't optimize away
> > > > the volatile read?
> > > You could make it p[5678] and scan RTL dumps for 5678?
> > 
> > I like that idea.  But it doesn't seem to work for machines which split
> > immediates.  I now think (after trying with signals, which would require
> > limiting the number of test platforms) that it's enough to simply check for
> > "mem/v" in any late dump.  I chose "shorten" as it seems to be the latest
> > one which is run on all platforms.
> 
> Yeah, that should also work.  I would have thought that the 5678 would be
> still present in some REG_EQUAL note.

Yes, that's what I also thought initially, but on ppc 56789 is constructed 
very early (cse1 or fwprop1) as 0x10000 - 8747, without any REG_EQUAL 
notes.


Ciao,
Michael.

Re: [patch] PR 33653: don't optimize volatile mem accesses

[Michael Matz]

Hi,

On Thu, 4 Oct 2007, Michael Matz wrote:

> So, I'll use the below as gcc.dg/pr33653.c.

bootstraped/regtested on x86_64-linux, checked in.


Ciao,
Michael.