[PATCH] D61975: [CodeGen] Fix hashing for MO_ExternalSymbol MachineOperands.

[Eli Friedman via Phabricator via llvm-commits]

efriedma created this revision.
efriedma added reviewers: chandlerc, craig.topper.
Herald added subscribers: dexonsmith, mehdi_amini.
Herald added a project: LLVM.

We were hashing the string pointer, not the string, so two instructions could be identical (isIdenticalTo), but have different hash codes.

This showed up as a very rare, non-deterministic assertion failure rehashing a DenseMap constructed by MachineOutliner.  So there's no "real" testcase, just a unittest which checks that the hash function behaves correctly.

I'm a little scared fixing this is going to cause a regression in outlining or MachineCSE, but hopefully we won't run into any issues.


Repository:
  rL LLVM

https://reviews.llvm.org/D61975

Files:
  lib/CodeGen/MachineOperand.cpp
  unittests/CodeGen/MachineOperandTest.cpp


Index: unittests/CodeGen/MachineOperandTest.cpp
===================================================================
--- unittests/CodeGen/MachineOperandTest.cpp
+++ unittests/CodeGen/MachineOperandTest.cpp
@@ -398,4 +398,13 @@
   ASSERT_TRUE(OS.str() == "intpred(eq)");
 }
 
+TEST(MachineOperandTest, HashValue) {
+  char SymName1[] = "test";
+  char SymName2[] = "test";
+  MachineOperand MO1 = MachineOperand::CreateES(SymName1);
+  MachineOperand MO2 = MachineOperand::CreateES(SymName2);
+  ASSERT_EQ(hash_value(MO1), hash_value(MO2));
+  ASSERT_TRUE(MO1.isIdenticalTo(MO2));
+}
+
 } // end namespace
Index: lib/CodeGen/MachineOperand.cpp
===================================================================
--- lib/CodeGen/MachineOperand.cpp
+++ lib/CodeGen/MachineOperand.cpp
@@ -348,7 +348,7 @@
     return hash_combine(MO.getType(), MO.getTargetFlags(), MO.getIndex());
   case MachineOperand::MO_ExternalSymbol:
     return hash_combine(MO.getType(), MO.getTargetFlags(), MO.getOffset(),
-                        MO.getSymbolName());
+                        StringRef(MO.getSymbolName()));
   case MachineOperand::MO_GlobalAddress:
     return hash_combine(MO.getType(), MO.getTargetFlags(), MO.getGlobal(),
                         MO.getOffset());


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D61975.199714.patch
Type: text/x-patch
Size: 1259 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190516/901c575c/attachment.bin>