Fix pessimistic DImode handling in combine.c:make_field_assignment

Message ID 87efvezmdl.fsf@linaro.org
State New
Headers show

Commit Message

Richard Sandiford May 24, 2017, 8:31 a.m.
The make_field_assignment code:

      src = force_to_mode (src, mode,
		       GET_MODE_PRECISION (mode) >= HOST_BITS_PER_WIDE_INT
		       ? HOST_WIDE_INT_M1U
		       : (HOST_WIDE_INT_1U << len) - 1,
		       0);

would ignore the field length len for DImode, even though DImode can be
handled using HWIs.  I think the code should be testing len instead.

The patch was originally part of the SVE machine_mode series.
Retesting showed that it changed the asm output on powerpc for a few
tests, so I thought it should go in separately.  Each test change
seemed to be an improvement.

Tested on aarch64-linux-gnu and x86_64-linux-gnu.  I no longer have
access to the compile farm to test on the powerpc boxes there.

Thanks,
Richard


2017-05-24  Richard Sandiford  <richard.sandiford@linaro.org>

gcc/
	* combine.c (make_field_assignment): Check len rather than the mode
	precision when calling force_to_mode.

Comments

Richard Sandiford May 30, 2017, 8:22 a.m. | #1
Richard Sandiford <richard.sandiford@linaro.org> writes:
> The make_field_assignment code:

>

>       src = force_to_mode (src, mode,

> 		       GET_MODE_PRECISION (mode) >= HOST_BITS_PER_WIDE_INT

> 		       ? HOST_WIDE_INT_M1U

> 		       : (HOST_WIDE_INT_1U << len) - 1,

> 		       0);

>

> would ignore the field length len for DImode, even though DImode can be

> handled using HWIs.  I think the code should be testing len instead.

>

> The patch was originally part of the SVE machine_mode series.

> Retesting showed that it changed the asm output on powerpc for a few

> tests, so I thought it should go in separately.  Each test change

> seemed to be an improvement.

>

> Tested on aarch64-linux-gnu and x86_64-linux-gnu.  I no longer have

> access to the compile farm to test on the powerpc boxes there.


Now tested on powerpc64le-linux-gnu too (thanks to Segher for the access).

> Thanks,

> Richard

>

>

> 2017-05-24  Richard Sandiford  <richard.sandiford@linaro.org>

>

> gcc/

> 	* combine.c (make_field_assignment): Check len rather than the mode

> 	precision when calling force_to_mode.

>

> Index: gcc/combine.c

> ===================================================================

> --- gcc/combine.c	2017-05-03 08:46:32.777861592 +0100

> +++ gcc/combine.c	2017-05-24 09:25:25.170351268 +0100

> @@ -9634,7 +9634,7 @@ make_field_assignment (rtx x)

>  						     other, pos),

>  			       dest);

>    src = force_to_mode (src, mode,

> -		       GET_MODE_PRECISION (mode) >= HOST_BITS_PER_WIDE_INT

> +		       len >= HOST_BITS_PER_WIDE_INT

>  		       ? HOST_WIDE_INT_M1U

>  		       : (HOST_WIDE_INT_1U << len) - 1,

>  		       0);
Richard Sandiford June 12, 2017, 6:36 a.m. | #2
Ping

Richard Sandiford <richard.sandiford@linaro.org> writes:
> Richard Sandiford <richard.sandiford@linaro.org> writes:

>> The make_field_assignment code:

>>

>>       src = force_to_mode (src, mode,

>> 		       GET_MODE_PRECISION (mode) >= HOST_BITS_PER_WIDE_INT

>> 		       ? HOST_WIDE_INT_M1U

>> 		       : (HOST_WIDE_INT_1U << len) - 1,

>> 		       0);

>>

>> would ignore the field length len for DImode, even though DImode can be

>> handled using HWIs.  I think the code should be testing len instead.

>>

>> The patch was originally part of the SVE machine_mode series.

>> Retesting showed that it changed the asm output on powerpc for a few

>> tests, so I thought it should go in separately.  Each test change

>> seemed to be an improvement.

>>

>> Tested on aarch64-linux-gnu and x86_64-linux-gnu.  I no longer have

>> access to the compile farm to test on the powerpc boxes there.

>

> Now tested on powerpc64le-linux-gnu too (thanks to Segher for the access).

>

>> Thanks,

>> Richard

>>

>>

>> 2017-05-24  Richard Sandiford  <richard.sandiford@linaro.org>

>>

>> gcc/

>> 	* combine.c (make_field_assignment): Check len rather than the mode

>> 	precision when calling force_to_mode.

>>

>> Index: gcc/combine.c

>> ===================================================================

>> --- gcc/combine.c	2017-05-03 08:46:32.777861592 +0100

>> +++ gcc/combine.c	2017-05-24 09:25:25.170351268 +0100

>> @@ -9634,7 +9634,7 @@ make_field_assignment (rtx x)

>>  						     other, pos),

>>  			       dest);

>>    src = force_to_mode (src, mode,

>> -		       GET_MODE_PRECISION (mode) >= HOST_BITS_PER_WIDE_INT

>> +		       len >= HOST_BITS_PER_WIDE_INT

>>  		       ? HOST_WIDE_INT_M1U

>>  		       : (HOST_WIDE_INT_1U << len) - 1,

>>  		       0);
Eric Botcazou June 12, 2017, 9:17 a.m. | #3
> 2017-05-24  Richard Sandiford  <richard.sandiford@linaro.org>

> 

> gcc/

> 	* combine.c (make_field_assignment): Check len rather than the mode

> 	precision when calling force_to_mode.


OK for mainline.

-- 
Eric Botcazou

Patch hide | download patch | download mbox

Index: gcc/combine.c
===================================================================
--- gcc/combine.c	2017-05-03 08:46:32.777861592 +0100
+++ gcc/combine.c	2017-05-24 09:25:25.170351268 +0100
@@ -9634,7 +9634,7 @@  make_field_assignment (rtx x)
 						     other, pos),
 			       dest);
   src = force_to_mode (src, mode,
-		       GET_MODE_PRECISION (mode) >= HOST_BITS_PER_WIDE_INT
+		       len >= HOST_BITS_PER_WIDE_INT
 		       ? HOST_WIDE_INT_M1U
 		       : (HOST_WIDE_INT_1U << len) - 1,
 		       0);