Patch Status 38456-12_RISCV_Fix_vlusedbynonrvvinsn_logic_of_vsetvl_pass-1

[github-actions[bot]]

Precommit CI Run information

Logs can be found in the associated Github Actions run: https://github.com/ewlu/gcc-precommit-ci/actions/runs/10811094381

Patch information

Applied patches: 1 -> 1 Associated series: https://patchwork.sourceware.org/project/gcc/list/?series=38456 Last patch applied: https://patchwork.sourceware.org/project/gcc/patch/51AE84BB9E509EBC+202409111936098703655@rivai.ai/ Patch id: 97433

Build Targets

Some targets are built as multilibs. If a build target ends with multilib, please refer to the table below to see all the targets within that multilib.	Target name	-march string
newlib-rv64gcv-lp64d-multilib	rv64gcv-lp64d, rv32gc-ilp32d, rv64gc-lp64d, rv32imc_zba_zbb_zbc_zbs-ilp32
linux-rv64gcv-lp64d-multilib	rv32gcv-ilp32d, rv64gcv-lp64d
linux-rv64gc_zba_zbb_zbc_zbs-lp64d-multilib	rv32gc_zba_zbb_zbc_zbs-ilp32d, rv64gc_zba_zbb_zbc_zbs-lp64d

Target Information

Target Shorthand	-march string
Bitmanip	gc_zba_zbb_zbc_zbs

Notes

Testsuite results use a more lenient allowlist to reduce error reporting with flakey tests. Please take a look at the current allowlist. Results come from a sum file comparator. Each patch is applied to a well known, non-broken baseline taken from our gcc postcommit framework (here) which runs the full gcc testsuite every 6 hours. If you have any questions or encounter any issues which may seem like false-positives, please contact us at patchworks-ci@rivosinc.com

[github-actions[bot]]

Lint Status

The following issues have been found with 38456-12_RISCV_Fix_vlusedbynonrvvinsn_logic_of_vsetvl_pass-1 using gcc's ./contrib/check_GNU_style.py. Please use your best judgement when resolving these issues. These are only warnings and do not need to be resolved in order to merge your patch.

Traceback (most recent call last):
  File "./gcc/contrib/check_GNU_style.py", line 45, in <module>
    main()
  File "./gcc/contrib/check_GNU_style.py", line 43, in main
    check_GNU_style_file(diff_file, format)
  File "/home/runner/work/gcc-precommit-ci/gcc-precommit-ci/riscv-gnu-toolchain/gcc/contrib/check_GNU_style_lib.py", line 279, in check_GNU_style_file
    patch = PatchSet(file)
  File "/home/runner/.local/lib/python3.8/site-packages/unidiff/patch.py", line 462, in __init__
    self._parse(data, encoding=encoding, metadata_only=metadata_only)
  File "/home/runner/.local/lib/python3.8/site-packages/unidiff/patch.py", line 552, in _parse
    current_file._parse_hunk(line, diff, encoding, metadata_only)
  File "/home/runner/.local/lib/python3.8/site-packages/unidiff/patch.py", line 318, in _parse_hunk
    raise UnidiffParseError(
unidiff.errors.UnidiffParseError: Hunk diff line expected: @@ -1035,7 +1038,10 @@ public:

Additional information

[github-actions[bot]]

Apply Status

Target	Status
Baseline hash: https://github.com/gcc-mirror/gcc/commit/f80e4ba94e41410219bdcdb1a0f204ea3f148666	Failed
Tip of tree hash: https://github.com/gcc-mirror/gcc/commit/3fd07d4f04f43816a038daf9b16c6d5bf2e96c9b	Failed

Command

> git am ../patches/*.patch --whitespace=fix -q --3way --empty=drop

Output

error: corrupt patch at line 79
error: could not build fake ancestor
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config advice.mergeConflict false"
Patch failed at 0001 RISC-V: Fix vl_used_by_non_rvv_insn logic of vsetvl pass
--- missing vsetvli a4, a4 here
        slli    a4,a4,1
        vsetvli zero,a4,e32,m1,ta,ma
        li      a2,-1
        addi    a5,a5,16
        vslide1down.vx  v1,v1,a2
        vslide1down.vx  v1,v1,zero
        vsetivli        zero,2,e64,m1,ta,ma
        vse64.v v1,0(a5)
        ret

When I revisit the codes here:

m_vl = ::get_vl
...
update_avl -> "m_vl" variable is modified
...
using wrong m_vl in the following.

A dedicated temporary variable dest_vl looks reasonable here.

LGTM.

The RISC-V folks will commit this patch for you.
Thanks.

juzhe.zhong@rivai.ai

From: Li, Pan2
Date: 2024-09-11 19:29
To: juzhe.zhong@rivai.ai
Subject: FW: [PATCH 1/2] RISC-V: Fix vl_used_by_non_rvv_insn logic of vsetvl pass
FYI.

-----Original Message-----
From: garthlei <garthlei@linux.alibaba.com> 
Sent: Wednesday, September 11, 2024 5:10 PM
To: gcc-patches <gcc-patches@gcc.gnu.org>
Subject: [PATCH 1/2] RISC-V: Fix vl_used_by_non_rvv_insn logic of vsetvl pass

This patch fixes a bug in the current vsetvl pass.  The current pass uses
`m_vl` to determine whether the dest operand has been used by non-RVV
instructions.  However, `m_vl` may have been modified as a result of an
`update_avl` call, and thus would be no longer the dest operand of the
original instruction.  This can lead to incorrect vsetvl eliminations, as is
shown in the testcase.  In this patch, we create a `dest_vl` variable for
this scenerio.

gcc/ChangeLog:

* config/riscv/riscv-vsetvl.cc: Use `dest_vl` for dest VL operand

gcc/testsuite/ChangeLog:

* gcc.target/riscv/rvv/vsetvl/vsetvl_bug-3.c: New test.
---
gcc/config/riscv/riscv-vsetvl.cc                | 16 +++++++++++-----
.../gcc.target/riscv/rvv/vsetvl/vsetvl_bug-3.c  | 17 +++++++++++++++++
2 files changed, 28 insertions(+), 5 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/vsetvl/vsetvl_bug-3.c

-- 
2.17.1

diff --git a/gcc/config/riscv/riscv-vsetvl.cc b/gcc/config/riscv/riscv-vsetvl.cc
index 017efa8bc17..ce831685439 100644
--- a/gcc/config/riscv/riscv-vsetvl.cc
+++ b/gcc/config/riscv/riscv-vsetvl.cc
@@ -1002,6 +1002,9 @@ public:
   void parse_insn (insn_info *insn)
   {
+    /* The VL dest of the insn */
+    rtx dest_vl = NULL_RTX;
+
     m_insn = insn;
     m_bb = insn->bb ();
     /* Return if it is debug insn for the consistency with optimize == 0.  */
@@ -1035,7 +1038,10 @@ public:
     if (m_avl)
       {
if (vsetvl_insn_p (insn->rtl ()) || has_vlmax_avl ())
-   m_vl = ::get_vl (insn->rtl ());
+   {
+     m_vl = ::get_vl (insn->rtl ());
+     dest_vl = m_vl;
+   }
if (has_nonvlmax_reg_avl ())
  m_avl_def = find_access (insn->uses (), REGNO (m_avl))->def ();
@@ -1132,22 +1138,22 @@ public:
       }
     /* Determine if dest operand(vl) has been used by non-RVV instructions.  */
-    if (has_vl ())
+    if (dest_vl)
       {
const hash_set<use_info *> vl_uses
-   = get_all_real_uses (get_insn (), REGNO (get_vl ()));
+   = get_all_real_uses (get_insn (), REGNO (dest_vl));
for (use_info *use : vl_uses)
  {
    gcc_assert (use->insn ()->is_real ());
    rtx_insn *rinsn = use->insn ()->rtl ();
    if (!has_vl_op (rinsn)
- || count_regno_occurrences (rinsn, REGNO (get_vl ())) != 1)
+ || count_regno_occurrences (rinsn, REGNO (dest_vl)) != 1)
      {
m_vl_used_by_non_rvv_insn = true;
break;
      }
    rtx avl = ::get_avl (rinsn);
-     if (!avl || !REG_P (avl) || REGNO (get_vl ()) != REGNO (avl))
+     if (!avl || !REG_P (avl) || REGNO (dest_vl) != REGNO (avl))
      {
m_vl_used_by_non_rvv_insn = true;
break;
diff --git a/gcc/testsuite/gcc.target/riscv/rvv/vsetvl/vsetvl_bug-3.c b/gcc/testsuite/gcc.target/riscv/rvv/vsetvl/vsetvl_bug-3.c
new file mode 100644
index 00000000000..c155f5613d2
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/rvv/vsetvl/vsetvl_bug-3.c
@@ -0,0 +1,17 @@
+/* { dg-do compile } */
+/* { dg-options "-march=rv32gcv -mabi=ilp32d -O2 -fdump-rtl-vsetvl-details" } */
+
+#include <riscv_vector.h>
+
+uint64_t a[2], b[2];
+
+void
+foo ()
+{
+  size_t vl = __riscv_vsetvl_e64m1 (2);
+  vuint64m1_t vx = __riscv_vle64_v_u64m1 (a, vl);
+  vx = __riscv_vslide1down_vx_u64m1 (vx, 0xffffffffull, vl);
+  __riscv_vse64_v_u64m1 (b, vx, vl);
+}
+
+/* { dg-final { scan-rtl-dump-not "Eliminate insn" "vsetvl" } }  */

Additional information