From c1015a0ac3baefb06821eebaad6d56f7289f0df9 Mon Sep 17 00:00:00 2001 From: Nuri Amari 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 b1d74f67377e2..79ca99d1566ce 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(L); + auto *MDStringR = dyn_cast(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(L); auto *CR = dyn_cast(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(L); + const MetadataAsValue *MetadataValueR = dyn_cast(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(L); const InlineAsm *InlineAsmR = dyn_cast(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 0000000000000..89fa6f8246961 --- /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 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 89fa6f8246961..1c29aec8d8f28 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