[PostRA Machine Sink] Sunk instruction corrupts liveness tracking

[JonPsson]

Bugzilla Link	36902
Resolution	FIXED
Resolved on	Apr 13, 2018 08:22
Version	trunk
OS	Linux
Attachments	reduced testcase (for clang), reduced testcase (for llc), llc test case (live-thrugh liveness)
CC	@francisvm,@JonPsson,@uweigand

Extended Description

This instruction:

renamable $r0l = COPY renamable $r12l, implicit killed $r12q

copies a subreg ($r12l) of $r12q which is killed. It is sunk into this block:

bb.8..preheader26: ; predecessors: %bb.6 successors: %bb.10(0x50000000), %bb.9(0x30000000) liveins: $r4d, $r0l, $r13l renamable $r0d = LGFR killed renamable $r0l renamable $r11d = LGFR killed renamable $r13l CHIMux renamable $r4l, 0, implicit-def $cc, implicit killed $r4d BRC 14, 10, %bb.10, implicit killed $cc

like

bb.8..preheader26: ; predecessors: %bb.6 successors: %bb.10(0x50000000), %bb.9(0x30000000) liveins: $r4d, $r13l, $r12l renamable $r0l = COPY renamable $r12l, implicit killed $r12q renamable $r0d = LGFR killed renamable $r0l renamable $r11d = LGFR killed renamable $r13l CHIMux renamable $r4l, 0, implicit-def $cc, implicit killed $r4d BRC 14, 10, %bb.10, implicit killed $cc

Note that another subreg of $r12q was already live into this MBB ($r13l). Since the sunk instruction now kills $r13l before its use, machine veririer says:

Bad machine code: Using an undefined physical register

• function: decode_one_b8block

• basic block: %bb.8 .preheader26 (0x2aa41875a08)

• instruction: renamable $r11d = LGFR killed renamable $r13l

• operand 1: killed renamable $r13l

clang line:

bin/clang -march=z13 -mllvm -disable-machine-dce -mllvm -disable-machine-sink -mllvm -extra-vectorizer-passes -mllvm -enable-tbaa=false -mllvm -verify-machineinstrs -O3 -w -o ./out.ll ./tc_machinesink.ll

llc line:

bin/llc -mtriple=s390x-linux-gnu -mcpu=z13 -start-before=postra-machine-sink ./tc_machinesink.mir -O3 -verify-machineinstrs

[llvmbot]

The patch was committed in https://reviews.llvm.org/rL330018.

[JonPsson]

Posted a fix (https://reviews.llvm.org/D44958) to extend the register dependency check for implicit operands. I didn't any code change in my environment (AArch64). Can anyone please apply this patch and test on SystemZ?

It seems that your patch does not introduce any new test failures on SystemZ, and it also seems to fix the unreduced test case.

[llvmbot]

Posted a fix (https://reviews.llvm.org/D44958) to extend the register dependency check for implicit operands. I didn't any code change in my environment (AArch64). Can anyone please apply this patch and test on SystemZ?

[uweigand]

There is one think I want to ask about register aliasing in SystemZ. As far as I checked the list of register aliased looks like this : $r12l : [$r12l] [$r12d] [$r12q] $r12q : [$r12l] [$r12d] [$r12q] [$r12h] [$r12d] [$r12q] [$r13l] [$r13d] [$r12q] [$r13h] [$r13d] [$r12q] $r13l : [$r13l] [$r13d] [$r12q]

r13l is aliased with r12q, and r12q is aliased with r12l. However, r13l is not aliased with r12l. Is this just expected alias relation in SystemZ?

The native size of general-purpose registers on Z is 64 bits (and we have 16 of those). r12d represents register 12 in its natural 64-bit size.

Many instructions only operate on the low 32 bits of a GPR. r12l represents the low 32 bits of register 12. A few instructions only operate on the high 32 bits of a GPR; r12h represents those for register 12.

Finally, some instructions operate on 128 bits, held in a register pair using even/odd numbers. r12q represents the pair of register 12 and register 13, interpreted as 128 bits.

Therefore, it is indeed the case that r12q aliases r12d (and therefore r12l and r12h) and also r13d (and therefore r13l and r13h). But r12d or its subregisters do not alias r13d or any of its subregisters.

[llvmbot]

There is one think I want to ask about register aliasing in SystemZ. As far as I checked the list of register aliased looks like this : $r12l : [$r12l] [$r12d] [$r12q] $r12q : [$r12l] [$r12d] [$r12q] [$r12h] [$r12d] [$r12q] [$r13l] [$r13d] [$r12q] [$r13h] [$r13d] [$r12q] $r13l : [$r13l] [$r13d] [$r12q]

r13l is aliased with r12q, and r12q is aliased with r12l. However, r13l is not aliased with r12l. Is this just expected alias relation in SystemZ?

[llvmbot]

Thanks Francis for the reduced test.

[francisvm]

X86 variant of the reduced test

[francisvm]

bb.0 (%ir-block.5): renamable $r0l = COPY renamable $r12l, implicit killed $r12q : renamable $r13l = SLLK renamable $r4l, $noreg, 1 : J %bb.1

I appears that $r13l alias with $r12q. We should have detected the register dependency between $r12q implicitly killed in the Copy and $r13l in SLLK. I will post a fix as soon as possible.

Right, makes more sense now. Attaching an x86 test.

[llvmbot]

bb.0 (%ir-block.5): renamable $r0l = COPY renamable $r12l, implicit killed $r12q : renamable $r13l = SLLK renamable $r4l, $noreg, 1 : J %bb.1

I appears that $r13l alias with $r12q. We should have detected the register dependency between $r12q implicitly killed in the Copy and $r13l in SLLK. I will post a fix as soon as possible.

[llvmbot]

In tc_machinesink.mir, PostRASink pass sink the copy in bb.0 :

renamable $r0l = COPY renamable $r12l, implicit killed $r12q

I guess some register aliased with $r12q must be defined in bb.0.

[llvmbot]

Started looking at this.

[francisvm]

Reduced MIR test Reduced this even further to 3 basic blocks. I don't really understand what went wrong here.