diff --git a/llvm/lib/Analysis/IRSimilarityIdentifier.cpp b/llvm/lib/Analysis/IRSimilarityIdentifier.cpp --- a/llvm/lib/Analysis/IRSimilarityIdentifier.cpp +++ b/llvm/lib/Analysis/IRSimilarityIdentifier.cpp @@ -526,19 +526,13 @@ for (Value *V : SourceOperands) { ArgVal = SourceValueToNumberMapping.find(V)->second; + // Instead of finding a current mapping, we attempt to insert a set. std::tie(ValueMappingIt, WasInserted) = CurrentSrcTgtNumberMapping.insert( std::make_pair(ArgVal, TargetValueNumbers)); - // Instead of finding a current mapping, we inserted a set. This means a - // mapping did not exist for the source Instruction operand, it has no - // current constraints we need to check. - if (WasInserted) - continue; - - // If a mapping already exists for the source operand to the values in the - // other IRSimilarityCandidate we need to iterate over the items in other - // IRSimilarityCandidate's Instruction to determine whether there is a valid - // mapping of Value to Value. + // We need to iterate over the items in other IRSimilarityCandidate's + // Instruction to determine whether there is a valid mapping of + // Value to Value. DenseSet NewSet; for (unsigned &Curr : ValueMappingIt->second) // If we can find the value in the mapping, we add it to the new set. @@ -558,7 +552,6 @@ if (ValueMappingIt->second.size() != 1) continue; - unsigned ValToRemove = *ValueMappingIt->second.begin(); // When there is only one item left in the mapping for and operand, remove // the value from the other operands. If it results in there being no diff --git a/llvm/unittests/Analysis/IRSimilarityIdentifierTest.cpp b/llvm/unittests/Analysis/IRSimilarityIdentifierTest.cpp --- a/llvm/unittests/Analysis/IRSimilarityIdentifierTest.cpp +++ b/llvm/unittests/Analysis/IRSimilarityIdentifierTest.cpp @@ -2619,6 +2619,29 @@ } } +// This test ensures that when the first instruction in a sequence is +// a commutative instruction with the same value (mcomm_inst_same_val), but the +// corresponding instruction (comm_inst_diff_val) is not, we mark the regions +// and not similar. +TEST(IRSimilarityIdentifier, CommutativeSameValueFirstMisMatch) { + StringRef ModuleString = R"( + define void @v_1_0(i64 %v_33) { + entry: + %comm_inst_same_val = mul i64 undef, undef + %add = add i64 %comm_inst_same_val, %v_33 + %comm_inst_diff_val = mul i64 0, undef + %mul.i = add i64 %comm_inst_diff_val, %comm_inst_diff_val + unreachable + })"; + LLVMContext Context; + std::unique_ptr M = makeLLVMModule(Context, ModuleString); + + std::vector> SimilarityCandidates; + getSimilarities(*M, SimilarityCandidates); + + ASSERT_TRUE(SimilarityCandidates.size() == 0); +} + // This test makes sure that intrinsic functions that are marked commutative // are still treated as non-commutative since they are function calls. TEST(IRSimilarityIdentifier, IntrinsicCommutative) {