diff mbox

Fix lower-subreg shift/zext handling (PR rtl-optimization/52113)

Message ID 20120204121042.GL18768@tyan-ft48-01.lab.bos.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek Feb. 4, 2012, 12:10 p.m. UTC 
Hi!

The lower subreg pass apparently relies on recog_memoized being performed
on all insns in the first pass (analysis), then it replaces some
of the regs with concat and then again on each insn performs recog_memoized
+ extract_insn.  This works, because recog_memoized then does nothing, just
returns the memoized number, and extract insn extracts the operands
(which may be now CONCATNs) and then adjusts the insns.
Unfortunately if find_decomposable_shift_zext returns true, this
recog_memoized in the first pass wasn't called.  If the insn (shift or zext)
has been recognized before, that still works, but as this testcase on AVR
shows, if the insn come e.g. from split1 splitting right before subreg
lowering, INSN_CODE might be -1 and if we don't do recog_memoized early,
we replace the operand with CONCATN and then it doesn't recog successfully
and extract_insn ICEs.

Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, tested
on the testcase with avr cross.  Ok for trunk?

2012-02-04  Jakub Jelinek  <jakub@redhat.com>

	PR rtl-optimization/52113
	* lower-subreg.c (decompose_multiword_subregs): Call recog_memoized
	even for decomposable shift/zext insns.

	* gcc.target/avr/pr52113.c: New test.


	Jakub

Comments

Richard Sandiford Feb. 4, 2012, 12:32 p.m. UTC | #1 
Jakub Jelinek <jakub@redhat.com> writes:
> The lower subreg pass apparently relies on recog_memoized being performed
> on all insns in the first pass (analysis), then it replaces some
> of the regs with concat and then again on each insn performs recog_memoized
> + extract_insn.  This works, because recog_memoized then does nothing, just
> returns the memoized number, and extract insn extracts the operands
> (which may be now CONCATNs) and then adjusts the insns.

Interesting.  For 4.8, it might be worth trying to remove the
calls to recog_memoized in the second pass -- or at least replace
them with an assert -- just to emphasise what the assumption is.
I'll probably never get round to it though.

> 2012-02-04  Jakub Jelinek  <jakub@redhat.com>
>
> 	PR rtl-optimization/52113
> 	* lower-subreg.c (decompose_multiword_subregs): Call recog_memoized
> 	even for decomposable shift/zext insns.
>
> 	* gcc.target/avr/pr52113.c: New test.

OK, thanks.

Richard
diff mbox

Patch

--- gcc/lower-subreg.c.jj	2011-12-21 08:43:48.000000000 +0100
+++ gcc/lower-subreg.c	2012-02-04 01:01:06.903472679 +0100
@@ -1135,10 +1135,11 @@  decompose_multiword_subregs (void)
 	      || GET_CODE (PATTERN (insn)) == USE)
 	    continue;
 
+	  recog_memoized (insn);
+
 	  if (find_decomposable_shift_zext (insn))
 	    continue;
 
-	  recog_memoized (insn);
 	  extract_insn (insn);
 
 	  set = simple_move (insn);
--- gcc/testsuite/gcc.c-torture/compile/pr52113.c.jj	2012-02-04 01:05:39.100980253 +0100
+++ gcc/testsuite/gcc.c-torture/compile/pr52113.c	2012-02-04 01:05:15.000000000 +0100
@@ -0,0 +1,11 @@ 
+/* PR rtl-optimization/52113 */
+
+unsigned long v1;
+unsigned char v2;
+
+void
+foo (void)
+{
+  if (v1 > (v2 * 1000L))
+    v1 = 0;
+}