Assertion in PHIElimination.cpp

Hi everyone

I'm hitting an assertion in PHIElimination.cpp:375.

"Terminator instructions cannot use virtual registers unless"
"they are the first terminator in a block!"

I was looking at the code around that location a bit and have not found
a reason why this assertion has to hold, except for a comment:
      // In our final twist, we have to decide which instruction kills the
      // register. In most cases this is the copy, however, the first
      // terminator instruction at the end of the block may also use the value.
      // In this case, we should mark *it* as being the killing block, not the
      // copy.

Again, no reason is given why only the first terminator is allowed to
use the register.

The offending code is:

(gdb) p opBlock.dump()
BB#8: derived from LLVM BB %for.cond
    Predecessors according to CFG: BB#7 BB#22 BB#19 BB#16 BB#11 BB#27 BB#26 BB#25 BB#24 BB#23
        %vreg18<def> = COPY %vreg80<kill>; GEXR16:%vreg18,%vreg80
        ADJCALLSTACKDOWN 0, %SP<imp-def>, %EX<imp-def>, %SP<imp-use>
        CALLi <ga:@clock_get_ticks>, <regmask>, %SP<imp-use>, %SP<imp-def>, %A<imp-def>, ...
        ADJCALLSTACKUP 0, 0, %SP<imp-def>, %EX<imp-def>, %SP<imp-use>
        %vreg57<def> = COPY %A<kill>; GR16:%vreg57
        %vreg58<def> = SUB16rr %vreg57, %vreg18<kill>, %EX<imp-def>; GR16:%vreg58,%vreg57 GEXR16:%vreg18
        %vreg59<def> = ADD16rm %vreg58<kill>, <fi#1>, 16, %EX<imp-def>; mem:LD1[%sunkaddr21](align=8)(tbaa=!"int") GR16:%vreg59,%vreg58
        MOV16mr <fi#1>, 16, %vreg59; mem:ST1[%sunkaddr21](align=8)(tbaa=!"int") GR16:%vreg59
        %vreg20<def> = COPY %vreg59; GEXR16:%vreg20 GR16:%vreg59
        %vreg21<def> = MOV16rm <fi#1>, 15; mem:LD1[%sunkaddr24](tbaa=!"int") GEXR16:%vreg21
        %vreg81<def> = COPY %vreg21; GEXR16:%vreg81,%vreg21
        BR_CCrr 2, %vreg59, %vreg21, <BB#10>; GR16:%vreg59 GEXR16:%vreg21
        BR_CCrr 7, %vreg59<kill>, %vreg21, <BB#10>; GR16:%vreg59 GEXR16:%vreg21
        JMP <BB#9>
    Successors according to CFG: BB#9 BB#10

Note the two BR_CCrr instructions in the end, those are the
terminators. %vreg21 is the register that is about to be killed.

In my opinion, the correct thing to do would be to mark the register in
the second BR_CCrr as killed. Am I missing something?

Cheers
Philipp

Hi everyone

I'm hitting an assertion in PHIElimination.cpp:375.

"Terminator instructions cannot use virtual registers unless"
"they are the first terminator in a block!"

I was looking at the code around that location a bit and have not found
a reason why this assertion has to hold, except for a comment:
     // In our final twist, we have to decide which instruction kills the
     // register. In most cases this is the copy, however, the first
     // terminator instruction at the end of the block may also use the value.
     // In this case, we should mark *it* as being the killing block, not the
     // copy.

Again, no reason is given why only the first terminator is allowed to
use the register.

It seems nobody has needed it before.

commit 2adfa7e9320e79859beb556367ea607a329866b3
Author: Chris Lattner <sabre@nondot.org>

    Add support for targets (like Alpha) that have terminator instructions which
    use virtual registers. We now allow the first instruction in a block of
    terminators to use virtual registers, and update phi elimination to correctly
    update livevar when eliminating phi's. This fixes a problem on a testcase
    Andrew sent me.
    
    git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@25083 91177308-0d34-0410-b5e6-96231b3b80d8

I don't think it is hard to fix, you simply need to find the last terminator reading SrcReg instead of insisting there can only be one.

Patches welcome!

/jakob

Thanks! Patch has been submitted.

Cheers,
Philipp