[PATCH GCC]Check tieable TRUNCATE only if inner rtx is register

[PATCH GCC]Check tieable TRUNCATE only if inner rtx is register

[Bin Cheng]

[-- Attachment #1: Type: text/plain, Size: 1210 bytes --]

Hi,
Revision 247881 possibly exposes bug in RTL or ARM backend, while the change itself
may be incomplete too.  Given a TRUNCATE rtx with complicated sub-rtx, backend may
want to know the complicated sub-rtx happens in context of TRUNCATE in order to give
different costs.  This patch adds restriction only checking tieable TRUNCATE if its operand
is register.  This is enough for middle-end, it builds up (truncate:SI (reg:DI)) in order to setup
conversion cost for GIMPLE optimizations.  On the other hand, targets like i386/arm don't
handle register truncation at the moment.

Note, this patch minimize the impact of cost change and workaround PR80754 for now.
So far the effect of change in r247881 is to allow smulsi3_highpart_v6 pattern to be used
rather than smullsidi, which means it actually gives RA more free in choosing registers.
If it's because of the cost change that smulsi3_highpart_v6 is now preferred, it could be
suggesting that r247881 is good.  Anyway, it's just my guess before looking into rtl passes.

Bootstrap and test ongoing.  Is it OK?

2017-05-13  Bin Cheng  <bin.cheng@arm.com>

	* rtlanal.c (rtx_cost): Check tieable TRUNCATE only if inner rtx
	is register.

[-- Attachment #2: pr80754.txt --]
[-- Type: text/plain, Size: 544 bytes --]

diff --git a/gcc/rtlanal.c b/gcc/rtlanal.c
index d9f57c3..b211efb 100644
--- a/gcc/rtlanal.c
+++ b/gcc/rtlanal.c
@@ -4165,7 +4165,10 @@ rtx_cost (rtx x, machine_mode mode, enum rtx_code outer_code,
       break;
 
     case TRUNCATE:
-      if (MODES_TIEABLE_P (mode, GET_MODE (XEXP (x, 0))))
+      /* If inner rtx isn't register, fall through and call target hook.
+	 Backend may want to know that sub-rtx is in truncate.  */
+      if (REG_P (XEXP (x, 0))
+	  && MODES_TIEABLE_P (mode, GET_MODE (XEXP (x, 0))))
 	{
 	  total = 0;
 	  break;

Re: [PATCH GCC]Check tieable TRUNCATE only if inner rtx is register

[Richard Sandiford]

Bin Cheng <Bin.Cheng@arm.com> writes:
> Hi,
> Revision 247881 possibly exposes bug in RTL or ARM backend, while the
> change itself
> may be incomplete too.  Given a TRUNCATE rtx with complicated sub-rtx,
> backend may
> want to know the complicated sub-rtx happens in context of TRUNCATE in
> order to give
> different costs.  This patch adds restriction only checking tieable
> TRUNCATE if its operand
> is register.  This is enough for middle-end, it builds up (truncate:SI
> (reg:DI)) in order to setup
> conversion cost for GIMPLE optimizations.  On the other hand, targets
> like i386/arm don't
> handle register truncation at the moment.
>
> Note, this patch minimize the impact of cost change and workaround
> PR80754 for now.
> So far the effect of change in r247881 is to allow smulsi3_highpart_v6
> pattern to be used
> rather than smullsidi, which means it actually gives RA more free in
> choosing registers.
> If it's because of the cost change that smulsi3_highpart_v6 is now
> preferred, it could be
> suggesting that r247881 is good.  Anyway, it's just my guess before
> looking into rtl passes.
>
> Bootstrap and test ongoing.  Is it OK?
>
> 2017-05-13  Bin Cheng  <bin.cheng@arm.com>
>
> 	* rtlanal.c (rtx_cost): Check tieable TRUNCATE only if inner rtx
> 	is register.
>
> diff --git a/gcc/rtlanal.c b/gcc/rtlanal.c
> index d9f57c3..b211efb 100644
> --- a/gcc/rtlanal.c
> +++ b/gcc/rtlanal.c
> @@ -4165,7 +4165,10 @@ rtx_cost (rtx x, machine_mode mode, enum rtx_code outer_code,
>        break;
>  
>      case TRUNCATE:
> -      if (MODES_TIEABLE_P (mode, GET_MODE (XEXP (x, 0))))
> +      /* If inner rtx isn't register, fall through and call target hook.
> +	 Backend may want to know that sub-rtx is in truncate.  */
> +      if (REG_P (XEXP (x, 0))
> +	  && MODES_TIEABLE_P (mode, GET_MODE (XEXP (x, 0))))
>  	{
>  	  total = 0;
>  	  break;

Even for !REG_P (i.e. even when you still call the hook), 0 seems like
a better default to pass than COSTS_N_INSNS (1).  Would it be worth
guarding just the break with REG_P?

Thanks,
Richard