[llvm] [FunctionComparator] Differentiate instructions passing different MDStrings (PR #69543)
Nuri Amari via llvm-commits
llvm-commits at lists.llvm.org
Thu Oct 19 10:13:49 PDT 2023
https://github.com/NuriAmari updated https://github.com/llvm/llvm-project/pull/69543
>From c1015a0ac3baefb06821eebaad6d56f7289f0df9 Mon Sep 17 00:00:00 2001
From: Nuri Amari <nuriamari at fb.com>
Date: Wed, 18 Oct 2023 17:39:51 -0700
Subject: [PATCH 1/2] [FunctionComparator] Distinguish between instructions
passing different MDStrings
Prior to this patch, differing metadata operands to two otherwise
identical instructions was not enough to consider the instructions
different in the eyes of the function comparator.
In this patch, we handle the case where two associated operands are
MDStrings of different value.
This patch does not differentiate more complex metadata operands.
---
.../Transforms/Utils/FunctionComparator.cpp | 30 +++++++++++++++-
.../mergefunc-preserve-vfe-intrinsics.ll | 34 +++++++++++++++++++
2 files changed, 63 insertions(+), 1 deletion(-)
create mode 100644 llvm/test/Transforms/MergeFunc/mergefunc-preserve-vfe-intrinsics.ll
diff --git a/llvm/lib/Transforms/Utils/FunctionComparator.cpp b/llvm/lib/Transforms/Utils/FunctionComparator.cpp
index b1d74f67377e27f..79ca99d1566ce25 100644
--- a/llvm/lib/Transforms/Utils/FunctionComparator.cpp
+++ b/llvm/lib/Transforms/Utils/FunctionComparator.cpp
@@ -160,10 +160,23 @@ int FunctionComparator::cmpAttrs(const AttributeList L,
int FunctionComparator::cmpMetadata(const Metadata *L,
const Metadata *R) const {
// TODO: the following routine coerce the metadata contents into constants
- // before comparison.
+ // or MDStrings before comparison.
// It ignores any other cases, so that the metadata nodes are considered
// equal even though this is not correct.
// We should structurally compare the metadata nodes to be perfect here.
+
+ auto *MDStringL = dyn_cast<MDString>(L);
+ auto *MDStringR = dyn_cast<MDString>(R);
+ if (MDStringL && MDStringR) {
+ if (MDStringL == MDStringR)
+ return 0;
+ return MDStringL->getString().compare(MDStringR->getString());
+ }
+ if (MDStringR)
+ return -1;
+ if (MDStringL)
+ return 1;
+
auto *CL = dyn_cast<ConstantAsMetadata>(L);
auto *CR = dyn_cast<ConstantAsMetadata>(R);
if (CL == CR)
@@ -820,6 +833,21 @@ int FunctionComparator::cmpValues(const Value *L, const Value *R) const {
if (ConstR)
return -1;
+ const MetadataAsValue *MetadataValueL = dyn_cast<MetadataAsValue>(L);
+ const MetadataAsValue *MetadataValueR = dyn_cast<MetadataAsValue>(R);
+ if (MetadataValueL && MetadataValueR) {
+ if (MetadataValueL == MetadataValueR)
+ return 0;
+
+ return cmpMetadata(MetadataValueL->getMetadata(),
+ MetadataValueR->getMetadata());
+ }
+
+ if (MetadataValueL)
+ return 1;
+ if (MetadataValueR)
+ return -1;
+
const InlineAsm *InlineAsmL = dyn_cast<InlineAsm>(L);
const InlineAsm *InlineAsmR = dyn_cast<InlineAsm>(R);
diff --git a/llvm/test/Transforms/MergeFunc/mergefunc-preserve-vfe-intrinsics.ll b/llvm/test/Transforms/MergeFunc/mergefunc-preserve-vfe-intrinsics.ll
new file mode 100644
index 000000000000000..89fa6f82469610d
--- /dev/null
+++ b/llvm/test/Transforms/MergeFunc/mergefunc-preserve-vfe-intrinsics.ll
@@ -0,0 +1,34 @@
+; This test contains three identical functions, aside from the metadata
+; they pass to a function call. This test verifies that the function merger
+; pass is able to merge the two functions that are truly identifical,
+; but the third that passes different metadata is preserved
+
+; RUN: opt -passes=mergefunc -S %s | FileCheck %s
+
+declare { ptr, i1 } @llvm.type.checked.load(ptr, i32, metadata)
+
+define i1 @merge_candidate_a(ptr %ptr, i32 %offset) {
+ %1 = call { ptr, i1 } @llvm.type.checked.load(ptr %ptr, i32 %offset, metadata !"common_metadata")
+ %2 = extractvalue { ptr, i1 } %1, 1
+ ret i1 %2
+}
+
+define i1 @merge_candidate_c(ptr %ptr, i32 %offset) {
+ %1 = call { ptr, i1 } @llvm.type.checked.load(ptr %ptr, i32 %offset, metadata !"different_metadata")
+ %2 = extractvalue { ptr, i1 } %1, 1
+ ret i1 %2
+}
+; CHECK-LABEL: @merge_candidate_c
+; CHECK-NOT: call i1 merge_candidate_a
+; CHECK: call { ptr, i1 } @llvm.type.checked.load
+; CHECK-NOT: call i1 merge_candidate_a
+; CHECK: ret
+
+define i1 @merge_candidate_b(ptr %ptr, i32 %offset) {
+ %1 = call { ptr, i1 } @llvm.type.checked.load(ptr %ptr, i32 %offset, metadata !"common_metadata")
+ %2 = extractvalue { ptr, i1 } %1, 1
+ ret i1 %2
+}
+; CHECK-LABEL: @merge_candidate_b
+; CHECK: call i1 @merge_candidate_a
+; CHECK: ret
>From 70ba2efbc606da0087cecec7b379ec96345e745b Mon Sep 17 00:00:00 2001
From: Nuri Amari <nuriamari at fb.com>
Date: Thu, 19 Oct 2023 10:11:07 -0700
Subject: [PATCH 2/2] Address PR Feedback
- Use update_test_checks.py to generate the file check lines
- Move the test description below the run lines
- Fix type identifical -> identical
---
.../mergefunc-preserve-vfe-intrinsics.ll | 50 +++++++++++--------
1 file changed, 30 insertions(+), 20 deletions(-)
diff --git a/llvm/test/Transforms/MergeFunc/mergefunc-preserve-vfe-intrinsics.ll b/llvm/test/Transforms/MergeFunc/mergefunc-preserve-vfe-intrinsics.ll
index 89fa6f82469610d..1c29aec8d8f28d1 100644
--- a/llvm/test/Transforms/MergeFunc/mergefunc-preserve-vfe-intrinsics.ll
+++ b/llvm/test/Transforms/MergeFunc/mergefunc-preserve-vfe-intrinsics.ll
@@ -1,34 +1,44 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 3
+; RUN: opt -passes=mergefunc -S %s | FileCheck %s
+
; This test contains three identical functions, aside from the metadata
; they pass to a function call. This test verifies that the function merger
-; pass is able to merge the two functions that are truly identifical,
+; pass is able to merge the two functions that are truly identical,
; but the third that passes different metadata is preserved
-; RUN: opt -passes=mergefunc -S %s | FileCheck %s
-
declare { ptr, i1 } @llvm.type.checked.load(ptr, i32, metadata)
define i1 @merge_candidate_a(ptr %ptr, i32 %offset) {
- %1 = call { ptr, i1 } @llvm.type.checked.load(ptr %ptr, i32 %offset, metadata !"common_metadata")
- %2 = extractvalue { ptr, i1 } %1, 1
- ret i1 %2
+; CHECK-LABEL: define i1 @merge_candidate_a(
+; CHECK-SAME: ptr [[PTR:%.*]], i32 [[OFFSET:%.*]]) {
+; CHECK-NEXT: [[TMP1:%.*]] = call { ptr, i1 } @llvm.type.checked.load(ptr [[PTR]], i32 [[OFFSET]], metadata !"common_metadata")
+; CHECK-NEXT: [[TMP2:%.*]] = extractvalue { ptr, i1 } [[TMP1]], 1
+; CHECK-NEXT: ret i1 [[TMP2]]
+;
+ %1 = call { ptr, i1 } @llvm.type.checked.load(ptr %ptr, i32 %offset, metadata !"common_metadata")
+ %2 = extractvalue { ptr, i1 } %1, 1
+ ret i1 %2
}
define i1 @merge_candidate_c(ptr %ptr, i32 %offset) {
- %1 = call { ptr, i1 } @llvm.type.checked.load(ptr %ptr, i32 %offset, metadata !"different_metadata")
- %2 = extractvalue { ptr, i1 } %1, 1
- ret i1 %2
+; CHECK-LABEL: define i1 @merge_candidate_c(
+; CHECK-SAME: ptr [[PTR:%.*]], i32 [[OFFSET:%.*]]) {
+; CHECK-NEXT: [[TMP1:%.*]] = call { ptr, i1 } @llvm.type.checked.load(ptr [[PTR]], i32 [[OFFSET]], metadata !"different_metadata")
+; CHECK-NEXT: [[TMP2:%.*]] = extractvalue { ptr, i1 } [[TMP1]], 1
+; CHECK-NEXT: ret i1 [[TMP2]]
+;
+ %1 = call { ptr, i1 } @llvm.type.checked.load(ptr %ptr, i32 %offset, metadata !"different_metadata")
+ %2 = extractvalue { ptr, i1 } %1, 1
+ ret i1 %2
}
-; CHECK-LABEL: @merge_candidate_c
-; CHECK-NOT: call i1 merge_candidate_a
-; CHECK: call { ptr, i1 } @llvm.type.checked.load
-; CHECK-NOT: call i1 merge_candidate_a
-; CHECK: ret
define i1 @merge_candidate_b(ptr %ptr, i32 %offset) {
- %1 = call { ptr, i1 } @llvm.type.checked.load(ptr %ptr, i32 %offset, metadata !"common_metadata")
- %2 = extractvalue { ptr, i1 } %1, 1
- ret i1 %2
+; CHECK-LABEL: define i1 @merge_candidate_b(
+; CHECK-SAME: ptr [[TMP0:%.*]], i32 [[TMP1:%.*]]) {
+; CHECK-NEXT: [[TMP3:%.*]] = tail call i1 @merge_candidate_a(ptr [[TMP0]], i32 [[TMP1]])
+; CHECK-NEXT: ret i1 [[TMP3]]
+;
+ %1 = call { ptr, i1 } @llvm.type.checked.load(ptr %ptr, i32 %offset, metadata !"common_metadata")
+ %2 = extractvalue { ptr, i1 } %1, 1
+ ret i1 %2
}
-; CHECK-LABEL: @merge_candidate_b
-; CHECK: call i1 @merge_candidate_a
-; CHECK: ret
More information about the llvm-commits
mailing list