[PATCH] D4351: bug fix for PR20020: anti-dependency-breaker causes miscompilation

[Danila Malyutin via Phabricator via llvm-commits]

danilaml added inline comments.
Herald added subscribers: pengfei, mcrosier.
Herald added a project: LLVM.


================
Comment at: llvm/trunk/lib/CodeGen/CriticalAntiDepBreaker.cpp:203
 
+    // If this reg is tied and live (Classes[Reg] is set to -1), we can't change
+    // it or any of its sub or super regs. We need to use KeepRegs to mark the
----------------
@spatel Hi, I was debugging some miscompile issue and came across what I think is very similar bug.
Do you remember why you check against `-1` here? In my test-case (which I'm trying to reduce), the Classes[Reg] is some regclass non -1 value. According to the comments for Classes:
```
/// For live regs that are only used in one register class in a
    /// live range, the register class. If the register is not live, the
    /// corresponding value is null. If the register is live but used in
    /// multiple register classes, the corresponding value is -1 casted to a
    /// pointer.
```
My question is: why is this code is checking for `-1` specifically, instead of `!= nullptr`? If it's not live, would it still be OK? I have trouble following the logic in this pass.
The comments for scan indicate that
```
// Note: AntiDepReg may be referenced by a two-address instruction such that
// it's use operand is tied to a def operand ...
// In this case, both the use and def operands are in
// RegRefs because the def is inserted by PrescanInstruction and not erased
// during ScanInstruction.
```
which is evidently not true in my test case (e.g. XOR has 3 same-reg operands, one pair tied, so the ScanInstruction would remove all RegRefs added by Prescan due to the 3rd non-tied reg, and skip the first tied def when adding refs back).


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D4351/new/

https://reviews.llvm.org/D4351