Patchwork [1/11] Use targetm.shift_truncation_mask more consistently

login
register
mail settings
Submitter Bernd Schmidt
Date July 1, 2011, 5:27 p.m.
Message ID <4E0E0389.5040505@codesourcery.com>
Download mbox | patch
Permalink /patch/102930/
State New
Headers show

Comments

Bernd Schmidt - July 1, 2011, 5:27 p.m.
At some point we've grown a shift_truncation_mask hook, but we're not
using it everywhere we're masking shift counts. This patch changes the
instances I found.


Bernd
* simplify-rtx.c (simplify_const_binary_operation): Use the
	shift_truncation_mask hook instead of performing modulo by
	width.  Compare against mode precision, not bitsize.
	* combine.c (combine_simplify_rtx, simplify_shift_const_1):
	Use shift_truncation_mask instead of constructing the value
	manually.
Richard Henderson - July 4, 2011, 3:28 p.m.
On 07/01/2011 10:27 AM, Bernd Schmidt wrote:
> 	* simplify-rtx.c (simplify_const_binary_operation): Use the
> 	shift_truncation_mask hook instead of performing modulo by
> 	width.  Compare against mode precision, not bitsize.
> 	* combine.c (combine_simplify_rtx, simplify_shift_const_1):
> 	Use shift_truncation_mask instead of constructing the value
> 	manually.

Ok.

r~
Richard Sandiford - July 6, 2011, 6:06 p.m.
Bernd Schmidt <bernds@codesourcery.com> writes:
> At some point we've grown a shift_truncation_mask hook, but we're not
> using it everywhere we're masking shift counts. This patch changes the
> instances I found.

The documentation reads:

 Note that, unlike @code{SHIFT_COUNT_TRUNCATED}, this function does
 @emph{not} apply to general shift rtxes; it applies only to instructions
 that are generated by the named shift patterns.

I think you need to update the documentation, and check that existing
target definitions do in fact apply to shift rtxes as well.

Richard
Bernd Schmidt - July 6, 2011, 11:56 p.m.
On 07/06/11 20:06, Richard Sandiford wrote:
> Bernd Schmidt <bernds@codesourcery.com> writes:
>> At some point we've grown a shift_truncation_mask hook, but we're not
>> using it everywhere we're masking shift counts. This patch changes the
>> instances I found.
> 
> The documentation reads:
> 
>  Note that, unlike @code{SHIFT_COUNT_TRUNCATED}, this function does
>  @emph{not} apply to general shift rtxes; it applies only to instructions
>  that are generated by the named shift patterns.

Ouch. That is one seriously misnamed hook then.

> I think you need to update the documentation, and check that existing
> target definitions do in fact apply to shift rtxes as well.

Until I can do that, I've reverted this patch.


Bernd
Richard Sandiford - July 7, 2011, 7:42 a.m.
Bernd Schmidt <bernds@codesourcery.com> writes:
> On 07/06/11 20:06, Richard Sandiford wrote:
>> Bernd Schmidt <bernds@codesourcery.com> writes:
>>> At some point we've grown a shift_truncation_mask hook, but we're not
>>> using it everywhere we're masking shift counts. This patch changes the
>>> instances I found.
>> 
>> The documentation reads:
>> 
>>  Note that, unlike @code{SHIFT_COUNT_TRUNCATED}, this function does
>>  @emph{not} apply to general shift rtxes; it applies only to instructions
>>  that are generated by the named shift patterns.
>
> Ouch. That is one seriously misnamed hook then.

Yeah.  I take the blame for that, sorry :-(

>> I think you need to update the documentation, and check that existing
>> target definitions do in fact apply to shift rtxes as well.
>
> Until I can do that, I've reverted this patch.

Thanks.

Richard

Patch

Index: gcc/simplify-rtx.c
===================================================================
--- gcc/simplify-rtx.c.orig
+++ gcc/simplify-rtx.c
@@ -3704,8 +3704,8 @@  simplify_const_binary_operation (enum rt
 	     shift_truncation_mask, since the shift might not be part of an
 	     ashlM3, lshrM3 or ashrM3 instruction.  */
 	  if (SHIFT_COUNT_TRUNCATED)
-	    arg1 = (unsigned HOST_WIDE_INT) arg1 % width;
-	  else if (arg1 < 0 || arg1 >= GET_MODE_BITSIZE (mode))
+	    arg1 &= targetm.shift_truncation_mask (mode);
+	  else if (arg1 < 0 || arg1 >= GET_MODE_PRECISION (mode))
 	    return 0;
 
 	  val = (code == ASHIFT
Index: gcc/combine.c
===================================================================
--- gcc/combine.c.orig
+++ gcc/combine.c
@@ -5941,9 +5941,7 @@  combine_simplify_rtx (rtx x, enum machin
       else if (SHIFT_COUNT_TRUNCATED && !REG_P (XEXP (x, 1)))
 	SUBST (XEXP (x, 1),
 	       force_to_mode (XEXP (x, 1), GET_MODE (XEXP (x, 1)),
-			      ((unsigned HOST_WIDE_INT) 1
-			       << exact_log2 (GET_MODE_BITSIZE (GET_MODE (x))))
-			      - 1,
+			      targetm.shift_truncation_mask (GET_MODE (x)),
 			      0));
       break;
 
@@ -9896,7 +9894,7 @@  simplify_shift_const_1 (enum rtx_code co
      want to do this inside the loop as it makes it more difficult to
      combine shifts.  */
   if (SHIFT_COUNT_TRUNCATED)
-    orig_count &= GET_MODE_BITSIZE (mode) - 1;
+    orig_count &= targetm.shift_truncation_mask (mode);
 
   /* If we were given an invalid count, don't do anything except exactly
      what was requested.  */