[llvm-bugs] [Bug 39085] New: Faulty lowering in PHIElimination results in "use after kill"

[via llvm-bugs]

https://bugs.llvm.org/show_bug.cgi?id=39085

            Bug ID: 39085
           Summary: Faulty lowering in PHIElimination results in "use
                    after kill"
           Product: new-bugs
           Version: trunk
          Hardware: PC
                OS: Linux
            Status: NEW
          Severity: enhancement
          Priority: P
         Component: new bugs
          Assignee: unassignedbugs at nondot.org
          Reporter: bjorn.a.pettersson at ericsson.com
                CC: llvm-bugs at lists.llvm.org

Overview:
=========

I got some machine verifier complaints and has reduced the test case to this
phi-elimination-killflags.mir

##########################################################################################################
# RUN: llc -mtriple amdgcn -run-pass livevars -run-pass phi-node-elimination
-verify-machineinstrs -o - %s 

---
name:            bar
tracksRegLiveness: true
body:             |
  bb.0:
    S_CBRANCH_SCC0 %bb.2, implicit undef $scc

  bb.1:
    %1:sreg_32_xm0 = S_MOV_B32 255
    S_BRANCH %bb.3

  bb.2:
    %2:sreg_32_xm0 = S_MOV_B32 254

  bb.3:
    %4:sreg_32_xm0 = PHI %2, %bb.2, %1, %bb.1
    %3:sreg_32_xm0 = PHI %2, %bb.2, %1, %bb.1
    S_NOP 0, implicit %4, implicit %3
...

#
############################################################################################################


How to reproduce:
=================

> llc -mtriple amdgcn -run-pass livevars -run-pass phi-node-elimination -verify-machineinstrs -o - phi-elimination-killflags.mir


Actual results:
===============

# After Eliminate PHI nodes for register allocation
# Machine code for function bar: NoPHIs, TracksLiveness

bb.0:
  successors: %bb.2(0x40000000), %bb.1(0x40000000); %bb.2(50.00%),
%bb.1(50.00%)

  S_CBRANCH_SCC0 %bb.2, implicit undef $scc

bb.1:
; predecessors: %bb.0
  successors: %bb.3(0x80000000); %bb.3(100.00%)

  %0:sreg_32_xm0 = S_MOV_B32 255
  %4:sreg_32_xm0 = COPY killed %0:sreg_32_xm0
  S_BRANCH %bb.3

bb.2:
; predecessors: %bb.0
  successors: %bb.3(0x80000000); %bb.3(100.00%)

  %1:sreg_32_xm0 = S_MOV_B32 254
  %4:sreg_32_xm0 = COPY killed %1:sreg_32_xm0

bb.3:
; predecessors: %bb.1, %bb.2

  %3:sreg_32_xm0 = COPY killed %4:sreg_32_xm0
  %2:sreg_32_xm0 = COPY %4:sreg_32_xm0
  S_NOP 0, implicit killed %2:sreg_32_xm0, implicit killed %3:sreg_32_xm0

# End machine code for function bar.

*** Bad machine code: Using a killed virtual register ***
- function:    bar
- basic block: %bb.3  (0x4e41510)
- instruction: %2:sreg_32_xm0 = COPY %4:sreg_32_xm0
- operand 1:   %4:sreg_32_xm0
LLVM ERROR: Found 1 machine code errors.


Expected results:
=================
No verifier error. And probably something like this in bb.3:

# Verify that the "killed" is put on the last use (in the second COPY).
#
# CHECK-LABEL: name:            bar
# CHECK:   bb.3:
# CHECK-NEXT: %3:sreg_32_xm0 = COPY %4:sreg_32_xm0
# CHECK-NEXT: %2:sreg_32_xm0 = COPY killed %4:sreg_32_xm0
# CHECK-NEXT: S_NOP 0, implicit killed %2:sreg_32_xm0, implicit killed
%3:sreg_32_xm0


Additional information:
=======================

I've been looking a little bit at PHIElimination::EliminatePHINodes and
PHIElimination::LowerPHINode, but code and comments are sometimes a little bit
confusing. I think it all started with a commit back in 2013-07 that solved
PR16508 (https://reviews.llvm.org/rL185363). Prior to that commit PHI nodes
where lowered starting with the first PHI node in the block, and the COPY
instructions that replaced the PHI node were inserted in the same order as the
original PHI nodes. Example
  %1 = PHI ...
  %2 = PHI ...
  %3 = PHI ...
=>
  %1 = COPY ...
  %2 = COPY ...
  %3 = COPY ...
but after rL185363 we still lower the PHI nodes in the same order, but the COPY
instructions are inserted in opposite order
=>
  %3 = COPY ...
  %2 = COPY ...
  %1 = COPY ...

The confusing part is that PHIElimination::LowerPHINode still talks about "the
last PHI node". And it does thing like "only need to update the LiveVariables
kill of SrcReg if this was the last PHI use". And it can do thing like "reusing
the incoming register".
But how comes that changing the order of inserted instructions did not affect
any of the above (including updates of LV and LIS)?

In the test case provided above the "killed" attribute is put on the COPY that
corresponds to the last lowered PHI. But that COPY is inserted before all COPY
instructions that corresponds to later PHI nodes in the input to
PHIElimination.


Even more information:
======================

It seems like the reproducer from rL185363 do not trigger the problem it solved
any longer.
I've prepared a fix for that here: https://reviews.llvm.org/D52553  (any
volunteers for reviewing that patch are welcome)

-- 
You are receiving this mail because:
You are on the CC list for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-bugs/attachments/20180926/f386136b/attachment-0001.html>