llvm-project/llvm/unittests/CodeGen
Diana Picus 4a5f522d4d MachineInstr: Make isEqual agree with getHashValue in MachineInstrExpressionTrait
MachineInstr::isIdenticalTo has a lot of logic for dealing with register
Defs (i.e. deciding whether to take them into account or ignore them).
This logic gets things wrong in some obscure cases, for instance if an
operand is not a Def for both the current MI and the one we are
comparing to.

I'm not sure if it's possible for this to happen for regular register
operands, but it may happen in the ARM backend for special operands
which use sentinel values for the register (i.e. 0, which is neither a
physical register nor a virtual one).

This causes MachineInstrExpressionTrait::isEqual (which uses
MachineInstr::isIdenticalTo) to return true for the following
instructions, which are the same except for the fact that one sets the
flags and the other one doesn't:
%1114 = ADDrsi %1113, %216, 17, 14, _, def _
%1115 = ADDrsi %1113, %216, 17, 14, _, _

OTOH, MachineInstrExpressionTrait::getHashValue returns different values
for the 2 instructions due to the different isDef on the last operand.
In practice this means that when trying to add those instructions to a
DenseMap, they will be considered different because of their different
hash values, but when growing the map we might get an assertion while
copying from the old buckets to the new buckets because isEqual
misleadingly returns true.

This patch makes sure that isEqual and getHashValue agree, by improving
the checks in MachineInstr::isIdenticalTo when we are ignoring virtual
register definitions (which is what the Trait uses). Firstly, instead of
checking isPhysicalRegister, we use !isVirtualRegister, so that we cover
both physical registers and sentinel values. Secondly, instead of
checking MachineOperand::isReg, we use MachineOperand::isIdenticalTo,
which checks isReg, isSubReg and isDef, which are the same values that
the hash function uses to compute the hash.

Note that the function is symmetric with this change, since if the
current operand is not a Def, we check MachineOperand::isIdenticalTo,
which returns false if the operands have different isDef's.

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

llvm-svn: 315579
2017-10-12 13:59:51 +00:00
..
GlobalISel [GlobalISel] Make GlobalISel a non-optional library. 2017-08-03 21:52:25 +00:00
CMakeLists.txt MachineInstr: Make isEqual agree with getHashValue in MachineInstrExpressionTrait 2017-10-12 13:59:51 +00:00
DIEHashTest.cpp Move Object format code to lib/BinaryFormat. 2017-06-07 03:48:56 +00:00
LowLevelTypeTest.cpp Added braces to work around gcc warning in googletest: suggest explicit braces to avoid ambiguous 'else'. NFC. 2017-06-15 21:00:40 +00:00
MachineInstrBundleIteratorTest.cpp Re-sort #include lines for unittests. This uses a slightly modified 2017-06-06 11:06:56 +00:00
MachineInstrTest.cpp MachineInstr: Make isEqual agree with getHashValue in MachineInstrExpressionTrait 2017-10-12 13:59:51 +00:00
MachineOperandTest.cpp [unittests] Adding a unittest for ChangeTaTargetIndex. NFC 2017-08-10 15:35:25 +00:00
ScalableVectorMVTsTest.cpp [SVE] Fix mismatched sign comparison warning in unit test from r300842. 2017-04-20 16:54:49 +00:00