[PATCH] D36224: [TwoAddressInstructionPass] Replace subregister uses when processing tied operands

Matthias Braun via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Sep 27 09:50:28 PDT 2018


MatzeB added a comment.

Note that embarassingly there are still a few passes where we skip machine verification in the default pass pipeline, `TwoAddressInstruction` being one of them. However if this is a new failure triggered by this particular patch then please fix it!

In https://reviews.llvm.org/D36224#1247994, @bjope wrote:

> In https://reviews.llvm.org/D36224#1246786, @bjope wrote:
>
> > I'm actually not sure if this should impact the update of liveness info. LV is deprecated and should not be used after this pass (so I don't know if it really need to be updated). And everything about LIS here is unused code unless using -early-live-intervals. And we do not have any tests for that (and when I tried -early-live-intervals I got lots of other failures). Anyway, since https://reviews.llvm.org/rL279804 did not change anything about liveness info, I guess a revert of https://reviews.llvm.org/rL279804 should not impact it either.
>
>
> I've looked a little bit more at this and if I add "-run-pass livevars" to make LiveVariables come into play, and also add "-verify-machineinstrs" then the result for "test2" will be:
>
>   # After Two-Address instruction pass
>   # Machine code for function test2: NoPHIs, TracksLiveness
>  
>   bb.0.entry:
>     liveins: $d0
>     %0:doubleregs = COPY killed $d0
>     %1:intregs = COPY killed %0.isub_lo:doubleregs
>     dead %1:intregs = S2_lsr_i_r_acc %1:intregs(tied-def 0), %0.isub_hi:doubleregs, 16
>  
>   # End machine code for function test2.
>  
>   *** Bad machine code: Using a killed virtual register ***
>   - function:    test2
>   - basic block: %bb.0 entry (0x5c773b8)
>   - instruction: dead %1:intregs = S2_lsr_i_r_acc %1:intregs(tied-def 0), %0.isub_hi:doubleregs, 16
>   - operand 2:   %0.isub_hi:doubleregs
>   LLVM ERROR: Found 1 machine code errors.
>
>
> I've been told that "dead" markings on subreg defs only tell that the subreg is dead, not the full register. Is it the same for "killed"?


No! dead, killed and undef are specifically not about the subregister part but about the full vreg. (In an ideal world for subregister liveness we would have a lanemask for dead,killed,undef instead of just the bit we have today. With only a bit available we mostly ignore them for subreg liveness purposes as we cannot express situations like a write with only some lanes dead anyway...)

> If so, then I'd assume that this is a verifier bug. Otherwise we need to remove `killed %0.isub_lo` from the COPY instruction here.

this is bad MI here.

> PS. Another thing is that we do not get `killed` on the `%0.isub_hi` use in the `S2_lsr_i_r_acc` in the result. But we do not even get that out from LiveVariables, so maybe we should not blame TwoAddressInstructionPass for that.

yes as mentioned above, there may be bad pre-existing things hiding here :-(


Repository:
  rL LLVM

https://reviews.llvm.org/D36224





More information about the llvm-commits mailing list