[llvm] ee2add0 - [GISEL] Fix bugs and clarify spec of G_EXTRACT_SUBVECTOR (#108848)

via llvm-commits llvm-commits at lists.llvm.org
Tue Sep 17 07:08:44 PDT 2024


Author: Michael Maitland
Date: 2024-09-17T10:08:39-04:00
New Revision: ee2add06836afdda6c86792441e6afdf6993f770

URL: https://github.com/llvm/llvm-project/commit/ee2add06836afdda6c86792441e6afdf6993f770
DIFF: https://github.com/llvm/llvm-project/commit/ee2add06836afdda6c86792441e6afdf6993f770.diff

LOG: [GISEL] Fix bugs and clarify spec of G_EXTRACT_SUBVECTOR (#108848)

The implementation was missing the fact that `G_EXTRACT_SUBVECTOR`
destination and source vector can be different types.

Also fix a bug in the MIR builder for `G_EXTRACT_SUBVECTOR` to generate
the correct opcode.

Clarify the G_EXTRACT_SUBVECTOR specification.

Added: 
    

Modified: 
    llvm/docs/GlobalISel/GenericOpcode.rst
    llvm/include/llvm/Target/GenericOpcodes.td
    llvm/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp
    llvm/lib/CodeGen/MachineVerifier.cpp
    llvm/test/CodeGen/AArch64/GlobalISel/legalizer-info-validation.mir
    llvm/test/MachineVerifier/test_g_extract_subvector.mir
    llvm/unittests/CodeGen/GlobalISel/MachineIRBuilderTest.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/docs/GlobalISel/GenericOpcode.rst b/llvm/docs/GlobalISel/GenericOpcode.rst
index b06ce1415b2bbe..c42adc10b10a29 100644
--- a/llvm/docs/GlobalISel/GenericOpcode.rst
+++ b/llvm/docs/GlobalISel/GenericOpcode.rst
@@ -706,6 +706,8 @@ vector must be valid indices of that vector. If this condition cannot be
 determined statically but is false at runtime, then the result vector is
 undefined.
 
+Mixing scalable vectors and fixed vectors are not allowed.
+
 .. code-block:: none
 
   %3:_(<vscale x 4 x i64>) = G_EXTRACT_SUBVECTOR %2:_(<vscale x 8 x i64>), 2

diff  --git a/llvm/include/llvm/Target/GenericOpcodes.td b/llvm/include/llvm/Target/GenericOpcodes.td
index a55d9d3b04e62f..f5e62dda6fd043 100644
--- a/llvm/include/llvm/Target/GenericOpcodes.td
+++ b/llvm/include/llvm/Target/GenericOpcodes.td
@@ -1541,7 +1541,7 @@ def G_INSERT_SUBVECTOR : GenericInstruction {
 // Generic extract subvector.
 def G_EXTRACT_SUBVECTOR : GenericInstruction {
   let OutOperandList = (outs type0:$dst);
-  let InOperandList = (ins type0:$src, untyped_imm_0:$idx);
+  let InOperandList = (ins type1:$src, untyped_imm_0:$idx);
   let hasSideEffects = false;
 }
 

diff  --git a/llvm/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp b/llvm/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp
index 925a1c7cf6aacc..59f2fc633f5de7 100644
--- a/llvm/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp
@@ -942,7 +942,7 @@ MachineInstrBuilder MachineIRBuilder::buildInsertSubvector(const DstOp &Res,
 MachineInstrBuilder MachineIRBuilder::buildExtractSubvector(const DstOp &Res,
                                                             const SrcOp &Src,
                                                             unsigned Idx) {
-  return buildInstr(TargetOpcode::G_INSERT_SUBVECTOR, Res,
+  return buildInstr(TargetOpcode::G_EXTRACT_SUBVECTOR, Res,
                     {Src, uint64_t(Idx)});
 }
 

diff  --git a/llvm/lib/CodeGen/MachineVerifier.cpp b/llvm/lib/CodeGen/MachineVerifier.cpp
index 759201ed9dadc7..6eed73c15f091d 100644
--- a/llvm/lib/CodeGen/MachineVerifier.cpp
+++ b/llvm/lib/CodeGen/MachineVerifier.cpp
@@ -1778,10 +1778,25 @@ void MachineVerifier::verifyPreISelGenericInstruction(const MachineInstr *MI) {
       break;
     }
 
-    if (IndexOp.getImm() != 0 &&
-        SrcTy.getElementCount().getKnownMinValue() % IndexOp.getImm() != 0) {
-      report("Index must be a multiple of the source vector's minimum vector "
-             "length",
+    if (SrcTy.isScalable() != DstTy.isScalable()) {
+      report("Vector types must both be fixed or both be scalable", MI);
+      break;
+    }
+
+    uint64_t Idx = IndexOp.getImm();
+    uint64_t DstMinLen = DstTy.getElementCount().getKnownMinValue();
+    if (Idx % DstMinLen != 0) {
+      report("Index must be a multiple of the destination vector's minimum "
+             "vector length",
+             MI);
+      break;
+    }
+
+    uint64_t SrcMinLen = SrcTy.getElementCount().getKnownMinValue();
+    if (SrcTy.isScalable() == DstTy.isScalable() &&
+        (Idx >= SrcMinLen || Idx + DstMinLen > SrcMinLen)) {
+      report("Source type and index must not cause extract to overrun to the "
+             "destination type",
              MI);
       break;
     }

diff  --git a/llvm/test/CodeGen/AArch64/GlobalISel/legalizer-info-validation.mir b/llvm/test/CodeGen/AArch64/GlobalISel/legalizer-info-validation.mir
index ee3087a81d2713..db2412de48b56a 100644
--- a/llvm/test/CodeGen/AArch64/GlobalISel/legalizer-info-validation.mir
+++ b/llvm/test/CodeGen/AArch64/GlobalISel/legalizer-info-validation.mir
@@ -651,7 +651,7 @@
 # DEBUG-NEXT: G_INSERT_SUBVECTOR (opcode {{[0-9]+}}): 2 type indices, 1 imm index
 # DEBUG-NEXT: .. type index coverage check SKIPPED: no rules defined
 # DEBUG-NEXT: .. imm index coverage check SKIPPED: no rules defined
-# DEBUG-NEXT: G_EXTRACT_SUBVECTOR (opcode {{[0-9]+}}): 1 type index, 1 imm index
+# DEBUG-NEXT: G_EXTRACT_SUBVECTOR (opcode {{[0-9]+}}): 2 type indices, 1 imm index
 # DEBUG-NEXT: .. type index coverage check SKIPPED: no rules defined
 # DEBUG-NEXT: .. imm index coverage check SKIPPED: no rules defined
 # DEBUG-NEXT: G_INSERT_VECTOR_ELT (opcode {{[0-9]+}}): 3 type indices, 0 imm indices

diff  --git a/llvm/test/MachineVerifier/test_g_extract_subvector.mir b/llvm/test/MachineVerifier/test_g_extract_subvector.mir
index 5a441ff29c1721..6a0b7ebfb4b0b2 100644
--- a/llvm/test/MachineVerifier/test_g_extract_subvector.mir
+++ b/llvm/test/MachineVerifier/test_g_extract_subvector.mir
@@ -27,6 +27,34 @@ body:             |
     ; CHECK: Element type of vectors must be the same
     %8:_(<vscale x 2 x s32>) = G_EXTRACT_SUBVECTOR %7, 0
 
-    ; CHECK: Index must be a multiple of the source vector's minimum vector length
+    ; CHECK: Index must be a multiple of the destination vector's minimum vector length
     %9:_(<vscale x 4 x s32>) = G_EXTRACT_SUBVECTOR  %1, 3
+
+    ; CHECK: Index must be a multiple of the destination vector's minimum vector length
+    %10:_(<vscale x 4 x s32>) = G_EXTRACT_SUBVECTOR  %1, 2
+
+    ; CHECK: Source type and index must not cause extract to overrun to the destination type
+    %11:_(<vscale x 1 x s32>) = G_EXTRACT_SUBVECTOR  %1, 4
+
+    %12:_(<vscale x 4 x s32>) = G_IMPLICIT_DEF
+
+    ; CHECK: Source type and index must not cause extract to overrun to the destination type
+    %13:_(<vscale x 3 x s32>) = G_EXTRACT_SUBVECTOR  %12, 3
+
+    %14:_(<2 x s32>) = G_IMPLICIT_DEF
+    %15:_(<4 x s32>) = G_IMPLICIT_DEF
+
+    ; CHECK: Source type and index must not cause extract to overrun to the destination type
+    %16:_(<2 x s32>) = G_EXTRACT_SUBVECTOR  %14, 4
+
+    ; CHECK: Source type and index must not cause extract to overrun to the destination type
+    %17:_(<3 x s32>) = G_EXTRACT_SUBVECTOR  %15, 3
+
+    ; CHECK: Vector types must both be fixed or both be scalable
+    %18:_(<vscale x 2 x s32>) = G_EXTRACT_SUBVECTOR %15, 0
+
+    ; CHECK: Vector types must both be fixed or both be scalable
+    %19:_(<2 x s32>) = G_EXTRACT_SUBVECTOR %12, 0
+
+
 ...

diff  --git a/llvm/unittests/CodeGen/GlobalISel/MachineIRBuilderTest.cpp b/llvm/unittests/CodeGen/GlobalISel/MachineIRBuilderTest.cpp
index 2c068ba7f88fe6..c85e6d486e0acf 100644
--- a/llvm/unittests/CodeGen/GlobalISel/MachineIRBuilderTest.cpp
+++ b/llvm/unittests/CodeGen/GlobalISel/MachineIRBuilderTest.cpp
@@ -480,3 +480,28 @@ TEST_F(AArch64GISelMITest, BuildFPEnv) {
 
   EXPECT_TRUE(CheckMachineFunction(*MF, CheckStr)) << *MF;
 }
+
+TEST_F(AArch64GISelMITest, BuildExtractSubvector) {
+  setUp();
+  if (!TM)
+    GTEST_SKIP();
+
+  LLT VecTy = LLT::fixed_vector(4, 32);
+  LLT SubVecTy = LLT::fixed_vector(2, 32);
+  auto Vec = B.buildUndef(VecTy);
+  B.buildExtractSubvector(SubVecTy, Vec, 0);
+
+  VecTy = LLT::scalable_vector(4, 32);
+  SubVecTy = LLT::scalable_vector(2, 32);
+  Vec = B.buildUndef(VecTy);
+  B.buildExtractSubvector(SubVecTy, Vec, 0);
+
+  auto CheckStr = R"(
+  ; CHECK: [[DEF:%[0-9]+]]:_(<4 x s32>) = G_IMPLICIT_DEF
+  ; CHECK: [[EXTRACT_SUBVECTOR:%[0-9]+]]:_(<2 x s32>) = G_EXTRACT_SUBVECTOR [[DEF]]:_(<4 x s32>), 0
+  ; CHECK: [[DEF1:%[0-9]+]]:_(<vscale x 4 x s32>) = G_IMPLICIT_DEF
+  ; CHECK: [[EXTRACT_SUBVECTOR1:%[0-9]+]]:_(<vscale x 2 x s32>) = G_EXTRACT_SUBVECTOR [[DEF1]]:_(<vscale x 4 x s32>), 0
+  )";
+
+  EXPECT_TRUE(CheckMachineFunction(*MF, CheckStr)) << *MF;
+}


        


More information about the llvm-commits mailing list