[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