public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH v2] RISC-V: bitmanip: improve constant-loading for (1ULL << 31) in DImode
@ 2022-05-29 21:51 Philipp Tomsich
  2022-06-02 13:17 ` Kito Cheng
  0 siblings, 1 reply; 6+ messages in thread
From: Philipp Tomsich @ 2022-05-29 21:51 UTC (permalink / raw)
  To: gcc-patches
  Cc: Andrew Pinski, Kito Cheng, Palmer Dabbelt, Vineet Gupta, Philipp Tomsich

The SINGLE_BIT_MASK_OPERAND() is overly restrictive, triggering for
bits above 31 only (to side-step any issues with the negative SImode
value 0x80000000/(-1ull << 31)/(1 << 31)).  This moves the special
handling of this SImode value (i.e. the check for (-1ull << 31) to
riscv.cc and relaxes the SINGLE_BIT_MASK_OPERAND() test.

With this, the code-generation for loading (1ULL << 31) from:
	li	a0,1
	slli	a0,a0,31
to:
	bseti	a0,zero,31

gcc/ChangeLog:

	* config/riscv/riscv.cc (riscv_build_integer_1): Rewrite value as
	(-1 << 31) for the single-bit case, when operating on (1 << 31)
	in SImode.
	* gcc/config/riscv/riscv.h (SINGLE_BIT_MASK_OPERAND): Allow for
	any single-bit value, moving the special case for (1 << 31) to
	riscv_build_integer_1 (in riscv.c).

Signed-off-by: Philipp Tomsich <philipp.tomsich@vrull.eu>

---

Changes in v2:
- Use HOST_WIDE_INT_1U/HOST_WIDE_INT_M1U instead of constants.
- Fix some typos in the comment above the rewrite of the value.
- Update the comment to clarify that we expect a LUI to be emitted for
  the SImode case (i.e. sign-extended for RV64) of (1 << 31).

 gcc/config/riscv/riscv.cc |  9 +++++++++
 gcc/config/riscv/riscv.h  | 11 ++++-------
 2 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index f83dc796d88..2e83ca07394 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -420,6 +420,15 @@ riscv_build_integer_1 (struct riscv_integer_op codes[RISCV_MAX_INTEGER_OPS],
       /* Simply BSETI.  */
       codes[0].code = UNKNOWN;
       codes[0].value = value;
+
+      /* RISC-V sign-extends all 32bit values that live in a 32bit
+	 register.  To avoid paradoxes, we thus need to use the
+	 sign-extended (negative) representation (-1 << 31) for the
+	 value, if we want to build (1 << 31) in SImode.  This will
+	 then expand to an LUI instruction.  */
+      if (mode == SImode && value == (HOST_WIDE_INT_1U << 31))
+	codes[0].value = (HOST_WIDE_INT_M1U << 31);
+
       return 1;
     }
 
diff --git a/gcc/config/riscv/riscv.h b/gcc/config/riscv/riscv.h
index 5083a1c24b0..6f7f4d3fbdc 100644
--- a/gcc/config/riscv/riscv.h
+++ b/gcc/config/riscv/riscv.h
@@ -528,13 +528,10 @@ enum reg_class
   (((VALUE) | ((1UL<<31) - IMM_REACH)) == ((1UL<<31) - IMM_REACH)	\
    || ((VALUE) | ((1UL<<31) - IMM_REACH)) + IMM_REACH == 0)
 
-/* If this is a single bit mask, then we can load it with bseti.  But this
-   is not useful for any of the low 31 bits because we can use addi or lui
-   to load them.  It is wrong for loading SImode 0x80000000 on rv64 because it
-   needs to be sign-extended.  So we restrict this to the upper 32-bits
-   only.  */
-#define SINGLE_BIT_MASK_OPERAND(VALUE) \
-  (pow2p_hwi (VALUE) && (ctz_hwi (VALUE) >= 32))
+/* If this is a single bit mask, then we can load it with bseti.  Special
+   handling of SImode 0x80000000 on RV64 is done in riscv_build_integer_1. */
+#define SINGLE_BIT_MASK_OPERAND(VALUE)					\
+  (pow2p_hwi (VALUE))
 
 /* Stack layout; function entry, exit and calling.  */
 
-- 
2.34.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] RISC-V: bitmanip: improve constant-loading for (1ULL << 31) in DImode
  2022-05-29 21:51 [PATCH v2] RISC-V: bitmanip: improve constant-loading for (1ULL << 31) in DImode Philipp Tomsich
@ 2022-06-02 13:17 ` Kito Cheng
  2022-06-02 19:23   ` Philipp Tomsich
  0 siblings, 1 reply; 6+ messages in thread
From: Kito Cheng @ 2022-06-02 13:17 UTC (permalink / raw)
  To: Philipp Tomsich; +Cc: GCC Patches, Vineet Gupta

LGTM

On Mon, May 30, 2022 at 5:52 AM Philipp Tomsich
<philipp.tomsich@vrull.eu> wrote:
>
> The SINGLE_BIT_MASK_OPERAND() is overly restrictive, triggering for
> bits above 31 only (to side-step any issues with the negative SImode
> value 0x80000000/(-1ull << 31)/(1 << 31)).  This moves the special
> handling of this SImode value (i.e. the check for (-1ull << 31) to
> riscv.cc and relaxes the SINGLE_BIT_MASK_OPERAND() test.
>
> With this, the code-generation for loading (1ULL << 31) from:
>         li      a0,1
>         slli    a0,a0,31
> to:
>         bseti   a0,zero,31
>
> gcc/ChangeLog:
>
>         * config/riscv/riscv.cc (riscv_build_integer_1): Rewrite value as
>         (-1 << 31) for the single-bit case, when operating on (1 << 31)
>         in SImode.
>         * gcc/config/riscv/riscv.h (SINGLE_BIT_MASK_OPERAND): Allow for
>         any single-bit value, moving the special case for (1 << 31) to
>         riscv_build_integer_1 (in riscv.c).
>
> Signed-off-by: Philipp Tomsich <philipp.tomsich@vrull.eu>
>
> ---
>
> Changes in v2:
> - Use HOST_WIDE_INT_1U/HOST_WIDE_INT_M1U instead of constants.
> - Fix some typos in the comment above the rewrite of the value.
> - Update the comment to clarify that we expect a LUI to be emitted for
>   the SImode case (i.e. sign-extended for RV64) of (1 << 31).
>
>  gcc/config/riscv/riscv.cc |  9 +++++++++
>  gcc/config/riscv/riscv.h  | 11 ++++-------
>  2 files changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> index f83dc796d88..2e83ca07394 100644
> --- a/gcc/config/riscv/riscv.cc
> +++ b/gcc/config/riscv/riscv.cc
> @@ -420,6 +420,15 @@ riscv_build_integer_1 (struct riscv_integer_op codes[RISCV_MAX_INTEGER_OPS],
>        /* Simply BSETI.  */
>        codes[0].code = UNKNOWN;
>        codes[0].value = value;
> +
> +      /* RISC-V sign-extends all 32bit values that live in a 32bit
> +        register.  To avoid paradoxes, we thus need to use the
> +        sign-extended (negative) representation (-1 << 31) for the
> +        value, if we want to build (1 << 31) in SImode.  This will
> +        then expand to an LUI instruction.  */
> +      if (mode == SImode && value == (HOST_WIDE_INT_1U << 31))
> +       codes[0].value = (HOST_WIDE_INT_M1U << 31);
> +
>        return 1;
>      }
>
> diff --git a/gcc/config/riscv/riscv.h b/gcc/config/riscv/riscv.h
> index 5083a1c24b0..6f7f4d3fbdc 100644
> --- a/gcc/config/riscv/riscv.h
> +++ b/gcc/config/riscv/riscv.h
> @@ -528,13 +528,10 @@ enum reg_class
>    (((VALUE) | ((1UL<<31) - IMM_REACH)) == ((1UL<<31) - IMM_REACH)      \
>     || ((VALUE) | ((1UL<<31) - IMM_REACH)) + IMM_REACH == 0)
>
> -/* If this is a single bit mask, then we can load it with bseti.  But this
> -   is not useful for any of the low 31 bits because we can use addi or lui
> -   to load them.  It is wrong for loading SImode 0x80000000 on rv64 because it
> -   needs to be sign-extended.  So we restrict this to the upper 32-bits
> -   only.  */
> -#define SINGLE_BIT_MASK_OPERAND(VALUE) \
> -  (pow2p_hwi (VALUE) && (ctz_hwi (VALUE) >= 32))
> +/* If this is a single bit mask, then we can load it with bseti.  Special
> +   handling of SImode 0x80000000 on RV64 is done in riscv_build_integer_1. */
> +#define SINGLE_BIT_MASK_OPERAND(VALUE)                                 \
> +  (pow2p_hwi (VALUE))
>
>  /* Stack layout; function entry, exit and calling.  */
>
> --
> 2.34.1
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] RISC-V: bitmanip: improve constant-loading for (1ULL << 31) in DImode
  2022-06-02 13:17 ` Kito Cheng
@ 2022-06-02 19:23   ` Philipp Tomsich
  2022-06-02 19:24     ` Philipp Tomsich
  0 siblings, 1 reply; 6+ messages in thread
From: Philipp Tomsich @ 2022-06-02 19:23 UTC (permalink / raw)
  To: Kito Cheng; +Cc: GCC Patches, Vineet Gupta

Thanks, applied to trunk!

On Thu, 2 Jun 2022 at 15:17, Kito Cheng <kito.cheng@gmail.com> wrote:
>
> LGTM
>
> On Mon, May 30, 2022 at 5:52 AM Philipp Tomsich
> <philipp.tomsich@vrull.eu> wrote:
> >
> > The SINGLE_BIT_MASK_OPERAND() is overly restrictive, triggering for
> > bits above 31 only (to side-step any issues with the negative SImode
> > value 0x80000000/(-1ull << 31)/(1 << 31)).  This moves the special
> > handling of this SImode value (i.e. the check for (-1ull << 31) to
> > riscv.cc and relaxes the SINGLE_BIT_MASK_OPERAND() test.
> >
> > With this, the code-generation for loading (1ULL << 31) from:
> >         li      a0,1
> >         slli    a0,a0,31
> > to:
> >         bseti   a0,zero,31
> >
> > gcc/ChangeLog:
> >
> >         * config/riscv/riscv.cc (riscv_build_integer_1): Rewrite value as
> >         (-1 << 31) for the single-bit case, when operating on (1 << 31)
> >         in SImode.
> >         * gcc/config/riscv/riscv.h (SINGLE_BIT_MASK_OPERAND): Allow for
> >         any single-bit value, moving the special case for (1 << 31) to
> >         riscv_build_integer_1 (in riscv.c).
> >
> > Signed-off-by: Philipp Tomsich <philipp.tomsich@vrull.eu>
> >
> > ---
> >
> > Changes in v2:
> > - Use HOST_WIDE_INT_1U/HOST_WIDE_INT_M1U instead of constants.
> > - Fix some typos in the comment above the rewrite of the value.
> > - Update the comment to clarify that we expect a LUI to be emitted for
> >   the SImode case (i.e. sign-extended for RV64) of (1 << 31).
> >
> >  gcc/config/riscv/riscv.cc |  9 +++++++++
> >  gcc/config/riscv/riscv.h  | 11 ++++-------
> >  2 files changed, 13 insertions(+), 7 deletions(-)
> >
> > diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> > index f83dc796d88..2e83ca07394 100644
> > --- a/gcc/config/riscv/riscv.cc
> > +++ b/gcc/config/riscv/riscv.cc
> > @@ -420,6 +420,15 @@ riscv_build_integer_1 (struct riscv_integer_op codes[RISCV_MAX_INTEGER_OPS],
> >        /* Simply BSETI.  */
> >        codes[0].code = UNKNOWN;
> >        codes[0].value = value;
> > +
> > +      /* RISC-V sign-extends all 32bit values that live in a 32bit
> > +        register.  To avoid paradoxes, we thus need to use the
> > +        sign-extended (negative) representation (-1 << 31) for the
> > +        value, if we want to build (1 << 31) in SImode.  This will
> > +        then expand to an LUI instruction.  */
> > +      if (mode == SImode && value == (HOST_WIDE_INT_1U << 31))
> > +       codes[0].value = (HOST_WIDE_INT_M1U << 31);
> > +
> >        return 1;
> >      }
> >
> > diff --git a/gcc/config/riscv/riscv.h b/gcc/config/riscv/riscv.h
> > index 5083a1c24b0..6f7f4d3fbdc 100644
> > --- a/gcc/config/riscv/riscv.h
> > +++ b/gcc/config/riscv/riscv.h
> > @@ -528,13 +528,10 @@ enum reg_class
> >    (((VALUE) | ((1UL<<31) - IMM_REACH)) == ((1UL<<31) - IMM_REACH)      \
> >     || ((VALUE) | ((1UL<<31) - IMM_REACH)) + IMM_REACH == 0)
> >
> > -/* If this is a single bit mask, then we can load it with bseti.  But this
> > -   is not useful for any of the low 31 bits because we can use addi or lui
> > -   to load them.  It is wrong for loading SImode 0x80000000 on rv64 because it
> > -   needs to be sign-extended.  So we restrict this to the upper 32-bits
> > -   only.  */
> > -#define SINGLE_BIT_MASK_OPERAND(VALUE) \
> > -  (pow2p_hwi (VALUE) && (ctz_hwi (VALUE) >= 32))
> > +/* If this is a single bit mask, then we can load it with bseti.  Special
> > +   handling of SImode 0x80000000 on RV64 is done in riscv_build_integer_1. */
> > +#define SINGLE_BIT_MASK_OPERAND(VALUE)                                 \
> > +  (pow2p_hwi (VALUE))
> >
> >  /* Stack layout; function entry, exit and calling.  */
> >
> > --
> > 2.34.1
> >

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] RISC-V: bitmanip: improve constant-loading for (1ULL << 31) in DImode
  2022-06-02 19:23   ` Philipp Tomsich
@ 2022-06-02 19:24     ` Philipp Tomsich
  2022-06-07 10:27       ` Kito Cheng
  0 siblings, 1 reply; 6+ messages in thread
From: Philipp Tomsich @ 2022-06-02 19:24 UTC (permalink / raw)
  To: Kito Cheng; +Cc: GCC Patches, Vineet Gupta

OK for backport?

On Thu, 2 Jun 2022 at 21:23, Philipp Tomsich <philipp.tomsich@vrull.eu> wrote:
>
> Thanks, applied to trunk!
>
> On Thu, 2 Jun 2022 at 15:17, Kito Cheng <kito.cheng@gmail.com> wrote:
> >
> > LGTM
> >
> > On Mon, May 30, 2022 at 5:52 AM Philipp Tomsich
> > <philipp.tomsich@vrull.eu> wrote:
> > >
> > > The SINGLE_BIT_MASK_OPERAND() is overly restrictive, triggering for
> > > bits above 31 only (to side-step any issues with the negative SImode
> > > value 0x80000000/(-1ull << 31)/(1 << 31)).  This moves the special
> > > handling of this SImode value (i.e. the check for (-1ull << 31) to
> > > riscv.cc and relaxes the SINGLE_BIT_MASK_OPERAND() test.
> > >
> > > With this, the code-generation for loading (1ULL << 31) from:
> > >         li      a0,1
> > >         slli    a0,a0,31
> > > to:
> > >         bseti   a0,zero,31
> > >
> > > gcc/ChangeLog:
> > >
> > >         * config/riscv/riscv.cc (riscv_build_integer_1): Rewrite value as
> > >         (-1 << 31) for the single-bit case, when operating on (1 << 31)
> > >         in SImode.
> > >         * gcc/config/riscv/riscv.h (SINGLE_BIT_MASK_OPERAND): Allow for
> > >         any single-bit value, moving the special case for (1 << 31) to
> > >         riscv_build_integer_1 (in riscv.c).
> > >
> > > Signed-off-by: Philipp Tomsich <philipp.tomsich@vrull.eu>
> > >
> > > ---
> > >
> > > Changes in v2:
> > > - Use HOST_WIDE_INT_1U/HOST_WIDE_INT_M1U instead of constants.
> > > - Fix some typos in the comment above the rewrite of the value.
> > > - Update the comment to clarify that we expect a LUI to be emitted for
> > >   the SImode case (i.e. sign-extended for RV64) of (1 << 31).
> > >
> > >  gcc/config/riscv/riscv.cc |  9 +++++++++
> > >  gcc/config/riscv/riscv.h  | 11 ++++-------
> > >  2 files changed, 13 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> > > index f83dc796d88..2e83ca07394 100644
> > > --- a/gcc/config/riscv/riscv.cc
> > > +++ b/gcc/config/riscv/riscv.cc
> > > @@ -420,6 +420,15 @@ riscv_build_integer_1 (struct riscv_integer_op codes[RISCV_MAX_INTEGER_OPS],
> > >        /* Simply BSETI.  */
> > >        codes[0].code = UNKNOWN;
> > >        codes[0].value = value;
> > > +
> > > +      /* RISC-V sign-extends all 32bit values that live in a 32bit
> > > +        register.  To avoid paradoxes, we thus need to use the
> > > +        sign-extended (negative) representation (-1 << 31) for the
> > > +        value, if we want to build (1 << 31) in SImode.  This will
> > > +        then expand to an LUI instruction.  */
> > > +      if (mode == SImode && value == (HOST_WIDE_INT_1U << 31))
> > > +       codes[0].value = (HOST_WIDE_INT_M1U << 31);
> > > +
> > >        return 1;
> > >      }
> > >
> > > diff --git a/gcc/config/riscv/riscv.h b/gcc/config/riscv/riscv.h
> > > index 5083a1c24b0..6f7f4d3fbdc 100644
> > > --- a/gcc/config/riscv/riscv.h
> > > +++ b/gcc/config/riscv/riscv.h
> > > @@ -528,13 +528,10 @@ enum reg_class
> > >    (((VALUE) | ((1UL<<31) - IMM_REACH)) == ((1UL<<31) - IMM_REACH)      \
> > >     || ((VALUE) | ((1UL<<31) - IMM_REACH)) + IMM_REACH == 0)
> > >
> > > -/* If this is a single bit mask, then we can load it with bseti.  But this
> > > -   is not useful for any of the low 31 bits because we can use addi or lui
> > > -   to load them.  It is wrong for loading SImode 0x80000000 on rv64 because it
> > > -   needs to be sign-extended.  So we restrict this to the upper 32-bits
> > > -   only.  */
> > > -#define SINGLE_BIT_MASK_OPERAND(VALUE) \
> > > -  (pow2p_hwi (VALUE) && (ctz_hwi (VALUE) >= 32))
> > > +/* If this is a single bit mask, then we can load it with bseti.  Special
> > > +   handling of SImode 0x80000000 on RV64 is done in riscv_build_integer_1. */
> > > +#define SINGLE_BIT_MASK_OPERAND(VALUE)                                 \
> > > +  (pow2p_hwi (VALUE))
> > >
> > >  /* Stack layout; function entry, exit and calling.  */
> > >
> > > --
> > > 2.34.1
> > >

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] RISC-V: bitmanip: improve constant-loading for (1ULL << 31) in DImode
  2022-06-02 19:24     ` Philipp Tomsich
@ 2022-06-07 10:27       ` Kito Cheng
  2022-06-14 11:18         ` Philipp Tomsich
  0 siblings, 1 reply; 6+ messages in thread
From: Kito Cheng @ 2022-06-07 10:27 UTC (permalink / raw)
  To: Philipp Tomsich; +Cc: Vineet Gupta, GCC Patches

> OK for backport?

OK, it seems no issue after a week :)


>
> On Thu, 2 Jun 2022 at 21:23, Philipp Tomsich <philipp.tomsich@vrull.eu> wrote:
> >
> > Thanks, applied to trunk!
> >
> > On Thu, 2 Jun 2022 at 15:17, Kito Cheng <kito.cheng@gmail.com> wrote:
> > >
> > > LGTM
> > >
> > > On Mon, May 30, 2022 at 5:52 AM Philipp Tomsich
> > > <philipp.tomsich@vrull.eu> wrote:
> > > >
> > > > The SINGLE_BIT_MASK_OPERAND() is overly restrictive, triggering for
> > > > bits above 31 only (to side-step any issues with the negative SImode
> > > > value 0x80000000/(-1ull << 31)/(1 << 31)).  This moves the special
> > > > handling of this SImode value (i.e. the check for (-1ull << 31) to
> > > > riscv.cc and relaxes the SINGLE_BIT_MASK_OPERAND() test.
> > > >
> > > > With this, the code-generation for loading (1ULL << 31) from:
> > > >         li      a0,1
> > > >         slli    a0,a0,31
> > > > to:
> > > >         bseti   a0,zero,31
> > > >
> > > > gcc/ChangeLog:
> > > >
> > > >         * config/riscv/riscv.cc (riscv_build_integer_1): Rewrite value as
> > > >         (-1 << 31) for the single-bit case, when operating on (1 << 31)
> > > >         in SImode.
> > > >         * gcc/config/riscv/riscv.h (SINGLE_BIT_MASK_OPERAND): Allow for
> > > >         any single-bit value, moving the special case for (1 << 31) to
> > > >         riscv_build_integer_1 (in riscv.c).
> > > >
> > > > Signed-off-by: Philipp Tomsich <philipp.tomsich@vrull.eu>
> > > >
> > > > ---
> > > >
> > > > Changes in v2:
> > > > - Use HOST_WIDE_INT_1U/HOST_WIDE_INT_M1U instead of constants.
> > > > - Fix some typos in the comment above the rewrite of the value.
> > > > - Update the comment to clarify that we expect a LUI to be emitted for
> > > >   the SImode case (i.e. sign-extended for RV64) of (1 << 31).
> > > >
> > > >  gcc/config/riscv/riscv.cc |  9 +++++++++
> > > >  gcc/config/riscv/riscv.h  | 11 ++++-------
> > > >  2 files changed, 13 insertions(+), 7 deletions(-)
> > > >
> > > > diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> > > > index f83dc796d88..2e83ca07394 100644
> > > > --- a/gcc/config/riscv/riscv.cc
> > > > +++ b/gcc/config/riscv/riscv.cc
> > > > @@ -420,6 +420,15 @@ riscv_build_integer_1 (struct riscv_integer_op codes[RISCV_MAX_INTEGER_OPS],
> > > >        /* Simply BSETI.  */
> > > >        codes[0].code = UNKNOWN;
> > > >        codes[0].value = value;
> > > > +
> > > > +      /* RISC-V sign-extends all 32bit values that live in a 32bit
> > > > +        register.  To avoid paradoxes, we thus need to use the
> > > > +        sign-extended (negative) representation (-1 << 31) for the
> > > > +        value, if we want to build (1 << 31) in SImode.  This will
> > > > +        then expand to an LUI instruction.  */
> > > > +      if (mode == SImode && value == (HOST_WIDE_INT_1U << 31))
> > > > +       codes[0].value = (HOST_WIDE_INT_M1U << 31);
> > > > +
> > > >        return 1;
> > > >      }
> > > >
> > > > diff --git a/gcc/config/riscv/riscv.h b/gcc/config/riscv/riscv.h
> > > > index 5083a1c24b0..6f7f4d3fbdc 100644
> > > > --- a/gcc/config/riscv/riscv.h
> > > > +++ b/gcc/config/riscv/riscv.h
> > > > @@ -528,13 +528,10 @@ enum reg_class
> > > >    (((VALUE) | ((1UL<<31) - IMM_REACH)) == ((1UL<<31) - IMM_REACH)      \
> > > >     || ((VALUE) | ((1UL<<31) - IMM_REACH)) + IMM_REACH == 0)
> > > >
> > > > -/* If this is a single bit mask, then we can load it with bseti.  But this
> > > > -   is not useful for any of the low 31 bits because we can use addi or lui
> > > > -   to load them.  It is wrong for loading SImode 0x80000000 on rv64 because it
> > > > -   needs to be sign-extended.  So we restrict this to the upper 32-bits
> > > > -   only.  */
> > > > -#define SINGLE_BIT_MASK_OPERAND(VALUE) \
> > > > -  (pow2p_hwi (VALUE) && (ctz_hwi (VALUE) >= 32))
> > > > +/* If this is a single bit mask, then we can load it with bseti.  Special
> > > > +   handling of SImode 0x80000000 on RV64 is done in riscv_build_integer_1. */
> > > > +#define SINGLE_BIT_MASK_OPERAND(VALUE)                                 \
> > > > +  (pow2p_hwi (VALUE))
> > > >
> > > >  /* Stack layout; function entry, exit and calling.  */
> > > >
> > > > --
> > > > 2.34.1
> > > >

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] RISC-V: bitmanip: improve constant-loading for (1ULL << 31) in DImode
  2022-06-07 10:27       ` Kito Cheng
@ 2022-06-14 11:18         ` Philipp Tomsich
  0 siblings, 0 replies; 6+ messages in thread
From: Philipp Tomsich @ 2022-06-14 11:18 UTC (permalink / raw)
  To: Kito Cheng; +Cc: Vineet Gupta, GCC Patches

Thanks, backport applied to releases/gcc-12!


On Tue, 7 Jun 2022 at 12:28, Kito Cheng <kito.cheng@gmail.com> wrote:

> > OK for backport?
>
> OK, it seems no issue after a week :)
>
>
> >
> > On Thu, 2 Jun 2022 at 21:23, Philipp Tomsich <philipp.tomsich@vrull.eu>
> wrote:
> > >
> > > Thanks, applied to trunk!
> > >
> > > On Thu, 2 Jun 2022 at 15:17, Kito Cheng <kito.cheng@gmail.com> wrote:
> > > >
> > > > LGTM
> > > >
> > > > On Mon, May 30, 2022 at 5:52 AM Philipp Tomsich
> > > > <philipp.tomsich@vrull.eu> wrote:
> > > > >
> > > > > The SINGLE_BIT_MASK_OPERAND() is overly restrictive, triggering for
> > > > > bits above 31 only (to side-step any issues with the negative
> SImode
> > > > > value 0x80000000/(-1ull << 31)/(1 << 31)).  This moves the special
> > > > > handling of this SImode value (i.e. the check for (-1ull << 31) to
> > > > > riscv.cc and relaxes the SINGLE_BIT_MASK_OPERAND() test.
> > > > >
> > > > > With this, the code-generation for loading (1ULL << 31) from:
> > > > >         li      a0,1
> > > > >         slli    a0,a0,31
> > > > > to:
> > > > >         bseti   a0,zero,31
> > > > >
> > > > > gcc/ChangeLog:
> > > > >
> > > > >         * config/riscv/riscv.cc (riscv_build_integer_1): Rewrite
> value as
> > > > >         (-1 << 31) for the single-bit case, when operating on (1
> << 31)
> > > > >         in SImode.
> > > > >         * gcc/config/riscv/riscv.h (SINGLE_BIT_MASK_OPERAND):
> Allow for
> > > > >         any single-bit value, moving the special case for (1 <<
> 31) to
> > > > >         riscv_build_integer_1 (in riscv.c).
> > > > >
> > > > > Signed-off-by: Philipp Tomsich <philipp.tomsich@vrull.eu>
> > > > >
> > > > > ---
> > > > >
> > > > > Changes in v2:
> > > > > - Use HOST_WIDE_INT_1U/HOST_WIDE_INT_M1U instead of constants.
> > > > > - Fix some typos in the comment above the rewrite of the value.
> > > > > - Update the comment to clarify that we expect a LUI to be emitted
> for
> > > > >   the SImode case (i.e. sign-extended for RV64) of (1 << 31).
> > > > >
> > > > >  gcc/config/riscv/riscv.cc |  9 +++++++++
> > > > >  gcc/config/riscv/riscv.h  | 11 ++++-------
> > > > >  2 files changed, 13 insertions(+), 7 deletions(-)
> > > > >
> > > > > diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> > > > > index f83dc796d88..2e83ca07394 100644
> > > > > --- a/gcc/config/riscv/riscv.cc
> > > > > +++ b/gcc/config/riscv/riscv.cc
> > > > > @@ -420,6 +420,15 @@ riscv_build_integer_1 (struct
> riscv_integer_op codes[RISCV_MAX_INTEGER_OPS],
> > > > >        /* Simply BSETI.  */
> > > > >        codes[0].code = UNKNOWN;
> > > > >        codes[0].value = value;
> > > > > +
> > > > > +      /* RISC-V sign-extends all 32bit values that live in a 32bit
> > > > > +        register.  To avoid paradoxes, we thus need to use the
> > > > > +        sign-extended (negative) representation (-1 << 31) for the
> > > > > +        value, if we want to build (1 << 31) in SImode.  This will
> > > > > +        then expand to an LUI instruction.  */
> > > > > +      if (mode == SImode && value == (HOST_WIDE_INT_1U << 31))
> > > > > +       codes[0].value = (HOST_WIDE_INT_M1U << 31);
> > > > > +
> > > > >        return 1;
> > > > >      }
> > > > >
> > > > > diff --git a/gcc/config/riscv/riscv.h b/gcc/config/riscv/riscv.h
> > > > > index 5083a1c24b0..6f7f4d3fbdc 100644
> > > > > --- a/gcc/config/riscv/riscv.h
> > > > > +++ b/gcc/config/riscv/riscv.h
> > > > > @@ -528,13 +528,10 @@ enum reg_class
> > > > >    (((VALUE) | ((1UL<<31) - IMM_REACH)) == ((1UL<<31) -
> IMM_REACH)      \
> > > > >     || ((VALUE) | ((1UL<<31) - IMM_REACH)) + IMM_REACH == 0)
> > > > >
> > > > > -/* If this is a single bit mask, then we can load it with bseti.
> But this
> > > > > -   is not useful for any of the low 31 bits because we can use
> addi or lui
> > > > > -   to load them.  It is wrong for loading SImode 0x80000000 on
> rv64 because it
> > > > > -   needs to be sign-extended.  So we restrict this to the upper
> 32-bits
> > > > > -   only.  */
> > > > > -#define SINGLE_BIT_MASK_OPERAND(VALUE) \
> > > > > -  (pow2p_hwi (VALUE) && (ctz_hwi (VALUE) >= 32))
> > > > > +/* If this is a single bit mask, then we can load it with bseti.
> Special
> > > > > +   handling of SImode 0x80000000 on RV64 is done in
> riscv_build_integer_1. */
> > > > > +#define SINGLE_BIT_MASK_OPERAND(VALUE)
>      \
> > > > > +  (pow2p_hwi (VALUE))
> > > > >
> > > > >  /* Stack layout; function entry, exit and calling.  */
> > > > >
> > > > > --
> > > > > 2.34.1
> > > > >
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-06-14 11:18 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-29 21:51 [PATCH v2] RISC-V: bitmanip: improve constant-loading for (1ULL << 31) in DImode Philipp Tomsich
2022-06-02 13:17 ` Kito Cheng
2022-06-02 19:23   ` Philipp Tomsich
2022-06-02 19:24     ` Philipp Tomsich
2022-06-07 10:27       ` Kito Cheng
2022-06-14 11:18         ` Philipp Tomsich

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).