From d8e8722791e4ce6694673d450fdcaf99e5edcbf9 Mon Sep 17 00:00:00 2001 From: Eli Friedman Date: Sat, 1 Jun 2019 00:08:54 +0000 Subject: [PATCH] [CodeGen] Fix hashing for MO_ExternalSymbol MachineOperands. 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 | 2 +- llvm/unittests/CodeGen/MachineOperandTest.cpp | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/llvm/lib/CodeGen/MachineOperand.cpp b/llvm/lib/CodeGen/MachineOperand.cpp index a83459e69170..9458745733f6 100644 --- a/llvm/lib/CodeGen/MachineOperand.cpp +++ b/llvm/lib/CodeGen/MachineOperand.cpp @@ -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()); diff --git a/llvm/unittests/CodeGen/MachineOperandTest.cpp b/llvm/unittests/CodeGen/MachineOperandTest.cpp index cfedfa9c8835..faa471f2260c 100644 --- a/llvm/unittests/CodeGen/MachineOperandTest.cpp +++ b/llvm/unittests/CodeGen/MachineOperandTest.cpp @@ -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