Skip to content

Conversation

@topperc
Copy link
Collaborator

@topperc topperc commented Dec 15, 2024

Ensure destination type is at least 2 bits.
Remove unnecessary check that both sources are the same type. The verifier already handles this generically.

Ensure destination type is at least 2 bytes.
Remove unnecessary check that both sources are the same type. The
verifier already handles this generically.
@topperc topperc requested review from aemerson and arsenm December 15, 2024 20:10
Copy link
Contributor

@s-barannikov s-barannikov left a comment

Choose a reason for hiding this comment

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

LGTM

@llvmbot
Copy link
Member

llvmbot commented Dec 16, 2024

@llvm/pr-subscribers-llvm-globalisel

Author: Craig Topper (topperc)

Changes

Ensure destination type is at least 2 bits.
Remove unnecessary check that both sources are the same type. The verifier already handles this generically.


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

2 Files Affected:

  • (modified) llvm/lib/CodeGen/MachineVerifier.cpp (+6-7)
  • (modified) llvm/test/MachineVerifier/test_uscmp.mir (+7-3)
diff --git a/llvm/lib/CodeGen/MachineVerifier.cpp b/llvm/lib/CodeGen/MachineVerifier.cpp
index fb4d96fdad0a5e..ce71f1aaee5949 100644
--- a/llvm/lib/CodeGen/MachineVerifier.cpp
+++ b/llvm/lib/CodeGen/MachineVerifier.cpp
@@ -1615,9 +1615,8 @@ void MachineVerifier::verifyPreISelGenericInstruction(const MachineInstr *MI) {
   case TargetOpcode::G_UCMP: {
     LLT DstTy = MRI->getType(MI->getOperand(0).getReg());
     LLT SrcTy = MRI->getType(MI->getOperand(1).getReg());
-    LLT SrcTy2 = MRI->getType(MI->getOperand(2).getReg());
 
-    if (SrcTy.isPointerOrPointerVector() || SrcTy2.isPointerOrPointerVector()) {
+    if (SrcTy.isPointerOrPointerVector()) {
       report("Generic scmp/ucmp does not support pointers as operands", MI);
       break;
     }
@@ -1627,6 +1626,11 @@ void MachineVerifier::verifyPreISelGenericInstruction(const MachineInstr *MI) {
       break;
     }
 
+    if (DstTy.getScalarSizeInBits() < 2) {
+      report("Result type must be at least 2 bits wide", MI);
+      break;
+    }
+
     if ((DstTy.isVector() != SrcTy.isVector()) ||
         (DstTy.isVector() &&
          DstTy.getElementCount() != SrcTy.getElementCount())) {
@@ -1634,11 +1638,6 @@ void MachineVerifier::verifyPreISelGenericInstruction(const MachineInstr *MI) {
       break;
     }
 
-    if (SrcTy != SrcTy2) {
-      report("Generic scmp/ucmp must have same input types", MI);
-      break;
-    }
-
     break;
   }
   case TargetOpcode::G_EXTRACT: {
diff --git a/llvm/test/MachineVerifier/test_uscmp.mir b/llvm/test/MachineVerifier/test_uscmp.mir
index aa686c4ec73e65..4e518749e7377e 100644
--- a/llvm/test/MachineVerifier/test_uscmp.mir
+++ b/llvm/test/MachineVerifier/test_uscmp.mir
@@ -19,13 +19,17 @@ body:             |
     %23:_(<2 x s32>) = G_IMPLICIT_DEF
     %24:_(<2 x s32>) = G_IMPLICIT_DEF
     ; CHECK: Generic vector scmp/ucmp must preserve number of lanes
-    %5:_(s1) = G_UCMP  %23, %24
+    %5:_(s2) = G_UCMP  %23, %24
 
     %15:_(s32) = G_CONSTANT i32 0
     %16:_(s64) = G_CONSTANT i64 2
-    ; CHECK: Generic scmp/ucmp must have same input types
-    %17:_(s1) = G_SCMP %15, %16
+    ; CHECK: Type mismatch in generic instruction
+    %17:_(s2) = G_SCMP %15, %16
 
+    %18:_(s32) = G_CONSTANT i32 0
+    %19:_(s32) = G_CONSTANT i32 2
+    ; CHECK: Result type must be at least 2 bits wide
+    %20:_(s1) = G_SCMP %18, %19
 
 
 ...

@topperc topperc merged commit 6dc24f6 into llvm:main Dec 16, 2024
9 checks passed
@topperc topperc deleted the pr/cmp-verifier branch December 16, 2024 06:08
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