* [PATCH] RISC-V: Fix splitter for 32-bit AND on 64-bit target.
@ 2019-07-08 11:11 Jim Wilson
2019-07-16 8:14 ` Kito Cheng
0 siblings, 1 reply; 4+ messages in thread
From: Jim Wilson @ 2019-07-08 11:11 UTC (permalink / raw)
To: gcc-patches; +Cc: Jim Wilson
Fixes github.com/riscv/riscv-gcc issue #161. We were accidentally using
BITS_PER_WORD to compute shift counts when we should have been using the
bitsize of the operand modes. This was wrong when we had an SImode shift
and a 64-bit target.
Tested with 32-bit elf and 64-bit linux cross compiler builds and checks.
There were no regressions. The new test fails without the patch and works
with the patch.
Committed.
Jim
Andrew Waterman <andrew@sifive.com>
gcc/
* config/riscv/riscv.md (lshrsi3_zero_extend_3+1): Use operands[1]
bitsize instead of BITS_PER_WORD.
gcc/testsuite/
* gcc.target/riscv/shift-shift-2.c: Add one more test.
---
gcc/config/riscv/riscv.md | 5 +++--
gcc/testsuite/gcc.target/riscv/shift-shift-2.c | 16 ++++++++++++++--
2 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md
index 0f4626656d6..78260fcf6fd 100644
--- a/gcc/config/riscv/riscv.md
+++ b/gcc/config/riscv/riscv.md
@@ -1776,10 +1776,11 @@
(set (match_dup 0)
(lshiftrt:GPR (match_dup 0) (match_dup 2)))]
{
- operands[2] = GEN_INT (BITS_PER_WORD
+ /* Op2 is a VOIDmode constant, so get the mode size from op1. */
+ operands[2] = GEN_INT (GET_MODE_BITSIZE (GET_MODE (operands[1]))
- exact_log2 (INTVAL (operands[2]) + 1));
})
-
+
;; Handle AND with 0xF...F0...0 where there are 32 to 63 zeros. This can be
;; split into two shifts. Otherwise it requires 3 instructions: li, sll, and.
(define_split
diff --git a/gcc/testsuite/gcc.target/riscv/shift-shift-2.c b/gcc/testsuite/gcc.target/riscv/shift-shift-2.c
index 3f07e7776e7..10a5bb728be 100644
--- a/gcc/testsuite/gcc.target/riscv/shift-shift-2.c
+++ b/gcc/testsuite/gcc.target/riscv/shift-shift-2.c
@@ -25,5 +25,17 @@ sub4 (unsigned long i)
{
return (i << 52) >> 52;
}
-/* { dg-final { scan-assembler-times "slli" 4 } } */
-/* { dg-final { scan-assembler-times "srli" 4 } } */
+
+unsigned int
+sub5 (unsigned int i)
+{
+ unsigned int j;
+ j = i >> 24;
+ j = j * (1 << 24);
+ j = i - j;
+ return j;
+}
+/* { dg-final { scan-assembler-times "slli" 5 } } */
+/* { dg-final { scan-assembler-times "srli" 5 } } */
+/* { dg-final { scan-assembler-times "slliw" 1 } } */
+/* { dg-final { scan-assembler-times "srliw" 1 } } */
--
2.17.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] RISC-V: Fix splitter for 32-bit AND on 64-bit target.
2019-07-08 11:11 [PATCH] RISC-V: Fix splitter for 32-bit AND on 64-bit target Jim Wilson
@ 2019-07-16 8:14 ` Kito Cheng
2019-07-16 10:53 ` Richard Biener
2019-07-16 18:37 ` Jim Wilson
0 siblings, 2 replies; 4+ messages in thread
From: Kito Cheng @ 2019-07-16 8:14 UTC (permalink / raw)
To: Jim Wilson, Jakub Jelinek, Richard Biener; +Cc: GCC Patches, kito.cheng
Hi
I'd like to back port this patch to GCC 8 and 9, should I send another
patch mail or just wait ack from release manager?
On Mon, Jul 8, 2019 at 6:48 PM Jim Wilson <jimw@sifive.com> wrote:
>
> Fixes github.com/riscv/riscv-gcc issue #161. We were accidentally using
> BITS_PER_WORD to compute shift counts when we should have been using the
> bitsize of the operand modes. This was wrong when we had an SImode shift
> and a 64-bit target.
>
> Tested with 32-bit elf and 64-bit linux cross compiler builds and checks.
> There were no regressions. The new test fails without the patch and works
> with the patch.
>
> Committed.
>
> Jim
>
> Andrew Waterman <andrew@sifive.com>
> gcc/
> * config/riscv/riscv.md (lshrsi3_zero_extend_3+1): Use operands[1]
> bitsize instead of BITS_PER_WORD.
> gcc/testsuite/
> * gcc.target/riscv/shift-shift-2.c: Add one more test.
> ---
> gcc/config/riscv/riscv.md | 5 +++--
> gcc/testsuite/gcc.target/riscv/shift-shift-2.c | 16 ++++++++++++++--
> 2 files changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md
> index 0f4626656d6..78260fcf6fd 100644
> --- a/gcc/config/riscv/riscv.md
> +++ b/gcc/config/riscv/riscv.md
> @@ -1776,10 +1776,11 @@
> (set (match_dup 0)
> (lshiftrt:GPR (match_dup 0) (match_dup 2)))]
> {
> - operands[2] = GEN_INT (BITS_PER_WORD
> + /* Op2 is a VOIDmode constant, so get the mode size from op1. */
> + operands[2] = GEN_INT (GET_MODE_BITSIZE (GET_MODE (operands[1]))
> - exact_log2 (INTVAL (operands[2]) + 1));
> })
> -
> +
> ;; Handle AND with 0xF...F0...0 where there are 32 to 63 zeros. This can be
> ;; split into two shifts. Otherwise it requires 3 instructions: li, sll, and.
> (define_split
> diff --git a/gcc/testsuite/gcc.target/riscv/shift-shift-2.c b/gcc/testsuite/gcc.target/riscv/shift-shift-2.c
> index 3f07e7776e7..10a5bb728be 100644
> --- a/gcc/testsuite/gcc.target/riscv/shift-shift-2.c
> +++ b/gcc/testsuite/gcc.target/riscv/shift-shift-2.c
> @@ -25,5 +25,17 @@ sub4 (unsigned long i)
> {
> return (i << 52) >> 52;
> }
> -/* { dg-final { scan-assembler-times "slli" 4 } } */
> -/* { dg-final { scan-assembler-times "srli" 4 } } */
> +
> +unsigned int
> +sub5 (unsigned int i)
> +{
> + unsigned int j;
> + j = i >> 24;
> + j = j * (1 << 24);
> + j = i - j;
> + return j;
> +}
> +/* { dg-final { scan-assembler-times "slli" 5 } } */
> +/* { dg-final { scan-assembler-times "srli" 5 } } */
> +/* { dg-final { scan-assembler-times "slliw" 1 } } */
> +/* { dg-final { scan-assembler-times "srliw" 1 } } */
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] RISC-V: Fix splitter for 32-bit AND on 64-bit target.
2019-07-16 8:14 ` Kito Cheng
@ 2019-07-16 10:53 ` Richard Biener
2019-07-16 18:37 ` Jim Wilson
1 sibling, 0 replies; 4+ messages in thread
From: Richard Biener @ 2019-07-16 10:53 UTC (permalink / raw)
To: Kito Cheng; +Cc: Jim Wilson, Jakub Jelinek, GCC Patches, kito.cheng
[-- Attachment #1: Type: text/plain, Size: 3215 bytes --]
On Tue, 16 Jul 2019, Kito Cheng wrote:
> Hi
>
> I'd like to back port this patch to GCC 8 and 9, should I send another
> patch mail or just wait ack from release manager?
Looks OK to backport since it only affects RISC-V and that's neither
a primary nor secondary arch and this looks like a wrong-code issue.
Richard.
>
>
> On Mon, Jul 8, 2019 at 6:48 PM Jim Wilson <jimw@sifive.com> wrote:
> >
> > Fixes github.com/riscv/riscv-gcc issue #161. We were accidentally using
> > BITS_PER_WORD to compute shift counts when we should have been using the
> > bitsize of the operand modes. This was wrong when we had an SImode shift
> > and a 64-bit target.
> >
> > Tested with 32-bit elf and 64-bit linux cross compiler builds and checks.
> > There were no regressions. The new test fails without the patch and works
> > with the patch.
> >
> > Committed.
> >
> > Jim
> >
> > Andrew Waterman <andrew@sifive.com>
> > gcc/
> > * config/riscv/riscv.md (lshrsi3_zero_extend_3+1): Use operands[1]
> > bitsize instead of BITS_PER_WORD.
> > gcc/testsuite/
> > * gcc.target/riscv/shift-shift-2.c: Add one more test.
> > ---
> > gcc/config/riscv/riscv.md | 5 +++--
> > gcc/testsuite/gcc.target/riscv/shift-shift-2.c | 16 ++++++++++++++--
> > 2 files changed, 17 insertions(+), 4 deletions(-)
> >
> > diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md
> > index 0f4626656d6..78260fcf6fd 100644
> > --- a/gcc/config/riscv/riscv.md
> > +++ b/gcc/config/riscv/riscv.md
> > @@ -1776,10 +1776,11 @@
> > (set (match_dup 0)
> > (lshiftrt:GPR (match_dup 0) (match_dup 2)))]
> > {
> > - operands[2] = GEN_INT (BITS_PER_WORD
> > + /* Op2 is a VOIDmode constant, so get the mode size from op1. */
> > + operands[2] = GEN_INT (GET_MODE_BITSIZE (GET_MODE (operands[1]))
> > - exact_log2 (INTVAL (operands[2]) + 1));
> > })
> > -
> > +
> > ;; Handle AND with 0xF...F0...0 where there are 32 to 63 zeros. This can be
> > ;; split into two shifts. Otherwise it requires 3 instructions: li, sll, and.
> > (define_split
> > diff --git a/gcc/testsuite/gcc.target/riscv/shift-shift-2.c b/gcc/testsuite/gcc.target/riscv/shift-shift-2.c
> > index 3f07e7776e7..10a5bb728be 100644
> > --- a/gcc/testsuite/gcc.target/riscv/shift-shift-2.c
> > +++ b/gcc/testsuite/gcc.target/riscv/shift-shift-2.c
> > @@ -25,5 +25,17 @@ sub4 (unsigned long i)
> > {
> > return (i << 52) >> 52;
> > }
> > -/* { dg-final { scan-assembler-times "slli" 4 } } */
> > -/* { dg-final { scan-assembler-times "srli" 4 } } */
> > +
> > +unsigned int
> > +sub5 (unsigned int i)
> > +{
> > + unsigned int j;
> > + j = i >> 24;
> > + j = j * (1 << 24);
> > + j = i - j;
> > + return j;
> > +}
> > +/* { dg-final { scan-assembler-times "slli" 5 } } */
> > +/* { dg-final { scan-assembler-times "srli" 5 } } */
> > +/* { dg-final { scan-assembler-times "slliw" 1 } } */
> > +/* { dg-final { scan-assembler-times "srliw" 1 } } */
> > --
> > 2.17.1
> >
>
--
Richard Biener <rguenther@suse.de>
SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany;
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah; HRB 21284 (AG Nürnberg)
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] RISC-V: Fix splitter for 32-bit AND on 64-bit target.
2019-07-16 8:14 ` Kito Cheng
2019-07-16 10:53 ` Richard Biener
@ 2019-07-16 18:37 ` Jim Wilson
1 sibling, 0 replies; 4+ messages in thread
From: Jim Wilson @ 2019-07-16 18:37 UTC (permalink / raw)
To: Kito Cheng; +Cc: Jakub Jelinek, Richard Biener, GCC Patches, Kito Cheng
On Tue, Jul 16, 2019 at 12:42 AM Kito Cheng <kito.cheng@gmail.com> wrote:
> I'd like to back port this patch to GCC 8 and 9, should I send another
> patch mail or just wait ack from release manager?
I think it is only relevant to the gcc-9 branch. The buggy pattern
was added June 2018, which is a while after the gcc-8 branch was made,
and not added to the gcc-8 branch because it was an optimization, not
a bug fix.
Jim
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-07-16 18:00 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-08 11:11 [PATCH] RISC-V: Fix splitter for 32-bit AND on 64-bit target Jim Wilson
2019-07-16 8:14 ` Kito Cheng
2019-07-16 10:53 ` Richard Biener
2019-07-16 18:37 ` Jim Wilson
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).