Skip to content

Conversation

@lukel97
Copy link
Contributor

@lukel97 lukel97 commented Sep 3, 2025

If an instruction's demanded VL is a virtual register defined by a vleff instruction, it might not dominate and fail to have its VL reduced.

In leiu of the output VL, we can try and use the AVL passed to the vleff itself since it will be at least greater than or equal the original VL.

I tried to create an LLVM IR test for this in but didn't have any luck because the scheduler kept on moving the instruction past the vleff, so it always dominated. So I've just included some mir tests instead.

…imizer

If an instruction's demanded VL is a virtual register defined by a vleff instruction, it might not dominate and fail to have its VL reduced.

In leiu of the output VL, we can try and use the AVL passed to the vleff itself since it will be at least greater than or equal the original VL.

I tried to create an LLVM IR test for this in but didn't have any luck because the scheduler kept on moving the instruction past the vleff, so it always dominated. So I've just included some mir tests instead.
Copy link
Contributor

@wangpc-pp wangpc-pp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Copy link
Collaborator

@preames preames left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@preames
Copy link
Collaborator

preames commented Sep 3, 2025

As a follow up, you could consider the same kind of logic as ensureDominates in RISCVVectorPeephole, or maybe rematerialization.

const MachineInstr *VLMI = MRI->getVRegDef(CommonVL->getReg());
if (RISCVInstrInfo::isFaultOnlyFirstLoad(*VLMI) &&
!MDT->dominates(VLMI, &MI))
CommonVL = VLMI->getOperand(4);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this the correct operand number for a masked fault only first load?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, fixed in 712d2fc

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch.

@llvmbot
Copy link
Member

llvmbot commented Sep 3, 2025

@llvm/pr-subscribers-backend-risc-v

Author: Luke Lau (lukel97)

Changes

If an instruction's demanded VL is a virtual register defined by a vleff instruction, it might not dominate and fail to have its VL reduced.

In leiu of the output VL, we can try and use the AVL passed to the vleff itself since it will be at least greater than or equal the original VL.

I tried to create an LLVM IR test for this in but didn't have any luck because the scheduler kept on moving the instruction past the vleff, so it always dominated. So I've just included some mir tests instead.


Full diff: https://github.com/llvm/llvm-project/pull/156618.diff

2 Files Affected:

  • (modified) llvm/lib/Target/RISCV/RISCVVLOptimizer.cpp (+9)
  • (modified) llvm/test/CodeGen/RISCV/rvv/vl-opt.mir (+58-1)
diff --git a/llvm/lib/Target/RISCV/RISCVVLOptimizer.cpp b/llvm/lib/Target/RISCV/RISCVVLOptimizer.cpp
index 9b70eb6c25b12..baf6aaac62779 100644
--- a/llvm/lib/Target/RISCV/RISCVVLOptimizer.cpp
+++ b/llvm/lib/Target/RISCV/RISCVVLOptimizer.cpp
@@ -1464,6 +1464,15 @@ bool RISCVVLOptimizer::tryReduceVL(MachineInstr &MI) const {
   assert((CommonVL->isImm() || CommonVL->getReg().isVirtual()) &&
          "Expected VL to be an Imm or virtual Reg");
 
+  // If the VL is defined by a vleff that doesn't dominate MI, try using the
+  // vleff's AVL. It will be greater than or equal to the output VL.
+  if (CommonVL->isReg()) {
+    const MachineInstr *VLMI = MRI->getVRegDef(CommonVL->getReg());
+    if (RISCVInstrInfo::isFaultOnlyFirstLoad(*VLMI) &&
+        !MDT->dominates(VLMI, &MI))
+      CommonVL = VLMI->getOperand(RISCVII::getVLOpNum(VLMI->getDesc()));
+  }
+
   if (!RISCV::isVLKnownLE(*CommonVL, VLOp)) {
     LLVM_DEBUG(dbgs() << "  Abort due to CommonVL not <= VLOp.\n");
     return false;
diff --git a/llvm/test/CodeGen/RISCV/rvv/vl-opt.mir b/llvm/test/CodeGen/RISCV/rvv/vl-opt.mir
index 60398cdf1db66..0acdca91ee84c 100644
--- a/llvm/test/CodeGen/RISCV/rvv/vl-opt.mir
+++ b/llvm/test/CodeGen/RISCV/rvv/vl-opt.mir
@@ -603,4 +603,61 @@ body: |
     $x10 = COPY %9
     PseudoRET implicit $x10
 ...
-
+---
+name: vleff_imm
+body: |
+  bb.0:
+    ; CHECK-LABEL: name: vleff_imm
+    ; CHECK: %x:vr = PseudoVADD_VV_M1 $noreg, $noreg, $noreg, 1, 3 /* e8 */, 3 /* ta, ma */
+    ; CHECK-NEXT: %y:vr, %vl:gprnox0 = PseudoVLE8FF_V_M1 $noreg, $noreg, 1, 3 /* e8 */, 3 /* ta, ma */
+    ; CHECK-NEXT: PseudoVSE8_V_M1 %x, $noreg, %vl, 3 /* e8 */
+    %x:vr = PseudoVADD_VV_M1 $noreg, $noreg, $noreg, -1, 3 /* e8 */, 3 /* ta, ma */
+    %y:vr, %vl:gprnox0 = PseudoVLE8FF_V_M1 $noreg, $noreg, 1, 3 /* e8 */, 3 /* ta, ma */
+    PseudoVSE8_V_M1 %x, $noreg, %vl, 3 /* e8 */
+...
+---
+name: vleff_reg_dominates
+body: |
+  bb.0:
+    liveins: $x8
+    ; CHECK-LABEL: name: vleff_reg_dominates
+    ; CHECK: liveins: $x8
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: %avl:gprnox0 = COPY $x8
+    ; CHECK-NEXT: %x:vr = PseudoVADD_VV_M1 $noreg, $noreg, $noreg, %avl, 3 /* e8 */, 3 /* ta, ma */
+    ; CHECK-NEXT: %y:vr, %vl:gprnox0 = PseudoVLE8FF_V_M1 $noreg, $noreg, %avl, 3 /* e8 */, 3 /* ta, ma */
+    ; CHECK-NEXT: PseudoVSE8_V_M1 %x, $noreg, %vl, 3 /* e8 */
+    %avl:gprnox0 = COPY $x8
+    %x:vr = PseudoVADD_VV_M1 $noreg, $noreg, $noreg, -1, 3 /* e8 */, 3 /* ta, ma */
+    %y:vr, %vl:gprnox0 = PseudoVLE8FF_V_M1 $noreg, $noreg, %avl, 3 /* e8 */, 3 /* ta, ma */
+    PseudoVSE8_V_M1 %x, $noreg, %vl, 3 /* e8 */
+...
+---
+name: vleff_reg_doesnt_dominate
+body: |
+  bb.0:
+    liveins: $x8
+    ; CHECK-LABEL: name: vleff_reg_doesnt_dominate
+    ; CHECK: liveins: $x8
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: %x:vr = PseudoVADD_VV_M1 $noreg, $noreg, $noreg, -1, 3 /* e8 */, 3 /* ta, ma */
+    ; CHECK-NEXT: %avl:gprnox0 = COPY $x8
+    ; CHECK-NEXT: %y:vr, %vl:gprnox0 = PseudoVLE8FF_V_M1 $noreg, $noreg, %avl, 3 /* e8 */, 3 /* ta, ma */
+    ; CHECK-NEXT: PseudoVSE8_V_M1 %x, $noreg, %vl, 3 /* e8 */
+    %x:vr = PseudoVADD_VV_M1 $noreg, $noreg, $noreg, -1, 3 /* e8 */, 3 /* ta, ma */
+    %avl:gprnox0 = COPY $x8
+    %y:vr, %vl:gprnox0 = PseudoVLE8FF_V_M1 $noreg, $noreg, %avl, 3 /* e8 */, 3 /* ta, ma */
+    PseudoVSE8_V_M1 %x, $noreg, %vl, 3 /* e8 */
+...
+---
+name: vleff_mask_imm
+body: |
+  bb.0:
+    ; CHECK-LABEL: name: vleff_mask_imm
+    ; CHECK: %x:vr = PseudoVADD_VV_M1 $noreg, $noreg, $noreg, 1, 3 /* e8 */, 3 /* ta, ma */
+    ; CHECK-NEXT: %y:vrnov0, %vl:gprnox0 = PseudoVLE8FF_V_M1_MASK $noreg, $noreg, $noreg, 1, 3 /* e8 */, 3 /* ta, ma */
+    ; CHECK-NEXT: PseudoVSE8_V_M1 %x, $noreg, %vl, 3 /* e8 */
+    %x:vr = PseudoVADD_VV_M1 $noreg, $noreg, $noreg, -1, 3 /* e8 */, 3 /* ta, ma */
+    %y:vrnov0, %vl:gprnox0 = PseudoVLE8FF_V_M1_MASK $noreg, $noreg, $noreg, 1, 3 /* e8 */, 3 /* ta, ma */
+    PseudoVSE8_V_M1 %x, $noreg, %vl, 3 /* e8 */
+...

@lukel97
Copy link
Contributor Author

lukel97 commented Sep 3, 2025

As a follow up, you could consider the same kind of logic as ensureDominates in RISCVVectorPeephole, or maybe rematerialization.

Is this in reference to #156639?

@preames
Copy link
Collaborator

preames commented Sep 3, 2025

As a follow up, you could consider the same kind of logic as ensureDominates in RISCVVectorPeephole, or maybe rematerialization.

Is this in reference to #156639?

Yep, wrong PR.

Copy link
Collaborator

@topperc topperc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@lukel97 lukel97 merged commit a95edec into llvm:main Sep 4, 2025
10 checks passed
@llvm-ci
Copy link
Collaborator

llvm-ci commented Sep 4, 2025

LLVM Buildbot has detected a new failure on builder lldb-aarch64-ubuntu running on linaro-lldb-aarch64-ubuntu while building llvm at step 6 "test".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/59/builds/23648

Here is the relevant piece of the build log for the reference
Step 6 (test) failure: build (failure)
...
PASS: lldb-api :: commands/frame/var/TestFrameVar.py (188 of 2314)
PASS: lldb-api :: commands/help/TestHelp.py (189 of 2314)
PASS: lldb-api :: commands/log/invalid-args/TestInvalidArgsLog.py (190 of 2314)
PASS: lldb-api :: commands/platform/basic/TestPlatformCommand.py (191 of 2314)
PASS: lldb-api :: commands/platform/basic/TestPlatformPython.py (192 of 2314)
PASS: lldb-api :: commands/memory/write/TestMemoryWrite.py (193 of 2314)
PASS: lldb-api :: commands/platform/file/close/TestPlatformFileClose.py (194 of 2314)
PASS: lldb-api :: commands/platform/file/read/TestPlatformFileRead.py (195 of 2314)
PASS: lldb-api :: commands/memory/read/TestMemoryRead.py (196 of 2314)
UNRESOLVED: lldb-api :: commands/gui/spawn-threads/TestGuiSpawnThreads.py (197 of 2314)
******************** TEST 'lldb-api :: commands/gui/spawn-threads/TestGuiSpawnThreads.py' FAILED ********************
Script:
--
/usr/bin/python3.10 /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/llvm-project/lldb/test/API/dotest.py -u CXXFLAGS -u CFLAGS --env LLVM_LIBS_DIR=/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/./lib --env LLVM_INCLUDE_DIR=/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/include --env LLVM_TOOLS_DIR=/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/./bin --arch aarch64 --build-dir /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/lldb-test-build.noindex --lldb-module-cache-dir /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/lldb-test-build.noindex/module-cache-lldb/lldb-api --clang-module-cache-dir /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/lldb-test-build.noindex/module-cache-clang/lldb-api --executable /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/./bin/lldb --compiler /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/./bin/clang --dsymutil /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/./bin/dsymutil --make /usr/bin/gmake --llvm-tools-dir /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/./bin --lldb-obj-root /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/tools/lldb --lldb-libs-dir /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/./lib --cmake-build-type Release /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/llvm-project/lldb/test/API/commands/gui/spawn-threads -p TestGuiSpawnThreads.py
--
Exit Code: 1

Command Output (stdout):
--
lldb version 22.0.0git (https://github.com/llvm/llvm-project.git revision a95edec28a4900d15f6fce17561964071600609a)
  clang revision a95edec28a4900d15f6fce17561964071600609a
  llvm revision a95edec28a4900d15f6fce17561964071600609a
Skipping the following test categories: ['libc++', 'msvcstl', 'dsym', 'gmodules', 'debugserver', 'objc']

--
Command Output (stderr):
--
FAIL: LLDB (/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/bin/clang-aarch64) :: test_gui (TestGuiSpawnThreads.TestGuiSpawnThreadsTest)
======================================================================
ERROR: test_gui (TestGuiSpawnThreads.TestGuiSpawnThreadsTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/llvm-project/lldb/packages/Python/lldbsuite/test/decorators.py", line 151, in wrapper
    return func(*args, **kwargs)
  File "/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/llvm-project/lldb/test/API/commands/gui/spawn-threads/TestGuiSpawnThreads.py", line 44, in test_gui
    self.child.expect_exact(f"thread #{i + 2}: tid =")
  File "/usr/local/lib/python3.10/dist-packages/pexpect/spawnbase.py", line 432, in expect_exact
    return exp.expect_loop(timeout)
  File "/usr/local/lib/python3.10/dist-packages/pexpect/expect.py", line 179, in expect_loop
    return self.eof(e)
  File "/usr/local/lib/python3.10/dist-packages/pexpect/expect.py", line 122, in eof
    raise exc
pexpect.exceptions.EOF: End Of File (EOF). Exception style platform.
<pexpect.pty_spawn.spawn object at 0xe9d7a52ada80>
command: /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/bin/lldb
args: ['/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/bin/lldb', '--no-lldbinit', '--no-use-colors', '-O', 'settings clear --all', '-O', 'settings set symbols.enable-external-lookup false', '-O', 'settings set target.inherit-tcc true', '-O', 'settings set target.disable-aslr false', '-O', 'settings set target.detach-on-error false', '-O', 'settings set target.auto-apply-fixits false', '-O', 'settings set plugin.process.gdb-remote.packet-timeout 60', '-O', 'settings set symbols.clang-modules-cache-path "/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/lldb-test-build.noindex/module-cache-lldb/lldb-api"', '-O', 'settings set use-color false', '-O', 'settings set show-statusline false', '--file', '/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/lldb-test-build.noindex/commands/gui/spawn-threads/TestGuiSpawnThreads.test_gui/a.out']
buffer (last 100 chars): b''
before (last 100 chars): b'thread_create.c:442:8\n#20 0x0000ef11ebda5edc ./misc/../sysdeps/unix/sysv/linux/aarch64/clone.S:82:0\n'
after: <class 'pexpect.exceptions.EOF'>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants