Skip to content

MachineVerifier: Fix check for range type #124894

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 2 commits into from
Jan 30, 2025

Conversation

arsenm
Copy link
Contributor

@arsenm arsenm commented Jan 29, 2025

We need to permit scalar extending loads with range annotations.

Fix expensive_checks failures after 11db7fb

@arsenm arsenm marked this pull request as ready for review January 29, 2025 08:08
Copy link
Contributor Author

arsenm commented Jan 29, 2025

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

@llvmbot
Copy link
Member

llvmbot commented Jan 29, 2025

@llvm/pr-subscribers-backend-amdgpu

@llvm/pr-subscribers-llvm-globalisel

Author: Matt Arsenault (arsenm)

Changes

We need to permit scalar extending loads with range annotations.

Fix expensive_checks failures after 11db7fb


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

2 Files Affected:

  • (modified) llvm/lib/CodeGen/MachineVerifier.cpp (+6-2)
  • (modified) llvm/test/MachineVerifier/AMDGPU/test_g_incompatible_range.mir (+9-1)
diff --git a/llvm/lib/CodeGen/MachineVerifier.cpp b/llvm/lib/CodeGen/MachineVerifier.cpp
index becf41b0a7bcdf..db65ebae7a7e99 100644
--- a/llvm/lib/CodeGen/MachineVerifier.cpp
+++ b/llvm/lib/CodeGen/MachineVerifier.cpp
@@ -1285,8 +1285,12 @@ void MachineVerifier::verifyPreISelGenericInstruction(const MachineInstr *MI) {
         if (MMO.getRanges()) {
           ConstantInt *i =
               mdconst::extract<ConstantInt>(MMO.getRanges()->getOperand(0));
-          if (i->getIntegerType()->getBitWidth() !=
-              ValTy.getScalarType().getSizeInBits()) {
+          const LLT RangeTy = LLT::scalar(i->getIntegerType()->getBitWidth());
+          const LLT MemTy = MMO.getMemoryType();
+          if (MemTy.getScalarType() != RangeTy ||
+              ValTy.isScalar() != MemTy.isScalar() ||
+              (ValTy.isVector() &&
+               ValTy.getNumElements() != MemTy.getNumElements())) {
             report("range is incompatible with the result type", MI);
           }
         }
diff --git a/llvm/test/MachineVerifier/AMDGPU/test_g_incompatible_range.mir b/llvm/test/MachineVerifier/AMDGPU/test_g_incompatible_range.mir
index 6813070ade9c50..0038173a077884 100644
--- a/llvm/test/MachineVerifier/AMDGPU/test_g_incompatible_range.mir
+++ b/llvm/test/MachineVerifier/AMDGPU/test_g_incompatible_range.mir
@@ -1,4 +1,4 @@
-# RUN: not --crash llc -mtriple=amdgcn-amd-amdhsa -mcpu=gfx942 -run-pass=none -filetype=null %s 2>&1 | FileCheck %s
+# RUN: not --crash llc -mtriple=amdgcn-amd-amdhsa -mcpu=gfx942 -run-pass=none -filetype=null %s 2>&1 | FileCheck -implicit-check-not="Bad machine code" %s
 --- |
   define void @mismatched_range_type() {
     ret void
@@ -21,10 +21,18 @@ body:             |
     ; CHECK: Bad machine code: range is incompatible with the result type
     %3:_(<2 x s32>) = G_LOAD %0(p1) :: (volatile load (s64), align 4, !range !0, addrspace 1)
 
+    ; CHECK: Bad machine code: range is incompatible with the result type
     %4:_(p0) = G_LOAD %0(p1) :: (volatile load (s64), align 4, !range !0, addrspace 1)
 
+    ; CHECK: Bad machine code: range is incompatible with the result type
     %5:_(<2 x p0>) = G_LOAD %0(p1) :: (volatile load (s64), align 4, !range !0, addrspace 1)
 
+    ; CHECK: Bad machine code: range is incompatible with the result type
+    %6:_(<3 x s64>) = G_LOAD %0(p1) :: (volatile load (s64), align 4, !range !0, addrspace 1)
+
+    ; CHECK: Bad machine code: range is incompatible with the result type
+    %7:_(<3 x s64>) = G_LOAD %0(p1) :: (volatile load (<2 x s64>), align 4, !range !0, addrspace 1)
+
     $vgpr0_vgpr1 = COPY %3
     SI_RETURN implicit $vgpr0_vgpr1
 

We need to permit scalar extending loads with range annotations.

Fix expensive_checks failures after 11db7fb
@arsenm arsenm force-pushed the users/arsenm/machine-verifier-fix-mmo-range-verifier branch from d9e7996 to 7fa5e14 Compare January 29, 2025 15:50
Copy link
Contributor Author

arsenm commented Jan 30, 2025

Merge activity

  • Jan 29, 10:54 PM EST: A user started a stack merge that includes this pull request via Graphite.
  • Jan 29, 10:56 PM EST: A user merged this pull request with Graphite.

@arsenm arsenm merged commit 6017480 into main Jan 30, 2025
8 checks passed
@arsenm arsenm deleted the users/arsenm/machine-verifier-fix-mmo-range-verifier branch January 30, 2025 03:56
Comment on lines +1291 to +1293
ValTy.isScalar() != MemTy.isScalar() ||
(ValTy.isVector() &&
ValTy.getNumElements() != MemTy.getNumElements())) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This part, validating ValTy against MemTy, does not belong inside if (MMO.getRanges()).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are many violations of the memory type that would need to be fixed

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