[llvm] 5fa6b24 - Address feedback in https://reviews.llvm.org/D133637

[Hongtao Yu via llvm-commits]

Author: YongKang Zhu
Date: 2022-09-13T16:12:41-07:00
New Revision: 5fa6b2435477c8d44fc827cda2f998591b1cf837

URL: https://github.com/llvm/llvm-project/commit/5fa6b2435477c8d44fc827cda2f998591b1cf837
DIFF: https://github.com/llvm/llvm-project/commit/5fa6b2435477c8d44fc827cda2f998591b1cf837.diff

LOG: 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

Added: 
    

Modified: 
    llvm/include/llvm/CodeGen/MachineOperand.h
    llvm/lib/CodeGen/MachineOperand.cpp
    llvm/lib/CodeGen/MachineStableHash.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/CodeGen/MachineOperand.h b/llvm/include/llvm/CodeGen/MachineOperand.h
index 9feab9e0ca067..c88e72cdc1d9c 100644
--- a/llvm/include/llvm/CodeGen/MachineOperand.h
+++ b/llvm/include/llvm/CodeGen/MachineOperand.h
@@ -641,10 +641,6 @@ class MachineOperand {
     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 7a85eaf95fd51..393a4f71b3cb2 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<MachineFunction *>(
       getMFIfAvailable(const_cast<const MachineOperand &>(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<stable_hash> 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 30c5404750e10..941aeaf15f320 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<llvm::stable_hash> 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<llvm::stable_hash> 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());
   }