On 03/26/14 12:28, Jakub Jelinek wrote:
On Wed, Mar 26, 2014 at 12:17:43PM -0600, Jeff Law wrote:
On 03/26/14 12:12, Jakub Jelinek wrote:
On Wed, Mar 26, 2014 at 11:02:48AM -0600, Jeff Law wrote:
Bootstrapped and regression tested on x86_64-unknown-linux-gnu.
Verified it fixes the original and reduced testcase.
Note, the testcase is missing from your patch.
But I'd question if this is the right place to canonicalize it.
The non-canonical order seems to be created in the generic code, where
do_tablejump does:
No, at that point it's still canonical because the x86 backend
hasn't simpified the (mult ...) subexpression. Its the
simplification of that subexpression to a constant that creates the
non-canonical RTL. That's why I fixed the x86 bits -- those are the
bits that simplify the (mult ...) into a (const_int) and thus
creates the non-canonical RTL.
(mult:SI (const_int 0) (const_int 4)) is IMHO far from being canonical.
And, I'd say it is likely other target legitimization hooks would also try
to simplify it similarly.
simplify_gen_binary is used in several other places during expansion,
so I don't see why it couldn't be desirable here.
Here's the updated patch. It uses simplify_gen_binary in expr.c to
simplify the address expression as we're building it. It also uses
copy_addr_to_reg in the x86 backend to avoid the possibility of
generating non-canonical RTL there too.
By accident I interrupted the regression test cycle, so that is still
running. OK for the trunk if that passes?
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 53d58b3..3caae44 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,12 @@
+2014-03-27 Jeff Law <l...@redhat.com>
+ Jakub Jalinek <ja...@redhat.com>
+
+ * expr.c (do_tablejump): Use simplify_gen_binary rather than
+ gen_rtx_{PLUS,MULT} to build up the address expression.
+
+ * i386/i386.c (ix86_legitimize_address): Use copy_addr_to_reg to avoid
+ creating non-canonical RTL.
+
2014-03-26 Richard Biener <rguent...@suse.de>
* tree-pretty-print.c (percent_K_format): Implement special
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 842be68..70b8f02 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -13925,13 +13925,13 @@ ix86_legitimize_address (rtx x, rtx oldx
ATTRIBUTE_UNUSED,
if (GET_CODE (XEXP (x, 0)) == MULT)
{
changed = 1;
- XEXP (x, 0) = force_operand (XEXP (x, 0), 0);
+ XEXP (x, 0) = copy_addr_to_reg (XEXP (x, 0));
}
if (GET_CODE (XEXP (x, 1)) == MULT)
{
changed = 1;
- XEXP (x, 1) = force_operand (XEXP (x, 1), 0);
+ XEXP (x, 1) = copy_addr_to_reg (XEXP (x, 1));
}
if (changed
diff --git a/gcc/expr.c b/gcc/expr.c
index cdb4551..ebf136e 100644
--- a/gcc/expr.c
+++ b/gcc/expr.c
@@ -11134,11 +11134,12 @@ do_tablejump (rtx index, enum machine_mode mode, rtx
range, rtx table_label,
GET_MODE_SIZE, because this indicates how large insns are. The other
uses should all be Pmode, because they are addresses. This code
could fail if addresses and insns are not the same size. */
- index = gen_rtx_PLUS
- (Pmode,
- gen_rtx_MULT (Pmode, index,
- gen_int_mode (GET_MODE_SIZE (CASE_VECTOR_MODE), Pmode)),
- gen_rtx_LABEL_REF (Pmode, table_label));
+ index = simplify_gen_binary (MULT, Pmode, index,
+ gen_int_mode (GET_MODE_SIZE (CASE_VECTOR_MODE),
+ Pmode));
+ index = simplify_gen_binary (PLUS, Pmode, index,
+ gen_rtx_LABEL_REF (Pmode, table_label));
+
#ifdef PIC_CASE_VECTOR_ADDRESS
if (flag_pic)
index = PIC_CASE_VECTOR_ADDRESS (index);
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index cdc8e9a..fc3c198 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2014-03-27 Jeff Law <l...@redhat.com>
+
+ PR target/60648
+ * g++.dg/pr60648.C: New test.
+
2014-03-26 Jakub Jelinek <ja...@redhat.com>
PR sanitizer/60636
diff --git a/gcc/testsuite/g++.dg/pr60648.C b/gcc/testsuite/g++.dg/pr60648.C
new file mode 100644
index 0000000..80c0561
--- /dev/null
+++ b/gcc/testsuite/g++.dg/pr60648.C
@@ -0,0 +1,73 @@
+/* { dg-do compile } */
+/* { dg-do compile { target i?86-*-* x86_64-*-* } } */
+/* { dg-options "-O3 -fPIC -m32" } */
+
+enum component
+{
+ Ex,
+ Ez,
+ Hy,
+ Permeability
+};
+enum derived_component
+{};
+enum direction
+{
+ X,
+ Y,
+ Z,
+ R,
+ P,
+ NO_DIRECTION
+};
+derived_component a;
+component *b;
+component c;
+direction d;
+inline direction fn1 (component p1)
+{
+ switch (p1)
+ {
+ case 0:
+ return Y;
+ case 1:
+ return Z;
+ case Permeability:
+ return NO_DIRECTION;
+ }
+ return X;
+}
+
+inline component fn2 (direction p1)
+{
+ switch (p1)
+ {
+ case 0:
+ case 1:
+ return component ();
+ case Z:
+ case R:
+ return component (1);
+ case P:
+ return component (3);
+ }
+}
+
+void fn3 ()
+{
+ direction e;
+ switch (0)
+ case 0:
+ switch (a)
+ {
+ case 0:
+ c = Ex;
+ b[1] = Hy;
+ }
+ e = fn1 (b[1]);
+ b[1] = fn2 (e);
+ d = fn1 (c);
+}
+
+
+
