Re: [PATCH] Cleanup expand_shift

[Richard Guenther <rguenther@suse.de>]

On Wed, 4 May 2011, Richard Guenther wrote:

> On Wed, 4 May 2011, Eric Botcazou wrote:
> 
> > > I think I did it that way because the old code tried to re-construct
> > > the type of the original amount.  I can surely simply use op1 here
> > > if that is preferred.
> > 
> > Right, but it used the value of OP1 so I think the new code should as well.
> 
> Ok, I'll change it that way.
> 
> > > Btw, do you happen to know any target that would excercise this code
> > > choosing from x86, ppc, s390 and ia64?
> > 
> > All have rotate instructions so this seems to be a wash.
> 
> Hm.  I guess people will scream if something breaks (I can't imagine
> what though).

I have applied the following after re-bootstrapping and testing on
x86_64-unknown-linux-gnu and re-checking the mipsel cross testcase.

Richard.

2011-05-05  Richard Guenther  <rguenther@suse.de>

	* expmed.c (expand_variable_shift): Rename to ...
	(expand_shift_1): ... this.  Take an expanded shift amount.
	For rotates recurse directly not building trees for the shift amount.
	(expand_variable_shift): Wrap around expand_shift_1.
	(expand_shift): Adjust.

Index: gcc/expmed.c
===================================================================
*** gcc/expmed.c.orig	2011-05-04 11:07:08.000000000 +0200
--- gcc/expmed.c	2011-05-04 17:54:27.000000000 +0200
*************** expand_dec (rtx target, rtx dec)
*** 2032,2045 ****
  \f
  /* Output a shift instruction for expression code CODE,
     with SHIFTED being the rtx for the value to shift,
!    and AMOUNT the tree for the amount to shift by.
     Store the result in the rtx TARGET, if that is convenient.
     If UNSIGNEDP is nonzero, do a logical shift; otherwise, arithmetic.
     Return the rtx for where the value is.  */
  
! rtx
! expand_variable_shift (enum tree_code code, enum machine_mode mode, rtx shifted,
! 		       tree amount, rtx target, int unsignedp)
  {
    rtx op1, temp = 0;
    int left = (code == LSHIFT_EXPR || code == LROTATE_EXPR);
--- 2032,2045 ----
  \f
  /* Output a shift instruction for expression code CODE,
     with SHIFTED being the rtx for the value to shift,
!    and AMOUNT the rtx for the amount to shift by.
     Store the result in the rtx TARGET, if that is convenient.
     If UNSIGNEDP is nonzero, do a logical shift; otherwise, arithmetic.
     Return the rtx for where the value is.  */
  
! static rtx
! expand_shift_1 (enum tree_code code, enum machine_mode mode, rtx shifted,
! 		rtx amount, rtx target, int unsignedp)
  {
    rtx op1, temp = 0;
    int left = (code == LSHIFT_EXPR || code == LROTATE_EXPR);
*************** expand_variable_shift (enum tree_code co
*** 2053,2059 ****
    int attempt;
    bool speed = optimize_insn_for_speed_p ();
  
!   op1 = expand_normal (amount);
    op1_mode = GET_MODE (op1);
  
    /* Determine whether the shift/rotate amount is a vector, or scalar.  If the
--- 2053,2059 ----
    int attempt;
    bool speed = optimize_insn_for_speed_p ();
  
!   op1 = amount;
    op1_mode = GET_MODE (op1);
  
    /* Determine whether the shift/rotate amount is a vector, or scalar.  If the
*************** expand_variable_shift (enum tree_code co
*** 2138,2162 ****
  		 code below.  */
  
  	      rtx subtarget = target == shifted ? 0 : target;
! 	      tree new_amount, other_amount;
  	      rtx temp1;
! 	      tree type = TREE_TYPE (amount);
! 	      if (GET_MODE (op1) != TYPE_MODE (type)
! 		  && GET_MODE (op1) != VOIDmode)
! 		op1 = convert_to_mode (TYPE_MODE (type), op1, 1);
! 	      new_amount = make_tree (type, op1);
  	      other_amount
! 		= fold_build2 (MINUS_EXPR, type,
! 			       build_int_cst (type, GET_MODE_BITSIZE (mode)),
! 			       new_amount);
  
  	      shifted = force_reg (mode, shifted);
  
! 	      temp = expand_variable_shift (left ? LSHIFT_EXPR : RSHIFT_EXPR,
! 					    mode, shifted, new_amount, 0, 1);
! 	      temp1 = expand_variable_shift (left ? RSHIFT_EXPR : LSHIFT_EXPR,
! 					     mode, shifted, other_amount,
! 					     subtarget, 1);
  	      return expand_binop (mode, ior_optab, temp, temp1, target,
  				   unsignedp, methods);
  	    }
--- 2138,2159 ----
  		 code below.  */
  
  	      rtx subtarget = target == shifted ? 0 : target;
! 	      rtx new_amount, other_amount;
  	      rtx temp1;
! 
! 	      new_amount = op1;
  	      other_amount
! 		= simplify_gen_binary (MINUS, GET_MODE (op1),
! 				       GEN_INT (GET_MODE_BITSIZE (mode)),
! 				       op1);
  
  	      shifted = force_reg (mode, shifted);
  
! 	      temp = expand_shift_1 (left ? LSHIFT_EXPR : RSHIFT_EXPR,
! 				     mode, shifted, new_amount, 0, 1);
! 	      temp1 = expand_shift_1 (left ? RSHIFT_EXPR : LSHIFT_EXPR,
! 				      mode, shifted, other_amount,
! 				      subtarget, 1);
  	      return expand_binop (mode, ior_optab, temp, temp1, target,
  				   unsignedp, methods);
  	    }
*************** rtx
*** 2211,2222 ****
  expand_shift (enum tree_code code, enum machine_mode mode, rtx shifted,
  	      int amount, rtx target, int unsignedp)
  {
!   /* ???  With re-writing expand_shift we could avoid going through a
!      tree for the shift amount and directly do GEN_INT (amount).  */
!   return expand_variable_shift (code, mode, shifted,
! 				build_int_cst (integer_type_node, amount),
! 				target, unsignedp);
  }
  \f
  /* Indicates the type of fixup needed after a constant multiplication.
     BASIC_VARIANT means no fixup is needed, NEGATE_VARIANT means that
--- 2208,2232 ----
  expand_shift (enum tree_code code, enum machine_mode mode, rtx shifted,
  	      int amount, rtx target, int unsignedp)
  {
!   return expand_shift_1 (code, mode,
! 			 shifted, GEN_INT (amount), target, unsignedp);
  }
+ 
+ /* Output a shift instruction for expression code CODE,
+    with SHIFTED being the rtx for the value to shift,
+    and AMOUNT the tree for the amount to shift by.
+    Store the result in the rtx TARGET, if that is convenient.
+    If UNSIGNEDP is nonzero, do a logical shift; otherwise, arithmetic.
+    Return the rtx for where the value is.  */
+ 
+ rtx
+ expand_variable_shift (enum tree_code code, enum machine_mode mode, rtx shifted,
+ 		       tree amount, rtx target, int unsignedp)
+ {
+   return expand_shift_1 (code, mode,
+ 			 shifted, expand_normal (amount), target, unsignedp);
+ }
+ 
  \f
  /* Indicates the type of fixup needed after a constant multiplication.
     BASIC_VARIANT means no fixup is needed, NEGATE_VARIANT means that