[llvm] [GlobalISel] Catching inconsistencies in load memory, result, and range metadata type (PR #121247)
Renat Idrisov via llvm-commits
llvm-commits at lists.llvm.org
Mon Jan 6 08:09:21 PST 2025
https://github.com/parsifal-47 updated https://github.com/llvm/llvm-project/pull/121247
>From d18e4f72c101adfd1cda014dd2c7f1b9e8d6f512 Mon Sep 17 00:00:00 2001
From: Renat Idrisov <parsifal-47 at users.noreply.github.com>
Date: Sat, 28 Dec 2024 04:07:30 +0000
Subject: [PATCH 1/4] Catching inconsistencies in load memory, result, and
range metadata type
---
llvm/lib/CodeGen/MachineVerifier.cpp | 6 +++++
.../test_g_load_vector_to_scalar.mir | 25 +++++++++++++++++++
2 files changed, 31 insertions(+)
create mode 100644 llvm/test/CodeGen/AMDGPU/GlobalISel/test_g_load_vector_to_scalar.mir
diff --git a/llvm/lib/CodeGen/MachineVerifier.cpp b/llvm/lib/CodeGen/MachineVerifier.cpp
index bec36b728ae328..1866d3ae2b285a 100644
--- a/llvm/lib/CodeGen/MachineVerifier.cpp
+++ b/llvm/lib/CodeGen/MachineVerifier.cpp
@@ -1274,6 +1274,12 @@ void MachineVerifier::verifyPreISelGenericInstruction(const MachineInstr *MI) {
if (TypeSize::isKnownGT(MMO.getSize().getValue(),
ValTy.getSizeInBytes()))
report("load memory size cannot exceed result size", MI);
+
+ if (MMO.getRanges() && (ValTy.isVector() != MMO.getType().isVector())) {
+ report("scalar operands cannot be loaded into vector values and vice "
+ "versa",
+ MI);
+ }
} else if (MI->getOpcode() == TargetOpcode::G_STORE) {
if (TypeSize::isKnownLT(ValTy.getSizeInBytes(),
MMO.getSize().getValue()))
diff --git a/llvm/test/CodeGen/AMDGPU/GlobalISel/test_g_load_vector_to_scalar.mir b/llvm/test/CodeGen/AMDGPU/GlobalISel/test_g_load_vector_to_scalar.mir
new file mode 100644
index 00000000000000..95429127c30d0c
--- /dev/null
+++ b/llvm/test/CodeGen/AMDGPU/GlobalISel/test_g_load_vector_to_scalar.mir
@@ -0,0 +1,25 @@
+# RUN: not --crash llc -mtriple=amdgcn-mesa-mesa3d -mcpu=tahiti -run-pass=amdgpu-prelegalizer-combiner -verify-machineinstrs %s -o -
+--- |
+ define void @range_metadata_sext_i33_signed_range_i64_load_as_v2i32() {
+ ret void
+ }
+
+ !1 = !{i64 -8589934591, i64 8589934592}
+
+...
+---
+name: range_metadata_sext_i33_signed_range_i64_load_as_v2i32
+tracksRegLiveness: true
+body: |
+ bb.0:
+ liveins: $vgpr0, $vgpr1
+
+ %1:_(s32) = COPY $vgpr0
+ %2:_(s32) = COPY $vgpr1
+ %0:_(p1) = G_MERGE_VALUES %1(s32), %2(s32)
+ ; CHECK: Bad machine code: scalar operands cannot be loaded into vector values
+ %3:_(<2 x s32>) = G_LOAD %0(p1) :: (volatile load (s64), align 4, !range !1, addrspace 1)
+ $vgpr0_vgpr1 = COPY %3
+ SI_RETURN implicit $vgpr0_vgpr1
+
+...
>From b76c3dfb3475e44d8c17a028f41f0223ddca1267 Mon Sep 17 00:00:00 2001
From: Renat Idrisov <parsifal-47 at users.noreply.github.com>
Date: Sun, 5 Jan 2025 15:53:04 +0000
Subject: [PATCH 2/4] Moving test case to proper location
---
.../test_g_load_vector_to_scalar.mir | 25 -------
.../test_g_load_vector_to_scalar.mir | 71 +++++++++++++++++++
2 files changed, 71 insertions(+), 25 deletions(-)
delete mode 100644 llvm/test/CodeGen/AMDGPU/GlobalISel/test_g_load_vector_to_scalar.mir
create mode 100644 llvm/test/MachineVerifier/test_g_load_vector_to_scalar.mir
diff --git a/llvm/test/CodeGen/AMDGPU/GlobalISel/test_g_load_vector_to_scalar.mir b/llvm/test/CodeGen/AMDGPU/GlobalISel/test_g_load_vector_to_scalar.mir
deleted file mode 100644
index 95429127c30d0c..00000000000000
--- a/llvm/test/CodeGen/AMDGPU/GlobalISel/test_g_load_vector_to_scalar.mir
+++ /dev/null
@@ -1,25 +0,0 @@
-# RUN: not --crash llc -mtriple=amdgcn-mesa-mesa3d -mcpu=tahiti -run-pass=amdgpu-prelegalizer-combiner -verify-machineinstrs %s -o -
---- |
- define void @range_metadata_sext_i33_signed_range_i64_load_as_v2i32() {
- ret void
- }
-
- !1 = !{i64 -8589934591, i64 8589934592}
-
-...
----
-name: range_metadata_sext_i33_signed_range_i64_load_as_v2i32
-tracksRegLiveness: true
-body: |
- bb.0:
- liveins: $vgpr0, $vgpr1
-
- %1:_(s32) = COPY $vgpr0
- %2:_(s32) = COPY $vgpr1
- %0:_(p1) = G_MERGE_VALUES %1(s32), %2(s32)
- ; CHECK: Bad machine code: scalar operands cannot be loaded into vector values
- %3:_(<2 x s32>) = G_LOAD %0(p1) :: (volatile load (s64), align 4, !range !1, addrspace 1)
- $vgpr0_vgpr1 = COPY %3
- SI_RETURN implicit $vgpr0_vgpr1
-
-...
diff --git a/llvm/test/MachineVerifier/test_g_load_vector_to_scalar.mir b/llvm/test/MachineVerifier/test_g_load_vector_to_scalar.mir
new file mode 100644
index 00000000000000..b255c0e5cd4021
--- /dev/null
+++ b/llvm/test/MachineVerifier/test_g_load_vector_to_scalar.mir
@@ -0,0 +1,71 @@
+# RUN: not --crash llc -mtriple=amdgcn-mesa-mesa3d -mcpu=tahiti -run-pass=none %s -o -
+--- |
+ define void @range_metadata_sext_i8_signed_range_i64_load_as_v2i32() {
+ ret void
+ }
+
+ define void @range_metadata_sext_i8_signed_range_i64_load_as_v2i32_extractlo() {
+ ret void
+ }
+
+ define void @range_metadata_sext_i33_signed_range_i64_load_as_v2i32() {
+ ret void
+ }
+
+ !0 = !{i64 -4294967295, i64 4294967296}
+ !1 = !{i64 -8589934591, i64 8589934592}
+
+...
+---
+name: range_metadata_sext_i33_signed_range_i64_load_as_v2i32
+tracksRegLiveness: true
+body: |
+ bb.0:
+ liveins: $vgpr0, $vgpr1
+
+ %1:_(s32) = COPY $vgpr0
+ %2:_(s32) = COPY $vgpr1
+ %0:_(p1) = G_MERGE_VALUES %1(s32), %2(s32)
+ ; CHECK: Bad machine code: scalar operands cannot be loaded into vector values
+ %3:_(<2 x s32>) = G_LOAD %0(p1) :: (volatile load (s64), align 4, !range !1, addrspace 1)
+ $vgpr0_vgpr1 = COPY %3
+ SI_RETURN implicit $vgpr0_vgpr1
+
+...
+
+---
+name: range_metadata_sext_i8_signed_range_i64_load_as_v2i32_extractlo
+tracksRegLiveness: true
+body: |
+ bb.0:
+ liveins: $vgpr0, $vgpr1
+
+ %1:_(s32) = COPY $vgpr0
+ %2:_(s32) = COPY $vgpr1
+ %0:_(p1) = G_MERGE_VALUES %1(s32), %2(s32)
+ ; CHECK: Bad machine code: scalar operands cannot be loaded into vector values
+ %3:_(<2 x s32>) = G_LOAD %0(p1) :: (volatile load (s64), align 4, !range !0, addrspace 1)
+ %zero:_(s32) = G_CONSTANT i32 0
+ %extract:_(s32) = G_EXTRACT_VECTOR_ELT %3, %zero
+ %6:_(s32) = G_SEXT_INREG %zero, 9
+ $vgpr0 = COPY %6
+ SI_RETURN implicit $vgpr0, implicit $vgpr1
+
+...
+
+---
+name: range_metadata_sext_i33_signed_range_i64_load_as_v2i32
+tracksRegLiveness: true
+body: |
+ bb.0:
+ liveins: $vgpr0, $vgpr1
+
+ %1:_(s32) = COPY $vgpr0
+ %2:_(s32) = COPY $vgpr1
+ %0:_(p1) = G_MERGE_VALUES %1(s32), %2(s32)
+ ; CHECK: Bad machine code: scalar operands cannot be loaded into vector values
+ %3:_(<2 x s32>) = G_LOAD %0(p1) :: (volatile load (s64), align 4, !range !1, addrspace 1)
+ $vgpr0_vgpr1 = COPY %3
+ SI_RETURN implicit $vgpr0_vgpr1
+
+...
>From b740e226c762079223b9aab6f5457d15a61f8ce8 Mon Sep 17 00:00:00 2001
From: Renat Idrisov <parsifal-47 at users.noreply.github.com>
Date: Mon, 6 Jan 2025 15:58:17 +0000
Subject: [PATCH 3/4] Addressing review feedback
---
llvm/lib/CodeGen/MachineVerifier.cpp | 16 +++++++++++++---
.../test_g_load_vector_to_scalar.mir | 13 +++++++------
2 files changed, 20 insertions(+), 9 deletions(-)
diff --git a/llvm/lib/CodeGen/MachineVerifier.cpp b/llvm/lib/CodeGen/MachineVerifier.cpp
index 1866d3ae2b285a..5a02fde6383897 100644
--- a/llvm/lib/CodeGen/MachineVerifier.cpp
+++ b/llvm/lib/CodeGen/MachineVerifier.cpp
@@ -1275,10 +1275,20 @@ void MachineVerifier::verifyPreISelGenericInstruction(const MachineInstr *MI) {
ValTy.getSizeInBytes()))
report("load memory size cannot exceed result size", MI);
- if (MMO.getRanges() && (ValTy.isVector() != MMO.getType().isVector())) {
- report("scalar operands cannot be loaded into vector values and vice "
- "versa",
+ if (MMO.getRanges()) {
+ auto operandBits = [](const MDOperand &o) -> unsigned {
+ ConstantInt *i = mdconst::dyn_extract<ConstantInt>(o);
+ if (!i->isNegative()) return i->getValue().getActiveBits();
+ APInt reversed(i->getValue());
+ reversed.negate();
+ return reversed.getActiveBits() + 1; // one more bit for the sign
+ };
+ unsigned bitsUsed = std::max(operandBits(MMO.getRanges()->getOperand(0)),
+ operandBits(MMO.getRanges()->getOperand(1)));
+ if (bitsUsed != ValTy.getScalarType().getSizeInBits()) {
+ report("range is incompatible with the value it gets assigned to",
MI);
+ }
}
} else if (MI->getOpcode() == TargetOpcode::G_STORE) {
if (TypeSize::isKnownLT(ValTy.getSizeInBytes(),
diff --git a/llvm/test/MachineVerifier/test_g_load_vector_to_scalar.mir b/llvm/test/MachineVerifier/test_g_load_vector_to_scalar.mir
index b255c0e5cd4021..ea4c986a3428bb 100644
--- a/llvm/test/MachineVerifier/test_g_load_vector_to_scalar.mir
+++ b/llvm/test/MachineVerifier/test_g_load_vector_to_scalar.mir
@@ -26,7 +26,7 @@ body: |
%1:_(s32) = COPY $vgpr0
%2:_(s32) = COPY $vgpr1
%0:_(p1) = G_MERGE_VALUES %1(s32), %2(s32)
- ; CHECK: Bad machine code: scalar operands cannot be loaded into vector values
+ ; CHECK: Bad machine code: range is incompatible with the value it gets assigned to
%3:_(<2 x s32>) = G_LOAD %0(p1) :: (volatile load (s64), align 4, !range !1, addrspace 1)
$vgpr0_vgpr1 = COPY %3
SI_RETURN implicit $vgpr0_vgpr1
@@ -43,7 +43,7 @@ body: |
%1:_(s32) = COPY $vgpr0
%2:_(s32) = COPY $vgpr1
%0:_(p1) = G_MERGE_VALUES %1(s32), %2(s32)
- ; CHECK: Bad machine code: scalar operands cannot be loaded into vector values
+ ; CHECK: Bad machine code: range is incompatible with the value it gets assigned to
%3:_(<2 x s32>) = G_LOAD %0(p1) :: (volatile load (s64), align 4, !range !0, addrspace 1)
%zero:_(s32) = G_CONSTANT i32 0
%extract:_(s32) = G_EXTRACT_VECTOR_ELT %3, %zero
@@ -54,7 +54,7 @@ body: |
...
---
-name: range_metadata_sext_i33_signed_range_i64_load_as_v2i32
+name: range_metadata_sext_i8_signed_range_i64_load_as_v2i32
tracksRegLiveness: true
body: |
bb.0:
@@ -63,9 +63,10 @@ body: |
%1:_(s32) = COPY $vgpr0
%2:_(s32) = COPY $vgpr1
%0:_(p1) = G_MERGE_VALUES %1(s32), %2(s32)
- ; CHECK: Bad machine code: scalar operands cannot be loaded into vector values
- %3:_(<2 x s32>) = G_LOAD %0(p1) :: (volatile load (s64), align 4, !range !1, addrspace 1)
- $vgpr0_vgpr1 = COPY %3
+ ; CHECK: Bad machine code: range is incompatible with the value it gets assigned to
+ %3:_(<2 x s32>) = G_LOAD %0(p1) :: (volatile load (s64), align 4, !range !0, addrspace 1)
+ %6:_(<2 x s32>) = G_SEXT_INREG %3, 9
+ $vgpr0_vgpr1 = COPY %6
SI_RETURN implicit $vgpr0_vgpr1
...
>From f935fc6ada4b1eec6e0a56aa69497fb787cb3e8b Mon Sep 17 00:00:00 2001
From: Renat Idrisov <parsifal-47 at users.noreply.github.com>
Date: Mon, 6 Jan 2025 16:09:06 +0000
Subject: [PATCH 4/4] Formatting
---
llvm/lib/CodeGen/MachineVerifier.cpp | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/llvm/lib/CodeGen/MachineVerifier.cpp b/llvm/lib/CodeGen/MachineVerifier.cpp
index 5a02fde6383897..d484c6811dda58 100644
--- a/llvm/lib/CodeGen/MachineVerifier.cpp
+++ b/llvm/lib/CodeGen/MachineVerifier.cpp
@@ -1277,17 +1277,19 @@ void MachineVerifier::verifyPreISelGenericInstruction(const MachineInstr *MI) {
if (MMO.getRanges()) {
auto operandBits = [](const MDOperand &o) -> unsigned {
- ConstantInt *i = mdconst::dyn_extract<ConstantInt>(o);
- if (!i->isNegative()) return i->getValue().getActiveBits();
+ ConstantInt *i = mdconst::dyn_extract<ConstantInt>(o);
+ if (!i->isNegative())
+ return i->getValue().getActiveBits();
APInt reversed(i->getValue());
reversed.negate();
return reversed.getActiveBits() + 1; // one more bit for the sign
};
- unsigned bitsUsed = std::max(operandBits(MMO.getRanges()->getOperand(0)),
- operandBits(MMO.getRanges()->getOperand(1)));
+ unsigned bitsUsed =
+ std::max(operandBits(MMO.getRanges()->getOperand(0)),
+ operandBits(MMO.getRanges()->getOperand(1)));
if (bitsUsed != ValTy.getScalarType().getSizeInBits()) {
- report("range is incompatible with the value it gets assigned to",
- MI);
+ report("range is incompatible with the value it gets assigned to",
+ MI);
}
}
} else if (MI->getOpcode() == TargetOpcode::G_STORE) {
More information about the llvm-commits
mailing list