Re: [PATCH] riscv: xtheadbb: Fix extendqi insn

[Philipp Tomsich]

Applied to master. Thanks!
Philipp.

On Fri, 8 Sept 2023 at 14:17, Kito Cheng  wrote:

> LGTM
>
> Christoph Muellner 於 2023年9月8日 週五，14:00寫道：
>
>> From: Christoph Müllner 
>>
>> Recently three SPEC CPU 2017 benchmarks broke when using xtheadbb:
>> * 500.perlbench_r
>> * 525.x264_r
>> * 557.xz_r
>>
>> Tracing the issue down revealed, that we emit a 'th.ext xN,xN,15,0'
>> for a extendqi insn, which is obviously wrong.
>> This patch splits the common 'extend2_th_ext'
>> insn into two 'extendqi' and 'extendhi' insns,
>> which emit the right extension instruction.
>> Additionally, this patch adds test cases for these insns.
>>
>> Signed-off-by: Christoph Müllner 
>>
>> gcc/ChangeLog:
>>
>> * config/riscv/thead.md
>> (*extend2_th_ext):
>> Remove broken INSN.
>> (*extendhi2_th_ext): New INSN.
>> (*extendqi2_th_ext): New INSN.
>>
>> gcc/testsuite/ChangeLog:
>>
>> * gcc.target/riscv/xtheadbb-ext-2.c: New test.
>> * gcc.target/riscv/xtheadbb-ext-3.c: New test.
>> ---
>>  gcc/config/riscv/thead.md   | 17 ++---
>>  gcc/testsuite/gcc.target/riscv/xtheadbb-ext-2.c | 12 
>>  gcc/testsuite/gcc.target/riscv/xtheadbb-ext-3.c | 12 
>>  3 files changed, 38 insertions(+), 3 deletions(-)
>>  create mode 100644 gcc/testsuite/gcc.target/riscv/xtheadbb-ext-2.c
>>  create mode 100644 gcc/testsuite/gcc.target/riscv/xtheadbb-ext-3.c
>>
>> diff --git a/gcc/config/riscv/thead.md b/gcc/config/riscv/thead.md
>> index 29f98dec3a8..05d1b32bd94 100644
>> --- a/gcc/config/riscv/thead.md
>> +++ b/gcc/config/riscv/thead.md
>> @@ -58,14 +58,25 @@ (define_insn "*th_ext4"
>>[(set_attr "type" "bitmanip")
>> (set_attr "mode" "")])
>>
>> -(define_insn "*extend2_th_ext"
>> +(define_insn "*extendhi2_th_ext"
>>[(set (match_operand:SUPERQI 0 "register_operand" "=r,r")
>> (sign_extend:SUPERQI
>> -   (match_operand:SHORT 1 "nonimmediate_operand" "r,m")))]
>> +   (match_operand:HI 1 "nonimmediate_operand" "r,m")))]
>>"TARGET_XTHEADBB"
>>"@
>> th.ext\t%0,%1,15,0
>> -   l\t%0,%1"
>> +   lh\t%0,%1"
>> +  [(set_attr "type" "bitmanip,load")
>> +   (set_attr "mode" "")])
>> +
>> +(define_insn "*extendqi2_th_ext"
>> +  [(set (match_operand:SUPERQI 0 "register_operand" "=r,r")
>> +   (sign_extend:SUPERQI
>> +   (match_operand:QI 1 "nonimmediate_operand" "r,m")))]
>> +  "TARGET_XTHEADBB"
>> +  "@
>> +   th.ext\t%0,%1,7,0
>> +   lb\t%0,%1"
>>[(set_attr "type" "bitmanip,load")
>> (set_attr "mode" "")])
>>
>> diff --git a/gcc/testsuite/gcc.target/riscv/xtheadbb-ext-2.c
>> b/gcc/testsuite/gcc.target/riscv/xtheadbb-ext-2.c
>> new file mode 100644
>> index 000..4645b9c56df
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/riscv/xtheadbb-ext-2.c
>> @@ -0,0 +1,12 @@
>> +/* { dg-do compile } */
>> +/* { dg-options "-march=rv64gc_xtheadbb" { target { rv64 } } } */
>> +/* { dg-options "-march=rv32gc_xtheadbb" { target { rv32 } } } */
>> +/* { dg-skip-if "" { *-*-* } { "-O0" "-Os" "-Og" "-Oz" } } */
>> +
>> +signed long extqi(signed char i)
>> +{
>> +return --i;
>> +}
>> +
>> +/* { dg-final { scan-assembler "th.ext\ta\[0-9\]+,a\[0-9\]+,7,0" } } */
>> +/* { dg-final { scan-assembler-not "th.ext\ta\[0-9\]+,a\[0-9\]+,15,0" }
>> } */
>> diff --git a/gcc/testsuite/gcc.target/riscv/xtheadbb-ext-3.c
>> b/gcc/testsuite/gcc.target/riscv/xtheadbb-ext-3.c
>> new file mode 100644
>> index 000..2c9ebbc563a
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/riscv/xtheadbb-ext-3.c
>> @@ -0,0 +1,12 @@
>> +/* { dg-do compile } */
>> +/* { dg-options "-march=rv64gc_xtheadbb" { target { rv64 } } } */
>> +/* { dg-options "-march=rv32gc_xtheadbb" { target { rv32 } } } */
>> +/* { dg-skip-if "" { *-*-* } { "-O0" "-Os" "-Og" "-Oz" } } */
>> +
>> +signed long exthi(signed short i)
>> +{
>> +return --i;
>> +}
>> +
>> +/* { dg-final { scan-assembler "th.ext\ta\[0-9\]+,a\[0-9\]+,15,0" } } */
>> +/* { dg-final { scan-assembler-not "th.ext\ta\[0-9\]+,a\[0-9\]+,7,0" } }
>> */
>> --
>> 2.41.0
>>
>>

Re: [PATCH] riscv: xtheadbb: Fix extendqi insn

[Kito Cheng via Gcc-patches]

LGTM

Christoph Muellner 於 2023年9月8日 週五，14:00寫道：

> From: Christoph Müllner 
>
> Recently three SPEC CPU 2017 benchmarks broke when using xtheadbb:
> * 500.perlbench_r
> * 525.x264_r
> * 557.xz_r
>
> Tracing the issue down revealed, that we emit a 'th.ext xN,xN,15,0'
> for a extendqi insn, which is obviously wrong.
> This patch splits the common 'extend2_th_ext'
> insn into two 'extendqi' and 'extendhi' insns,
> which emit the right extension instruction.
> Additionally, this patch adds test cases for these insns.
>
> Signed-off-by: Christoph Müllner 
>
> gcc/ChangeLog:
>
> * config/riscv/thead.md
> (*extend2_th_ext):
> Remove broken INSN.
> (*extendhi2_th_ext): New INSN.
> (*extendqi2_th_ext): New INSN.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.target/riscv/xtheadbb-ext-2.c: New test.
> * gcc.target/riscv/xtheadbb-ext-3.c: New test.
> ---
>  gcc/config/riscv/thead.md   | 17 ++---
>  gcc/testsuite/gcc.target/riscv/xtheadbb-ext-2.c | 12 
>  gcc/testsuite/gcc.target/riscv/xtheadbb-ext-3.c | 12 
>  3 files changed, 38 insertions(+), 3 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.target/riscv/xtheadbb-ext-2.c
>  create mode 100644 gcc/testsuite/gcc.target/riscv/xtheadbb-ext-3.c
>
> diff --git a/gcc/config/riscv/thead.md b/gcc/config/riscv/thead.md
> index 29f98dec3a8..05d1b32bd94 100644
> --- a/gcc/config/riscv/thead.md
> +++ b/gcc/config/riscv/thead.md
> @@ -58,14 +58,25 @@ (define_insn "*th_ext4"
>[(set_attr "type" "bitmanip")
> (set_attr "mode" "")])
>
> -(define_insn "*extend2_th_ext"
> +(define_insn "*extendhi2_th_ext"
>[(set (match_operand:SUPERQI 0 "register_operand" "=r,r")
> (sign_extend:SUPERQI
> -   (match_operand:SHORT 1 "nonimmediate_operand" "r,m")))]
> +   (match_operand:HI 1 "nonimmediate_operand" "r,m")))]
>"TARGET_XTHEADBB"
>"@
> th.ext\t%0,%1,15,0
> -   l\t%0,%1"
> +   lh\t%0,%1"
> +  [(set_attr "type" "bitmanip,load")
> +   (set_attr "mode" "")])
> +
> +(define_insn "*extendqi2_th_ext"
> +  [(set (match_operand:SUPERQI 0 "register_operand" "=r,r")
> +   (sign_extend:SUPERQI
> +   (match_operand:QI 1 "nonimmediate_operand" "r,m")))]
> +  "TARGET_XTHEADBB"
> +  "@
> +   th.ext\t%0,%1,7,0
> +   lb\t%0,%1"
>[(set_attr "type" "bitmanip,load")
> (set_attr "mode" "")])
>
> diff --git a/gcc/testsuite/gcc.target/riscv/xtheadbb-ext-2.c
> b/gcc/testsuite/gcc.target/riscv/xtheadbb-ext-2.c
> new file mode 100644
> index 000..4645b9c56df
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/xtheadbb-ext-2.c
> @@ -0,0 +1,12 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gc_xtheadbb" { target { rv64 } } } */
> +/* { dg-options "-march=rv32gc_xtheadbb" { target { rv32 } } } */
> +/* { dg-skip-if "" { *-*-* } { "-O0" "-Os" "-Og" "-Oz" } } */
> +
> +signed long extqi(signed char i)
> +{
> +return --i;
> +}
> +
> +/* { dg-final { scan-assembler "th.ext\ta\[0-9\]+,a\[0-9\]+,7,0" } } */
> +/* { dg-final { scan-assembler-not "th.ext\ta\[0-9\]+,a\[0-9\]+,15,0" } }
> */
> diff --git a/gcc/testsuite/gcc.target/riscv/xtheadbb-ext-3.c
> b/gcc/testsuite/gcc.target/riscv/xtheadbb-ext-3.c
> new file mode 100644
> index 000..2c9ebbc563a
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/xtheadbb-ext-3.c
> @@ -0,0 +1,12 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gc_xtheadbb" { target { rv64 } } } */
> +/* { dg-options "-march=rv32gc_xtheadbb" { target { rv32 } } } */
> +/* { dg-skip-if "" { *-*-* } { "-O0" "-Os" "-Og" "-Oz" } } */
> +
> +signed long exthi(signed short i)
> +{
> +return --i;
> +}
> +
> +/* { dg-final { scan-assembler "th.ext\ta\[0-9\]+,a\[0-9\]+,15,0" } } */
> +/* { dg-final { scan-assembler-not "th.ext\ta\[0-9\]+,a\[0-9\]+,7,0" } }
> */
> --
> 2.41.0
>
>

[PATCH] riscv: xtheadbb: Fix extendqi insn

[Christoph Muellner]

From: Christoph Müllner 

Recently three SPEC CPU 2017 benchmarks broke when using xtheadbb:
* 500.perlbench_r
* 525.x264_r
* 557.xz_r

Tracing the issue down revealed, that we emit a 'th.ext xN,xN,15,0'
for a extendqi insn, which is obviously wrong.
This patch splits the common 'extend2_th_ext'
insn into two 'extendqi' and 'extendhi' insns,
which emit the right extension instruction.
Additionally, this patch adds test cases for these insns.

Signed-off-by: Christoph Müllner 

gcc/ChangeLog:

* config/riscv/thead.md (*extend2_th_ext):
Remove broken INSN.
(*extendhi2_th_ext): New INSN.
(*extendqi2_th_ext): New INSN.

gcc/testsuite/ChangeLog:

* gcc.target/riscv/xtheadbb-ext-2.c: New test.
* gcc.target/riscv/xtheadbb-ext-3.c: New test.
---
 gcc/config/riscv/thead.md   | 17 ++---
 gcc/testsuite/gcc.target/riscv/xtheadbb-ext-2.c | 12 
 gcc/testsuite/gcc.target/riscv/xtheadbb-ext-3.c | 12 
 3 files changed, 38 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/riscv/xtheadbb-ext-2.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/xtheadbb-ext-3.c

diff --git a/gcc/config/riscv/thead.md b/gcc/config/riscv/thead.md
index 29f98dec3a8..05d1b32bd94 100644
--- a/gcc/config/riscv/thead.md
+++ b/gcc/config/riscv/thead.md
@@ -58,14 +58,25 @@ (define_insn "*th_ext4"
   [(set_attr "type" "bitmanip")
(set_attr "mode" "")])
 
-(define_insn "*extend2_th_ext"
+(define_insn "*extendhi2_th_ext"
   [(set (match_operand:SUPERQI 0 "register_operand" "=r,r")
(sign_extend:SUPERQI
-   (match_operand:SHORT 1 "nonimmediate_operand" "r,m")))]
+   (match_operand:HI 1 "nonimmediate_operand" "r,m")))]
   "TARGET_XTHEADBB"
   "@
th.ext\t%0,%1,15,0
-   l\t%0,%1"
+   lh\t%0,%1"
+  [(set_attr "type" "bitmanip,load")
+   (set_attr "mode" "")])
+
+(define_insn "*extendqi2_th_ext"
+  [(set (match_operand:SUPERQI 0 "register_operand" "=r,r")
+   (sign_extend:SUPERQI
+   (match_operand:QI 1 "nonimmediate_operand" "r,m")))]
+  "TARGET_XTHEADBB"
+  "@
+   th.ext\t%0,%1,7,0
+   lb\t%0,%1"
   [(set_attr "type" "bitmanip,load")
(set_attr "mode" "")])
 
diff --git a/gcc/testsuite/gcc.target/riscv/xtheadbb-ext-2.c 
b/gcc/testsuite/gcc.target/riscv/xtheadbb-ext-2.c
new file mode 100644
index 000..4645b9c56df
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/xtheadbb-ext-2.c
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gc_xtheadbb" { target { rv64 } } } */
+/* { dg-options "-march=rv32gc_xtheadbb" { target { rv32 } } } */
+/* { dg-skip-if "" { *-*-* } { "-O0" "-Os" "-Og" "-Oz" } } */
+
+signed long extqi(signed char i)
+{
+return --i;
+}
+
+/* { dg-final { scan-assembler "th.ext\ta\[0-9\]+,a\[0-9\]+,7,0" } } */
+/* { dg-final { scan-assembler-not "th.ext\ta\[0-9\]+,a\[0-9\]+,15,0" } } */
diff --git a/gcc/testsuite/gcc.target/riscv/xtheadbb-ext-3.c 
b/gcc/testsuite/gcc.target/riscv/xtheadbb-ext-3.c
new file mode 100644
index 000..2c9ebbc563a
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/xtheadbb-ext-3.c
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gc_xtheadbb" { target { rv64 } } } */
+/* { dg-options "-march=rv32gc_xtheadbb" { target { rv32 } } } */
+/* { dg-skip-if "" { *-*-* } { "-O0" "-Os" "-Og" "-Oz" } } */
+
+signed long exthi(signed short i)
+{
+return --i;
+}
+
+/* { dg-final { scan-assembler "th.ext\ta\[0-9\]+,a\[0-9\]+,15,0" } } */
+/* { dg-final { scan-assembler-not "th.ext\ta\[0-9\]+,a\[0-9\]+,7,0" } } */
-- 
2.41.0