This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Fix (?) load_kills_store
Hello,
> On Mon, 2003-07-07 at 11:45, Zdenek Dvorak wrote:
> > what you say seems to be correct, except that IMHO the cases 3 and 4
> > don't occur.
>
> cases 1 and 2 come from store_killed_after. cases 3 and 4 come from
> store_killed_before. I built a native x86 linux toolchain, and then
> compiled loop.i with it at -O2. I put a breakpoint in
> store_killed_before. I stepped through it, and I hit an instance of
> case 3. So case 3 obviously occurs. Since case 3 can occur, it should
> be obvious that case 4 can also occur.
>
> > if (GET_CODE (SET_DEST (pat)) == MEM && !expr_equiv_p (SET_DEST (pat), x))
> > ! if (output_dependence (x, SET_DEST (pat)))
>
> This is an improvement, however, it looks like the original code is
> obviously buggy. This will fail if the SET_DEST holds a
> STRICT_LOW_PART, ZERO_EXTRACT, or SIGN_EXTRACT. In that case, we will
> ignore the SET_DEST, and may miss an aliased mem.
can mem be inside STRICT_LOW_PART?
> So we are now up to 5 different bugs, and your patch only fixes 2 of
> them. The question is what to do about the other 3. It would be better
> to fix them, but maybe we can get away with documenting them since we
> don't have testcases for them?
what about this patch (completely untested)?
Zdenek
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 8 Jul 2003 18:17:59 -0000
*************** static rtx extract_mentioned_regs PARAMS
*** 685,693 ****
static rtx extract_mentioned_regs_helper PARAMS ((rtx, rtx));
static void find_moveable_store PARAMS ((rtx, int *, int *));
static int compute_store_table PARAMS ((void));
! static bool load_kills_store PARAMS ((rtx, rtx));
! static bool find_loads PARAMS ((rtx, rtx));
! static bool store_killed_in_insn PARAMS ((rtx, rtx, rtx));
static bool store_killed_after PARAMS ((rtx, rtx, rtx, basic_block,
int *, rtx *));
static bool store_killed_before PARAMS ((rtx, rtx, rtx, basic_block,
--- 685,693 ----
static rtx extract_mentioned_regs_helper PARAMS ((rtx, rtx));
static void find_moveable_store PARAMS ((rtx, int *, int *));
static int compute_store_table PARAMS ((void));
! static bool load_kills_store PARAMS ((rtx, rtx, int));
! static bool find_loads PARAMS ((rtx, rtx, int));
! static bool store_killed_in_insn PARAMS ((rtx, rtx, rtx, int));
static bool store_killed_after PARAMS ((rtx, rtx, rtx, basic_block,
int *, rtx *));
static bool store_killed_before PARAMS ((rtx, rtx, rtx, basic_block,
*************** compute_store_table ()
*** 7391,7413 ****
return ret;
}
! /* Check to see if the load X is aliased with STORE_PATTERN. */
static bool
! 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;
}
/* Go through the entire insn X, looking for any loads which might alias
! STORE_PATTERN. Return true if found. */
static bool
! find_loads (x, store_pattern)
rtx x, store_pattern;
{
const char * fmt;
int i, j;
--- 7392,7429 ----
return ret;
}
! /* Check to see if the load X is aliased with STORE_PATTERN.
! AFTER is true if we are checking the case when STORE_PATTERN occurs
! after the X. */
static bool
! load_kills_store (x, store_pattern, after)
rtx x, store_pattern;
+ int after;
{
! if (after)
! {
! if (true_dependence (store_pattern, GET_MODE (store_pattern), x,
! rtx_addr_varies_p))
! return true;
! }
! else
! {
! if (anti_dependence (x, store_pattern))
! return true;
! }
return false;
}
/* Go through the entire insn X, looking for any loads which might alias
! STORE_PATTERN. Return true if found.
! AFTER is true if we are checking the case when STORE_PATTERN occurs
! after the insn X. */
static bool
! find_loads (x, store_pattern, after)
rtx x, store_pattern;
+ int after;
{
const char * fmt;
int i, j;
*************** find_loads (x, store_pattern)
*** 7421,7427 ****
if (GET_CODE (x) == MEM)
{
! if (load_kills_store (x, store_pattern))
return true;
}
--- 7437,7443 ----
if (GET_CODE (x) == MEM)
{
! if (load_kills_store (x, store_pattern, after))
return true;
}
*************** find_loads (x, store_pattern)
*** 7431,7450 ****
for (i = GET_RTX_LENGTH (GET_CODE (x)) - 1; i >= 0 && !ret; i--)
{
if (fmt[i] == 'e')
! ret |= find_loads (XEXP (x, i), store_pattern);
else if (fmt[i] == 'E')
for (j = XVECLEN (x, i) - 1; j >= 0; j--)
! ret |= find_loads (XVECEXP (x, i, j), store_pattern);
}
return ret;
}
/* Check if INSN kills the store pattern X (is aliased with it).
Return true if it it does. */
static bool
! store_killed_in_insn (x, x_regs, insn)
rtx x, x_regs, insn;
{
rtx reg, base;
--- 7447,7470 ----
for (i = GET_RTX_LENGTH (GET_CODE (x)) - 1; i >= 0 && !ret; i--)
{
if (fmt[i] == 'e')
! ret |= find_loads (XEXP (x, i), store_pattern, after);
else if (fmt[i] == 'E')
for (j = XVECLEN (x, i) - 1; j >= 0; j--)
! ret |= find_loads (XVECEXP (x, i, j), store_pattern, after);
}
return ret;
}
/* Check if INSN kills the store pattern X (is aliased with it).
+ AFTER is true if we are checking the case when store X occurs
+ after the insn.
+
Return true if it it does. */
static bool
! store_killed_in_insn (x, x_regs, insn, after)
rtx x, x_regs, insn;
+ int after;
{
rtx reg, base;
*************** store_killed_in_insn (x, x_regs, insn)
*** 7476,7490 ****
if (GET_CODE (PATTERN (insn)) == SET)
{
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);
}
else
! return find_loads (PATTERN (insn), x);
}
/* Returns true if the expression X is loaded or clobbered on or after INSN
--- 7508,7540 ----
if (GET_CODE (PATTERN (insn)) == SET)
{
rtx pat = PATTERN (insn);
+ rtx dest = SET_DEST (pat);
+
+ if (GET_CODE (dest) == SIGN_EXTRACT
+ || GET_CODE (dest) == ZERO_EXTRACT)
+ dest = XEXP (dest, 0);
+ else if (GET_CODE (dest) == STRICT_LOW_PART)
+ dest = XEXP (XEXP (dest, 0), 0);
+
/* Check for memory stores to aliased objects. */
! if (GET_CODE (dest) == MEM
! && !expr_equiv_p (dest, x))
! {
! if (after)
! {
! if (output_dependence (dest, x))
! return true;
! }
! else
! {
! if (output_dependence (x, dest))
! return true;
! }
! }
! return find_loads (SET_SRC (pat), x, after);
}
else
! return find_loads (PATTERN (insn), x, after);
}
/* Returns true if the expression X is loaded or clobbered on or after INSN
*************** store_killed_after (x, x_regs, insn, bb,
*** 7511,7517 ****
/* Scan from the end, so that fail_insn is determined correctly. */
for (act = last; act != PREV_INSN (insn); act = PREV_INSN (act))
! if (store_killed_in_insn (x, x_regs, act))
{
if (fail_insn)
*fail_insn = act;
--- 7561,7567 ----
/* Scan from the end, so that fail_insn is determined correctly. */
for (act = last; act != PREV_INSN (insn); act = PREV_INSN (act))
! if (store_killed_in_insn (x, x_regs, act, false))
{
if (fail_insn)
*fail_insn = act;
*************** store_killed_before (x, x_regs, insn, bb
*** 7536,7542 ****
return true;
for ( ; insn != PREV_INSN (first); insn = PREV_INSN (insn))
! if (store_killed_in_insn (x, x_regs, insn))
return true;
return false;
--- 7586,7592 ----
return true;
for ( ; insn != PREV_INSN (first); insn = PREV_INSN (insn))
! if (store_killed_in_insn (x, x_regs, insn, true))
return true;
return false;