[UnreachableBlockElim] Use COPY if PHI input is undef

[uabelho]

Summary

If we have

%vreg0<def> = PHI %vreg2<undef>, <BB#0>, %vreg3, <BB#2>; GR32:%vreg0,%vreg2,%vreg3
%vreg3<def,tied1> = ADD32ri8 %vreg0<kill,tied0>, 1, %EFLAGS<imp-def>; GR32:%vreg3,%vreg0

then we can't just change %vreg0 into %vreg3, since %vreg2 is actually
undef. We would have to also copy the undef flag to be able to change the
register.

Instead we deal with this case like other cases where we can't just
replace the register: we insert a COPY. The code creating the COPY already
copied all flags from the PHI input, so the undef flag will be transferred
as it should.

[kparzysz]

Looks good. Thanks.

Could you change the testcase to MIR and only run the unreachable block elimination?

[uabelho]

In D38235#884338, @kparzysz wrote:

Looks good. Thanks.

Could you change the testcase to MIR and only run the unreachable block elimination?

I've tried but I don't understand what happens when I try to make a MIR test.

First weirdness:

Even if I try to stop before unreachable machine block elimination with
llc -optimize-regalloc -O0 -o - -stop-before=unreachable-mbb-elimination
it doesn't stop before? WIth -debug-pass=Executions I see:

[...]
[2017-10-03 10:06:20.575253544] 0x3c95710 Executing Pass 'Process Implicit Definitions' on Function 'f'...
[2017-10-03 10:06:20.575288256] 0x3c95710 Made Modification 'Process Implicit Definitions' on Function 'f'...
[2017-10-03 10:06:20.575317590] 0x3c95710 Freeing Pass 'Process Implicit Definitions' on Function 'f'...
[2017-10-03 10:06:20.575343782] 0x3c95710 Executing Pass 'Remove unreachable machine basic blocks' on Function 'f'...
[2017-10-03 10:06:20.575384081] 0x3c95710 Made Modification 'Remove unreachable machine basic blocks' on Function 'f'...
[2017-10-03 10:06:20.575412158] 0x3c95710 Executing Pass 'Live Variable Analysis' on Function 'f'...
[2017-10-03 10:06:20.575482280] 0x3c95710 Executing Pass 'MachineDominator Tree Construction' on Function 'f'...
[2017-10-03 10:06:20.575531100] 0x3c95710 Executing Pass 'Machine Natural Loop Construction' on Function 'f'...
[2017-10-03 10:06:20.575574333] 0x3c95710 Executing Pass 'Eliminate PHI nodes for register allocation' on Function 'f'...
[2017-10-03 10:06:20.575609045] 0x3c95710 Freeing Pass 'Eliminate PHI nodes for register allocation' on Function 'f'...
[2017-10-03 10:06:20.575635795] 0x3c95710 Freeing Pass 'Remove unreachable machine basic blocks' on Function 'f'...
[...]
Shouldn't it stop before "Executing Pass 'Remove unreachable machine basic blocks' on Function 'f'" ?

Second weirdness:
If I try to work around the above by instead stopping after "Process Implicit Definitions" and then just
run run-pass=unreachable-mbb-elimination I get

Machine code for function f: IsSSA

BB#0: derived from LLVM BB %bb0

    JMP_1 <BB#1>
Successors according to CFG: BB#1(0x80000000 / 0x80000000 = 100.00%)

BB#1: derived from LLVM BB %bb1

Predecessors according to CFG: BB#0 BB#2
    %vreg0<def> = PHI %vreg2<undef>, <BB#0>, %vreg3, <BB#2>; GR32:%vreg0,%vreg2,%vreg3
    %vreg3<def,tied1> = ADD32ri8 %vreg0<kill,tied0>, 1, %EFLAGS<imp-def>; GR32:%vreg3,%vreg0
    JMP_1 <BB#3>
Successors according to CFG: BB#3(0x80000000 / 0x80000000 = 100.00%)

BB#2: derived from LLVM BB %bb2

    JMP_1 <BB#1>
Successors according to CFG: BB#1(0x80000000 / 0x80000000 = 100.00%)

BB#3: derived from LLVM BB %bb3

Predecessors according to CFG: BB#1
    RETQ

End machine code for function f.

• • Bad machine code: PHI operand is not live-out from predecessor ***
• function: f
• basic block: BB#1 bb1 (0x4995cf8)
• instruction: %vreg0<def> = PHI
• operand 1: %vreg2<undef>

LLVM ERROR: Found 1 machine code errors.

So the MIR output from processimpdefs is not accepted as input.

So the smallest MIR test I've managed to do is if I stop before processimpdefs
and then run with that output as input with
-run-pass=processimpdefs -run-pass=unreachable-mbb-elimination.

I'll upload that new testcase in a minute.

[uabelho]

Changed the testcase to MIR.

[uabelho]

In D38235#884338, @kparzysz wrote:

Looks good. Thanks.

Could you change the testcase to MIR and only run the unreachable block elimination?

Do you prefer the new version?

[kparzysz]

Yes. This is great! Thanks.

[uabelho]

Several build bots broke due to my new test case. I think it's because it assumed the default target would be x86.
I reverted in r314880 and then recommited with a (hopefully) fixed testcase in r314882.

llvm/trunk/lib/CodeGen/UnreachableBlockElim.cpp

while (phi != BB->end() && phi->isPHI()) {
         unsigned OutputReg = Output.getReg();
         assert(Output.getSubReg() == 0 && "Cannot have output subregister");
         ModifiedPHI = true;
 
         if (InputReg != OutputReg) {
           MachineRegisterInfo &MRI = F.getRegInfo();
           unsigned InputSub = Input.getSubReg();
           if (InputSub == 0 &&
-              MRI.constrainRegClass(InputReg, MRI.getRegClass(OutputReg))) {
+              MRI.constrainRegClass(InputReg, MRI.getRegClass(OutputReg)) &&
+              !Input.isUndef()) {
             MRI.replaceRegWith(OutputReg, InputReg);
           } else {
             // The input register to the PHI has a subregister or it can't be
-            // constrained to the proper register class:
+            // constrained to the proper register class or it is undef:
             // insert a COPY instead of simply replacing the output
             // with the input.
             const TargetInstrInfo *TII = F.getSubtarget().getInstrInfo();
             BuildMI(*BB, BB->getFirstNonPHI(), phi->getDebugLoc(),
                     TII->get(TargetOpcode::COPY), OutputReg)
                 .addReg(InputReg, getRegState(Input), InputSub);
           }
           phi++->eraseFromParent();

llvm/trunk/test/CodeGen/MIR/X86/unreachable-mbb-undef-phi.mir

+# RUN: llc %s -o - -run-pass=processimpdefs -run-pass=unreachable-mbb-elimination | FileCheck %s
+---
+name:            f
+tracksRegLiveness: true
+registers:
+  - { id: 0, class: gr32, preferred-register: '' }
+  - { id: 1, class: gr32, preferred-register: '' }
+  - { id: 2, class: gr32, preferred-register: '' }
+body:             |
+  bb.0:
+    %0 = IMPLICIT_DEF
+    JMP_1 %bb.1
+
+  bb.1:
+    %1 = PHI %0, %bb.0, %2, %bb.2
+    %2 = ADD32ri8 killed %1, 1, implicit-def %eflags
+    JMP_1 %bb.3
+
+  bb.2:
+    JMP_1 %bb.1
+
+  bb.3:
+...
+
+# bb2 above is dead and should be removed and the PHI should be replaced with a
+# COPY from an undef value since the bb0 value in the PHI is undef.
+
+# CHECK:  bb.0:
+# CHECK:    successors: %bb.1
+# CHECK:    JMP_1 %bb.1
+
+# CHECK:  bb.1:
+# CHECK:    successors: %bb.2
+# CHECK:    [[TMP1:%[0-9]+]] = COPY undef %{{[0-9]+}}
+# CHECK:    %{{[0-9]+}} = ADD32ri8 killed [[TMP1]], 1
+# CHECK:    JMP_1 %bb.2
+
+# CHECK:  bb.2: