* [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).