From 5fa6b2435477c8d44fc827cda2f998591b1cf837 Mon Sep 17 00:00:00 2001 From: YongKang Zhu Date: Tue, 13 Sep 2022 16:08:17 -0700 Subject: [PATCH] Address feedback in https://reviews.llvm.org/D133637 https://reviews.llvm.org/D133637 fixes the problem where we should hash raw content of register mask instead of the pointer to it. Fix the same issue in `llvm::hash_value()`. Remove the added API `MachineOperand::getRegMaskSize()` to avoid potential confusion. Add an assert to emphasize that we probably should hash a machine operand iff it has associated machine function, but keep the fallback logic in the original change. Reviewed By: MatzeB Differential Revision: https://reviews.llvm.org/D133747 --- llvm/include/llvm/CodeGen/MachineOperand.h | 4 ---- llvm/lib/CodeGen/MachineOperand.cpp | 34 +++++++++++++++++------------- llvm/lib/CodeGen/MachineStableHash.cpp | 24 +++++++++++++-------- 3 files changed, 34 insertions(+), 28 deletions(-) diff --git a/llvm/include/llvm/CodeGen/MachineOperand.h b/llvm/include/llvm/CodeGen/MachineOperand.h index 9feab9e..c88e72c 100644 --- a/llvm/include/llvm/CodeGen/MachineOperand.h +++ b/llvm/include/llvm/CodeGen/MachineOperand.h @@ -641,10 +641,6 @@ public: return Contents.RegMask; } - /// Return the size of regmask array if we are able to figure it out from - /// this operand. Return zero otherwise. - unsigned getRegMaskSize() const; - /// Returns number of elements needed for a regmask array. static unsigned getRegMaskSize(unsigned NumRegs) { return (NumRegs + 31) / 32; diff --git a/llvm/lib/CodeGen/MachineOperand.cpp b/llvm/lib/CodeGen/MachineOperand.cpp index 7a85eaf..393a4f7 100644 --- a/llvm/lib/CodeGen/MachineOperand.cpp +++ b/llvm/lib/CodeGen/MachineOperand.cpp @@ -18,6 +18,7 @@ #include "llvm/CodeGen/MachineFrameInfo.h" #include "llvm/CodeGen/MachineJumpTableInfo.h" #include "llvm/CodeGen/MachineRegisterInfo.h" +#include "llvm/CodeGen/StableHashing.h" #include "llvm/CodeGen/TargetInstrInfo.h" #include "llvm/CodeGen/TargetRegisterInfo.h" #include "llvm/Config/llvm-config.h" @@ -45,6 +46,7 @@ static const MachineFunction *getMFIfAvailable(const MachineOperand &MO) { return MF; return nullptr; } + static MachineFunction *getMFIfAvailable(MachineOperand &MO) { return const_cast( getMFIfAvailable(const_cast(MO))); @@ -279,17 +281,6 @@ void MachineOperand::ChangeToRegister(Register Reg, bool isDef, bool isImp, RegInfo->addRegOperandToUseList(this); } -/// getRegMaskSize - Return the size of regmask array if we are able to figure -/// it out from this operand. Return zero otherwise. -unsigned MachineOperand::getRegMaskSize() const { - if (const MachineFunction *MF = getMFIfAvailable(*this)) { - const TargetRegisterInfo *TRI = MF->getSubtarget().getRegisterInfo(); - unsigned RegMaskSize = (TRI->getNumRegs() + 31) / 32; - return RegMaskSize; - } - return 0; -} - /// isIdenticalTo - Return true if this operand is identical to the specified /// operand. Note that this should stay in sync with the hash_value overload /// below. @@ -333,8 +324,9 @@ bool MachineOperand::isIdenticalTo(const MachineOperand &Other) const { if (RegMask == OtherRegMask) return true; - const unsigned RegMaskSize = getRegMaskSize(); - if (RegMaskSize != 0) { + if (const MachineFunction *MF = getMFIfAvailable(*this)) { + const TargetRegisterInfo *TRI = MF->getSubtarget().getRegisterInfo(); + unsigned RegMaskSize = MachineOperand::getRegMaskSize(TRI->getNumRegs()); // Deep compare of the two RegMasks return std::equal(RegMask, RegMask + RegMaskSize, OtherRegMask); } @@ -390,8 +382,20 @@ hash_code llvm::hash_value(const MachineOperand &MO) { return hash_combine(MO.getType(), MO.getTargetFlags(), MO.getBlockAddress(), MO.getOffset()); case MachineOperand::MO_RegisterMask: - case MachineOperand::MO_RegisterLiveOut: - return hash_combine(MO.getType(), MO.getTargetFlags(), MO.getRegMask()); + case MachineOperand::MO_RegisterLiveOut: { + if (const MachineFunction *MF = getMFIfAvailable(MO)) { + const TargetRegisterInfo *TRI = MF->getSubtarget().getRegisterInfo(); + unsigned RegMaskSize = MachineOperand::getRegMaskSize(TRI->getNumRegs()); + const uint32_t *RegMask = MO.getRegMask(); + std::vector RegMaskHashes(RegMask, RegMask + RegMaskSize); + return hash_combine(MO.getType(), MO.getTargetFlags(), + stable_hash_combine_array(RegMaskHashes.data(), + RegMaskHashes.size())); + } + + assert(0 && "MachineOperand not associated with any MachineFunction"); + return hash_combine(MO.getType(), MO.getTargetFlags()); + } case MachineOperand::MO_Metadata: return hash_combine(MO.getType(), MO.getTargetFlags(), MO.getMetadata()); case MachineOperand::MO_MCSymbol: diff --git a/llvm/lib/CodeGen/MachineStableHash.cpp b/llvm/lib/CodeGen/MachineStableHash.cpp index 30c5404..941aeaf 100644 --- a/llvm/lib/CodeGen/MachineStableHash.cpp +++ b/llvm/lib/CodeGen/MachineStableHash.cpp @@ -120,17 +120,23 @@ stable_hash llvm::stableHashValue(const MachineOperand &MO) { case MachineOperand::MO_RegisterMask: case MachineOperand::MO_RegisterLiveOut: { - const uint32_t *RegMask = MO.getRegMask(); - const unsigned RegMaskSize = MO.getRegMaskSize(); - - if (RegMaskSize != 0) { - std::vector RegMaskHashes(RegMask, - RegMask + RegMaskSize); - return hash_combine(MO.getType(), MO.getTargetFlags(), - stable_hash_combine_array(RegMaskHashes.data(), - RegMaskHashes.size())); + if (const MachineInstr *MI = MO.getParent()) { + if (const MachineBasicBlock *MBB = MI->getParent()) { + if (const MachineFunction *MF = MBB->getParent()) { + const TargetRegisterInfo *TRI = MF->getSubtarget().getRegisterInfo(); + unsigned RegMaskSize = + MachineOperand::getRegMaskSize(TRI->getNumRegs()); + const uint32_t *RegMask = MO.getRegMask(); + std::vector RegMaskHashes(RegMask, + RegMask + RegMaskSize); + return hash_combine(MO.getType(), MO.getTargetFlags(), + stable_hash_combine_array(RegMaskHashes.data(), + RegMaskHashes.size())); + } + } } + assert(0 && "MachineOperand not associated with any MachineFunction"); return hash_combine(MO.getType(), MO.getTargetFlags()); } -- 2.7.4