This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: fix long-term -fgcse-sm bug
On Apr 13, 2007, Steven Bosscher <stevenb.gcc@gmail.com> wrote:
> But unfortunately, as rth already pointed out the loop you crafted
> took a bit of effort to dig.
Would it make it any better if I factored it into a macro? I suppose
it's not the only situation in which we want to iterate over all
members of a PARALLEL, if we have one, or over the single pattern
otherwise.
#define FOR_EACH_PAT_IN_PAR(pat, index, maybepar) \
for ((index) = (GET_CODE (maybepar) == PARALLEL \
? XVECLEN ((maybepar), 0) : 1), \
(pat) = (GET_CODE (maybepar) == PARALLEL \
? XVECEXP ((maybepar), 0, \
XVECLEN ((maybepar), 0) - (index)) \
: (maybepar)); \
(index) > 0; \
(void)(--(index) > 0 \
? ((pat) = XVECEXP ((maybepar), 0, \
XVECLEN ((maybepar), 0) - (index))) \
: (pat)))
[...]
if (GET_CODE (PATTERN (insn)) == SET
|| GET_CODE (PATTERN (insn)) == PARALLEL)
{
int count;
rtx pat;
FOR_EACH_PAT_IN_PAR (pat, count, PATTERN (insn))
{
if (GET_CODE (pat) == SET)
{
rtx dest = SET_DEST (pat);
if (GET_CODE (dest) == ZERO_EXTRACT)
dest = XEXP (dest, 0);
/* Check for memory stores to aliased objects. */
if (MEM_P (dest)
&& !expr_equiv_p (dest, x))
{
if (after)
{
if (output_dependence (dest, x))
return true;
}
else
{
if (output_dependence (x, dest))
return true;
}
}
}
if (find_loads (pat, x, after))
return true;
}
}
> Can you see if the attached (untested) patch also works for you?
I haven't actually tested it, because I see it changes the behavior
and might needlessly call find_loads twice for a SET, if it returns
false when called from store_killed_in_pat(). Also, it wouldn't call
find_loads() for anything but SETs within PARALLELs, which looks
wrong.
Here's what I'm testing instead.
for gcc/ChangeLog
from Alexandre Oliva <aoliva@redhat.com>
* gcse.c (store_killed_in_insn): Handle PARALLELs.
(store_killed_in_pat): New.
for gcc/testsuite/ChangeLog
from Alexandre Oliva <aoliva@redhat.com>
* gcc.target/i386/movsi-sm-1.c: New.
Index: gcc/gcse.c
===================================================================
--- gcc/gcse.c.orig 2007-04-09 02:44:26.000000000 -0300
+++ gcc/gcse.c 2007-04-20 05:05:12.000000000 -0300
@@ -5909,6 +5909,39 @@ find_loads (rtx x, rtx store_pattern, in
return ret;
}
+static inline bool
+store_killed_in_pat (rtx x, rtx pat, int after)
+{
+ if (GET_CODE (pat) == SET)
+ {
+ rtx dest = SET_DEST (pat);
+
+ if (GET_CODE (dest) == ZERO_EXTRACT)
+ dest = XEXP (dest, 0);
+
+ /* Check for memory stores to aliased objects. */
+ if (MEM_P (dest)
+ && !expr_equiv_p (dest, x))
+ {
+ if (after)
+ {
+ if (output_dependence (dest, x))
+ return true;
+ }
+ else
+ {
+ if (output_dependence (x, dest))
+ return true;
+ }
+ }
+ }
+
+ if (find_loads (pat, x, after))
+ return true;
+
+ return false;
+}
+
/* 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 does. */
@@ -5916,7 +5949,7 @@ find_loads (rtx x, rtx store_pattern, in
static bool
store_killed_in_insn (rtx x, rtx x_regs, rtx insn, int after)
{
- rtx reg, base, note;
+ rtx reg, base, note, pat;
if (!INSN_P (insn))
return false;
@@ -5943,32 +5976,20 @@ store_killed_in_insn (rtx x, rtx x_regs,
return false;
}
- if (GET_CODE (PATTERN (insn)) == SET)
+ pat = PATTERN (insn);
+ if (GET_CODE (pat) == SET)
{
- rtx pat = PATTERN (insn);
- rtx dest = SET_DEST (pat);
-
- if (GET_CODE (dest) == ZERO_EXTRACT)
- dest = XEXP (dest, 0);
-
- /* Check for memory stores to aliased objects. */
- if (MEM_P (dest)
- && !expr_equiv_p (dest, x))
- {
- if (after)
- {
- if (output_dependence (dest, x))
- return true;
- }
- else
- {
- if (output_dependence (x, dest))
- return true;
- }
- }
- if (find_loads (SET_SRC (pat), x, after))
+ if (store_killed_in_pat (x, pat, after))
return true;
}
+ else if (GET_CODE (pat) == PARALLEL)
+ {
+ int i;
+
+ for (i = 0; i < XVECLEN (pat, 0); i++)
+ if (store_killed_in_pat (x, XVECEXP (pat, 0, i), after))
+ return true;
+ }
else if (find_loads (PATTERN (insn), x, after))
return true;
Index: gcc/testsuite/gcc.target/i386/movsi-sm-1.c
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ gcc/testsuite/gcc.target/i386/movsi-sm-1.c 2007-04-20 04:58:26.000000000 -0300
@@ -0,0 +1,31 @@
+/* { dg-do run } */
+/* { dg-options "-O2 -fgcse-sm -minline-all-stringops" } */
+
+/* Store motion used to fail to recognize killed expressions within
+ parallels such as those generated for memory copying. */
+
+static const char s[1024] __attribute__ ((__aligned__ (32)))
+ = "This is what we should get!";
+
+int bug (int arg) {
+ char str[sizeof(s) > 4 ? sizeof(s) : 4] __attribute__ ((__aligned__ (32)));
+
+ __builtin_memcpy (str, "Bug", 4);
+
+ if (arg <= 2)
+ __builtin_memcpy (str, s, sizeof (s));
+
+ if (arg <= 1)
+ __builtin_memcpy (str, "Err", 4);
+
+ __builtin_puts (str);
+
+ return str[0] != s[0];
+}
+
+int main () {
+ if (bug (2))
+ __builtin_abort ();
+
+ return 0;
+}
--
Alexandre Oliva http://www.lsd.ic.unicamp.br/~oliva/
FSF Latin America Board Member http://www.fsfla.org/
Red Hat Compiler Engineer aoliva@{redhat.com, gcc.gnu.org}
Free Software Evangelist oliva@{lsd.ic.unicamp.br, gnu.org}