Skip to content

RISCV: Remove faulty assert that ignored subregister uses #141658

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 27, 2025

Conversation

arsenm
Copy link
Contributor

@arsenm arsenm commented May 27, 2025

This was asserting the raw virtual register class was a scalar
class, instead of computing the net result of the register class
plus the subregister index on the operand. The machine verifier
should be checking this was a valid combination in the first place,
so just drop the assert.

This was asserting the raw virtual register class was a scalar
class, instead of computing the net result of the register class
plus the subregister index on the operand. The machine verifier
should be checking this was a valid combination in the first place,
so just drop the assert.
Copy link
Contributor Author

arsenm commented May 27, 2025

This stack of pull requests is managed by Graphite. Learn more about stacking.

@arsenm arsenm requested review from lukel97, preames and topperc May 27, 2025 19:23
@arsenm arsenm marked this pull request as ready for review May 27, 2025 19:24
@llvmbot
Copy link
Member

llvmbot commented May 27, 2025

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

Author: Matt Arsenault (arsenm)

Changes

This was asserting the raw virtual register class was a scalar
class, instead of computing the net result of the register class
plus the subregister index on the operand. The machine verifier
should be checking this was a valid combination in the first place,
so just drop the assert.


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

2 Files Affected:

  • (modified) llvm/lib/Target/RISCV/RISCVVLOptimizer.cpp (-5)
  • (added) llvm/test/CodeGen/RISCV/rvv/vl-optimizer-subreg-assert.mir (+32)
diff --git a/llvm/lib/Target/RISCV/RISCVVLOptimizer.cpp b/llvm/lib/Target/RISCV/RISCVVLOptimizer.cpp
index f7cbfa1546de6..0bb81a58c29a4 100644
--- a/llvm/lib/Target/RISCV/RISCVVLOptimizer.cpp
+++ b/llvm/lib/Target/RISCV/RISCVVLOptimizer.cpp
@@ -1298,12 +1298,7 @@ RISCVVLOptimizer::getMinimumVLForUser(const MachineOperand &UserOp) const {
   // Instructions like reductions may use a vector register as a scalar
   // register. In this case, we should treat it as only reading the first lane.
   if (isVectorOpUsedAsScalarOp(UserOp)) {
-    [[maybe_unused]] Register R = UserOp.getReg();
-    [[maybe_unused]] const TargetRegisterClass *RC = MRI->getRegClass(R);
-    assert(RISCV::VRRegClass.hasSubClassEq(RC) &&
-           "Expect LMUL 1 register class for vector as scalar operands!");
     LLVM_DEBUG(dbgs() << "    Used this operand as a scalar operand\n");
-
     return MachineOperand::CreateImm(1);
   }
 
diff --git a/llvm/test/CodeGen/RISCV/rvv/vl-optimizer-subreg-assert.mir b/llvm/test/CodeGen/RISCV/rvv/vl-optimizer-subreg-assert.mir
new file mode 100644
index 0000000000000..b816741285b43
--- /dev/null
+++ b/llvm/test/CodeGen/RISCV/rvv/vl-optimizer-subreg-assert.mir
@@ -0,0 +1,32 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 5
+# RUN: llc -mtriple=riscv64 -mattr=+v -run-pass=riscv-vl-optimizer -o - %s | FileCheck %s
+# Check that there is no assert on subregister uses.
+
+---
+name: vl_optimizer_subreg_assert
+tracksRegLiveness: true
+body:             |
+  bb.0:
+    liveins: $v8m2
+
+    ; CHECK-LABEL: name: vl_optimizer_subreg_assert
+    ; CHECK: liveins: $v8m2
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: [[DEF:%[0-9]+]]:vrm8 = IMPLICIT_DEF
+    ; CHECK-NEXT: [[DEF1:%[0-9]+]]:vmv0 = IMPLICIT_DEF
+    ; CHECK-NEXT: [[DEF2:%[0-9]+]]:vrm8 = IMPLICIT_DEF
+    ; CHECK-NEXT: [[PseudoVMERGE_VVM_M8_:%[0-9]+]]:vrm8nov0 = PseudoVMERGE_VVM_M8 $noreg, killed [[DEF2]], [[DEF]], [[DEF1]], -1, 6 /* e64 */
+    ; CHECK-NEXT: [[PseudoVREDMAXU_VS_M8_E64_:%[0-9]+]]:vr = PseudoVREDMAXU_VS_M8_E64 $noreg, [[PseudoVMERGE_VVM_M8_]], [[PseudoVMERGE_VVM_M8_]].sub_vrm1_0, -1, 6 /* e64 */, 1 /* ta, mu */
+    ; CHECK-NEXT: [[PseudoVMV_X_S:%[0-9]+]]:gpr = PseudoVMV_X_S killed [[PseudoVREDMAXU_VS_M8_E64_]], 6 /* e64 */
+    ; CHECK-NEXT: $x10 = COPY [[PseudoVMV_X_S]]
+    ; CHECK-NEXT: PseudoRET implicit $x10
+    %0:vrm8 = IMPLICIT_DEF
+    %1:vmv0 = IMPLICIT_DEF
+    %2:vrm8 = IMPLICIT_DEF
+    %3:vrm8nov0 = PseudoVMERGE_VVM_M8 $noreg, killed %2, %0, %1, -1, 6 /* e64 */
+    %4:vr = PseudoVREDMAXU_VS_M8_E64 $noreg, %3, %3.sub_vrm1_0, -1, 6 /* e64 */, 1 /* ta, mu */
+    %5:gpr = PseudoVMV_X_S killed %4, 6 /* e64 */
+    $x10 = COPY %5
+    PseudoRET implicit $x10
+
+...

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

@arsenm arsenm merged commit d79312b into main May 27, 2025
13 of 14 checks passed
@arsenm arsenm deleted the users/arsenm/riscv/remove-subreg-ignoring-assert branch May 27, 2025 21:34
@llvm-ci
Copy link
Collaborator

llvm-ci commented May 27, 2025

LLVM Buildbot has detected a new failure on builder ml-opt-dev-x86-64 running on ml-opt-dev-x86-64-b1 while building llvm at step 6 "test-build-unified-tree-check-all".

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

Here is the relevant piece of the build log for the reference
Step 6 (test-build-unified-tree-check-all) failure: test (failure)
...
PASS: LLVM :: CodeGen/PowerPC/and-extend-combine.ll (16989 of 58336)
PASS: LLVM :: CodeGen/PowerPC/aix-xcoff-visibility.ll (16990 of 58336)
PASS: LLVM :: CodeGen/PowerPC/alias.ll (16991 of 58336)
PASS: LLVM :: CodeGen/PowerPC/and_add.ll (16992 of 58336)
PASS: LLVM :: CodeGen/PowerPC/and_sra.ll (16993 of 58336)
PASS: LLVM :: CodeGen/PowerPC/andc.ll (16994 of 58336)
PASS: LLVM :: CodeGen/PowerPC/anyext_srl.ll (16995 of 58336)
PASS: LLVM :: CodeGen/PowerPC/aix64-xcoff-roptr.ll (16996 of 58336)
PASS: LLVM :: CodeGen/PowerPC/aix32-p8-scalar_vector_conversions.ll (16997 of 58336)
UNRESOLVED: LLVM :: CodeGen/PowerPC/atomic-compare-exchange-weak.ll (16998 of 58336)
******************** TEST 'LLVM :: CodeGen/PowerPC/atomic-compare-exchange-weak.ll' FAILED ********************
Test has unterminated 'RUN:' directive (with '\') from line 7 to 9
********************
PASS: LLVM :: CodeGen/NVPTX/wmma-ptx86-sm101a.py (16999 of 58336)
PASS: LLVM :: CodeGen/PowerPC/anon_aggr.ll (17000 of 58336)
PASS: LLVM :: CodeGen/PowerPC/ashr-neg1.ll (17001 of 58336)
PASS: LLVM :: CodeGen/PowerPC/aix_scalar_vector_permuted.ll (17002 of 58336)
PASS: LLVM :: CodeGen/PowerPC/arg_promotion.ll (17003 of 58336)
PASS: LLVM :: CodeGen/PowerPC/arr-fp-arg-no-copy.ll (17004 of 58336)
PASS: LLVM :: CodeGen/PowerPC/and-mask.ll (17005 of 58336)
PASS: LLVM :: CodeGen/PowerPC/asm-printer-topological-order.ll (17006 of 58336)
PASS: LLVM :: CodeGen/PowerPC/asm-Zy.ll (17007 of 58336)
PASS: LLVM :: CodeGen/PowerPC/asm-template-I.ll (17008 of 58336)
PASS: LLVM :: CodeGen/PowerPC/asym-regclass-copy.ll (17009 of 58336)
PASS: LLVM :: CodeGen/PowerPC/alloca-crspill.ll (17010 of 58336)
PASS: LLVM :: CodeGen/PowerPC/asm-constraints.ll (17011 of 58336)
PASS: LLVM :: CodeGen/PowerPC/atomic-1.ll (17012 of 58336)
PASS: LLVM :: CodeGen/PowerPC/and_sext.ll (17013 of 58336)
PASS: LLVM :: CodeGen/PowerPC/atomicrmw-uinc-udec-wrap.ll (17014 of 58336)
PASS: LLVM :: CodeGen/PowerPC/atomics-constant.ll (17015 of 58336)
PASS: LLVM :: CodeGen/PowerPC/atomicrmw-cond-sub-clamp.ll (17016 of 58336)
PASS: LLVM :: CodeGen/PowerPC/atomic-float.ll (17017 of 58336)
PASS: LLVM :: CodeGen/PowerPC/asm-dialect.ll (17018 of 58336)
PASS: LLVM :: CodeGen/PowerPC/atomic-minmax.ll (17019 of 58336)
PASS: LLVM :: CodeGen/PowerPC/basic-toc-data-local-linkage.ll (17020 of 58336)
PASS: LLVM :: CodeGen/PowerPC/atomics-indexed.ll (17021 of 58336)
PASS: LLVM :: CodeGen/PowerPC/basic-toc-data-private-linkage.ll (17022 of 58336)
PASS: LLVM :: CodeGen/PowerPC/bcd-intrinsics.ll (17023 of 58336)
PASS: LLVM :: CodeGen/PowerPC/aix-overflow-toc-data.py (17024 of 58336)
PASS: LLVM :: CodeGen/PowerPC/big-endian-actual-args.ll (17025 of 58336)
PASS: LLVM :: CodeGen/PowerPC/atomics-fences.ll (17026 of 58336)
PASS: LLVM :: CodeGen/PowerPC/big-endian-call-result.ll (17027 of 58336)
PASS: LLVM :: CodeGen/PowerPC/basic-toc-data-def.ll (17028 of 58336)
PASS: LLVM :: CodeGen/PowerPC/bdzlr.ll (17029 of 58336)
PASS: LLVM :: CodeGen/PowerPC/atomics.ll (17030 of 58336)
PASS: LLVM :: CodeGen/PowerPC/basic-toc-data-extern.ll (17031 of 58336)
PASS: LLVM :: CodeGen/PowerPC/available-externally.ll (17032 of 58336)
PASS: LLVM :: CodeGen/PowerPC/big-endian-formal-args.ll (17033 of 58336)
PASS: LLVM :: CodeGen/PowerPC/big-endian-store-forward.ll (17034 of 58336)

sivan-shani pushed a commit to sivan-shani/llvm-project that referenced this pull request Jun 3, 2025
This was asserting the raw virtual register class was a scalar
class, instead of computing the net result of the register class
plus the subregister index on the operand. The machine verifier
should be checking this was a valid combination in the first place,
so just drop the assert.
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.

4 participants