Skip to content

[GlobalIsel] Add G_SCMP and G_UCMP instructions #98894

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 4 commits into from
Jul 18, 2024

Conversation

tschuett
Copy link

@llvmbot
Copy link
Member

llvmbot commented Jul 15, 2024

@llvm/pr-subscribers-backend-aarch64

@llvm/pr-subscribers-llvm-globalisel

Author: Thorsten Schütt (tschuett)

Changes

#83227


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

11 Files Affected:

  • (modified) llvm/docs/GlobalISel/GenericOpcode.rst (+20)
  • (modified) llvm/include/llvm/CodeGen/GlobalISel/MachineIRBuilder.h (+28)
  • (modified) llvm/include/llvm/Support/TargetOpcodes.def (+6)
  • (modified) llvm/include/llvm/Target/GenericOpcodes.td (+14)
  • (modified) llvm/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp (+12)
  • (modified) llvm/lib/CodeGen/MachineVerifier.cpp (+20)
  • (modified) llvm/test/CodeGen/AArch64/GlobalISel/legalizer-info-validation.mir (+6)
  • (modified) llvm/test/MachineVerifier/test_g_extract_subvector.mir (+3-1)
  • (modified) llvm/test/MachineVerifier/test_g_insert_subvector.mir (+2-1)
  • (modified) llvm/test/MachineVerifier/test_g_vscale.mir (+2-1)
  • (added) llvm/test/MachineVerifier/test_uscmp.mir (+19)
diff --git a/llvm/docs/GlobalISel/GenericOpcode.rst b/llvm/docs/GlobalISel/GenericOpcode.rst
index b05394aeee003..4ef16ffc69bd8 100644
--- a/llvm/docs/GlobalISel/GenericOpcode.rst
+++ b/llvm/docs/GlobalISel/GenericOpcode.rst
@@ -348,6 +348,26 @@ G_ICMP
 Perform integer comparison producing non-zero (true) or zero (false). It's
 target specific whether a true value is 1, ~0U, or some other non-zero value.
 
+G_SCMP
+^^^^^^
+
+Perform signed 3-way integer comparison producing -1 (smaller), 0 (equal), or 1 (larger).
+
+.. code-block:: none
+
+  %5:_(s32) = G_SCMP %6, %2
+
+
+G_UCMP
+^^^^^^
+
+Perform unsigned 3-way integer comparison producing -1 (smaller), 0 (equal), or 1 (larger).
+
+.. code-block:: none
+
+  %7:_(s32) = G_UCMP %2, %6
+
+
 G_SELECT
 ^^^^^^^^
 
diff --git a/llvm/include/llvm/CodeGen/GlobalISel/MachineIRBuilder.h b/llvm/include/llvm/CodeGen/GlobalISel/MachineIRBuilder.h
index e74136f34b234..56a77b8596a18 100644
--- a/llvm/include/llvm/CodeGen/GlobalISel/MachineIRBuilder.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/MachineIRBuilder.h
@@ -1273,6 +1273,34 @@ class MachineIRBuilder {
                                 const SrcOp &Op0, const SrcOp &Op1,
                                 std::optional<unsigned> Flags = std::nullopt);
 
+  /// Build and insert a \p Res = G_SCMP \p Op0, \p Op1
+  ///
+  /// \pre setBasicBlock or setMI must have been called.
+
+  /// \pre \p Res must be a generic virtual register with scalar or
+  ///      vector type. Typically this starts as s2 or <N x s2>.
+  /// \pre \p Op0 and Op1 must be generic virtual registers with the
+  ///      same number of elements as \p Res. If \p Res is a scalar,
+  ///      \p Op0 must be a scalar.
+  ///
+  /// \return a MachineInstrBuilder for the newly created instruction.
+  MachineInstrBuilder buildSCmp(const DstOp &Res, const SrcOp &Op0,
+                                const SrcOp &Op1);
+
+  /// Build and insert a \p Res = G_UCMP \p Op0, \p Op1
+  ///
+  /// \pre setBasicBlock or setMI must have been called.
+
+  /// \pre \p Res must be a generic virtual register with scalar or
+  ///      vector type. Typically this starts as s2 or <N x s2>.
+  /// \pre \p Op0 and Op1 must be generic virtual registers with the
+  ///      same number of elements as \p Res. If \p Res is a scalar,
+  ///      \p Op0 must be a scalar.
+  ///
+  /// \return a MachineInstrBuilder for the newly created instruction.
+  MachineInstrBuilder buildUCmp(const DstOp &Res, const SrcOp &Op0,
+                                const SrcOp &Op1);
+
   /// Build and insert a \p Res = G_IS_FPCLASS \p Src, \p Mask
   MachineInstrBuilder buildIsFPClass(const DstOp &Res, const SrcOp &Src,
                                      unsigned Mask) {
diff --git a/llvm/include/llvm/Support/TargetOpcodes.def b/llvm/include/llvm/Support/TargetOpcodes.def
index e7f40e87ed24a..ba3713cd68506 100644
--- a/llvm/include/llvm/Support/TargetOpcodes.def
+++ b/llvm/include/llvm/Support/TargetOpcodes.def
@@ -503,6 +503,12 @@ HANDLE_TARGET_OPCODE(G_ICMP)
 /// Generic floating-point comparison, also applicable to vectors.
 HANDLE_TARGET_OPCODE(G_FCMP)
 
+/// Generic signed 3-way comparison.
+HANDLE_TARGET_OPCODE(G_SCMP)
+
+/// Generic unsigned 3-way comparison.
+HANDLE_TARGET_OPCODE(G_UCMP)
+
 /// Generic select.
 HANDLE_TARGET_OPCODE(G_SELECT)
 
diff --git a/llvm/include/llvm/Target/GenericOpcodes.td b/llvm/include/llvm/Target/GenericOpcodes.td
index e1710ff2d8abf..f26284e001dbd 100644
--- a/llvm/include/llvm/Target/GenericOpcodes.td
+++ b/llvm/include/llvm/Target/GenericOpcodes.td
@@ -430,6 +430,20 @@ def G_FCMP : GenericInstruction {
   let hasSideEffects = false;
 }
 
+// Generic signed three-way comparison.
+def G_SCMP : GenericInstruction {
+  let OutOperandList = (outs type0:$dst);
+  let InOperandList = (ins type1:$src1, type1:$src2);
+  let hasSideEffects = false;
+}
+
+// Generic unsigned three-way comparison.
+def G_UCMP : GenericInstruction {
+  let OutOperandList = (outs type0:$dst);
+  let InOperandList = (ins type1:$src1, type1:$src2);
+  let hasSideEffects = false;
+}
+
 // Generic select
 def G_SELECT : GenericInstruction {
   let OutOperandList = (outs type0:$dst);
diff --git a/llvm/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp b/llvm/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp
index 06a6c1f93ef1f..7eb6cd4e0d798 100644
--- a/llvm/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp
@@ -911,6 +911,18 @@ MachineInstrBuilder MachineIRBuilder::buildFCmp(CmpInst::Predicate Pred,
   return buildInstr(TargetOpcode::G_FCMP, Res, {Pred, Op0, Op1}, Flags);
 }
 
+MachineInstrBuilder MachineIRBuilder::buildSCmp(const DstOp &Res,
+                                                const SrcOp &Op0,
+                                                const SrcOp &Op1) {
+  return buildInstr(TargetOpcode::G_SCMP, Res, {Op0, Op1});
+}
+
+MachineInstrBuilder MachineIRBuilder::buildUCmp(const DstOp &Res,
+                                                const SrcOp &Op0,
+                                                const SrcOp &Op1) {
+  return buildInstr(TargetOpcode::G_UCMP, Res, {Op0, Op1});
+}
+
 MachineInstrBuilder
 MachineIRBuilder::buildSelect(const DstOp &Res, const SrcOp &Tst,
                               const SrcOp &Op0, const SrcOp &Op1,
diff --git a/llvm/lib/CodeGen/MachineVerifier.cpp b/llvm/lib/CodeGen/MachineVerifier.cpp
index 0a5b8bdbc9371..1a183d83d0891 100644
--- a/llvm/lib/CodeGen/MachineVerifier.cpp
+++ b/llvm/lib/CodeGen/MachineVerifier.cpp
@@ -1544,6 +1544,26 @@ void MachineVerifier::verifyPreISelGenericInstruction(const MachineInstr *MI) {
 
     break;
   }
+  case TargetOpcode::G_SCMP:
+  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 ((DstTy.isVector() != SrcTy.isVector()) ||
+        (DstTy.isVector() &&
+         DstTy.getElementCount() != SrcTy.getElementCount())) {
+      report("Generic vector scmp/ucmp must preserve number of lanes", MI);
+      break;
+    }
+
+    if (SrcTy != SrcTy2) {
+      report("Generic scmp/ucmp must have same input types", MI);
+      break;
+    }
+
+    break;
+  }
   case TargetOpcode::G_EXTRACT: {
     const MachineOperand &SrcOp = MI->getOperand(1);
     if (!SrcOp.isReg()) {
diff --git a/llvm/test/CodeGen/AArch64/GlobalISel/legalizer-info-validation.mir b/llvm/test/CodeGen/AArch64/GlobalISel/legalizer-info-validation.mir
index 1f048528ea153..26687c915ecb2 100644
--- a/llvm/test/CodeGen/AArch64/GlobalISel/legalizer-info-validation.mir
+++ b/llvm/test/CodeGen/AArch64/GlobalISel/legalizer-info-validation.mir
@@ -351,6 +351,12 @@
 # DEBUG-NEXT: G_FCMP (opcode {{[0-9]+}}): 2 type indices, 0 imm indices
 # DEBUG-NEXT: .. type index coverage check SKIPPED: user-defined predicate detected
 # DEBUG-NEXT: .. imm index coverage check SKIPPED: user-defined predicate detected
+# DEBUG-NEXT: G_SCMP (opcode {{[0-9]+}}): 2 type indices, 0 imm indices
+# DEBUG-NEXT: .. type index coverage check SKIPPED: no rules defined
+# DEBUG-NEXT: .. imm index coverage check SKIPPED: no rules defined
+# DEBUG-NEXT: G_UCMP (opcode {{[0-9]+}}): 2 type indices, 0 imm indices
+# DEBUG-NEXT: .. type index coverage check SKIPPED: no rules defined
+# DEBUG-NEXT: .. imm index coverage check SKIPPED: no rules defined
 # DEBUG-NEXT: G_SELECT (opcode {{[0-9]+}}): 2 type indices, 0 imm indices
 # DEBUG-NEXT: .. type index coverage check SKIPPED: user-defined predicate detected
 # DEBUG-NEXT: .. imm index coverage check SKIPPED: user-defined predicate detected
diff --git a/llvm/test/MachineVerifier/test_g_extract_subvector.mir b/llvm/test/MachineVerifier/test_g_extract_subvector.mir
index bc167d2eb7bcd..fe04471563e49 100644
--- a/llvm/test/MachineVerifier/test_g_extract_subvector.mir
+++ b/llvm/test/MachineVerifier/test_g_extract_subvector.mir
@@ -1,4 +1,6 @@
-# RUN: not --crash llc -o - -run-pass=none -verify-machineinstrs %s 2>&1 | FileCheck %s
+# RUN: not --crash llc -o - -run-pass=none -mtriple=arm64 -verify-machineinstrs %s 2>&1 | FileCheck %s
+# REQUIRES: aarch64-registered-target
+
 ---
 name:            g_extract_subvector
 tracksRegLiveness: true
diff --git a/llvm/test/MachineVerifier/test_g_insert_subvector.mir b/llvm/test/MachineVerifier/test_g_insert_subvector.mir
index dce30cdb6b1e5..25cb4ca3c4529 100644
--- a/llvm/test/MachineVerifier/test_g_insert_subvector.mir
+++ b/llvm/test/MachineVerifier/test_g_insert_subvector.mir
@@ -1,4 +1,5 @@
-# RUN: not --crash llc -o - -run-pass=none -verify-machineinstrs %s 2>&1 | FileCheck %s
+# RUN: not --crash llc -o - -run-pass=none -mtriple=arm64 -verify-machineinstrs %s 2>&1 | FileCheck %s
+# REQUIRES: aarch64-registered-target
 
 ---
 name:            g_splat_vector
diff --git a/llvm/test/MachineVerifier/test_g_vscale.mir b/llvm/test/MachineVerifier/test_g_vscale.mir
index 78854620913a1..f4ff76766a84e 100644
--- a/llvm/test/MachineVerifier/test_g_vscale.mir
+++ b/llvm/test/MachineVerifier/test_g_vscale.mir
@@ -1,4 +1,5 @@
-# RUN: not --crash llc -verify-machineinstrs -run-pass none -o /dev/null %s 2>&1 | FileCheck %s
+# RUN: not --crash llc -verify-machineinstrs -mtriple=arm64 -run-pass none -o /dev/null %s 2>&1 | FileCheck %s
+# REQUIRES: aarch64-registered-target
 
 ---
 name:            g_vscale
diff --git a/llvm/test/MachineVerifier/test_uscmp.mir b/llvm/test/MachineVerifier/test_uscmp.mir
new file mode 100644
index 0000000000000..0716399c865d5
--- /dev/null
+++ b/llvm/test/MachineVerifier/test_uscmp.mir
@@ -0,0 +1,19 @@
+# RUN: not --crash llc -verify-machineinstrs -mtriple=arm64 -run-pass none -o /dev/null %s 2>&1 | FileCheck %s
+# REQUIRES: aarch64-registered-target
+
+---
+name:            test_uscmp
+body:             |
+  bb.0:
+
+    %3:_(<2 x s32>) = G_IMPLICIT_DEF
+    %4:_(<2 x s32>) = G_IMPLICIT_DEF
+    ; CHECK: Generic vector scmp/ucmp must preserve number of lanes
+    %5:_(s1) = G_UCMP  %3, %4
+
+    %12:_(s32) = G_CONSTANT i32 0
+    %13:_(s64) = G_CONSTANT i64 2
+    ; CHECK: Generic scmp/ucmp must have same input types
+    %14:_(s1) = G_SCMP %12, %13
+
+...

@llvmbot
Copy link
Member

llvmbot commented Jul 15, 2024

@llvm/pr-subscribers-llvm-support

Author: Thorsten Schütt (tschuett)

Changes

#83227


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

11 Files Affected:

  • (modified) llvm/docs/GlobalISel/GenericOpcode.rst (+20)
  • (modified) llvm/include/llvm/CodeGen/GlobalISel/MachineIRBuilder.h (+28)
  • (modified) llvm/include/llvm/Support/TargetOpcodes.def (+6)
  • (modified) llvm/include/llvm/Target/GenericOpcodes.td (+14)
  • (modified) llvm/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp (+12)
  • (modified) llvm/lib/CodeGen/MachineVerifier.cpp (+20)
  • (modified) llvm/test/CodeGen/AArch64/GlobalISel/legalizer-info-validation.mir (+6)
  • (modified) llvm/test/MachineVerifier/test_g_extract_subvector.mir (+3-1)
  • (modified) llvm/test/MachineVerifier/test_g_insert_subvector.mir (+2-1)
  • (modified) llvm/test/MachineVerifier/test_g_vscale.mir (+2-1)
  • (added) llvm/test/MachineVerifier/test_uscmp.mir (+19)
diff --git a/llvm/docs/GlobalISel/GenericOpcode.rst b/llvm/docs/GlobalISel/GenericOpcode.rst
index b05394aeee003..4ef16ffc69bd8 100644
--- a/llvm/docs/GlobalISel/GenericOpcode.rst
+++ b/llvm/docs/GlobalISel/GenericOpcode.rst
@@ -348,6 +348,26 @@ G_ICMP
 Perform integer comparison producing non-zero (true) or zero (false). It's
 target specific whether a true value is 1, ~0U, or some other non-zero value.
 
+G_SCMP
+^^^^^^
+
+Perform signed 3-way integer comparison producing -1 (smaller), 0 (equal), or 1 (larger).
+
+.. code-block:: none
+
+  %5:_(s32) = G_SCMP %6, %2
+
+
+G_UCMP
+^^^^^^
+
+Perform unsigned 3-way integer comparison producing -1 (smaller), 0 (equal), or 1 (larger).
+
+.. code-block:: none
+
+  %7:_(s32) = G_UCMP %2, %6
+
+
 G_SELECT
 ^^^^^^^^
 
diff --git a/llvm/include/llvm/CodeGen/GlobalISel/MachineIRBuilder.h b/llvm/include/llvm/CodeGen/GlobalISel/MachineIRBuilder.h
index e74136f34b234..56a77b8596a18 100644
--- a/llvm/include/llvm/CodeGen/GlobalISel/MachineIRBuilder.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/MachineIRBuilder.h
@@ -1273,6 +1273,34 @@ class MachineIRBuilder {
                                 const SrcOp &Op0, const SrcOp &Op1,
                                 std::optional<unsigned> Flags = std::nullopt);
 
+  /// Build and insert a \p Res = G_SCMP \p Op0, \p Op1
+  ///
+  /// \pre setBasicBlock or setMI must have been called.
+
+  /// \pre \p Res must be a generic virtual register with scalar or
+  ///      vector type. Typically this starts as s2 or <N x s2>.
+  /// \pre \p Op0 and Op1 must be generic virtual registers with the
+  ///      same number of elements as \p Res. If \p Res is a scalar,
+  ///      \p Op0 must be a scalar.
+  ///
+  /// \return a MachineInstrBuilder for the newly created instruction.
+  MachineInstrBuilder buildSCmp(const DstOp &Res, const SrcOp &Op0,
+                                const SrcOp &Op1);
+
+  /// Build and insert a \p Res = G_UCMP \p Op0, \p Op1
+  ///
+  /// \pre setBasicBlock or setMI must have been called.
+
+  /// \pre \p Res must be a generic virtual register with scalar or
+  ///      vector type. Typically this starts as s2 or <N x s2>.
+  /// \pre \p Op0 and Op1 must be generic virtual registers with the
+  ///      same number of elements as \p Res. If \p Res is a scalar,
+  ///      \p Op0 must be a scalar.
+  ///
+  /// \return a MachineInstrBuilder for the newly created instruction.
+  MachineInstrBuilder buildUCmp(const DstOp &Res, const SrcOp &Op0,
+                                const SrcOp &Op1);
+
   /// Build and insert a \p Res = G_IS_FPCLASS \p Src, \p Mask
   MachineInstrBuilder buildIsFPClass(const DstOp &Res, const SrcOp &Src,
                                      unsigned Mask) {
diff --git a/llvm/include/llvm/Support/TargetOpcodes.def b/llvm/include/llvm/Support/TargetOpcodes.def
index e7f40e87ed24a..ba3713cd68506 100644
--- a/llvm/include/llvm/Support/TargetOpcodes.def
+++ b/llvm/include/llvm/Support/TargetOpcodes.def
@@ -503,6 +503,12 @@ HANDLE_TARGET_OPCODE(G_ICMP)
 /// Generic floating-point comparison, also applicable to vectors.
 HANDLE_TARGET_OPCODE(G_FCMP)
 
+/// Generic signed 3-way comparison.
+HANDLE_TARGET_OPCODE(G_SCMP)
+
+/// Generic unsigned 3-way comparison.
+HANDLE_TARGET_OPCODE(G_UCMP)
+
 /// Generic select.
 HANDLE_TARGET_OPCODE(G_SELECT)
 
diff --git a/llvm/include/llvm/Target/GenericOpcodes.td b/llvm/include/llvm/Target/GenericOpcodes.td
index e1710ff2d8abf..f26284e001dbd 100644
--- a/llvm/include/llvm/Target/GenericOpcodes.td
+++ b/llvm/include/llvm/Target/GenericOpcodes.td
@@ -430,6 +430,20 @@ def G_FCMP : GenericInstruction {
   let hasSideEffects = false;
 }
 
+// Generic signed three-way comparison.
+def G_SCMP : GenericInstruction {
+  let OutOperandList = (outs type0:$dst);
+  let InOperandList = (ins type1:$src1, type1:$src2);
+  let hasSideEffects = false;
+}
+
+// Generic unsigned three-way comparison.
+def G_UCMP : GenericInstruction {
+  let OutOperandList = (outs type0:$dst);
+  let InOperandList = (ins type1:$src1, type1:$src2);
+  let hasSideEffects = false;
+}
+
 // Generic select
 def G_SELECT : GenericInstruction {
   let OutOperandList = (outs type0:$dst);
diff --git a/llvm/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp b/llvm/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp
index 06a6c1f93ef1f..7eb6cd4e0d798 100644
--- a/llvm/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp
@@ -911,6 +911,18 @@ MachineInstrBuilder MachineIRBuilder::buildFCmp(CmpInst::Predicate Pred,
   return buildInstr(TargetOpcode::G_FCMP, Res, {Pred, Op0, Op1}, Flags);
 }
 
+MachineInstrBuilder MachineIRBuilder::buildSCmp(const DstOp &Res,
+                                                const SrcOp &Op0,
+                                                const SrcOp &Op1) {
+  return buildInstr(TargetOpcode::G_SCMP, Res, {Op0, Op1});
+}
+
+MachineInstrBuilder MachineIRBuilder::buildUCmp(const DstOp &Res,
+                                                const SrcOp &Op0,
+                                                const SrcOp &Op1) {
+  return buildInstr(TargetOpcode::G_UCMP, Res, {Op0, Op1});
+}
+
 MachineInstrBuilder
 MachineIRBuilder::buildSelect(const DstOp &Res, const SrcOp &Tst,
                               const SrcOp &Op0, const SrcOp &Op1,
diff --git a/llvm/lib/CodeGen/MachineVerifier.cpp b/llvm/lib/CodeGen/MachineVerifier.cpp
index 0a5b8bdbc9371..1a183d83d0891 100644
--- a/llvm/lib/CodeGen/MachineVerifier.cpp
+++ b/llvm/lib/CodeGen/MachineVerifier.cpp
@@ -1544,6 +1544,26 @@ void MachineVerifier::verifyPreISelGenericInstruction(const MachineInstr *MI) {
 
     break;
   }
+  case TargetOpcode::G_SCMP:
+  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 ((DstTy.isVector() != SrcTy.isVector()) ||
+        (DstTy.isVector() &&
+         DstTy.getElementCount() != SrcTy.getElementCount())) {
+      report("Generic vector scmp/ucmp must preserve number of lanes", MI);
+      break;
+    }
+
+    if (SrcTy != SrcTy2) {
+      report("Generic scmp/ucmp must have same input types", MI);
+      break;
+    }
+
+    break;
+  }
   case TargetOpcode::G_EXTRACT: {
     const MachineOperand &SrcOp = MI->getOperand(1);
     if (!SrcOp.isReg()) {
diff --git a/llvm/test/CodeGen/AArch64/GlobalISel/legalizer-info-validation.mir b/llvm/test/CodeGen/AArch64/GlobalISel/legalizer-info-validation.mir
index 1f048528ea153..26687c915ecb2 100644
--- a/llvm/test/CodeGen/AArch64/GlobalISel/legalizer-info-validation.mir
+++ b/llvm/test/CodeGen/AArch64/GlobalISel/legalizer-info-validation.mir
@@ -351,6 +351,12 @@
 # DEBUG-NEXT: G_FCMP (opcode {{[0-9]+}}): 2 type indices, 0 imm indices
 # DEBUG-NEXT: .. type index coverage check SKIPPED: user-defined predicate detected
 # DEBUG-NEXT: .. imm index coverage check SKIPPED: user-defined predicate detected
+# DEBUG-NEXT: G_SCMP (opcode {{[0-9]+}}): 2 type indices, 0 imm indices
+# DEBUG-NEXT: .. type index coverage check SKIPPED: no rules defined
+# DEBUG-NEXT: .. imm index coverage check SKIPPED: no rules defined
+# DEBUG-NEXT: G_UCMP (opcode {{[0-9]+}}): 2 type indices, 0 imm indices
+# DEBUG-NEXT: .. type index coverage check SKIPPED: no rules defined
+# DEBUG-NEXT: .. imm index coverage check SKIPPED: no rules defined
 # DEBUG-NEXT: G_SELECT (opcode {{[0-9]+}}): 2 type indices, 0 imm indices
 # DEBUG-NEXT: .. type index coverage check SKIPPED: user-defined predicate detected
 # DEBUG-NEXT: .. imm index coverage check SKIPPED: user-defined predicate detected
diff --git a/llvm/test/MachineVerifier/test_g_extract_subvector.mir b/llvm/test/MachineVerifier/test_g_extract_subvector.mir
index bc167d2eb7bcd..fe04471563e49 100644
--- a/llvm/test/MachineVerifier/test_g_extract_subvector.mir
+++ b/llvm/test/MachineVerifier/test_g_extract_subvector.mir
@@ -1,4 +1,6 @@
-# RUN: not --crash llc -o - -run-pass=none -verify-machineinstrs %s 2>&1 | FileCheck %s
+# RUN: not --crash llc -o - -run-pass=none -mtriple=arm64 -verify-machineinstrs %s 2>&1 | FileCheck %s
+# REQUIRES: aarch64-registered-target
+
 ---
 name:            g_extract_subvector
 tracksRegLiveness: true
diff --git a/llvm/test/MachineVerifier/test_g_insert_subvector.mir b/llvm/test/MachineVerifier/test_g_insert_subvector.mir
index dce30cdb6b1e5..25cb4ca3c4529 100644
--- a/llvm/test/MachineVerifier/test_g_insert_subvector.mir
+++ b/llvm/test/MachineVerifier/test_g_insert_subvector.mir
@@ -1,4 +1,5 @@
-# RUN: not --crash llc -o - -run-pass=none -verify-machineinstrs %s 2>&1 | FileCheck %s
+# RUN: not --crash llc -o - -run-pass=none -mtriple=arm64 -verify-machineinstrs %s 2>&1 | FileCheck %s
+# REQUIRES: aarch64-registered-target
 
 ---
 name:            g_splat_vector
diff --git a/llvm/test/MachineVerifier/test_g_vscale.mir b/llvm/test/MachineVerifier/test_g_vscale.mir
index 78854620913a1..f4ff76766a84e 100644
--- a/llvm/test/MachineVerifier/test_g_vscale.mir
+++ b/llvm/test/MachineVerifier/test_g_vscale.mir
@@ -1,4 +1,5 @@
-# RUN: not --crash llc -verify-machineinstrs -run-pass none -o /dev/null %s 2>&1 | FileCheck %s
+# RUN: not --crash llc -verify-machineinstrs -mtriple=arm64 -run-pass none -o /dev/null %s 2>&1 | FileCheck %s
+# REQUIRES: aarch64-registered-target
 
 ---
 name:            g_vscale
diff --git a/llvm/test/MachineVerifier/test_uscmp.mir b/llvm/test/MachineVerifier/test_uscmp.mir
new file mode 100644
index 0000000000000..0716399c865d5
--- /dev/null
+++ b/llvm/test/MachineVerifier/test_uscmp.mir
@@ -0,0 +1,19 @@
+# RUN: not --crash llc -verify-machineinstrs -mtriple=arm64 -run-pass none -o /dev/null %s 2>&1 | FileCheck %s
+# REQUIRES: aarch64-registered-target
+
+---
+name:            test_uscmp
+body:             |
+  bb.0:
+
+    %3:_(<2 x s32>) = G_IMPLICIT_DEF
+    %4:_(<2 x s32>) = G_IMPLICIT_DEF
+    ; CHECK: Generic vector scmp/ucmp must preserve number of lanes
+    %5:_(s1) = G_UCMP  %3, %4
+
+    %12:_(s32) = G_CONSTANT i32 0
+    %13:_(s64) = G_CONSTANT i64 2
+    ; CHECK: Generic scmp/ucmp must have same input types
+    %14:_(s1) = G_SCMP %12, %13
+
+...

@nikic
Copy link
Contributor

nikic commented Jul 15, 2024

cc @Poseydon42 who was also working on GlobalISel support for this...

@@ -1,4 +1,5 @@
# RUN: not --crash llc -verify-machineinstrs -run-pass none -o /dev/null %s 2>&1 | FileCheck %s
# RUN: not --crash llc -verify-machineinstrs -mtriple=arm64 -run-pass none -o /dev/null %s 2>&1 | FileCheck %s
# REQUIRES: aarch64-registered-target
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need to add target and requires?

#84542 (comment)

Copy link
Author

Choose a reason for hiding this comment

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

I knew that this was coming. ninja check-llvm-machineverifier was complaining that the llc s had no target.

Copy link
Contributor

Choose a reason for hiding this comment

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

Was this occurring prior to this patch?

Copy link
Author

Choose a reason for hiding this comment

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

IDK. The first time, I called ninja check-llvm-machineverifier. This is my first instruction.

Copy link
Author

@tschuett tschuett Jul 15, 2024

Choose a reason for hiding this comment

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

Note that I register exactly AArch64 and AMDGPU.

Copy link
Author

@tschuett tschuett Jul 15, 2024

Choose a reason for hiding this comment

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

********************
********************
Failed Tests (4):
  LLVM :: MachineVerifier/test_g_extract_subvector.mir
  LLVM :: MachineVerifier/test_g_insert_subvector.mir
  LLVM :: MachineVerifier/test_g_vscale.mir
  LLVM :: MachineVerifier/test_uscmp.mir


Testing Time: 1.92s

Total Discovered Tests: 98
  Unsupported      : 15 (15.31%)
  Passed           : 76 (77.55%)
  Expectedly Failed:  3 (3.06%)
  Failed           :  4 (4.08%)
FAILED: test/CMakeFiles/check-llvm-machineverifier

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe that it is happening to you. Could you please share your cmake command?

Copy link
Author

Choose a reason for hiding this comment

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

cmake -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_ASSERTIONS=On -DLLVM_TARGETS_TO_BUILD="AArch64;AMDGPU" -G Ninja -DLLVM_ENABLE_PROJECTS="llvm" -DLLVM_ENABLE_LTO=Thin -DCMAKE_CXX_FLAGS="-march=native" -DCMAKE_EXPORT_COMPILE_COMMANDS=On -DLLVM_ENABLE_SPHINX=ON ./llvm

Copy link
Contributor

Choose a reason for hiding this comment

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

Every MachineVerifier test needs a REQUIRES, simply because we haven't bothered to add target specific subdirectories with the appropriate lit.local.cfg in each

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe the default target is still the host target, which I'm assuming is x86. You would have to manually override to set one. The real condition here is probably REQUIRES: default_triple

@tschuett
Copy link
Author

Next steps could be:

  • Teach the IRTranslator to translate the intrinsics to G_[US]CMP.
  • Add lower methods for default implementations.
  • Add to targets:
getActionDefinitionsBuilder({G_SCMP, G_UCMP})
  .lower();

@tschuett tschuett requested a review from Poseydon42 July 15, 2024 15:05
Copy link
Contributor

@arsenm arsenm left a comment

Choose a reason for hiding this comment

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

lgtm except for the touching of unrelated tests

@@ -1,4 +1,6 @@
# RUN: not --crash llc -o - -run-pass=none -verify-machineinstrs %s 2>&1 | FileCheck %s
# RUN: not --crash llc -o - -run-pass=none -mtriple=arm64 -verify-machineinstrs %s 2>&1 | FileCheck %s
# REQUIRES: aarch64-registered-target
Copy link
Contributor

Choose a reason for hiding this comment

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

This shouldn't be in this patch, and is the rare case that shouldn't require a target

Copy link
Author

Choose a reason for hiding this comment

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

I mentioned above that my llc complained about being unable to get a target.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is still unrelated to this PR and should be separate, but see my comment above about it's really REQUIRES: default_triple

report("Generic scmp/ucmp must have same input types", MI);
break;
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Are pointers valid?

Copy link
Author

Choose a reason for hiding this comment

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

I linked the LangRef PR. Langref claims it is integer-only.

%12:_(p0) = G_IMPLICIT_DEF
%13:_(p0) = G_IMPLICIT_DEF
; CHECK: Generic scmp/ucmp does not support pointers
%14:_(p0) = G_SCMP %12, %13
Copy link
Contributor

Choose a reason for hiding this comment

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

Checking the operands separately from the result would be better

@tschuett
Copy link
Author

There are still failures with:

  LLVM :: TableGen/GlobalISelCombinerEmitter/match-table-cxx.td
  LLVM :: TableGen/GlobalISelEmitter.td

Is the solution update script or manual work? The failures are probably related to adding new instructions.

tschuett added a commit to tschuett/llvm-project that referenced this pull request Jul 16, 2024
temporary solution.
For discussion see llvm#98894
tschuett added a commit that referenced this pull request Jul 17, 2024
temporary solution.
For discussion see #98894

Permanent solution could be:
REQUIRES: default_triple
Thorsten Schütt added 3 commits July 17, 2024 07:04
Copy link
Contributor

@michaelmaitland michaelmaitland left a comment

Choose a reason for hiding this comment

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

LGTM

@tschuett tschuett merged commit 1cc1072 into llvm:main Jul 18, 2024
8 checks passed
@tschuett tschuett deleted the gisel-uscmp branch July 18, 2024 14:22
tschuett added a commit to tschuett/llvm-project that referenced this pull request Jul 18, 2024
tschuett added a commit to tschuett/llvm-project that referenced this pull request Jul 18, 2024
tschuett added a commit that referenced this pull request Jul 19, 2024
yuxuanchen1997 pushed a commit that referenced this pull request Jul 25, 2024
Summary:
temporary solution.
For discussion see #98894

Permanent solution could be:
REQUIRES: default_triple

Test Plan: 

Reviewers: 

Subscribers: 

Tasks: 

Tags: 


Differential Revision: https://phabricator.intern.facebook.com/D60251709
yuxuanchen1997 pushed a commit that referenced this pull request Jul 25, 2024
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.

5 participants