[CodeGen] Fix hashing for MO_ExternalSymbol MachineOperands.
authorEli Friedman <efriedma@quicinc.com>
Sat, 1 Jun 2019 00:08:54 +0000 (00:08 +0000)
committerEli Friedman <efriedma@quicinc.com>
Sat, 1 Jun 2019 00:08:54 +0000 (00:08 +0000)
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.

Differential Revision: https://reviews.llvm.org/D61975

llvm-svn: 362281

llvm/lib/CodeGen/MachineOperand.cpp
llvm/unittests/CodeGen/MachineOperandTest.cpp

index a83459e6917093a1ce73a8e4a8c7521622973af9..9458745733f644864a271c915df3139447be58ea 100644 (file)
@@ -361,7 +361,7 @@ hash_code llvm::hash_value(const MachineOperand &MO) {
     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());
index cfedfa9c8835b4494033b8cb48c0ba62493f4b4a..faa471f2260c7280f50cc5a13adc4d0fb164dde3 100644 (file)
@@ -398,4 +398,14 @@ TEST(MachineOperandTest, PrintPredicate) {
   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_NE(SymName1, SymName2);
+  ASSERT_EQ(hash_value(MO1), hash_value(MO2));
+  ASSERT_TRUE(MO1.isIdenticalTo(MO2));
+}
+
 } // end namespace