[PATCH] Fix (?) load_kills_store

Zdenek Dvorak rakdver@atrey.karlin.mff.cuni.cz
Mon Jul 7 18:45:00 GMT 2003


Hello,

> On Fri, 2003-07-04 at 10:21, Zdenek Dvorak wrote:
> > this looks overly complicated to me.  What we test in that function is
> > whether changing the memory (store) might affect the load -- I don't
> > see how their order is relevant.
> 
> The comment is misleading.  From my reading of the code, there seems to
> be 4 different cases all of which end up in load_kills_store.
> 
> 1) We have a store X followed by a load Y, and we want to know if they
> conflict.  We call true_dependence (load Y, store X).
> 2) We have a store X followed by a store Y, and we want to know if they
> conflict.  We call true_dependent (store Y, store X).
> 3) We have a store X preceded by a load Y, and we want to know if they
> conflict.  We call true_dependence (load Y, store X).
> 4) We have a store X preceded by a store Y, and we want to know if they
> conflict.  We call true_dependence (store Y, store X).
>
> The current code is wrong for all 4 cases.  It is wrong for case 1
> because the arguments are in the wrong order.  It is wrong for case 2
> because the arguments are in the wrong order, and it should be a call to
> output_dependence.  It is wrong for case 3 because it should be a call
> to anti_dependence.  It is wrong for case 4 because it should be a call
> to output_dependence.
>
> Your patch fixes case 1, but does not fix any of the others.  It makes 2
> partly better, and it makes 3 and 4 partly worse.  I don't see how this
> is any better for the other 3 cases. Do you have any evidence to show
> that my analysis is wrong, or that your patch is safe for the other 3
> cases?

what you say seems to be correct, except that IMHO the cases 3 and 4
don't occur. The right patch then should be

Index: gcse.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/gcse.c,v
retrieving revision 1.255
diff -c -3 -p -r1.255 gcse.c
*** gcse.c	3 Jul 2003 05:26:30 -0000	1.255
--- gcse.c	7 Jul 2003 18:42:55 -0000
*************** static bool
*** 7397,7403 ****
  load_kills_store (x, store_pattern)
       rtx x, store_pattern;
  {
!   if (true_dependence (x, GET_MODE (x), store_pattern, rtx_addr_varies_p))
      return true;
    return false;
  }
--- 7398,7405 ----
  load_kills_store (x, store_pattern)
       rtx x, store_pattern;
  {
!   if (true_dependence (store_pattern, GET_MODE (store_pattern), x,
! 		       rtx_addr_varies_p))
      return true;
    return false;
  }
*************** store_killed_in_insn (x, x_regs, insn)
*** 7478,7485 ****
        rtx pat = PATTERN (insn);
        /* Check for memory stores to aliased objects.  */
        if (GET_CODE (SET_DEST (pat)) == MEM && !expr_equiv_p (SET_DEST (pat), x))
! 	/* pretend its a load and check for aliasing.  */
! 	if (find_loads (SET_DEST (pat), x))
  	  return true;
        return find_loads (SET_SRC (pat), x);
      }
--- 7492,7498 ----
        rtx pat = PATTERN (insn);
        /* Check for memory stores to aliased objects.  */
        if (GET_CODE (SET_DEST (pat)) == MEM && !expr_equiv_p (SET_DEST (pat), x))
! 	if (output_dependence (x, SET_DEST (pat)))
  	  return true;
        return find_loads (SET_SRC (pat), x);
      }



More information about the Gcc-patches mailing list