Re: RFA: Fix rtl-optimization/57425 (SPEC perl problem on MIPS)

2013-07-12 Thread Steve Ellcey 
Joern,

Has anyone reported any problems to you about this patch?  I am running into
a problem running the perl benchmark from SPEC2006 and I have tracked it down
to this June 16 patch (SVN 200133,
GIT ddba76b84c757d93b4247713d558724776149b62).  I am building a GCC cross
compiler running on x86 linux and targeting mips-mti-linux-gnu.

I haven't been able to cut down perl into a smaller test case yet but starting
with this version of GCC, if I build perl with -O2 or -O3 and then run it with
this perl input:

% cat x.pl
#!./perl
{
eval {
use Math::BigInt;
$y = pack('w*', Math::BigInt::-new(50));
};
}
1;


I get:

% ./perlbench_base.sellcey_reload x.pl
*** Error in `./perlbench_base.sellcey_reload': malloc(): memory corruption 
(fast): 0x00642390 ***

and perl seems to go into an infinite loop.  Before your change perl
ran and exited cleanly.  I will try to create a smaller example but I wanted
to see if you (or someone else on gcc-patches) was seeing anything like
this on any other targets.

Steve Ellcey
sell...@mips.com

Re: RFA: Fix rtl-optimization/57425

2013-06-20 Thread Richard Biener 
On Wed, Jun 19, 2013 at 9:58 PM, Joern Rennecke
joern.renne...@embecosm.com wrote:
 Quoting Michael Matz m...@suse.de:

 That's not good.  You now have different order of parameters between
 anti_dependence and canon_anti_dependence.  That will be mightily
 confusing, please instead change the caller.  Currently these predicates
 take their arguments in the order of the corresponding instructions, that
 should better be retained:

 true_dependence:  write-then-(depending)read
 anti_dependence:  read-then-(clobbering)write
 write_dependence: write-then-(clobbering)write


 All right, attached is the patch with the arguments in instruction-order.
 Again, bootstrapped/regtested on i686-pc-linux-gnu .

Ok.

Thanks,
Richard.

 2013-06-19  Joern Rennecke joern.renne...@embecosm.com

 PR rtl-optimization/57425
 PR rtl-optimization/57569
 * alias.c (write_dependence_p): Remove parameters mem_mode and
 canon_mem_addr.  Add parameters x_mode, x_addr and x_canonicalized.
 Changed all callers.
 (canon_anti_dependence): Get comments and semantics in sync.
 Add parameter mem_canonicalized.  Changed all callers.
 * rtl.h (canon_anti_dependence): Update prototype.

 Index: alias.c
 ===
 --- alias.c (revision 200133)
 +++ alias.c (working copy)
 @@ -156,8 +156,9 @@ static int insert_subset_children (splay
  static alias_set_entry get_alias_set_entry (alias_set_type);
  static bool nonoverlapping_component_refs_p (const_rtx, const_rtx);
  static tree decl_for_component_ref (tree);
 -static int write_dependence_p (const_rtx, enum machine_mode, rtx,
 const_rtx,
 -  bool, bool);
 +static int write_dependence_p (const_rtx,
 +  const_rtx, enum machine_mode, rtx,
 +  bool, bool, bool);

  static void memory_modified_1 (rtx, const_rtx, void *);

 @@ -2555,20 +2556,22 @@ canon_true_dependence (const_rtx mem, en

  /* Returns nonzero if a write to X might alias a previous read from
 (or, if WRITEP is true, a write to) MEM.
 -   If MEM_CANONCALIZED is nonzero, CANON_MEM_ADDR is the canonicalized
 -   address of MEM, and MEM_MODE the mode for that access.  */
 +   If X_CANONCALIZED is true, then X_ADDR is the canonicalized address of
 X,
 +   and X_MODE the mode for that access.
 +   If MEM_CANONICALIZED is true, MEM is canonicalized.  */

  static int
 -write_dependence_p (const_rtx mem, enum machine_mode mem_mode,
 -   rtx canon_mem_addr, const_rtx x,
 -   bool mem_canonicalized, bool writep)
 +write_dependence_p (const_rtx mem,
 +   const_rtx x, enum machine_mode x_mode, rtx x_addr,
 +   bool mem_canonicalized, bool x_canonicalized, bool
 writep)
  {
 -  rtx x_addr, mem_addr;
 +  rtx mem_addr;
rtx base;
int ret;

 -  gcc_checking_assert (mem_canonicalized ? (canon_mem_addr != NULL_RTX)
 -  : (canon_mem_addr == NULL_RTX  mem_mode ==
 VOIDmode));
 +  gcc_checking_assert (x_canonicalized
 +  ? (x_addr != NULL_RTX  x_mode != VOIDmode)
 +  : (x_addr == NULL_RTX  x_mode == VOIDmode));

if (MEM_VOLATILE_P (x)  MEM_VOLATILE_P (mem))
  return 1;
 @@ -2593,17 +2596,21 @@ write_dependence_p (const_rtx mem, enum
if (MEM_ADDR_SPACE (mem) != MEM_ADDR_SPACE (x))
  return 1;

 -  x_addr = XEXP (x, 0);
mem_addr = XEXP (mem, 0);
 -  if (!((GET_CODE (x_addr) == VALUE
 - GET_CODE (mem_addr) != VALUE
 - reg_mentioned_p (x_addr, mem_addr))
 -   || (GET_CODE (x_addr) != VALUE
 -GET_CODE (mem_addr) == VALUE
 -reg_mentioned_p (mem_addr, x_addr
 +  if (!x_addr)
  {
 -  x_addr = get_addr (x_addr);
 -  mem_addr = get_addr (mem_addr);
 +  x_addr = XEXP (x, 0);
 +  if (!((GET_CODE (x_addr) == VALUE
 + GET_CODE (mem_addr) != VALUE
 + reg_mentioned_p (x_addr, mem_addr))
 +   || (GET_CODE (x_addr) != VALUE
 +GET_CODE (mem_addr) == VALUE
 +reg_mentioned_p (mem_addr, x_addr
 +   {
 + x_addr = get_addr (x_addr);
 + if (!mem_canonicalized)
 +   mem_addr = get_addr (mem_addr);
 +   }
  }

base = find_base_term (mem_addr);
 @@ -2619,17 +2626,16 @@ write_dependence_p (const_rtx mem, enum
   GET_MODE (mem)))
  return 0;

 -  x_addr = canon_rtx (x_addr);
 -  if (mem_canonicalized)
 -mem_addr = canon_mem_addr;
 -  else
 +  if (!x_canonicalized)
  {
 -  mem_addr = canon_rtx (mem_addr);
 -  mem_mode = GET_MODE (mem);
 +  x_addr = canon_rtx (x_addr);
 +  x_mode = GET_MODE (x);
  }
 +  if (!mem_canonicalized)
 +mem_addr = canon_rtx (mem_addr);

 -  if ((ret = memrefs_conflict_p (GET_MODE_SIZE (mem_mode), mem_addr,
 -SIZE_FOR_MODE (x), x_addr,

Re: RFA: Fix rtl-optimization/57425

2013-06-19 Thread Joern Rennecke 

I.e. the arguments after your patch are exactly swapped.  This is usually
harmless, but not always, so that should be corrected before check in.
The change in cselib.c:cselib_invalidate_mem has the same problem.


Well, I have already committed the patch, so attached is a patch to fix
things up.
Looking at the read MEM canonicalization further, these are obviously
canonicalized in cse.c:invalidate, but this is not so clear in cselib.
I can see some canonicalization going on where things are recorded, but
there is no comment in cselib.h:cselib_val nor cselib.c:first_containing_mem
to say that it's guaranteed that all the expression in the locs lists are
canonicalized, so I'll assume that's not a safe assumption, and I've
added an mem_canonicalized parameter to canon_anti_dependence to
indicate if MEM (the previously read location) is canonicalized.

Bootstrapped / regtested on i686-pc-linux-gnu.
2013-06-19  Joern Rennecke joern.renne...@embecosm.com

PR rtl-optimization/57425
PR rtl-optimization/57569
* alias.c (write_dependence_p): Remove parameters mem_mode and
canon_mem_addr.  Add parameters x_mode, x_addr and x_canonicalized.
Changed all callers.
(canon_anti_dependence): Get comments and semantics in sync,
to be a proper drop-in replacement for canon_true_dependence
for use by cse(lib).
Add parameter  mem_canonicalized.  Changed all callers.
* rtl.h (canon_anti_dependence): Update prototype.

Index: alias.c
===
--- alias.c (revision 200133)
+++ alias.c (working copy)
@@ -156,8 +156,9 @@ static int insert_subset_children (splay
 static alias_set_entry get_alias_set_entry (alias_set_type);
 static bool nonoverlapping_component_refs_p (const_rtx, const_rtx);
 static tree decl_for_component_ref (tree);
-static int write_dependence_p (const_rtx, enum machine_mode, rtx, const_rtx,
-  bool, bool);
+static int write_dependence_p (const_rtx,
+  const_rtx, enum machine_mode, rtx,
+  bool, bool, bool);
 
 static void memory_modified_1 (rtx, const_rtx, void *);
 
@@ -2555,20 +2556,22 @@ canon_true_dependence (const_rtx mem, en
 
 /* Returns nonzero if a write to X might alias a previous read from
(or, if WRITEP is true, a write to) MEM.
-   If MEM_CANONCALIZED is nonzero, CANON_MEM_ADDR is the canonicalized
-   address of MEM, and MEM_MODE the mode for that access.  */
+   If X_CANONCALIZED is nonzero, then X_ADDR is the canonicalized address of X,
+   and X_MODE the mode for that access.
+   If MEM_CANONICALIZED is true, MEM is canonicalized.  */
 
 static int
-write_dependence_p (const_rtx mem, enum machine_mode mem_mode,
-   rtx canon_mem_addr, const_rtx x,
-   bool mem_canonicalized, bool writep)
+write_dependence_p (const_rtx mem,
+   const_rtx x, enum machine_mode x_mode, rtx x_addr,
+   bool mem_canonicalized, bool x_canonicalized, bool writep)
 {
-  rtx x_addr, mem_addr;
+  rtx mem_addr;
   rtx base;
   int ret;
 
-  gcc_checking_assert (mem_canonicalized ? (canon_mem_addr != NULL_RTX)
-  : (canon_mem_addr == NULL_RTX  mem_mode == VOIDmode));
+  gcc_checking_assert (x_canonicalized
+  ? (x_addr != NULL_RTX  x_mode != VOIDmode)
+  : (x_addr == NULL_RTX  x_mode == VOIDmode));
 
   if (MEM_VOLATILE_P (x)  MEM_VOLATILE_P (mem))
 return 1;
@@ -2593,17 +2596,21 @@ write_dependence_p (const_rtx mem, enum
   if (MEM_ADDR_SPACE (mem) != MEM_ADDR_SPACE (x))
 return 1;
 
-  x_addr = XEXP (x, 0);
   mem_addr = XEXP (mem, 0);
-  if (!((GET_CODE (x_addr) == VALUE
- GET_CODE (mem_addr) != VALUE
- reg_mentioned_p (x_addr, mem_addr))
-   || (GET_CODE (x_addr) != VALUE
-GET_CODE (mem_addr) == VALUE
-reg_mentioned_p (mem_addr, x_addr
+  if (!x_addr)
 {
-  x_addr = get_addr (x_addr);
-  mem_addr = get_addr (mem_addr);
+  x_addr = XEXP (x, 0);
+  if (!((GET_CODE (x_addr) == VALUE
+ GET_CODE (mem_addr) != VALUE
+ reg_mentioned_p (x_addr, mem_addr))
+   || (GET_CODE (x_addr) != VALUE
+GET_CODE (mem_addr) == VALUE
+reg_mentioned_p (mem_addr, x_addr
+   {
+ x_addr = get_addr (x_addr);
+ if (!mem_canonicalized)
+   mem_addr = get_addr (mem_addr);
+   }
 }
 
   base = find_base_term (mem_addr);
@@ -2619,17 +2626,16 @@ write_dependence_p (const_rtx mem, enum
  GET_MODE (mem)))
 return 0;
 
-  x_addr = canon_rtx (x_addr);
-  if (mem_canonicalized)
-mem_addr = canon_mem_addr;
-  else
+  if (!x_canonicalized)
 {
-  mem_addr = canon_rtx (mem_addr);
-  mem_mode = GET_MODE (mem);
+  x_addr = canon_rtx (x_addr);
+  x_mode = GET_MODE (x);
 }
+  if

Re: RFA: Fix rtl-optimization/57425

2013-06-19 Thread Michael Matz 
On Wed, 19 Jun 2013, Joern Rennecke wrote:

  I.e. the arguments after your patch are exactly swapped.  This is usually
  harmless, but not always, so that should be corrected before check in.
  The change in cselib.c:cselib_invalidate_mem has the same problem.
 
 Well, I have already committed the patch, so attached is a patch to fix
 things up.


  int  
   
  anti_dependence (const_rtx mem, const_rtx x) 
   
  {
   

...

  int  
   
 -canon_anti_dependence (const_rtx mem, enum machine_mode mem_mode,
   
 -  rtx mem_addr, const_rtx x) 
   
 +canon_anti_dependence (const_rtx x, enum machine_mode x_mode, rtx x_addr,
   
 +  const_rtx mem, bool mem_canonicalized) 
   
 {  

That's not good.  You now have different order of parameters between 
anti_dependence and canon_anti_dependence.  That will be mightily 
confusing, please instead change the caller.  Currently these predicates 
take their arguments in the order of the corresponding instructions, that 
should better be retained:

true_dependence:  write-then-(depending)read
anti_dependence:  read-then-(clobbering)write
write_dependence: write-then-(clobbering)write

We could change the order of arguments to something else, like first the 
clobber, then the clobbered, but then that should be done for all the 
predicates at the same time (and I would suggest to not do it).


Ciao,
Michael.

Re: RFA: Fix rtl-optimization/57425

2013-06-19 Thread Joern Rennecke 

Quoting Michael Matz m...@suse.de:


That's not good.  You now have different order of parameters between
anti_dependence and canon_anti_dependence.  That will be mightily
confusing, please instead change the caller.  Currently these predicates
take their arguments in the order of the corresponding instructions, that
should better be retained:

true_dependence:  write-then-(depending)read
anti_dependence:  read-then-(clobbering)write
write_dependence: write-then-(clobbering)write


All right, attached is the patch with the arguments in instruction-order.
Again, bootstrapped/regtested on i686-pc-linux-gnu .
2013-06-19  Joern Rennecke joern.renne...@embecosm.com

PR rtl-optimization/57425
PR rtl-optimization/57569
* alias.c (write_dependence_p): Remove parameters mem_mode and
canon_mem_addr.  Add parameters x_mode, x_addr and x_canonicalized.
Changed all callers.
(canon_anti_dependence): Get comments and semantics in sync.
Add parameter mem_canonicalized.  Changed all callers.
* rtl.h (canon_anti_dependence): Update prototype.

Index: alias.c
===
--- alias.c (revision 200133)
+++ alias.c (working copy)
@@ -156,8 +156,9 @@ static int insert_subset_children (splay
 static alias_set_entry get_alias_set_entry (alias_set_type);
 static bool nonoverlapping_component_refs_p (const_rtx, const_rtx);
 static tree decl_for_component_ref (tree);
-static int write_dependence_p (const_rtx, enum machine_mode, rtx, const_rtx,
-  bool, bool);
+static int write_dependence_p (const_rtx,
+  const_rtx, enum machine_mode, rtx,
+  bool, bool, bool);
 
 static void memory_modified_1 (rtx, const_rtx, void *);
 
@@ -2555,20 +2556,22 @@ canon_true_dependence (const_rtx mem, en
 
 /* Returns nonzero if a write to X might alias a previous read from
(or, if WRITEP is true, a write to) MEM.
-   If MEM_CANONCALIZED is nonzero, CANON_MEM_ADDR is the canonicalized
-   address of MEM, and MEM_MODE the mode for that access.  */
+   If X_CANONCALIZED is true, then X_ADDR is the canonicalized address of X,
+   and X_MODE the mode for that access.
+   If MEM_CANONICALIZED is true, MEM is canonicalized.  */
 
 static int
-write_dependence_p (const_rtx mem, enum machine_mode mem_mode,
-   rtx canon_mem_addr, const_rtx x,
-   bool mem_canonicalized, bool writep)
+write_dependence_p (const_rtx mem,
+   const_rtx x, enum machine_mode x_mode, rtx x_addr,
+   bool mem_canonicalized, bool x_canonicalized, bool writep)
 {
-  rtx x_addr, mem_addr;
+  rtx mem_addr;
   rtx base;
   int ret;
 
-  gcc_checking_assert (mem_canonicalized ? (canon_mem_addr != NULL_RTX)
-  : (canon_mem_addr == NULL_RTX  mem_mode == VOIDmode));
+  gcc_checking_assert (x_canonicalized
+  ? (x_addr != NULL_RTX  x_mode != VOIDmode)
+  : (x_addr == NULL_RTX  x_mode == VOIDmode));
 
   if (MEM_VOLATILE_P (x)  MEM_VOLATILE_P (mem))
 return 1;
@@ -2593,17 +2596,21 @@ write_dependence_p (const_rtx mem, enum
   if (MEM_ADDR_SPACE (mem) != MEM_ADDR_SPACE (x))
 return 1;
 
-  x_addr = XEXP (x, 0);
   mem_addr = XEXP (mem, 0);
-  if (!((GET_CODE (x_addr) == VALUE
- GET_CODE (mem_addr) != VALUE
- reg_mentioned_p (x_addr, mem_addr))
-   || (GET_CODE (x_addr) != VALUE
-GET_CODE (mem_addr) == VALUE
-reg_mentioned_p (mem_addr, x_addr
+  if (!x_addr)
 {
-  x_addr = get_addr (x_addr);
-  mem_addr = get_addr (mem_addr);
+  x_addr = XEXP (x, 0);
+  if (!((GET_CODE (x_addr) == VALUE
+ GET_CODE (mem_addr) != VALUE
+ reg_mentioned_p (x_addr, mem_addr))
+   || (GET_CODE (x_addr) != VALUE
+GET_CODE (mem_addr) == VALUE
+reg_mentioned_p (mem_addr, x_addr
+   {
+ x_addr = get_addr (x_addr);
+ if (!mem_canonicalized)
+   mem_addr = get_addr (mem_addr);
+   }
 }
 
   base = find_base_term (mem_addr);
@@ -2619,17 +2626,16 @@ write_dependence_p (const_rtx mem, enum
  GET_MODE (mem)))
 return 0;
 
-  x_addr = canon_rtx (x_addr);
-  if (mem_canonicalized)
-mem_addr = canon_mem_addr;
-  else
+  if (!x_canonicalized)
 {
-  mem_addr = canon_rtx (mem_addr);
-  mem_mode = GET_MODE (mem);
+  x_addr = canon_rtx (x_addr);
+  x_mode = GET_MODE (x);
 }
+  if (!mem_canonicalized)
+mem_addr = canon_rtx (mem_addr);
 
-  if ((ret = memrefs_conflict_p (GET_MODE_SIZE (mem_mode), mem_addr,
-SIZE_FOR_MODE (x), x_addr, 0)) != -1)
+  if ((ret = memrefs_conflict_p (SIZE_FOR_MODE (mem), mem_addr,
+GET_MODE_SIZE (x_mode), x_addr, 0)) != -1)
 return ret;
 
   if (nonoverlapping_memrefs_p (x,

Re: RFA: Fix rtl-optimization/57425

2013-06-18 Thread Michael Matz 
On Sun, 16 Jun 2013, Joern Rennecke wrote:

 Quoting Eric Botcazou ebotca...@adacore.com:
 
  Could you also check that your patch also fixes PR opt/57569 and, if so, add
  the reference to the ChangeLog as well as the testcase?
 
 Attached is what I'm currently testing. bootstrap on i686-pc-linux-gnu
 finished, now regtesting.
 On x86_64-pc-linux-gnu, bootstrap is still in progress  I also still have
 to verify if the pr57569.c testcase FAILs/PASSes for unpatched/patched
 svn trunk.

Careful.  The patch uses the wrong order of arguments in the call
to the new function:

  /* Returns nonzero if a write to X might alias a previous read from  
   
 -   (or, if WRITEP is nonzero, a write to) MEM.  */   
   
 +   (or, if WRITEP is true, a write to) MEM.  
   
 +   If MEM_CANONCALIZED is nonzero, CANON_MEM_ADDR is the canonicalized   
   
 +   address of MEM, and MEM_MODE the mode for that access.  */
   
   
   
  static int   
   
 -write_dependence_p (const_rtx mem, const_rtx x, int writep)  
   
 +write_dependence_p (const_rtx mem, enum machine_mode mem_mode,   
   
 +   rtx canon_mem_addr, const_rtx x,  
   
 +   bool mem_canonicalized, bool writep)

So, first argument is the thing in question (the one that might be 
clobbered), the second (or in the new interface the fourth) is the write 
that might clobber it ...

 /* Likewise, but we already have a canonicalized MEM_ADDR for MEM.
  
 +   Also, consider MEM in MEM_MODE (which might be from an enclosing  
   
 +   STRICT_LOW_PART / ZERO_EXTRACT).  */  
   
 + 
   
 +int  
   
 +canon_anti_dependence (const_rtx mem, enum machine_mode mem_mode,
   
 +  rtx mem_addr, const_rtx x) 
   
 +{
   
 +  return write_dependence_p (mem, mem_mode, mem_addr, x,  

... same here, first the read, then the potentially destroying write ...

if (*x  MEM_P (*x))  
   
 -return canon_true_dependence (d-exp, d-mode, d-addr, *x, NULL_RTX);   
   
 +return canon_anti_dependence (d-exp, d-mode, d-addr, *x); 
   
else

... but here you use the same order as for true_dependence, to cite from 
the function comment:

 /* True dependence: X is read after store in MEM takes place.  */
 int
 true_dependence (const_rtx mem, enum machine_mode mem_mode, const_rtx x)

So, first the potentially clobbering write, then the read.  And indeed in 
check_dependence d-exp is the write and x the read that is potentially 
clobbered.

I.e. the arguments after your patch are exactly swapped.  This is usually 
harmless, but not always, so that should be corrected before check in.
The change in cselib.c:cselib_invalidate_mem has the same problem.

Generally I would prefer simple interfaces to the query functions, 
dependence problems are hard enough to think about without functions 
needing four arguments.  Does it really save much to not canonicalize the 
mem address for some calls?


Ciao,
Michael.

Re: RFA: Fix rtl-optimization/57425

2013-06-18 Thread Joern Rennecke 

Quoting Michael Matz m...@suse.de:


So, first the potentially clobbering write, then the read.  And indeed in
check_dependence d-exp is the write and x the read that is potentially
clobbered.


Oops, you are right.  I got confused because what is X in cse.c:invalidate
ends up as d-exp in cse.c:check_dependence, and what is p-canon_exp in
cse.c:invalidate ends up as X in cse.c:check_dependence.

I was a bit puzzled why we could have a STRICT_LOW_PART for MEM.  Now it
makes more sense.

I also see now that both the read expression from the hash table and the
write address are canonicalized, so if we have a canon_* interface, we
want to use both.


Generally I would prefer simple interfaces to the query functions,
dependence problems are hard enough to think about without functions
needing four arguments.  Does it really save much to not canonicalize the
mem address for some calls?


I haven't measured it myself, but I suppose it must be there for a  
good reason.

Of course a simpler interface would be nice, however...
When cse encounters a write, it goes through the entire hash table of
expressions and checks dependencies.  So for (extended?) basic blocks with
lots of expressions  writes, we got O(n^2) dependency checks.

Re: RFA: Fix rtl-optimization/57425

2013-06-16 Thread Eric Botcazou 
 Bootstrapped/regtested on i686-pc-linux-gnu.

For the record, and as you diagnosed, the change proposed in
  http://gcc.gnu.org/ml/gcc-patches/2012-06/msg00367.html
means that we must now be very careful with memory dependency checking in the 
various RTL optimization passes.  Another example is PR rtl-opt/57569.

The patch is OK on principle but I think that we should use the same interface 
for write_dependence_p as for true_dependence_1, i.e. add a mem_mode parameter 
instead of a mem_size and add both mem_addr and mem_canonicalized (and since 
it doesn't seem that we need x_addr for now, let's set it aside).  Btw I agree 
that the interface is probably not optimal, but it has been there for a while.

Could you also check that your patch also fixes PR opt/57569 and, if so, add 
the reference to the ChangeLog as well as the testcase?

Thanks for working on this.

-- 
Eric Botcazou

Re: RFA: Fix rtl-optimization/57425

2013-06-16 Thread Joern Rennecke 

Quoting Eric Botcazou ebotca...@adacore.com:

The patch is OK on principle but I think that we should use the same  
 interface
for write_dependence_p as for true_dependence_1, i.e. add a mem_mode  
 parameter

instead of a mem_size and add both mem_addr and mem_canonicalized (and since
it doesn't seem that we need x_addr for now, let's set it aside).
Btw I agree
that the interface is probably not optimal, but it has been there   
for a while.



Well, I see some merit in the way true_dependence_1 does it with regards to
the opportunities that this allows function cloning / specialization, it
just thought that the tiny speed benefit it could bring to write_dependence_p
didn't justify the extra code and churn to have this - or an enum parameter
to distinguish the three cases.  Using DEP_ANTI / DEP_OUTPUT from sched-int.h
would mean unwanted dependencies, so it'd be a new ad-hoc enum... .

OK, so if we go with consistency with a bool mem_canonicalized, I suppose
I should also make writep bool, i.e.:

static int
write_dependence_p (const_rtx mem, unsigned mem_size, rtx canon_mem_addr,
const_rtx x, bool mem_canonicalized, bool writep)

Re: RFA: Fix rtl-optimization/57425

2013-06-16 Thread Joern Rennecke 

Quoting Eric Botcazou ebotca...@adacore.com:


Could you also check that your patch also fixes PR opt/57569 and, if so, add
the reference to the ChangeLog as well as the testcase?


Attached is what I'm currently testing. bootstrap on i686-pc-linux-gnu
finished, now regtesting.
On x86_64-pc-linux-gnu, bootstrap is still in progress  I also still have
to verify if the pr57569.c testcase FAILs/PASSes for unpatched/patched
svn trunk.
2013-06-15  Joern Rennecke joern.renne...@embecosm.com

gcc:
PR rtl-optimization/57425
PR rtl-optimization/57569
* alias.c (write_dependence_p): Add new parameters mem_size,
canon_mem_addr and mem_canonicalized.  Change type of writep to bool.
Changed all callers.
(canon_anti_dependence): New function.
* cse.c (check_dependence): Use canon_anti_dependence.
* cselib.c (cselib_invalidate_mem): Likewise.
* rtl.h (canon_anti_dependence): Declare.
gcc/testsuite:
PR rtl-optimization/57425
PR rtl-optimization/57569
* gcc.dg/torture/pr57425-1.c, gcc.dg/torture/pr57425-2.c: New files.
* gcc.dg/torture/pr57425-3.c, gcc.dg/torture/pr57569.c: Likewise.

Index: alias.c
===
--- alias.c (revision 200126)
+++ alias.c (working copy)
@@ -156,7 +156,8 @@ static int insert_subset_children (splay
 static alias_set_entry get_alias_set_entry (alias_set_type);
 static bool nonoverlapping_component_refs_p (const_rtx, const_rtx);
 static tree decl_for_component_ref (tree);
-static int write_dependence_p (const_rtx, const_rtx, int);
+static int write_dependence_p (const_rtx, enum machine_mode, rtx, const_rtx,
+  bool, bool);
 
 static void memory_modified_1 (rtx, const_rtx, void *);
 
@@ -2553,15 +2554,22 @@ canon_true_dependence (const_rtx mem, en
 }
 
 /* Returns nonzero if a write to X might alias a previous read from
-   (or, if WRITEP is nonzero, a write to) MEM.  */
+   (or, if WRITEP is true, a write to) MEM.
+   If MEM_CANONCALIZED is nonzero, CANON_MEM_ADDR is the canonicalized
+   address of MEM, and MEM_MODE the mode for that access.  */
 
 static int
-write_dependence_p (const_rtx mem, const_rtx x, int writep)
+write_dependence_p (const_rtx mem, enum machine_mode mem_mode,
+   rtx canon_mem_addr, const_rtx x,
+   bool mem_canonicalized, bool writep)
 {
   rtx x_addr, mem_addr;
   rtx base;
   int ret;
 
+  gcc_checking_assert (mem_canonicalized ? (canon_mem_addr != NULL_RTX)
+  : (canon_mem_addr == NULL_RTX  mem_mode == VOIDmode));
+
   if (MEM_VOLATILE_P (x)  MEM_VOLATILE_P (mem))
 return 1;
 
@@ -2612,9 +2620,15 @@ write_dependence_p (const_rtx mem, const
 return 0;
 
   x_addr = canon_rtx (x_addr);
-  mem_addr = canon_rtx (mem_addr);
+  if (mem_canonicalized)
+mem_addr = canon_mem_addr;
+  else
+{
+  mem_addr = canon_rtx (mem_addr);
+  mem_mode = GET_MODE (mem);
+}
 
-  if ((ret = memrefs_conflict_p (SIZE_FOR_MODE (mem), mem_addr,
+  if ((ret = memrefs_conflict_p (GET_MODE_SIZE (mem_mode), mem_addr,
 SIZE_FOR_MODE (x), x_addr, 0)) != -1)
 return ret;
 
@@ -2629,7 +2643,20 @@ write_dependence_p (const_rtx mem, const
 int
 anti_dependence (const_rtx mem, const_rtx x)
 {
-  return write_dependence_p (mem, x, /*writep=*/0);
+  return write_dependence_p (mem, VOIDmode, NULL_RTX, x,
+/*mem_canonicalized=*/false, /*writep=*/false);
+}
+
+/* Likewise, but we already have a canonicalized MEM_ADDR for MEM.
+   Also, consider MEM in MEM_MODE (which might be from an enclosing
+   STRICT_LOW_PART / ZERO_EXTRACT).  */
+
+int
+canon_anti_dependence (const_rtx mem, enum machine_mode mem_mode,
+  rtx mem_addr, const_rtx x)
+{
+  return write_dependence_p (mem, mem_mode, mem_addr, x,
+/*mem_canonicalized=*/true, /*writep=*/false);
 }
 
 /* Output dependence: X is written after store in MEM takes place.  */
@@ -2637,7 +2664,8 @@ anti_dependence (const_rtx mem, const_rt
 int
 output_dependence (const_rtx mem, const_rtx x)
 {
-  return write_dependence_p (mem, x, /*writep=*/1);
+  return write_dependence_p (mem, VOIDmode, NULL_RTX, x,
+/*mem_canonicalized=*/false, /*writep=*/true);
 }
 
 
Index: cse.c
===
--- cse.c   (revision 200126)
+++ cse.c   (working copy)
@@ -1824,7 +1824,7 @@ flush_hash_table (void)
   }
 }
 
-/* Function called for each rtx to check whether true dependence exist.  */
+/* Function called for each rtx to check whether an anti dependence exist.  */
 struct check_dependence_data
 {
   enum machine_mode mode;
@@ -1837,7 +1837,7 @@ check_dependence (rtx *x, void *data)
 {
   struct check_dependence_data *d = (struct check_dependence_data *) data;
   if (*x  MEM_P (*x))
-return

RFA: Fix rtl-optimization/57425

2013-06-15 Thread Joern Rennecke 

Bootstrapped/regtested on i686-pc-linux-gnu.
2013-06-15  Joern Rennecke joern.renne...@embecosm.com

gcc:
PR rtl-optimization/57425
* alias.c (write_dependence_p): Add new parameters mem_size and
canon_mem_addr.  Changed all callers.
(canon_anti_dependence): New function.
* cse.c (check_dependence): Use canon_anti_dependence.
* cselib.c (cselib_invalidate_mem): Likewise.
* rtl.h (canon_anti_dependence): Declare.
gcc/testsuite:
PR rtl-optimization/57425
* gcc.dg/torture/pr57425-1.c, gcc.dg/torture/pr57425-2.c: New files.
* gcc.dg/torture/pr57425-3.c: Likewise.

Index: alias.c
===
--- alias.c (revision 200126)
+++ alias.c (working copy)
@@ -156,7 +156,7 @@ static int insert_subset_children (splay
 static alias_set_entry get_alias_set_entry (alias_set_type);
 static bool nonoverlapping_component_refs_p (const_rtx, const_rtx);
 static tree decl_for_component_ref (tree);
-static int write_dependence_p (const_rtx, const_rtx, int);
+static int write_dependence_p (const_rtx, unsigned, rtx, const_rtx, int);
 
 static void memory_modified_1 (rtx, const_rtx, void *);
 
@@ -2553,10 +2553,12 @@ canon_true_dependence (const_rtx mem, en
 }
 
 /* Returns nonzero if a write to X might alias a previous read from
-   (or, if WRITEP is nonzero, a write to) MEM.  */
+   (or, if WRITEP is nonzero, a write to) MEM.
+   If CANON_MEM_ADDR is nonzero, it is the canonicalized address of MEM.  */
 
 static int
-write_dependence_p (const_rtx mem, const_rtx x, int writep)
+write_dependence_p (const_rtx mem, unsigned mem_size, rtx canon_mem_addr,
+   const_rtx x, int writep)
 {
   rtx x_addr, mem_addr;
   rtx base;
@@ -2612,9 +2614,14 @@ write_dependence_p (const_rtx mem, const
 return 0;
 
   x_addr = canon_rtx (x_addr);
-  mem_addr = canon_rtx (mem_addr);
+  if (canon_mem_addr)
+mem_addr = canon_mem_addr;
+  else
+mem_addr = canon_rtx (mem_addr);
+  if (!mem_size)
+mem_size = SIZE_FOR_MODE (mem);
 
-  if ((ret = memrefs_conflict_p (SIZE_FOR_MODE (mem), mem_addr,
+  if ((ret = memrefs_conflict_p (mem_size, mem_addr,
 SIZE_FOR_MODE (x), x_addr, 0)) != -1)
 return ret;
 
@@ -2629,7 +2636,19 @@ write_dependence_p (const_rtx mem, const
 int
 anti_dependence (const_rtx mem, const_rtx x)
 {
-  return write_dependence_p (mem, x, /*writep=*/0);
+  return write_dependence_p (mem, 0, NULL_RTX, x, /*writep=*/0);
+}
+
+/* Likewise, but we already have a canonicalized MEM_ADDR for MEM.
+   Also, consider MEM in MEM_MODE (which might be from an enclosing
+   STRICT_LOW_PART / ZERO_EXTRACT).  */
+
+int
+canon_anti_dependence (const_rtx mem, enum machine_mode mem_mode,
+  rtx mem_addr, const_rtx x)
+{
+  return write_dependence_p (mem, GET_MODE_SIZE (mem_mode), mem_addr,
+x, /*writep=*/0);
 }
 
 /* Output dependence: X is written after store in MEM takes place.  */
@@ -2637,7 +2656,7 @@ anti_dependence (const_rtx mem, const_rt
 int
 output_dependence (const_rtx mem, const_rtx x)
 {
-  return write_dependence_p (mem, x, /*writep=*/1);
+  return write_dependence_p (mem, 0, NULL_RTX, x, /*writep=*/1);
 }
 
 
Index: cse.c
===
--- cse.c   (revision 200126)
+++ cse.c   (working copy)
@@ -1824,7 +1824,7 @@ flush_hash_table (void)
   }
 }
 
-/* Function called for each rtx to check whether true dependence exist.  */
+/* Function called for each rtx to check whether an anti dependence exist.  */
 struct check_dependence_data
 {
   enum machine_mode mode;
@@ -1837,7 +1837,7 @@ check_dependence (rtx *x, void *data)
 {
   struct check_dependence_data *d = (struct check_dependence_data *) data;
   if (*x  MEM_P (*x))
-return canon_true_dependence (d-exp, d-mode, d-addr, *x, NULL_RTX);
+return canon_anti_dependence (d-exp, d-mode, d-addr, *x);
   else
 return 0;
 }
Index: cselib.c
===
--- cselib.c(revision 200126)
+++ cselib.c(working copy)
@@ -2263,8 +2263,8 @@ cselib_invalidate_mem (rtx mem_rtx)
  continue;
}
  if (num_mems  PARAM_VALUE (PARAM_MAX_CSELIB_MEMORY_LOCATIONS)
-  ! canon_true_dependence (mem_rtx, GET_MODE (mem_rtx),
- mem_addr, x, NULL_RTX))
+  ! canon_anti_dependence (mem_rtx, GET_MODE (mem_rtx),
+ mem_addr, x))
{
  has_mem = true;
  num_mems++;
Index: rtl.h
===
--- rtl.h   (revision 200126)
+++ rtl.h   (working copy)
@@ -2705,6 +2705,8 @@ extern int canon_true_dependence (const_
  const_rtx, rtx);
 extern int read_dependence (const_rtx, const_rtx);