[llvm] [GISEL] Fix bugs and clarify spec of G_EXTRACT_SUBVECTOR (PR #108848)
Michael Maitland via llvm-commits
llvm-commits at lists.llvm.org
Tue Sep 17 07:08:39 PDT 2024
https://github.com/michaelmaitland updated https://github.com/llvm/llvm-project/pull/108848
>From fc3b43e6ced2dea4fd3aca63d1a051bd8f726326 Mon Sep 17 00:00:00 2001
From: Michael Maitland <michaeltmaitland at gmail.com>
Date: Mon, 16 Sep 2024 08:45:56 -0700
Subject: [PATCH 01/10] [RISCV][GISEL] Fix bug in G_EXTRACT_SUBVECTOR
definition
The implementation was missing the fact that G_EXTRACT_SUBVECTOR destination
and source vector can be different types.
---
llvm/include/llvm/Target/GenericOpcodes.td | 2 +-
llvm/lib/CodeGen/MachineVerifier.cpp | 5 +++++
llvm/test/MachineVerifier/test_g_extract_subvector.mir | 3 +++
3 files changed, 9 insertions(+), 1 deletion(-)
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/MachineVerifier.cpp b/llvm/lib/CodeGen/MachineVerifier.cpp
index 759201ed9dadc7..6168027369172a 100644
--- a/llvm/lib/CodeGen/MachineVerifier.cpp
+++ b/llvm/lib/CodeGen/MachineVerifier.cpp
@@ -1778,6 +1778,11 @@ void MachineVerifier::verifyPreISelGenericInstruction(const MachineInstr *MI) {
break;
}
+ if (DstTy.isScalable() && SrcTy.isScalable() && DstTy != SrcTy) {
+ report("Scalable vector types must match", 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 "
diff --git a/llvm/test/MachineVerifier/test_g_extract_subvector.mir b/llvm/test/MachineVerifier/test_g_extract_subvector.mir
index 5a441ff29c1721..938bfe3632000e 100644
--- a/llvm/test/MachineVerifier/test_g_extract_subvector.mir
+++ b/llvm/test/MachineVerifier/test_g_extract_subvector.mir
@@ -29,4 +29,7 @@ body: |
; CHECK: Index must be a multiple of the source vector's minimum vector length
%9:_(<vscale x 4 x s32>) = G_EXTRACT_SUBVECTOR %1, 3
+
+ ; CHECK: Scalable vector types must match
+ %10:_(<vscale x 4 x s32>) = G_EXTRACT_SUBVECTOR %1, 2
...
>From 158a4ba23f3e021635d93bc64d41b20fb4bae831 Mon Sep 17 00:00:00 2001
From: Michael Maitland <michaeltmaitland at gmail.com>
Date: Mon, 16 Sep 2024 08:49:57 -0700
Subject: [PATCH 02/10] [GISel] Fix bug where wrong opcode was created in
buildExtractSubvector
---
.../CodeGen/GlobalISel/MachineIRBuilder.cpp | 2 +-
.../GlobalISel/MachineIRBuilderTest.cpp | 24 +++++++++++++++++++
2 files changed, 25 insertions(+), 1 deletion(-)
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/unittests/CodeGen/GlobalISel/MachineIRBuilderTest.cpp b/llvm/unittests/CodeGen/GlobalISel/MachineIRBuilderTest.cpp
index 2c068ba7f88fe6..3296b8e64cd5c0 100644
--- a/llvm/unittests/CodeGen/GlobalISel/MachineIRBuilderTest.cpp
+++ b/llvm/unittests/CodeGen/GlobalISel/MachineIRBuilderTest.cpp
@@ -480,3 +480,27 @@ 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);
+ Vec = B.buildUndef(VecTy);
+ B.buildExtractSubvector(VecTy, Vec, 0);
+
+ auto CheckStr = R"(
+ ; CHECK: [[DEF:%[0-9]+]]:_(<4 x s32>) = G_IMPLICIT_DEF
+ ; CHECK-NEXT: [[EXTRACT_SUBVECTOR:%[0-9]+]]:_(<2 x s32>) = G_EXTRACT_SUBVECTOR [[DEF]](<4 x s32>), 0
+ ; CHECK-NEXT: [[DEF1:%[0-9]+]]:_(<vscale x 4 x s32>) = G_IMPLICIT_DEF
+ ; CHECK-NEXT: [[EXTRACT_SUBVECTOR1:%[0-9]+]]:_(<vscale x 4 x s32>) = G_EXTRACT_SUBVECTOR [[DEF1]](<vscale x 4 x s32>), 0
+ )";
+
+ EXPECT_TRUE(CheckMachineFunction(*MF, CheckStr)) << *MF;
+}
>From 8ebf7c945353e180ae8af8a6fad17f14d753c587 Mon Sep 17 00:00:00 2001
From: Michael Maitland <michaeltmaitland at gmail.com>
Date: Mon, 16 Sep 2024 13:22:50 -0700
Subject: [PATCH 03/10] fixup! respond to comments
---
llvm/lib/CodeGen/MachineVerifier.cpp | 12 +++---------
.../MachineVerifier/test_g_extract_subvector.mir | 4 ++--
2 files changed, 5 insertions(+), 11 deletions(-)
diff --git a/llvm/lib/CodeGen/MachineVerifier.cpp b/llvm/lib/CodeGen/MachineVerifier.cpp
index 6168027369172a..49f19f6539b693 100644
--- a/llvm/lib/CodeGen/MachineVerifier.cpp
+++ b/llvm/lib/CodeGen/MachineVerifier.cpp
@@ -1778,15 +1778,9 @@ void MachineVerifier::verifyPreISelGenericInstruction(const MachineInstr *MI) {
break;
}
- if (DstTy.isScalable() && SrcTy.isScalable() && DstTy != SrcTy) {
- report("Scalable vector types must match", 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 (IndexOp.getImm() % DstTy.getElementCount().getKnownMinValue() != 0) {
+ report("Index must be a multiple of the destinations vector's minimum "
+ "vector length",
MI);
break;
}
diff --git a/llvm/test/MachineVerifier/test_g_extract_subvector.mir b/llvm/test/MachineVerifier/test_g_extract_subvector.mir
index 938bfe3632000e..c50a518c99ac2d 100644
--- a/llvm/test/MachineVerifier/test_g_extract_subvector.mir
+++ b/llvm/test/MachineVerifier/test_g_extract_subvector.mir
@@ -27,9 +27,9 @@ 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 destinations vector's minimum vector length
%9:_(<vscale x 4 x s32>) = G_EXTRACT_SUBVECTOR %1, 3
- ; CHECK: Scalable vector types must match
+ ; CHECK: Index must be a multiple of the destinations vector's minimum vector length
%10:_(<vscale x 4 x s32>) = G_EXTRACT_SUBVECTOR %1, 2
...
>From 2a21df26817502db86dc1c439a313e1453d44fe4 Mon Sep 17 00:00:00 2001
From: Michael Maitland <michaeltmaitland at gmail.com>
Date: Mon, 16 Sep 2024 14:15:13 -0700
Subject: [PATCH 04/10] fixup! fix typo
---
llvm/lib/CodeGen/MachineVerifier.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/llvm/lib/CodeGen/MachineVerifier.cpp b/llvm/lib/CodeGen/MachineVerifier.cpp
index 49f19f6539b693..f88a5420e0223e 100644
--- a/llvm/lib/CodeGen/MachineVerifier.cpp
+++ b/llvm/lib/CodeGen/MachineVerifier.cpp
@@ -1779,7 +1779,7 @@ void MachineVerifier::verifyPreISelGenericInstruction(const MachineInstr *MI) {
}
if (IndexOp.getImm() % DstTy.getElementCount().getKnownMinValue() != 0) {
- report("Index must be a multiple of the destinations vector's minimum "
+ report("Index must be a multiple of the destination vector's minimum "
"vector length",
MI);
break;
>From b32f2b6bb19c7ba1c9f5389125d5356b9eff78a8 Mon Sep 17 00:00:00 2001
From: Michael Maitland <michaeltmaitland at gmail.com>
Date: Mon, 16 Sep 2024 14:30:29 -0700
Subject: [PATCH 05/10] fixup! respond to comments
---
llvm/lib/CodeGen/MachineVerifier.cpp | 12 +++++++++++-
.../MachineVerifier/test_g_extract_subvector.mir | 7 +++++--
.../CodeGen/GlobalISel/MachineIRBuilderTest.cpp | 5 +++--
3 files changed, 19 insertions(+), 5 deletions(-)
diff --git a/llvm/lib/CodeGen/MachineVerifier.cpp b/llvm/lib/CodeGen/MachineVerifier.cpp
index f88a5420e0223e..feb85d6765f300 100644
--- a/llvm/lib/CodeGen/MachineVerifier.cpp
+++ b/llvm/lib/CodeGen/MachineVerifier.cpp
@@ -1778,13 +1778,23 @@ void MachineVerifier::verifyPreISelGenericInstruction(const MachineInstr *MI) {
break;
}
- if (IndexOp.getImm() % DstTy.getElementCount().getKnownMinValue() != 0) {
+ int64_t Idx = IndexOp.getImm();
+ auto DstMinLen = DstTy.getElementCount().getKnownMinValue();
+ if (Idx % DstMinLen != 0) {
report("Index must be a multiple of the destination vector's minimum "
"vector length",
MI);
break;
}
+ auto SrcMinLen = SrcTy.getElementCount().getKnownMinValue();
+ if (Idx < SrcMinLen && Idx + DstMinLen <= SrcMinLen) {
+ report("Source type and index must not cause extract to overrun to the "
+ "destination type",
+ MI);
+ break;
+ }
+
break;
}
case TargetOpcode::G_SHUFFLE_VECTOR: {
diff --git a/llvm/test/MachineVerifier/test_g_extract_subvector.mir b/llvm/test/MachineVerifier/test_g_extract_subvector.mir
index c50a518c99ac2d..7a92e264f1e729 100644
--- a/llvm/test/MachineVerifier/test_g_extract_subvector.mir
+++ b/llvm/test/MachineVerifier/test_g_extract_subvector.mir
@@ -27,9 +27,12 @@ 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 destinations 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 destinations vector's minimum vector length
+ ; 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, 1
...
diff --git a/llvm/unittests/CodeGen/GlobalISel/MachineIRBuilderTest.cpp b/llvm/unittests/CodeGen/GlobalISel/MachineIRBuilderTest.cpp
index 3296b8e64cd5c0..227a9d652b3230 100644
--- a/llvm/unittests/CodeGen/GlobalISel/MachineIRBuilderTest.cpp
+++ b/llvm/unittests/CodeGen/GlobalISel/MachineIRBuilderTest.cpp
@@ -492,14 +492,15 @@ TEST_F(AArch64GISelMITest, BuildExtractSubvector) {
B.buildExtractSubvector(SubVecTy, Vec, 0);
VecTy = LLT::scalable_vector(4, 32);
+ SubVecTy = LLT::scalable_vector(2, 32);
Vec = B.buildUndef(VecTy);
- B.buildExtractSubvector(VecTy, Vec, 0);
+ B.buildExtractSubvector(SubVecTy, Vec, 0);
auto CheckStr = R"(
; CHECK: [[DEF:%[0-9]+]]:_(<4 x s32>) = G_IMPLICIT_DEF
; CHECK-NEXT: [[EXTRACT_SUBVECTOR:%[0-9]+]]:_(<2 x s32>) = G_EXTRACT_SUBVECTOR [[DEF]](<4 x s32>), 0
; CHECK-NEXT: [[DEF1:%[0-9]+]]:_(<vscale x 4 x s32>) = G_IMPLICIT_DEF
- ; CHECK-NEXT: [[EXTRACT_SUBVECTOR1:%[0-9]+]]:_(<vscale x 4 x s32>) = G_EXTRACT_SUBVECTOR [[DEF1]](<vscale x 4 x s32>), 0
+ ; CHECK-NEXT: [[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;
>From 95cc03cd6d6a3cbee4d8b9c707ca76bee87dc5a8 Mon Sep 17 00:00:00 2001
From: Michael Maitland <michaeltmaitland at gmail.com>
Date: Mon, 16 Sep 2024 15:01:20 -0700
Subject: [PATCH 06/10] fixup! respond to comments
---
llvm/lib/CodeGen/MachineVerifier.cpp | 3 ++-
llvm/test/MachineVerifier/test_g_extract_subvector.mir | 7 ++++++-
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/llvm/lib/CodeGen/MachineVerifier.cpp b/llvm/lib/CodeGen/MachineVerifier.cpp
index feb85d6765f300..fa2a2eb6ade4f4 100644
--- a/llvm/lib/CodeGen/MachineVerifier.cpp
+++ b/llvm/lib/CodeGen/MachineVerifier.cpp
@@ -1788,7 +1788,8 @@ void MachineVerifier::verifyPreISelGenericInstruction(const MachineInstr *MI) {
}
auto SrcMinLen = SrcTy.getElementCount().getKnownMinValue();
- if (Idx < SrcMinLen && Idx + DstMinLen <= SrcMinLen) {
+ 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);
diff --git a/llvm/test/MachineVerifier/test_g_extract_subvector.mir b/llvm/test/MachineVerifier/test_g_extract_subvector.mir
index 7a92e264f1e729..97d7f0d1268e65 100644
--- a/llvm/test/MachineVerifier/test_g_extract_subvector.mir
+++ b/llvm/test/MachineVerifier/test_g_extract_subvector.mir
@@ -34,5 +34,10 @@ body: |
%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, 1
+ %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
...
>From c47aa1be6b9912a063ba72035247aba3c4844a17 Mon Sep 17 00:00:00 2001
From: Michael Maitland <michaeltmaitland at gmail.com>
Date: Mon, 16 Sep 2024 15:22:35 -0700
Subject: [PATCH 07/10] fixup! respond to reviews
---
llvm/lib/CodeGen/MachineVerifier.cpp | 6 +++---
.../AArch64/GlobalISel/legalizer-info-validation.mir | 2 +-
.../CodeGen/GlobalISel/MachineIRBuilderTest.cpp | 9 ++++++---
3 files changed, 10 insertions(+), 7 deletions(-)
diff --git a/llvm/lib/CodeGen/MachineVerifier.cpp b/llvm/lib/CodeGen/MachineVerifier.cpp
index fa2a2eb6ade4f4..40d076fc5c1726 100644
--- a/llvm/lib/CodeGen/MachineVerifier.cpp
+++ b/llvm/lib/CodeGen/MachineVerifier.cpp
@@ -1778,8 +1778,8 @@ void MachineVerifier::verifyPreISelGenericInstruction(const MachineInstr *MI) {
break;
}
- int64_t Idx = IndexOp.getImm();
- auto DstMinLen = DstTy.getElementCount().getKnownMinValue();
+ 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",
@@ -1787,7 +1787,7 @@ void MachineVerifier::verifyPreISelGenericInstruction(const MachineInstr *MI) {
break;
}
- auto SrcMinLen = SrcTy.getElementCount().getKnownMinValue();
+ 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 "
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/unittests/CodeGen/GlobalISel/MachineIRBuilderTest.cpp b/llvm/unittests/CodeGen/GlobalISel/MachineIRBuilderTest.cpp
index 227a9d652b3230..8da4d369da088f 100644
--- a/llvm/unittests/CodeGen/GlobalISel/MachineIRBuilderTest.cpp
+++ b/llvm/unittests/CodeGen/GlobalISel/MachineIRBuilderTest.cpp
@@ -497,10 +497,13 @@ TEST_F(AArch64GISelMITest, BuildExtractSubvector) {
B.buildExtractSubvector(SubVecTy, Vec, 0);
auto CheckStr = R"(
+ ; CHECK: [[COPY0:%[0-9]+]]:_(s64) = COPY $x0
+ ; CHECK: [[COPY1:%[0-9]+]]:_(s64) = COPY $x1
+ ; CHECK: [[COPY2:%[0-9]+]]:_(s64) = COPY $x2
; CHECK: [[DEF:%[0-9]+]]:_(<4 x s32>) = G_IMPLICIT_DEF
- ; CHECK-NEXT: [[EXTRACT_SUBVECTOR:%[0-9]+]]:_(<2 x s32>) = G_EXTRACT_SUBVECTOR [[DEF]](<4 x s32>), 0
- ; CHECK-NEXT: [[DEF1:%[0-9]+]]:_(<vscale x 4 x s32>) = G_IMPLICIT_DEF
- ; CHECK-NEXT: [[EXTRACT_SUBVECTOR1:%[0-9]+]]:_(<vscale x 2 x s32>) = G_EXTRACT_SUBVECTOR [[DEF1]](<vscale x 4 x s32>), 0
+ ; 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;
>From 1d8fee3cf0a6c03dde1cda697a0bade1d5db28ae Mon Sep 17 00:00:00 2001
From: Michael Maitland <michaeltmaitland at gmail.com>
Date: Mon, 16 Sep 2024 15:39:25 -0700
Subject: [PATCH 08/10] fixup! do the check for fixed vectors too
---
llvm/lib/CodeGen/MachineVerifier.cpp | 4 ++--
llvm/test/MachineVerifier/test_g_extract_subvector.mir | 9 +++++++++
2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/llvm/lib/CodeGen/MachineVerifier.cpp b/llvm/lib/CodeGen/MachineVerifier.cpp
index 40d076fc5c1726..838383b301a9d9 100644
--- a/llvm/lib/CodeGen/MachineVerifier.cpp
+++ b/llvm/lib/CodeGen/MachineVerifier.cpp
@@ -1778,7 +1778,7 @@ void MachineVerifier::verifyPreISelGenericInstruction(const MachineInstr *MI) {
break;
}
- uint64_t Idx = IndexOp.getImm();
+ int64_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 "
@@ -1788,7 +1788,7 @@ void MachineVerifier::verifyPreISelGenericInstruction(const MachineInstr *MI) {
}
uint64_t SrcMinLen = SrcTy.getElementCount().getKnownMinValue();
- if (SrcTy.isScalable() && DstTy.isScalable() &&
+ 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",
diff --git a/llvm/test/MachineVerifier/test_g_extract_subvector.mir b/llvm/test/MachineVerifier/test_g_extract_subvector.mir
index 97d7f0d1268e65..385aa948d77fc3 100644
--- a/llvm/test/MachineVerifier/test_g_extract_subvector.mir
+++ b/llvm/test/MachineVerifier/test_g_extract_subvector.mir
@@ -40,4 +40,13 @@ body: |
; 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
...
>From 6dd01cb2a1d6a4ac27b36fa6fb9d3359c7281e84 Mon Sep 17 00:00:00 2001
From: Michael Maitland <michaeltmaitland at gmail.com>
Date: Mon, 16 Sep 2024 16:21:41 -0700
Subject: [PATCH 09/10] fixup! disallow mixed vectors
---
llvm/docs/GlobalISel/GenericOpcode.rst | 2 ++
llvm/lib/CodeGen/MachineVerifier.cpp | 7 ++++++-
llvm/test/MachineVerifier/test_g_extract_subvector.mir | 8 ++++++++
3 files changed, 16 insertions(+), 1 deletion(-)
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/lib/CodeGen/MachineVerifier.cpp b/llvm/lib/CodeGen/MachineVerifier.cpp
index 838383b301a9d9..6eed73c15f091d 100644
--- a/llvm/lib/CodeGen/MachineVerifier.cpp
+++ b/llvm/lib/CodeGen/MachineVerifier.cpp
@@ -1778,7 +1778,12 @@ void MachineVerifier::verifyPreISelGenericInstruction(const MachineInstr *MI) {
break;
}
- int64_t Idx = IndexOp.getImm();
+ 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 "
diff --git a/llvm/test/MachineVerifier/test_g_extract_subvector.mir b/llvm/test/MachineVerifier/test_g_extract_subvector.mir
index 385aa948d77fc3..6a0b7ebfb4b0b2 100644
--- a/llvm/test/MachineVerifier/test_g_extract_subvector.mir
+++ b/llvm/test/MachineVerifier/test_g_extract_subvector.mir
@@ -49,4 +49,12 @@ body: |
; 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
+
+
...
>From f08a6a48dff66fce4785048aaac0f8053d182fd1 Mon Sep 17 00:00:00 2001
From: Michael Maitland <michaeltmaitland at gmail.com>
Date: Tue, 17 Sep 2024 07:07:44 -0700
Subject: [PATCH 10/10] fixup! fix check line
---
llvm/unittests/CodeGen/GlobalISel/MachineIRBuilderTest.cpp | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/llvm/unittests/CodeGen/GlobalISel/MachineIRBuilderTest.cpp b/llvm/unittests/CodeGen/GlobalISel/MachineIRBuilderTest.cpp
index 8da4d369da088f..c85e6d486e0acf 100644
--- a/llvm/unittests/CodeGen/GlobalISel/MachineIRBuilderTest.cpp
+++ b/llvm/unittests/CodeGen/GlobalISel/MachineIRBuilderTest.cpp
@@ -497,13 +497,10 @@ TEST_F(AArch64GISelMITest, BuildExtractSubvector) {
B.buildExtractSubvector(SubVecTy, Vec, 0);
auto CheckStr = R"(
- ; CHECK: [[COPY0:%[0-9]+]]:_(s64) = COPY $x0
- ; CHECK: [[COPY1:%[0-9]+]]:_(s64) = COPY $x1
- ; CHECK: [[COPY2:%[0-9]+]]:_(s64) = COPY $x2
; 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: [[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
+ ; 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