[preliminary patch] Fix rtl-optimization/27538.

[Kazu Hirata <kazu at codesourcery dot com>]

Hi,

Attached is a preliminary patch to fix rtl-optimization/27538.

Consider:

volatile short y;

int
foo (void)
{
  return y;
}

"arm-none-eabi-gcc -O1" turns this function into "return 0;" due to a
bug in the combiner.

At one point during the combiner pass, try_combiner gets:

=== i1 ===
(insn 10 8 11 2
      (set (reg/f:SI 103)
	   (symbol_ref:SI ("y") <var_decl 0xf7f71de0 y>)) 145 {*arm_movsi_insn} (nil)
	   (nil))

=== i2 ===
(insn 11 10 13 2
      (set (reg:SI 105 [ y ])
	   (zero_extend:SI (mem/v/c/i:HI (reg/f:SI 103) [0 y+0 S2 A16]))) 123 {*arm_zero_extendhisi2} (insn_list:REG_DEP_TRUE 10 (nil))
	   (expr_list:REG_DEAD (reg/f:SI 103)
			       (nil)))

=== i3 ===
(insn 13 11 14 2
      (set (reg:SI 106)
	   (ashift:SI (reg:SI 105 [ y ])
		      (const_int 16 [0x10]))) 101 {*arm_shiftsi3} (insn_list:REG_DEP_TRUE 11 (nil))
		      (expr_list:REG_DEAD (reg:SI 105 [ y ])
					  (nil)))

During substitution and simplification, combine_simplify_rtx is asked
to simplify

(ashift:SI (zero_extend:SI (mem/v/c/i:HI (reg/f:SI 103) [0 y+0 S2 A16]))
	   (const_int 16 [0x10]))

expand_compound_operation is then called to expand zero_extend into
left and right shifts.  In turn, expand_compound_operation calls
simplify_shift_const.  Now, let's examine what happens in
simplify_shift_const.  It is called with

  x:           NULL_RTX
  code:        ASHIFT
  result_mode: SImode
  varop:       (mem/v/c/i:HI ...)
  count:       16

Note that gen_lowpart_for_combine refuses to make a subreg of a
volatile mem.  Eventually simplify_shift_const_1 returns NULL_RTX, so
simplify_shift_const tries to make a shift on its own by calling
simplify_gen_binary.  However, notice that simplify_shift_const has
just passed (mem/v/c/i:HI ...) as is, resulting in

  (ashift:HI (mem/v/c/i:HI ...)
	     (const_int 16))

which is really 0.

The preliminary patch below makes a new version of
simplify_shift_const called maybe_simplify_shift_const, which is aware
that gen_lowpart_for_combine may fail.

The reason I created this new function instead of modifying
simplify_shift_const itself is because there are so many (about 30)
uses of simplify_shift_const that I did not want to affect them.

Now, I understand this is a hack, but I hope this is enough to start a
discussion towards a correct fix.  Maybe the correct fix is to make
simplify_shift_const aware of gen_lowpart_for_combine failure, and
update each use of simplify_shift_const, but I'd like a second opinion
before I make a big change.

By the way, this bug does not seem to be present in 4.1., so this is a
regression.

Comments?

Kazu Hirata

2006-05-12  Kazu Hirata  <kazu@codesourcery.com>

	* combine.c (expand_compound_operation): Use
	maybe_simplify_shift_const instead of simplify_shift_const.
	(maybe_simplify_shift_const): New.

Index: combine.c
===================================================================
--- combine.c	(revision 113700)
+++ combine.c	(working copy)
@@ -409,6 +409,8 @@ static int merge_outer_ops (enum rtx_cod
 static rtx simplify_shift_const_1 (enum rtx_code, enum machine_mode, rtx, int);
 static rtx simplify_shift_const (rtx, enum rtx_code, enum machine_mode, rtx,
 				 int);
+static rtx maybe_simplify_shift_const (rtx, enum rtx_code, enum machine_mode, rtx,
+				       int);
 static int recog_for_combine (rtx *, rtx, rtx *);
 static rtx gen_lowpart_for_combine (enum machine_mode, rtx);
 static enum rtx_code simplify_comparison (enum rtx_code, rtx *, rtx *);
@@ -5736,13 +5745,16 @@ expand_compound_operation (rtx x)
 
   modewidth = GET_MODE_BITSIZE (GET_MODE (x));
   if (modewidth + len >= pos)
-    tem = simplify_shift_const (NULL_RTX, unsignedp ? LSHIFTRT : ASHIFTRT,
-				GET_MODE (x),
-				simplify_shift_const (NULL_RTX, ASHIFT,
-						      GET_MODE (x),
-						      XEXP (x, 0),
-						      modewidth - pos - len),
-				modewidth - len);
+    {
+      tem = maybe_simplify_shift_const (NULL_RTX, ASHIFT, GET_MODE (x),
+					XEXP (x, 0),
+					modewidth - pos - len);
+      if (tem == NULL_RTX)
+	return x;
+      tem = simplify_shift_const (NULL_RTX, unsignedp ? LSHIFTRT : ASHIFTRT,
+				  GET_MODE (x), tem,
+				  modewidth - len);
+    }
 
   else if (unsignedp && len < HOST_BITS_PER_WIDE_INT)
     tem = simplify_and_const_int (NULL_RTX, GET_MODE (x),
@@ -9245,6 +9257,32 @@ simplify_shift_const (rtx x, enum rtx_co
   return x;
 }
 
+/* Just like simplify_shift_const except that this function may return
+   NULL_RTX in cases where we cannot even make a straightforward
+   shift.  */
+
+static rtx
+maybe_simplify_shift_const (rtx x, enum rtx_code code,
+			    enum machine_mode result_mode,
+			    rtx varop, int count)
+{
+  rtx tem = simplify_shift_const_1 (code, result_mode, varop, count);
+  if (tem)
+    return tem;
+
+  if (!x)
+    {
+      /* Make a SUBREG if necessary.  If we can't make it, fail.  */
+      varop = gen_lowpart (result_mode, varop);
+      if (varop == NULL_RTX || GET_CODE (varop) == CLOBBER)
+	return NULL_RTX;
+      
+      x = simplify_gen_binary (code, GET_MODE (varop), varop, GEN_INT (count));
+    }
+  if (GET_MODE (x) != result_mode)
+    x = gen_lowpart (result_mode, x);
+  return x;
+}
 
 /* Like recog, but we receive the address of a pointer to a new pattern.
    We try to match the rtx that the pointer points to.