[PATCH] D71396: [llvm][NFCi][lMIRVRegNamerUtils] Leverage MachineInstrExpressionTrait for hashing a MachineInstr.

[Puyan Lotfi via Phabricator via llvm-commits]

plotfi updated this revision to Diff 233656.
Herald added subscribers: nhaehnle, jvesely.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71396/new/

https://reviews.llvm.org/D71396

Files:
  llvm/lib/CodeGen/MIRVRegNamerUtils.cpp
  llvm/test/CodeGen/MIR/AArch64/mirnamer.mir
  llvm/test/CodeGen/MIR/AMDGPU/mir-canon-multi.mir


Index: llvm/test/CodeGen/MIR/AMDGPU/mir-canon-multi.mir
===================================================================
--- llvm/test/CodeGen/MIR/AMDGPU/mir-canon-multi.mir
+++ llvm/test/CodeGen/MIR/AMDGPU/mir-canon-multi.mir
@@ -15,9 +15,9 @@
     ; CHECK: %bb0_{{[0-9]+}}__1:sreg_64_xexec = S_LOAD_DWORDX2_IMM %bb0_{{[0-9]+}}__1, 11, 0, 0
     ; CHECK: %bb0_{{[0-9]+}}__1:vgpr_32 = COPY %bb0_{{[0-9]+}}__1
     ; CHECK: %bb0_{{[0-9]+}}__1:vgpr_32 = COPY %bb0_{{[0-9]+}}__1
-    ; CHECK: %bb0_{{[0-9]+}}__2:vgpr_32 = COPY %bb0_{{[0-9]+}}__1
+    ; CHECK: %bb0_{{[0-9]+}}__1:vgpr_32 = COPY %bb0_{{[0-9]+}}__1
     ; CHECK: %bb0_{{[0-9]+}}__1:vreg_64 = REG_SEQUENCE %bb0_{{[0-9]+}}__1, %subreg.sub0, %bb0_{{[0-9]+}}__1, %subreg.sub1
-    ; CHECK: %bb0_{{[0-9]+}}__1:sgpr_128 = REG_SEQUENCE %bb0_{{[0-9]+}}__1, %subreg.sub0, %bb0_{{[0-9]+}}__1, %subreg.sub1, %bb0_{{[0-9]+}}__1, %subreg.sub2, %bb0_{{[0-9]+}}__2, %subreg.sub3
+    ; CHECK: %bb0_{{[0-9]+}}__1:sgpr_128 = REG_SEQUENCE %bb0_{{[0-9]+}}__1, %subreg.sub0, %bb0_{{[0-9]+}}__1, %subreg.sub1, %bb0_{{[0-9]+}}__1, %subreg.sub2, %bb0_{{[0-9]+}}__1, %subreg.sub3
     ; CHECK: BUFFER_STORE_DWORD_ADDR64 %bb0_{{[0-9]+}}__1, %bb0_{{[0-9]+}}__1, %bb0_{{[0-9]+}}__1, 0, 0, 0, 0, 0, 0, 0, implicit $exec
     ; CHECK: S_ENDPGM 0
     %10:sreg_32_xm0 = S_MOV_B32 61440
Index: llvm/test/CodeGen/MIR/AArch64/mirnamer.mir
===================================================================
--- llvm/test/CodeGen/MIR/AArch64/mirnamer.mir
+++ llvm/test/CodeGen/MIR/AArch64/mirnamer.mir
@@ -34,9 +34,9 @@
     ;CHECK-NEXT: %bb0_{{[0-9]+}}__1:gpr32 = MOVi32imm 3
     ;CHECK-NEXT: %bb0_{{[0-9]+}}__1:gpr32 = nsw ADDWrr
     ;CHECK-NEXT: %bb0_{{[0-9]+}}__4:gpr32 = LDRWui
-    ;CHECK-NEXT: %bb0_{{[0-9]+}}__2:gpr32 = nsw ADDWrr
+    ;CHECK-NEXT: %bb0_{{[0-9]+}}__1:gpr32 = nsw ADDWrr
     ;CHECK-NEXT: %bb0_{{[0-9]+}}__1:gpr32 = MOVi32imm 4
-    ;CHECK-NEXT: %bb0_{{[0-9]+}}__3:gpr32 = nsw ADDWrr
+    ;CHECK-NEXT: %bb0_{{[0-9]+}}__1:gpr32 = nsw ADDWrr
     ;CHECK-NEXT: %bb0_{{[0-9]+}}__5:gpr32 = LDRWui
     ;CHECK-NEXT: %bb0_{{[0-9]+}}__1:gpr32 = MOVi32imm 5
 
@@ -77,8 +77,8 @@
     ;CHECK-NEXT: %bb0_{{[0-9]+}}__1:gpr32 = LDRWui %stack.0, 0
     ;CHECK-NEXT: %bb0_{{[0-9]+}}__1:gpr32 = COPY %bb0_{{[0-9]+}}__1
     ;CHECK-NEXT: %bb0_{{[0-9]+}}__1:gpr32 = COPY %bb0_{{[0-9]+}}__1
-    ;CHECK-NEXT: %bb0_{{[0-9]+}}__2:gpr32 = COPY %bb0_{{[0-9]+}}__1
-    ;CHECK-NEXT: $w0 = COPY %bb0_{{[0-9]+}}__2
+    ;CHECK-NEXT: %bb0_{{[0-9]+}}__1:gpr32 = COPY %bb0_{{[0-9]+}}__1
+    ;CHECK-NEXT: $w0 = COPY %bb0_{{[0-9]+}}__1
 
     %0:gpr32 = LDRWui %stack.0, 0 :: (dereferenceable load 8)
     %1:gpr32 = COPY %0
Index: llvm/lib/CodeGen/MIRVRegNamerUtils.cpp
===================================================================
--- llvm/lib/CodeGen/MIRVRegNamerUtils.cpp
+++ llvm/lib/CodeGen/MIRVRegNamerUtils.cpp
@@ -50,27 +50,16 @@
   std::string S;
   raw_string_ostream OS(S);
 
-  // Gets a hashable artifact from a given MachineOperand (ie an unsigned).
-  auto GetHashableMO = [this](const MachineOperand &MO) -> unsigned {
-    if (MO.isImm())
-      return MO.getImm();
-    if (MO.isTargetIndex())
-      return MO.getOffset() | (MO.getTargetFlags() << 16);
-    if (MO.isReg() && Register::isVirtualRegister(MO.getReg()))
-      return MRI.getVRegDef(MO.getReg())->getOpcode();
-    if (MO.isReg())
-      return MO.getReg();
-    // TODO:
-    // We could explicitly handle all the types of the MachineOperand,
-    // here but we can just return a common number until we find a
-    // compelling test case where this is bad. The only side effect here
-    // is contributing to a hash collision but there's enough information
-    // (Opcodes,other registers etc) that this will likely not be a problem.
-    return 0;
-  };
-
-  SmallVector<unsigned, 16> MIOperands = {MI.getOpcode(), MI.getFlags()};
-  llvm::transform(MI.uses(), std::back_inserter(MIOperands), GetHashableMO);
+  // Here we walk over all of the flags and operands (non vreg-defs) of the MI
+  // to collect artifacts for hashing. Where we used to walk over all of the
+  // MachoneOperand uses of an MI we now instead use MachineInstrExpressionTrait
+  // since that covers more types of operands and is less likely to result in
+  // collisions. MachineInstrExpressionTrait is what is used to produce hashes
+  // by MachineCSE. So in effect we are hashing on the MI flags, the MI
+  // memoprands, and the hash of all of the other non-vreg def operands of the
+  // MI (the latter through MachineInstrExpressionTrait).
+  SmallVector<unsigned, 16> MIOperands = {MI.getFlags()};
+  MIOperands.push_back(MachineInstrExpressionTrait::getHashValue(&MI));
 
   for (const auto *Op : MI.memoperands()) {
     MIOperands.push_back((unsigned)Op->getSize());


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D71396.233656.patch
Type: text/x-patch
Size: 4772 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20191212/aafa5cb1/attachment.bin>