This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] PR rtl-optimization/53352
On 05/17/2012 03:02 PM, Richard Sandiford wrote:
> After agonising over this for a couple of days, I think it's probably
> the correct fix. What we're doing now would be valid if the only use of
> equiv_constant(x) were to substitute for x. The name and current use
> of the function obviously require equality though.
>
> Patch is OK, thanks. It might be worth extending fold_rtx and
> equiv_constant to set a flag that says whether the returned value
> is equivalent or simply one of many valid replacement values.
> We'd then be able to replace uses of registers like 142 with 0,
> while not replacing uses of 0 with 142. I don't know how much it
> would buy us though. That kind of thing is probably more of a job
> for fwprop.
Thanks for the review.
> Please add UL to the hex constants in the testcase. Not sure whether
> it matters for 16-bit int targets or not, but better safe than sorry :-)
Fixed.
> Also, rather than:
>
>> + /* Otherwise see if we already have a constant for the inner REG.
>> + Don't bother with paradoxical subregs because we have no way
>> + of knowing what the upper bytes are. */
>
> how about:
>
> /* Otherwise see if we already have a constant for the inner REG,
> and if that is enough to calculate an equivalent constant for
> the subreg. Note that the upper bits of paradoxical subregs
> are undefined, so they cannot be said to equal anything. */
Sounds good to me.
v2 OK? If so, would you mind committing for me? I don't have write access.
--
Meador Inge
CodeSourcery / Mentor Embedded
http://www.mentor.com/embedded-software
Index: testsuite/gcc.dg/pr53352.c
===================================================================
--- testsuite/gcc.dg/pr53352.c (revision 0)
+++ testsuite/gcc.dg/pr53352.c (revision 0)
@@ -0,0 +1,41 @@
+/* { dg-do run } */
+/* { dg-options "-O1" } */
+
+#include <stdlib.h>
+
+typedef union
+{
+ struct
+ {
+ unsigned char a;
+ unsigned char b;
+ unsigned char c;
+ unsigned char d;
+ } parts;
+ unsigned long whole;
+} T;
+
+T *g_t;
+
+void bar (unsigned long x)
+{
+ if (x != 0)
+ abort ();
+}
+
+int main ()
+{
+ T one;
+ T two;
+ T tmp1, tmp2;
+
+ one.whole = 0xFFE0E0E0UL;
+ two.whole = 0xFF000000UL;
+ tmp1.parts = two.parts;
+ tmp2.parts = one.parts;
+ tmp2.parts.c = tmp1.parts.c;
+ one.parts = tmp2.parts;
+
+ g_t = &one;
+ bar (0);
+}
Index: cse.c
===================================================================
--- cse.c (revision 187470)
+++ cse.c (working copy)
@@ -3786,8 +3786,12 @@ equiv_constant (rtx x)
}
}
- /* Otherwise see if we already have a constant for the inner REG. */
+ /* Otherwise see if we already have a constant for the inner REG,
+ and if that is enough to calculate an equivalent constant for
+ the subreg. Note that the upper bits of paradoxical subregs
+ are undefined, so they cannot be said to equal anything. */
if (REG_P (SUBREG_REG (x))
+ && (GET_MODE_SIZE (mode) <= GET_MODE_SIZE (imode))
&& (new_rtx = equiv_constant (SUBREG_REG (x))) != 0)
return simplify_subreg (mode, new_rtx, imode, SUBREG_BYTE (x));