[1/3] expr: Allow scalar_int_mode target mode when converting a constant

Message ID 20200721182415.xekt7zd5eiwqr2my@jozef-acer-manjaro
State New
Headers show
Series
  • MSP430: Improve code-generation for shift instructions
Related show

Commit Message

Jozef Lawrynowicz July 21, 2020, 6:24 p.m.
is_int_mode does not allow MODE_PARTIAL_INT modes, so convert_modes was
not allowing a constant value to be converted to a MODE_PARTIAL_INT for
use as operand 2 in patterns such as ashlpsi3. The constant had
to be copied into a register before it could be used, but now can be
used directly as an operand without any copying.

Successfully regtested on trunk for x86_64-pc-linux-gnu and msp430-elf.
Ok to apply?
From 485feafad6ef0966ff0e1d2ae383cd2b6dbc5c96 Mon Sep 17 00:00:00 2001
From: Jozef Lawrynowicz <jozef.l@mittosystems.com>

Date: Wed, 15 Jul 2020 11:19:16 +0100
Subject: [PATCH 1/3] expr: Allow scalar_int_mode target mode when converting a
 constant

is_int_mode does not allow MODE_PARTIAL_INT modes, so convert_modes was
not allowing a constant value to be converted to a MODE_PARTIAL_INT for
use as operand 2 in patterns such as ashlpsi3. The constant had
to be copied into a register before it could be used, but now can be
used directly as an operand without any copying.

gcc/ChangeLog:

	* expr.c (convert_modes): Allow a constant integer to be converted to
	any scalar int mode.

---
 gcc/expr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.27.0

Comments

Richard Sandiford July 22, 2020, 8:33 a.m. | #1
Jozef Lawrynowicz <jozef.l@mittosystems.com> writes:
> is_int_mode does not allow MODE_PARTIAL_INT modes, so convert_modes was

> not allowing a constant value to be converted to a MODE_PARTIAL_INT for

> use as operand 2 in patterns such as ashlpsi3. The constant had

> to be copied into a register before it could be used, but now can be

> used directly as an operand without any copying.


Yeah.  I guess this dates back to when MODE_PARTIAL_INTs didn't have
a known precision.

> diff --git a/gcc/expr.c b/gcc/expr.c

> index c7c3e9fd655..5a252f0935f 100644

> --- a/gcc/expr.c

> +++ b/gcc/expr.c

> @@ -696,7 +696,7 @@ convert_modes (machine_mode mode, machine_mode oldmode, rtx x, int unsignedp)

>      return x;

>  

>    if (CONST_SCALAR_INT_P (x)

> -      && is_int_mode (mode, &int_mode))

> +      && is_a <scalar_int_mode> (mode, &int_mode))

>      {

>        /* If the caller did not tell us the old mode, then there is not

>  	 much to do with respect to canonicalization.  We have to


I think we also need to change the condition in:

      /* If the caller did not tell us the old mode, then there is not
	 much to do with respect to canonicalization.  We have to
	 assume that all the bits are significant.  */
      if (GET_MODE_CLASS (oldmode) != MODE_INT)

to is_a <scalar_int_mode> (old_mode)

OK with that, thanks.

Richard
Jozef Lawrynowicz July 22, 2020, 9:48 a.m. | #2
On Wed, Jul 22, 2020 at 09:33:47AM +0100, Richard Sandiford wrote:
> Jozef Lawrynowicz <jozef.l@mittosystems.com> writes:

> > is_int_mode does not allow MODE_PARTIAL_INT modes, so convert_modes was

> > not allowing a constant value to be converted to a MODE_PARTIAL_INT for

> > use as operand 2 in patterns such as ashlpsi3. The constant had

> > to be copied into a register before it could be used, but now can be

> > used directly as an operand without any copying.

> 

> Yeah.  I guess this dates back to when MODE_PARTIAL_INTs didn't have

> a known precision.


Is that what the section on MODE_PARTIAL_INT in the description for the
"subreg" RTX refers to?  From "14.8 Registers and Memory" of gccint:

  A MODE_PARTIAL_INT mode behaves as if it were as wide as the corresponding
  MODE_INT mode, except that it has an unknown number of undefined bits.

If so, that whole section seems out of date. I can work on getting it
fixed up.

> 

> > diff --git a/gcc/expr.c b/gcc/expr.c

> > index c7c3e9fd655..5a252f0935f 100644

> > --- a/gcc/expr.c

> > +++ b/gcc/expr.c

> > @@ -696,7 +696,7 @@ convert_modes (machine_mode mode, machine_mode oldmode, rtx x, int unsignedp)

> >      return x;

> >  

> >    if (CONST_SCALAR_INT_P (x)

> > -      && is_int_mode (mode, &int_mode))

> > +      && is_a <scalar_int_mode> (mode, &int_mode))

> >      {

> >        /* If the caller did not tell us the old mode, then there is not

> >  	 much to do with respect to canonicalization.  We have to

> 

> I think we also need to change the condition in:

> 

>       /* If the caller did not tell us the old mode, then there is not

> 	 much to do with respect to canonicalization.  We have to

> 	 assume that all the bits are significant.  */

>       if (GET_MODE_CLASS (oldmode) != MODE_INT)

> 

> to is_a <scalar_int_mode> (old_mode)

> 

> OK with that, thanks.


Thanks, I'll regtest that change and apply if all looks good.

Jozef
> 

> Richard
Richard Sandiford July 24, 2020, 2:14 p.m. | #3
Jozef Lawrynowicz <jozef.l@mittosystems.com> writes:
> On Wed, Jul 22, 2020 at 09:33:47AM +0100, Richard Sandiford wrote:

>> Jozef Lawrynowicz <jozef.l@mittosystems.com> writes:

>> > is_int_mode does not allow MODE_PARTIAL_INT modes, so convert_modes was

>> > not allowing a constant value to be converted to a MODE_PARTIAL_INT for

>> > use as operand 2 in patterns such as ashlpsi3. The constant had

>> > to be copied into a register before it could be used, but now can be

>> > used directly as an operand without any copying.

>> 

>> Yeah.  I guess this dates back to when MODE_PARTIAL_INTs didn't have

>> a known precision.

>

> Is that what the section on MODE_PARTIAL_INT in the description for the

> "subreg" RTX refers to?  From "14.8 Registers and Memory" of gccint:

>

>   A MODE_PARTIAL_INT mode behaves as if it were as wide as the corresponding

>   MODE_INT mode, except that it has an unknown number of undefined bits.


Yeah.  Before d8487c949ad5 the number of significant bits in a partial
mode was hidden from target-independent code.

> If so, that whole section seems out of date. I can work on getting it

> fixed up.


Sounds good :-)

Thanks,
Richard

Patch

diff --git a/gcc/expr.c b/gcc/expr.c
index c7c3e9fd655..5a252f0935f 100644
--- a/gcc/expr.c
+++ b/gcc/expr.c
@@ -696,7 +696,7 @@  convert_modes (machine_mode mode, machine_mode oldmode, rtx x, int unsignedp)
     return x;
 
   if (CONST_SCALAR_INT_P (x)
-      && is_int_mode (mode, &int_mode))
+      && is_a <scalar_int_mode> (mode, &int_mode))
     {
       /* If the caller did not tell us the old mode, then there is not
 	 much to do with respect to canonicalization.  We have to