Subzero: Fix a lowering bug involving xchg and xadd instructions.
The x86-32 xchg and xadd instructions are modeled using two source operands, one of which is a memory operand and the other ultimately a physical register. These instructions have a side effect of modifying both operands.
During lowering, we need to specially express that the instruction modifies the Variable operand (since it doesn't appear as the instruction's Dest variable). This makes the register allocator aware of the Variable being multi-def, and prevents it from sharing a register with an overlapping live range.
This was being partially expressed by adding a FakeDef instruction. However, FakeDef instructions are still allowed to be dead-code eliminated, and if this happens, the Variable may appear to be single-def, triggering the unsafe register sharing.
The solution is to prevent the FakeDef instruction from being eliminated, via a FakeUse instruction.
It turns out that the current register allocator isn't aggressive enough to manifest the bug with cmpxchg instructions, but the fix and tests are there just in case.
BUG= none
R=jvoung@chromium.org
Review URL: https://codereview.chromium.org/1020853011
diff --git a/LOWERING.rst b/LOWERING.rst
index d51cd46..190ff23 100644
--- a/LOWERING.rst
+++ b/LOWERING.rst
@@ -218,3 +218,22 @@
If ``v_result_high`` is live but ``v_result_low`` is dead, adding ``t1`` as an
argument to ``InstFakeDef`` suffices to keep the ``call`` instruction live.
+
+Instructions modifying source operands
+--------------------------------------
+
+Some native instructions may modify one or more source operands. For example,
+the x86 ``xadd`` and ``xchg`` instructions modify both source operands. Some
+analysis needs to identify every place a ``Variable`` is modified, and it uses
+the presence of a ``Dest`` variable for this analysis. Since ICE instructions
+have at most one ``Dest``, the ``xadd`` and ``xchg`` instructions need special
+treatment.
+
+A ``Variable`` that is not the ``Dest`` can be marked as modified by adding an
+``InstFakeDef``. However, this is not sufficient, as the ``Variable`` may have
+no more live uses, which could result in the ``InstFakeDef`` being dead-code
+eliminated. The solution is to add an ``InstFakeUse`` as well.
+
+To summarize, for every source ``Variable`` that is not equal to the
+instruction's ``Dest``, append an ``InstFakeDef`` and ``InstFakeUse``
+instruction to provide the necessary analysis information.
