Skip to content

Commit

Permalink
[FunctionComparator] Differentiate instructions passing different MDS…
Browse files Browse the repository at this point in the history
…trings (llvm#69543)

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. This breaks LLVM
virtual function elimination, among other features.

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.

---------

Co-authored-by: Nuri Amari <[email protected]>
  • Loading branch information
NuriAmari and Nuri Amari committed Oct 25, 2023
1 parent e47653c commit 3b9cd92
Show file tree
Hide file tree
Showing 2 changed files with 73 additions and 1 deletion.
30 changes: 29 additions & 1 deletion llvm/lib/Transforms/Utils/FunctionComparator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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);

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +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 identical,
; but the third that passes different metadata is preserved

declare { ptr, i1 } @llvm.type.checked.load(ptr, i32, metadata)

define i1 @merge_candidate_a(ptr %ptr, i32 %offset) {
; 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) {
; 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
}

define i1 @merge_candidate_b(ptr %ptr, i32 %offset) {
; 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
}

0 comments on commit 3b9cd92

Please sign in to comment.