Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions llvm/include/llvm/IR/StructuralHash.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ class Module;
/// to true includes instruction and operand type information.
stable_hash StructuralHash(const Function &F, bool DetailedHash = false);

/// Returns a hash of the global variable \p G.
stable_hash StructuralHash(const GlobalVariable &G);

/// Returns a hash of the module \p M by hashing all functions and global
/// variables contained within. \param M The module to hash. \param DetailedHash
/// Whether or not to encode additional information in the function hashes that
Expand Down
13 changes: 10 additions & 3 deletions llvm/lib/CodeGen/MachineStableHash.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@
#include "llvm/CodeGen/Register.h"
#include "llvm/Config/llvm-config.h"
#include "llvm/IR/Constants.h"
#include "llvm/IR/GlobalVariable.h"
#include "llvm/IR/StructuralHash.h"
#include "llvm/MC/MCSymbol.h"
#include "llvm/Support/Alignment.h"
#include "llvm/Support/ErrorHandling.h"
Expand Down Expand Up @@ -97,9 +99,14 @@ stable_hash llvm::stableHashValue(const MachineOperand &MO) {
++StableHashBailingGlobalAddress;
return 0;
}
auto Name = GV->getName();
return stable_hash_combine(MO.getType(), MO.getTargetFlags(),
stable_hash_name(Name), MO.getOffset());
stable_hash GVHash = 0;
if (auto *GVar = dyn_cast<GlobalVariable>(GV))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like we can generate meaningful hash for globals regardless of the presence of name. Should we move the hasName check after this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved hasName check below.

GVHash = StructuralHash(*GVar);
if (!GVHash)
GVHash = stable_hash_name(GV->getName());

return stable_hash_combine(MO.getType(), MO.getTargetFlags(), GVHash,
MO.getOffset());
}

case MachineOperand::MO_TargetIndex: {
Expand Down
51 changes: 44 additions & 7 deletions llvm/lib/IR/StructuralHash.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ class StructuralHashImpl {
/// Assign a unique ID to each Value in the order they are first seen.
DenseMap<const Value *, int> ValueToId;

stable_hash hashType(Type *ValueType) {
static stable_hash hashType(Type *ValueType) {
SmallVector<stable_hash> Hashes;
Hashes.emplace_back(ValueType->getTypeID());
if (ValueType->isIntegerTy())
Expand All @@ -65,19 +65,44 @@ class StructuralHashImpl {
}
}

stable_hash hashAPInt(const APInt &I) {
static stable_hash hashAPInt(const APInt &I) {
SmallVector<stable_hash> Hashes;
Hashes.emplace_back(I.getBitWidth());
auto RawVals = ArrayRef<uint64_t>(I.getRawData(), I.getNumWords());
Hashes.append(RawVals.begin(), RawVals.end());
return stable_hash_combine(Hashes);
}

stable_hash hashAPFloat(const APFloat &F) {
static stable_hash hashAPFloat(const APFloat &F) {
return hashAPInt(F.bitcastToAPInt());
}

stable_hash hashGlobalValue(const GlobalValue *GV) {
static stable_hash hashGlobalVariable(const GlobalVariable &GVar) {
if (!GVar.hasInitializer())
return hashGlobalValue(&GVar);

// Hash the contents of a string.
if (GVar.getName().starts_with(".str"))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a weird way to detect if a GV is a string. Is there a better way? Could we check if the type if [* x i8] or something?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a string condition, using ConstantDataSequential.

return hashConstant(GVar.getInitializer());
Copy link
Contributor

@mingmingl-llvm mingmingl-llvm Dec 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.str is the prefix of almost all (if not all) string literals in the LLVM IR that I've seen. I'm wondering if there is a place to verify it's true (e.g., .str prefix means it's a string literal, and a string literal starts with .str).

Ask for my information since I'm making the same assumption for one recent prototype.

Copy link
Contributor Author

@kyulee-com kyulee-com Dec 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I only use hasInitializer() to determine if the global variable (beginning with .str) has an initializer. While I don't specifically verify if it's a string literal, this approach should suffice for most scenarios. It's still safe for our purposes, even if it's not a string literal. The only concern is avoiding an initializer that recursively references the current global variable itself. I assume that the cases filtered out here and below do not include such instances.


// Hash structural contents of Objective-C metadata in specific sections.
// This can be extended to other metadata if needed.
static constexpr const char *SectionNames[] = {
"__cfstring", "__cstring", "__objc_classrefs",
"__objc_methname", "__objc_selrefs",
};
if (GVar.hasSection()) {
StringRef SectionName = GVar.getSection();
for (const char *Name : SectionNames) {
if (SectionName.contains(Name))
return hashConstant(GVar.getInitializer());
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
for (const char *Name : SectionNames) {
if (SectionName.contains(Name))
return hashConstant(GVar.getInitializer());
}
for (const char *Name : SectionNames)
if (SectionName.contains(Name))
return hashConstant(GVar.getInitializer());

}

return hashGlobalValue(&GVar);
}

static stable_hash hashGlobalValue(const GlobalValue *GV) {
if (!GV->hasName())
return 0;
return stable_hash_name(GV->getName());
Expand All @@ -87,7 +112,7 @@ class StructuralHashImpl {
// FunctionComparator::cmpConstants() in FunctionComparator.cpp, but here
// we're interested in computing a hash rather than comparing two Constants.
// Some of the logic is simplified, e.g, we don't expand GEPOperator.
stable_hash hashConstant(Constant *C) {
static stable_hash hashConstant(const Constant *C) {
SmallVector<stable_hash> Hashes;

Type *Ty = C->getType();
Expand All @@ -98,14 +123,21 @@ class StructuralHashImpl {
return stable_hash_combine(Hashes);
}

if (auto *GVar = dyn_cast<GlobalVariable>(C)) {
Hashes.emplace_back(hashGlobalVariable(*GVar));
return stable_hash_combine(Hashes);
}

if (auto *G = dyn_cast<GlobalValue>(C)) {
Hashes.emplace_back(hashGlobalValue(G));
return stable_hash_combine(Hashes);
}

if (const auto *Seq = dyn_cast<ConstantDataSequential>(C)) {
Hashes.emplace_back(xxh3_64bits(Seq->getRawDataValues()));
return stable_hash_combine(Hashes);
if (Seq->isString()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about other ConstantDataSequentials which are not string?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you see below in the switch statement, ConstantArrayVal or ConstantVectorVal (that inherits ConstantDataSequentials) should cover the rest by handling each Constant element.

Hashes.emplace_back(stable_hash_name(Seq->getAsString()));
return stable_hash_combine(Hashes);
}
}

switch (C->getValueID()) {
Expand Down Expand Up @@ -266,6 +298,7 @@ class StructuralHashImpl {
Hashes.emplace_back(Hash);
Hashes.emplace_back(GlobalHeaderHash);
Hashes.emplace_back(GV.getValueType()->getTypeID());
Hashes.emplace_back(hashGlobalVariable(GV));

// Update the combined hash in place.
Hash = stable_hash_combine(Hashes);
Expand Down Expand Up @@ -297,6 +330,10 @@ stable_hash llvm::StructuralHash(const Function &F, bool DetailedHash) {
return H.getHash();
}

stable_hash llvm::StructuralHash(const GlobalVariable &GVar) {
return StructuralHashImpl::hashGlobalVariable(GVar);
}

stable_hash llvm::StructuralHash(const Module &M, bool DetailedHash) {
StructuralHashImpl H(DetailedHash);
H.update(M);
Expand Down
91 changes: 91 additions & 0 deletions llvm/test/CodeGen/AArch64/cgdata-merge-gvar.ll
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
; This test verifies that global variables are hashed based on their initial contents,
; allowing them to be merged even if they appear different due to their names.
; Now they become identical functions that can be merged without creating a parameter.

; RUN: rm -rf %t && split-file %s %t

; RUN: llc -mtriple=arm64-apple-darwin -enable-global-merge-func=true -global-merging-skip-no-params=false < %t/string.ll | FileCheck %s
; RUN: llc -mtriple=arm64-apple-darwin -enable-global-merge-func=true -global-merging-skip-no-params=false < %t/ns-const.ll | FileCheck %s
; RUN: llc -mtriple=arm64-apple-darwin -enable-global-merge-func=true -global-merging-skip-no-params=false < %t/objc-ref.ll | FileCheck %s
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These seem like three separate tests that don't need to share any data (except that their CHECK lines happen to be the same). Can we separate them into their own files?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Separated the test cases into their own files.


; CHECK: _f1.Tgm
; CHECK: _f2.Tgm

;--- string.ll

@.str = private unnamed_addr constant [6 x i8] c"hello\00", align 1
@.str.1 = private unnamed_addr constant [6 x i8] c"hello\00", align 1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add another global string with a different value to ensure it isn't merged?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a failing case.


declare noundef i32 @goo(ptr noundef)

define i32 @f1() {
entry:
%call = tail call noundef i32 @goo(ptr noundef nonnull @.str)
%add = add nsw i32 %call, 1
ret i32 %add
}

define i32 @f2() {
entry:
%call = tail call noundef i32 @goo(ptr noundef nonnull @.str.1)
%add = add nsw i32 %call, 1
ret i32 %add
}

;--- ns-const.ll

%struct.__NSConstantString_tag = type { ptr, i32, ptr, i64 }
@__CFConstantStringClassReference = external global [0 x i32]
@.str.2 = private unnamed_addr constant [9 x i8] c"cfstring\00", section "__TEXT,__cstring,cstring_literals", align 1
@_unnamed_cfstring_ = private global %struct.__NSConstantString_tag { ptr @__CFConstantStringClassReference, i32 1992, ptr @.str.2, i64 8 }, section "__DATA,__cfstring", align 8

@.str.3 = private unnamed_addr constant [9 x i8] c"cfstring\00", section "__TEXT,__cstring,cstring_literals", align 1
@_unnamed_cfstring_.2 = private global %struct.__NSConstantString_tag { ptr @__CFConstantStringClassReference, i32 1992, ptr @.str.3, i64 8 }, section "__DATA,__cfstring", align 8

declare noundef i32 @hoo(ptr noundef)

define i32 @f1() {
entry:
%call = tail call i32 @hoo(ptr noundef nonnull @_unnamed_cfstring_)
%add = sub nsw i32 %call, 1
ret i32 %add
}

define i32 @f2() {
entry:
%call = tail call i32 @hoo(ptr noundef nonnull @_unnamed_cfstring_.2)
%add = sub nsw i32 %call, 1
ret i32 %add
}

;--- objc-ref.ll

%struct._class_t = type { ptr, ptr, ptr, ptr, ptr }

@"OBJC_CLASS_$_MyClass" = external global %struct._class_t
@"OBJC_CLASSLIST_REFERENCES_$_" = internal global ptr @"OBJC_CLASS_$_MyClass", section "__DATA,__objc_classrefs,regular,no_dead_strip", align 8
@"OBJC_CLASSLIST_REFERENCES_$_.1" = internal global ptr @"OBJC_CLASS_$_MyClass", section "__DATA,__objc_classrefs,regular,no_dead_strip", align 8

@OBJC_METH_VAR_NAME_ = private unnamed_addr constant [6 x i8] c"hello\00", section "__TEXT,__objc_methname,cstring_literals", align 1
@OBJC_METH_VAR_NAME_.1 = private unnamed_addr constant [6 x i8] c"hello\00", section "__TEXT,__objc_methname,cstring_literals", align 1

@OBJC_SELECTOR_REFERENCES_ = internal externally_initialized global ptr @OBJC_METH_VAR_NAME_, section "__DATA,__objc_selrefs,literal_pointers,no_dead_strip", align 8
@OBJC_SELECTOR_REFERENCES_.1 = internal externally_initialized global ptr @OBJC_METH_VAR_NAME_.1, section "__DATA,__objc_selrefs,literal_pointers,no_dead_strip", align 8

define i32 @f1() {
entry:
%0 = load ptr, ptr @"OBJC_CLASSLIST_REFERENCES_$_", align 8
%1 = load ptr, ptr @OBJC_SELECTOR_REFERENCES_, align 8
%call = tail call noundef i32 @objc_msgSend(ptr noundef %0, ptr noundef %1)
ret i32 %call
}

declare ptr @objc_msgSend(ptr, ptr, ...)

define i32 @f2() {
entry:
%0 = load ptr, ptr @"OBJC_CLASSLIST_REFERENCES_$_.1", align 8
%1 = load ptr, ptr @OBJC_SELECTOR_REFERENCES_.1, align 8
%call = tail call noundef i32 @objc_msgSend(ptr noundef %0, ptr noundef %1)
ret i32 %call
}
52 changes: 52 additions & 0 deletions llvm/test/CodeGen/AArch64/cgdata-outline-gvar.ll
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
; This test verifies that global variables are hashed based on their initial contents,
; allowing them to be outlined even if they appear different due to their names.

; RUN: split-file %s %t

; Check if the outlined function is created locally.
; RUN: llc -mtriple=arm64-apple-darwin -enable-machine-outliner -codegen-data-generate=true -aarch64-enable-collect-loh=false -filetype=obj %t/local-two.ll -o %t_write_base
; RUN: llvm-objdump -d %t_write_base | FileCheck %s

; RUN: llvm-cgdata --merge %t_write_base -o %t_cgdata_base

; Read the cgdata in the machine outliner for optimistically outlining in local-one.ll.
; RUN: llc -mtriple=arm64-apple-darwin -enable-machine-outliner -codegen-data-use-path=%t_cgdata_base -aarch64-enable-collect-loh=false -append-content-hash-outlined-name=false -filetype=obj %t/local-one.ll -o %t_read_base
; RUN: llvm-objdump -d %t_read_base | FileCheck %s

; The names of globals `.str` and `.str.4` are different, but their initial contents are identical.
; The outlined function now starts with a reference to that global ("hello\00").
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit confused. Where is the reference to the "hello\00" string in this outlined function?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Restructured the test cases.

; CHECK: _OUTLINED_FUNCTION_{{.*}}:
; CHECK-NEXT: adrp x1
; CHECK-NEXT: add x1, x1
; CHECK-NEXT: mov w2
; CHECK-NEXT: mov w3
; CHECK-NEXT: mov w4
; CHECK-NEXT: b

;--- local-two.ll
@.str = private unnamed_addr constant [6 x i8] c"hello\00", align 1
@.str.1 = private unnamed_addr constant [3 x i8] c"f1\00", align 1
@.str.2 = private unnamed_addr constant [3 x i8] c"f2\00", align 1

declare noundef i32 @goo(ptr noundef, ptr noundef, i32, i32, i32)
define i32 @f1() minsize {
entry:
%call = tail call noundef i32 @goo(ptr noundef nonnull @.str.1, ptr noundef nonnull @.str, i32 1, i32 2, i32 3)
ret i32 %call
}
define i32 @f2() minsize {
entry:
%call = tail call noundef i32 @goo(ptr noundef nonnull @.str.2, ptr noundef nonnull @.str, i32 1, i32 2, i32 3)
ret i32 %call
}

;--- local-one.ll
@.str.3 = private unnamed_addr constant [3 x i8] c"f3\00", align 1
@.str.4 = private unnamed_addr constant [6 x i8] c"hello\00", align 1

declare noundef i32 @goo(ptr noundef, ptr noundef, i32, i32, i32)
define i32 @f1() minsize {
entry:
%call = tail call noundef i32 @goo(ptr noundef nonnull @.str.3, ptr noundef nonnull @.str.4, i32 1, i32 2, i32 3)
ret i32 %call
}
Loading