handling address mode changes inside extract_bit_field

Message ID CABXYE2VQo+55AACbN_H-DJXVZwu5TAbTYP94Op8Voh62cZ5_zw@mail.gmail.com
State New
Headers show

Commit Message

Jim Wilson March 1, 2017, 10:06 p.m.
This is a proposed patch for the bug 79794 which I just submitted.
This isn't a regression, so this can wait for after the gcc 7 branch
if necessary.

The problem here is that a reg+offset MEM target is passed to
extract_bit_field with a vector register source.  On aarch64, we have
an instruction for this, but it accepts a reg address only, so the
address gets loaded into a reg inside extract_bit_field.  We then
return to expand_expr which does
      ! rtx_equal_p (temp, target)
which fails because of the address mode change, so we end up copying
target into a reg and then back to itself.

expand_expr has a solution for this problem.  There is an alt_rtl
variable that can be set when temp is logically the same as target.
This variable is currently not passed into extract_bit_field.  This
patch does that.

There is an additional complication that the actual address load into
a reg occurs inside maybe_expand_insn, and it doesn't seem reasonable
to pass alt_reg into that.  However, I can grab a bit from the
expand_operand structure to indicate when an operand is the target,
and then clear it if target is replaced with a reg.

The resulting patch works, but ends up a bit more invasive than I
hoped.  The patch has passed a bootstrap and make check test on x86_64
and aarch64.

Jim

Patch hide | download patch | download mbox

Proposed patch for RTL expand bug affecting aarch64 vector code.

	PR middle-end/79794
    	* expmed.c (extract_bit_field_1): Add alt_rtl argument.  Before
	maybe_expand_insn call, set ops[0].target.  If still set after call,
	set alt_rtl.  Add extra arg to recursive calls.
	(extract_bit_field): Add alt_rtl argument.  Pass to
	extract_bit_field.
	* expmed.h (extract_bit_field): Fix prototype.
	* expr.c (emit_group_load_1, copy_blkmode_from_reg)
	(copy_blkmode_to_reg, read_complex_part, store_field): Pass extra NULL
	to extract_bit_field_calls.
	(expand_expr_real_1): Pass alt_rtl to expand_expr_real instead of 0.
	Pass alt_rtl to extract_bit_field calls.
	* calls.c (store_unaligned_arguments_into_psuedos)
	load_register_parameters): Pass extra NULL to extract_bit_field calls.
	* optabs.c (maybe_legitimize_operand): Clear op->target when call
	gen_reg_rtx.
	* optabs.h (struct expand_operand): Add target bitfield.

Index: gcc/calls.c
===================================================================
--- gcc/calls.c	(revision 245764)
+++ gcc/calls.c	(working copy)
@@ -1161,7 +1161,7 @@  store_unaligned_arguments_into_pseudos (struct arg
 
 	    args[i].aligned_regs[j] = reg;
 	    word = extract_bit_field (word, bitsize, 0, 1, NULL_RTX,
-				      word_mode, word_mode, false);
+				      word_mode, word_mode, false, NULL);
 
 	    /* There is no need to restrict this code to loading items
 	       in TYPE_ALIGN sized hunks.  The bitfield instructions can
@@ -2554,7 +2554,8 @@  load_register_parameters (struct arg_data *args, i
 		  unsigned int bitoff = (nregs - 1) * BITS_PER_WORD;
 		  unsigned int bitsize = size * BITS_PER_UNIT - bitoff;
 		  rtx x = extract_bit_field (mem, bitsize, bitoff, 1, dest,
-					     word_mode, word_mode, false);
+					     word_mode, word_mode, false,
+					     NULL);
 		  if (BYTES_BIG_ENDIAN)
 		    x = expand_shift (LSHIFT_EXPR, word_mode, x,
 				      BITS_PER_WORD - bitsize, dest, 1);
Index: gcc/expmed.c
===================================================================
--- gcc/expmed.c	(revision 245764)
+++ gcc/expmed.c	(working copy)
@@ -1528,7 +1528,7 @@  static rtx
 extract_bit_field_1 (rtx str_rtx, unsigned HOST_WIDE_INT bitsize,
 		     unsigned HOST_WIDE_INT bitnum, int unsignedp, rtx target,
 		     machine_mode mode, machine_mode tmode,
-		     bool reverse, bool fallback_p)
+		     bool reverse, bool fallback_p, rtx *alt_rtl)
 {
   rtx op0 = str_rtx;
   machine_mode int_mode;
@@ -1604,10 +1604,13 @@  extract_bit_field_1 (rtx str_rtx, unsigned HOST_WI
       unsigned HOST_WIDE_INT pos = bitnum / GET_MODE_BITSIZE (innermode);
 
       create_output_operand (&ops[0], target, innermode);
+      ops[0].target = 1;
       create_input_operand (&ops[1], op0, outermode);
       create_integer_operand (&ops[2], pos);
       if (maybe_expand_insn (icode, 3, ops))
 	{
+	  if (alt_rtl && ops[0].target)
+	    *alt_rtl = target;
 	  target = ops[0].value;
       	  if (GET_MODE (target) != mode)
 	    return gen_lowpart (tmode, target);
@@ -1729,7 +1732,7 @@  extract_bit_field_1 (rtx str_rtx, unsigned HOST_WI
 	    = extract_bit_field_1 (op0, MIN (BITS_PER_WORD,
 					     bitsize - i * BITS_PER_WORD),
 				   bitnum + bit_offset, 1, target_part,
-				   mode, word_mode, reverse, fallback_p);
+				   mode, word_mode, reverse, fallback_p, NULL);
 
 	  gcc_assert (target_part);
 	  if (!result_part)
@@ -1832,7 +1835,7 @@  extract_bit_field_1 (rtx str_rtx, unsigned HOST_WI
 	  xop0 = copy_to_reg (xop0);
 	  rtx result = extract_bit_field_1 (xop0, bitsize, bitpos,
 					    unsignedp, target,
-					    mode, tmode, reverse, false);
+					    mode, tmode, reverse, false, NULL);
 	  if (result)
 	    return result;
 	  delete_insns_since (last);
@@ -1888,7 +1891,8 @@  extract_bit_field_1 (rtx str_rtx, unsigned HOST_WI
 rtx
 extract_bit_field (rtx str_rtx, unsigned HOST_WIDE_INT bitsize,
 		   unsigned HOST_WIDE_INT bitnum, int unsignedp, rtx target,
-		   machine_mode mode, machine_mode tmode, bool reverse)
+		   machine_mode mode, machine_mode tmode, bool reverse,
+		   rtx *alt_rtl)
 {
   machine_mode mode1;
 
@@ -1923,7 +1927,7 @@  extract_bit_field (rtx str_rtx, unsigned HOST_WIDE
     }
 
   return extract_bit_field_1 (str_rtx, bitsize, bitnum, unsignedp,
-			      target, mode, tmode, reverse, true);
+			      target, mode, tmode, reverse, true, alt_rtl);
 }
 
 /* Use shifts and boolean operations to extract a field of BITSIZE bits
Index: gcc/expmed.h
===================================================================
--- gcc/expmed.h	(revision 245764)
+++ gcc/expmed.h	(working copy)
@@ -725,7 +725,7 @@  extern void store_bit_field (rtx, unsigned HOST_WI
 			     machine_mode, rtx, bool);
 extern rtx extract_bit_field (rtx, unsigned HOST_WIDE_INT,
 			      unsigned HOST_WIDE_INT, int, rtx,
-			      machine_mode, machine_mode, bool);
+			      machine_mode, machine_mode, bool, rtx *);
 extern rtx extract_low_bits (machine_mode, machine_mode, rtx);
 extern rtx expand_mult (machine_mode, rtx, rtx, rtx, int);
 extern rtx expand_mult_highpart_adjust (machine_mode, rtx, rtx, rtx, rtx, int);
Index: gcc/expr.c
===================================================================
--- gcc/expr.c	(revision 245764)
+++ gcc/expr.c	(working copy)
@@ -2192,7 +2192,8 @@  emit_group_load_1 (rtx *tmps, rtx dst, rtx orig_sr
 		      && (!REG_P (tmps[i]) || GET_MODE (tmps[i]) != mode)))
 		tmps[i] = extract_bit_field (tmps[i], bytelen * BITS_PER_UNIT,
 					     subpos * BITS_PER_UNIT,
-					     1, NULL_RTX, mode, mode, false);
+					     1, NULL_RTX, mode, mode, false,
+					     NULL);
 	    }
 	  else
 	    {
@@ -2202,7 +2203,8 @@  emit_group_load_1 (rtx *tmps, rtx dst, rtx orig_sr
 	      mem = assign_stack_temp (GET_MODE (src), slen);
 	      emit_move_insn (mem, src);
 	      tmps[i] = extract_bit_field (mem, bytelen * BITS_PER_UNIT,
-					   0, 1, NULL_RTX, mode, mode, false);
+					   0, 1, NULL_RTX, mode, mode, false,
+					   NULL);
 	    }
 	}
       /* FIXME: A SIMD parallel will eventually lead to a subreg of a
@@ -2245,7 +2247,7 @@  emit_group_load_1 (rtx *tmps, rtx dst, rtx orig_sr
       else
 	tmps[i] = extract_bit_field (src, bytelen * BITS_PER_UNIT,
 				     bytepos * BITS_PER_UNIT, 1, NULL_RTX,
-				     mode, mode, false);
+				     mode, mode, false, NULL);
 
       if (shift)
 	tmps[i] = expand_shift (LSHIFT_EXPR, mode, tmps[i],
@@ -2697,7 +2699,7 @@  copy_blkmode_from_reg (rtx target, rtx srcreg, tre
 		       extract_bit_field (src, bitsize,
 					  xbitpos % BITS_PER_WORD, 1,
 					  NULL_RTX, copy_mode, copy_mode,
-					  false),
+					  false, NULL),
 		       false);
     }
 }
@@ -2776,7 +2778,7 @@  copy_blkmode_to_reg (machine_mode mode, tree src)
 		       extract_bit_field (src_word, bitsize,
 					  bitpos % BITS_PER_WORD, 1,
 					  NULL_RTX, word_mode, word_mode,
-					  false),
+					  false, NULL),
 		       false);
     }
 
@@ -3225,7 +3227,7 @@  read_complex_part (rtx cplx, bool imag_p)
     }
 
   return extract_bit_field (cplx, ibitsize, imag_p ? ibitsize : 0,
-			    true, NULL_RTX, imode, imode, false);
+			    true, NULL_RTX, imode, imode, false, NULL);
 }
 
 /* A subroutine of emit_move_insn_1.  Yet another lowpart generator.
@@ -6911,7 +6913,7 @@  store_field (rtx target, HOST_WIDE_INT bitsize, HO
 	{
 	  machine_mode temp_mode = smallest_mode_for_size (bitsize, MODE_INT);
 	  temp = extract_bit_field (temp, bitsize, 0, 1, NULL_RTX, temp_mode,
-				    temp_mode, false);
+				    temp_mode, false, NULL);
 	}
 
       /* Store the value in the bitfield.  */
@@ -9760,7 +9762,8 @@  expand_expr_real_1 (tree exp, rtx target, machine_
 	    case GIMPLE_SINGLE_RHS:
 	      {
 		r = expand_expr_real (gimple_assign_rhs1 (g), target,
-				      tmode, modifier, NULL, inner_reference_p);
+				      tmode, modifier, alt_rtl,
+				      inner_reference_p);
 		break;
 	      }
 	    default:
@@ -10190,7 +10193,7 @@  expand_expr_real_1 (tree exp, rtx target, machine_
 					0, TYPE_UNSIGNED (TREE_TYPE (exp)),
 					(modifier == EXPAND_STACK_PARM
 					 ? NULL_RTX : target),
-					mode, mode, false);
+					mode, mode, false, alt_rtl);
 	  }
 	if (reverse
 	    && modifier != EXPAND_MEMORY
@@ -10687,7 +10690,7 @@  expand_expr_real_1 (tree exp, rtx target, machine_
 	    op0 = extract_bit_field (op0, bitsize, bitpos, unsignedp,
 				     (modifier == EXPAND_STACK_PARM
 				      ? NULL_RTX : target),
-				     ext_mode, ext_mode, reversep);
+				     ext_mode, ext_mode, reversep, alt_rtl);
 
 	    /* If the result has a record type and the mode of OP0 is an
 	       integral mode then, if BITSIZE is narrower than this mode
@@ -10908,7 +10911,7 @@  expand_expr_real_1 (tree exp, rtx target, machine_
       else if (reduce_bit_field)
 	return extract_bit_field (op0, TYPE_PRECISION (type), 0,
 				  TYPE_UNSIGNED (type), NULL_RTX,
-				  mode, mode, false);
+				  mode, mode, false, NULL);
       /* As a last resort, spill op0 to memory, and reload it in a
 	 different mode.  */
       else if (!MEM_P (op0))
Index: gcc/optabs.c
===================================================================
--- gcc/optabs.c	(revision 245764)
+++ gcc/optabs.c	(working copy)
@@ -6942,6 +6942,7 @@  maybe_legitimize_operand (enum insn_code icode, un
 	return true;
 
       op->value = gen_reg_rtx (mode);
+      op->target = 0;
       break;
 
     case EXPAND_INPUT:
Index: gcc/optabs.h
===================================================================
--- gcc/optabs.h	(revision 245764)
+++ gcc/optabs.h	(working copy)
@@ -48,8 +48,11 @@  struct expand_operand {
      rather than signed.  Only meaningful for certain types.  */
   unsigned int unsigned_p : 1;
 
+  /* Is the target operand.  */
+  unsigned int target : 1;
+
   /* Unused; available for future use.  */
-  unsigned int unused : 7;
+  unsigned int unused : 6;
 
   /* The mode passed to the convert_*_operand function.  It has a
      type-dependent meaning.  */