-
Notifications
You must be signed in to change notification settings - Fork 14.4k
Adding Matching and Inference Functionality to Propeller-Patch 1 #140886
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,108 @@ | ||||||||
#ifndef LLVM_CODEGEN_MACHINEBLOCKHASHINFO_H | ||||||||
#define LLVM_CODEGEN_MACHINEBLOCKHASHINFO_H | ||||||||
|
||||||||
#include "llvm/CodeGen/MachineFunctionPass.h" | ||||||||
|
||||||||
namespace llvm { | ||||||||
|
||||||||
/// An object wrapping several components of a basic block hash. The combined | ||||||||
/// (blended) hash is represented and stored as one uint64_t, while individual | ||||||||
/// components are of smaller size (e.g., uint16_t or uint8_t). | ||||||||
struct BlendedBlockHash { | ||||||||
private: | ||||||||
static uint64_t combineHashes(uint16_t Hash1, uint16_t Hash2, uint16_t Hash3, | ||||||||
uint16_t Hash4) { | ||||||||
uint64_t Hash = 0; | ||||||||
|
||||||||
Hash |= uint64_t(Hash4); | ||||||||
Hash <<= 16; | ||||||||
|
||||||||
Hash |= uint64_t(Hash3); | ||||||||
Hash <<= 16; | ||||||||
|
||||||||
Hash |= uint64_t(Hash2); | ||||||||
Hash <<= 16; | ||||||||
|
||||||||
Hash |= uint64_t(Hash1); | ||||||||
|
||||||||
return Hash; | ||||||||
} | ||||||||
|
||||||||
static void parseHashes(uint64_t Hash, uint16_t &Hash1, uint16_t &Hash2, | ||||||||
uint16_t &Hash3, uint16_t &Hash4) { | ||||||||
Hash1 = Hash & 0xffff; | ||||||||
Hash >>= 16; | ||||||||
|
||||||||
Hash2 = Hash & 0xffff; | ||||||||
Hash >>= 16; | ||||||||
|
||||||||
Hash3 = Hash & 0xffff; | ||||||||
Hash >>= 16; | ||||||||
|
||||||||
Hash4 = Hash & 0xffff; | ||||||||
Hash >>= 16; | ||||||||
} | ||||||||
|
||||||||
public: | ||||||||
explicit BlendedBlockHash() {} | ||||||||
|
||||||||
explicit BlendedBlockHash(uint64_t CombinedHash) { | ||||||||
parseHashes(CombinedHash, Offset, OpcodeHash, InstrHash, NeighborHash); | ||||||||
} | ||||||||
|
||||||||
/// Combine the blended hash into uint64_t. | ||||||||
uint64_t combine() const { | ||||||||
return combineHashes(Offset, OpcodeHash, InstrHash, NeighborHash); | ||||||||
} | ||||||||
|
||||||||
/// Compute a distance between two given blended hashes. The smaller the | ||||||||
/// distance, the more similar two blocks are. For identical basic blocks, | ||||||||
/// the distance is zero. | ||||||||
uint64_t distance(const BlendedBlockHash &BBH) const { | ||||||||
assert(OpcodeHash == BBH.OpcodeHash && | ||||||||
"incorrect blended hash distance computation"); | ||||||||
uint64_t Dist = 0; | ||||||||
// Account for NeighborHash | ||||||||
Dist += NeighborHash == BBH.NeighborHash ? 0 : 1; | ||||||||
Dist <<= 16; | ||||||||
// Account for InstrHash | ||||||||
Dist += InstrHash == BBH.InstrHash ? 0 : 1; | ||||||||
Dist <<= 16; | ||||||||
// Account for Offset | ||||||||
Dist += (Offset >= BBH.Offset ? Offset - BBH.Offset : BBH.Offset - Offset); | ||||||||
return Dist; | ||||||||
} | ||||||||
|
||||||||
/// The offset of the basic block from the function start. | ||||||||
uint16_t Offset{0}; | ||||||||
/// (Loose) Hash of the basic block instructions, excluding operands. | ||||||||
uint16_t OpcodeHash{0}; | ||||||||
/// (Strong) Hash of the basic block instructions, including opcodes and | ||||||||
/// operands. | ||||||||
uint16_t InstrHash{0}; | ||||||||
/// Hash of the (loose) basic block together with (loose) hashes of its | ||||||||
/// successors and predecessors. | ||||||||
uint16_t NeighborHash{0}; | ||||||||
}; | ||||||||
|
||||||||
class MachineBlockHashInfo : public MachineFunctionPass { | ||||||||
DenseMap<unsigned, uint64_t> MBBHashInfo; | ||||||||
|
||||||||
public: | ||||||||
static char ID; | ||||||||
MachineBlockHashInfo(); | ||||||||
|
||||||||
StringRef getPassName() const override { | ||||||||
return "Basic Block Hash Compute"; | ||||||||
} | ||||||||
|
||||||||
void getAnalysisUsage(AnalysisUsage &AU) const override; | ||||||||
|
||||||||
bool runOnMachineFunction(MachineFunction &F) override; | ||||||||
|
||||||||
uint64_t getMBBHash(const MachineBasicBlock &MBB); | ||||||||
}; | ||||||||
|
||||||||
} // end namespace llvm | ||||||||
|
||||||||
#endif // LLVM_CODEGEN_MACHINEBLOCKHASHINFO_H | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. DONE |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,104 @@ | ||
#include "llvm/CodeGen/MachineBlockHashInfo.h" | ||
#include "llvm/CodeGen/Passes.h" | ||
#include "llvm/Target/TargetMachine.h" | ||
#include "llvm/InitializePasses.h" | ||
|
||
using namespace llvm; | ||
|
||
using OperandHashFuncTy = function_ref<uint64_t(uint64_t &, const MachineOperand &)>; | ||
|
||
uint64_t hashBlock(const MachineBasicBlock &MBB, OperandHashFuncTy OperandHashFunc) { | ||
uint64_t Hash = 0; | ||
for (const MachineInstr &MI : MBB) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you also hash the terminator instructions (branches at the end of the block)? They may be reversed across builds. |
||
if (MI.isPseudo()) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You presumably want isMetaInstruction. isPseudo only tells you if it's an instruction with a final machine encoding or not There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks. Here, we want to hash all instructions that are actually executed by the machine, so I think using isPseudo is fine. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's not fine, isPseudo is essentially a backend implementation detail flag that very little code should ever need to consider. It will most likely eventually expand to some real machine sequence There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For stale profile, the ordering of basic blocks is prone to changes. In the Matching and Inference technique, hashes of basic blocks are used to accurately match basic blocks even when their ordering changes. We disregard pseudo-instructions, nop instructions, and unconditional jumps, as these are frequently added or removed due to basic block reordering. For more detailed explanations, refer to Section 3.1 of the Matching and Inference paper (https://arxiv.org/abs/2401.17168). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. isPseudo does not mean what you think it means, and I'm sure you want isMetaInstruction There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you very much for pointing out the issue. We carefully reviewed the isPseudo method of the MachineInstr class and realized we had previously misunderstood its meaning. We have made corrections to the code accordingly. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
continue; | ||
// Ignore unconditional jumps | ||
if (MI.isUnconditionalBranch()) | ||
continue; | ||
Hash = hashing::detail::hash_16_bytes(Hash, MI.getOpcode()); | ||
for (unsigned i = 0; i < MI.getNumOperands(); i++) { | ||
Hash = OperandHashFunc(Hash, MI.getOperand(i)); | ||
} | ||
} | ||
return Hash; | ||
} | ||
|
||
/// Hashing a 64-bit integer to a 16-bit one. | ||
uint16_t hash_64_to_16(const uint64_t Hash) { | ||
uint16_t Res = (uint16_t)(Hash & 0xFFFF); | ||
Res ^= (uint16_t)((Hash >> 16) & 0xFFFF); | ||
Res ^= (uint16_t)((Hash >> 32) & 0xFFFF); | ||
Res ^= (uint16_t)((Hash >> 48) & 0xFFFF); | ||
return Res; | ||
} | ||
|
||
uint64_t hashInstOperand(uint64_t &Hash, const MachineOperand &Operand) { | ||
return hashing::detail::hash_16_bytes(Hash, hash_value(Operand)); | ||
} | ||
|
||
INITIALIZE_PASS(MachineBlockHashInfo, "machine-block-hash", | ||
"Machine Block Hash Analysis", true, true) | ||
|
||
char MachineBlockHashInfo::ID = 0; | ||
|
||
MachineBlockHashInfo::MachineBlockHashInfo() : MachineFunctionPass(ID) { | ||
initializeMachineBlockHashInfoPass(*PassRegistry::getPassRegistry()); | ||
} | ||
|
||
void MachineBlockHashInfo::getAnalysisUsage(AnalysisUsage &AU) const { | ||
AU.setPreservesAll(); | ||
MachineFunctionPass::getAnalysisUsage(AU); | ||
} | ||
|
||
bool MachineBlockHashInfo::runOnMachineFunction(MachineFunction &F) { | ||
DenseMap<MachineBasicBlock *, BlendedBlockHash> BlendedHashes; | ||
DenseMap<MachineBasicBlock *, uint64_t> OpcodeHashes; | ||
uint16_t Offset = 0; | ||
// Initialize hash components | ||
for (MachineBasicBlock &MBB : F) { | ||
BlendedBlockHash BlendedHash; | ||
// offset of the machine basic block | ||
BlendedHash.Offset = Offset; | ||
Offset += MBB.size(); | ||
// Hashing opcodes | ||
uint64_t OpcodeHash = hashBlock(MBB, [](uint64_t &Hash, const MachineOperand &Op) { return Hash; }); | ||
OpcodeHashes[&MBB] = OpcodeHash; | ||
BlendedHash.OpcodeHash = hash_64_to_16(OpcodeHash); | ||
// Hash complete instructions | ||
uint64_t InstrHash = hashBlock(MBB, hashInstOperand); | ||
BlendedHash.InstrHash = hash_64_to_16(InstrHash); | ||
BlendedHashes[&MBB] = BlendedHash; | ||
} | ||
|
||
// Initialize neighbor hash | ||
for (MachineBasicBlock &MBB : F) { | ||
uint64_t Hash = OpcodeHashes[&MBB]; | ||
// Append hashes of successors | ||
for (MachineBasicBlock *SuccMBB : MBB.successors()) { | ||
uint64_t SuccHash = OpcodeHashes[SuccMBB]; | ||
Hash = hashing::detail::hash_16_bytes(Hash, SuccHash); | ||
} | ||
// Append hashes of predecessors | ||
for (MachineBasicBlock *PredMBB : MBB.predecessors()) { | ||
uint64_t PredHash = OpcodeHashes[PredMBB]; | ||
Hash = hashing::detail::hash_16_bytes(Hash, PredHash); | ||
} | ||
BlendedHashes[&MBB].NeighborHash = hash_64_to_16(Hash); | ||
} | ||
|
||
// Assign hashes | ||
for (MachineBasicBlock &MBB : F) { | ||
if (MBB.getBBID()) { | ||
MBBHashInfo[MBB.getBBID()->BaseID] = BlendedHashes[&MBB].combine(); | ||
} | ||
} | ||
|
||
return false; | ||
} | ||
|
||
uint64_t MachineBlockHashInfo::getMBBHash(const MachineBasicBlock &MBB) { | ||
if (MBB.getBBID()) { | ||
return MBBHashInfo[MBB.getBBID()->BaseID]; | ||
} | ||
return 0; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -873,6 +873,7 @@ decodeBBAddrMapImpl(const ELFFile<ELFT> &EF, | |
uint32_t Offset = readULEB128As<uint32_t>(Data, Cur, ULEBSizeErr); | ||
uint32_t Size = readULEB128As<uint32_t>(Data, Cur, ULEBSizeErr); | ||
uint32_t MD = readULEB128As<uint32_t>(Data, Cur, ULEBSizeErr); | ||
uint64_t Hash = readULEB128As<uint64_t>(Data, Cur, ULEBSizeErr); | ||
if (Version >= 1) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You need to define a new version number for this to support backward-compatibility. |
||
// Offset is calculated relative to the end of the previous BB. | ||
Offset += PrevBBEndOffset; | ||
|
@@ -884,7 +885,7 @@ decodeBBAddrMapImpl(const ELFFile<ELFT> &EF, | |
MetadataDecodeErr = MetadataOrErr.takeError(); | ||
break; | ||
} | ||
BBEntries.push_back({ID, Offset, Size, *MetadataOrErr}); | ||
BBEntries.push_back({ID, Offset, Size, *MetadataOrErr, Hash}); | ||
} | ||
TotalNumBlocks += BBEntries.size(); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A lot of the code in this file repeats the implementation in BOLT (which has actually been improved since the original commits), e.g., https://github.com/llvm/llvm-project/blob/main/bolt/lib/Profile/StaleProfileMatching.cpp
Do you think there is a way to unify the implementations? (totally understand if not, since there might be some specifics)