As mentioned in the BZ, we have the following insns:
(insn 24 23 25 6 (set (reg:DI 0 ax [orig:100 D.2269 ] [100])
(zero_extend:DI (reg/v:SI 0 ax [orig:91 v ] [91]))) j.c:22 133
{*zero_extendsidi2}
(nil))
$13 = void
(gdb) p debug_rtx (curr_insn)
(insn 33 32 35 7 (set (reg/v:SI 0 ax [orig:91 v ] [91])
(sign_extend:SI (reg:HI 0 ax [orig:88 D.2271 ] [88]))) j.c:19
146 {extendhisi2}
(nil))
We eliminate insn 24 by changing insn 33 into:
(insn 33 32 35 7 (set (reg:DI 0 ax)
(zero_extend:DI (sign_extend:SI (reg:HI 0 ax [orig:88 D.2271 ]
[88])))) j.c:19 -1
(nil))
Later we call combine_reaching_defs to see if we can eliminate insn 33.
Unfortunately the tests for the srcreg != destreg case assume that
there are no nested extensions and we run afoul of RTL checking.
There's two cases to consider with nested extensions. When the two
registers are the same, we want to do nothing as the old code handles
that case correctly. When the registers are different, we want to avoid
the optimization simply because not all the code for handling the srcreg
!= destreg case has been vetted for nested extensions.
So clearly the way to go is to fix the test in combine_reaching_defs not
to barf on nested extensions. When the srcreg != destreg we still go
into the true arm which will always return false if there was a nested
extension (only way to get nested extensions at this point in code will
be if state->modified != EXT_MODIFIED_NONE). When srcreg == destreg we
fall-thru to the original ree code which does the right thing.
A similar test in find_and_remove_re needs updating as well. It's
simpler because we're simply disallowing srcreg != destreg with nested
extensions. We don't need to find the regno, just check if we have
(extend (reg)) or not.
Bootstrapped and regression tested (rtl checking enabled) on
x86_64-unknown-linux-gnu. OK for the trunk?
Thanks,
Jeff
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index a5ad4fa..ee2165c 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,12 @@
+2014-02-10 Jeff Law <l...@redhat.com>
+
+ PR rtl-optimization/60131
+ * ree.c (get_extended_src_reg): New function.
+ (combine_reaching_defs): Use it rather than assuming location
+ of REG.
+ (find_and_remove_re): Verify first operand of extension is
+ a REG before adding the insns to the copy list.
+
2014-02-10 Bernd Edlinger <bernd.edlin...@hotmail.de>
PR middle-end/60080
diff --git a/gcc/ree.c b/gcc/ree.c
index 421eb6c..fcde9a0 100644
--- a/gcc/ree.c
+++ b/gcc/ree.c
@@ -670,6 +670,18 @@ merge_def_and_ext (ext_cand *cand, rtx def_insn, ext_state
*state)
return false;
}
+/* Given SRC, which should be one or more extensions of a REG, strip
+ away the extensions and return the REG. */
+
+static inline rtx
+get_extended_src_reg (rtx src)
+{
+ while (GET_CODE (src) == SIGN_EXTEND || GET_CODE (src) == ZERO_EXTEND)
+ src = XEXP (src, 0);
+ gcc_assert (REG_P (src));
+ return src;
+}
+
/* This function goes through all reaching defs of the source
of the candidate for elimination (CAND) and tries to combine
the extension with the definition instruction. The changes
@@ -698,17 +710,23 @@ combine_reaching_defs (ext_cand *cand, const_rtx set_pat,
ext_state *state)
/* If the destination operand of the extension is a different
register than the source operand, then additional restrictions
- are needed. */
- if ((REGNO (SET_DEST (PATTERN (cand->insn)))
- != REGNO (XEXP (SET_SRC (PATTERN (cand->insn)), 0))))
+ are needed. Note we have to handle cases where we have nested
+ extensions in the source operand. */
+ if (REGNO (SET_DEST (PATTERN (cand->insn)))
+ != REGNO (get_extended_src_reg (SET_SRC (PATTERN (cand->insn)))))
{
/* In theory we could handle more than one reaching def, it
just makes the code to update the insn stream more complex. */
if (state->defs_list.length () != 1)
return false;
- /* We require the candidate not already be modified. This may
- be overly conservative. */
+ /* We require the candidate not already be modified. It may,
+ for example have been changed from a (sign_extend (reg))
+ into (zero_extend (sign_extend (reg)).
+
+ Handling that case shouldn't be terribly difficult, but the code
+ here and the code to emit copies would need auditing. Until
+ we see a need, this is the safe thing to do. */
if (state->modified[INSN_UID (cand->insn)].kind != EXT_MODIFIED_NONE)
return false;
@@ -999,8 +1017,13 @@ find_and_remove_re (void)
if (dump_file)
fprintf (dump_file, "Eliminated the extension.\n");
num_realized++;
- if (REGNO (SET_DEST (PATTERN (curr_cand->insn)))
- != REGNO (XEXP (SET_SRC (PATTERN (curr_cand->insn)), 0)))
+ /* If the RHS of the current candidate is not (extend (reg)), then
+ we do not allow the optimization of extensions where
+ the source and destination registers do not match. Thus
+ checking REG_P here is correct. */
+ if (REG_P (XEXP (SET_SRC (PATTERN (curr_cand->insn)), 0))
+ && (REGNO (SET_DEST (PATTERN (curr_cand->insn)))
+ != REGNO (XEXP (SET_SRC (PATTERN (curr_cand->insn)), 0))))
{
reinsn_copy_list.safe_push (curr_cand->insn);
reinsn_copy_list.safe_push (state.defs_list[0]);
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index dd2c268..dfa1fb2 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2014-02-10 Jeff Law <l...@redhat.com>
+
+ PR rtl-optimization/60131
+ * g++.dg/torture/pr60131.C: New test.
+
2014-02-10 Rainer Orth <r...@cebitec.uni-bielefeld.de>
* gcc.dg/binop-xor1.c: Don't xfail scan-tree-dump-times.
diff --git a/gcc/testsuite/g++.dg/torture/pr60131.C
b/gcc/testsuite/g++.dg/torture/pr60131.C
new file mode 100644
index 0000000..23dde31
--- /dev/null
+++ b/gcc/testsuite/g++.dg/torture/pr60131.C
@@ -0,0 +1,23 @@
+// { dg-do compile }
+struct A { short a; };
+int **b;
+unsigned long c;
+
+bool foo ();
+unsigned bar (unsigned i);
+extern void baz () __attribute__((noreturn));
+
+int *
+test (unsigned x, struct A *y)
+{
+ unsigned v;
+ if (foo () || y[x].a == -1)
+ {
+ c = bar (x);
+ return 0;
+ }
+ v = y[x].a;
+ if (v >= 23)
+ baz ();
+ return b[v];
+}
