Handle a COPY with undef source operand in LowerCopy().

[jonpa]

Summary

Llvm-stress discovered that a COPY may end up in ExpandPostRA::LowerCopy() with an undef source operand. It is not possible for the target to handle this, as this flag is not passed to TII->copyPhysReg().

This patch solves this by treating such a COPY as an identity COPY, as Matthias suggested on llvm-dev.

Reduces llvm-stress test case is also included.

[jonpa]

ping!

Matthias, this is per you suggestion - what do you think?

[MatzeB]

In D32892#749387, @jonpa wrote:

ping!

This is barely one workday old (in my timezone).

Matthias, this is per you suggestion - what do you think?

The code changes look good to me, but this would better be tested with a .mir test.

[jonpa]

This is barely one workday old (in my timezone).

Sorry - got carried away since I was working a little over the weekend :-)

The code changes look good to me, but this would better be tested with a .mir test.

Thanks. I changed the test.

Btw, I wonder what the best way to test that the machine-verifier passes? Currently I just do a CHECK for some expected string. Is this what "not llc" might be used for?

[MatzeB]

I was thinking more about a minimal test that just focused on that one transformation that postrapseudos should do. Whether you also run the verifier or not shouldn't matter at that point. Should look something like this (I haven't actually tested this though, so you may need some tweaks):

# RUN: llc -mtriple=s390x-linux-gnu -mcpu=z13 -verify-machineinstrs -run-pass=postrapseudos -o - %s | FileCheck %s
---
# CHECK-LABEL: name: undef_copy
# CHECK: %r13d = KILL undef %r0d, implicit killed %r12q, implicit-def %r12q
name: undef_copy
tracksRegLiveness: true
body: |
  bb.0:
    liveins: %r12q
    %r13d = COPY undef %r0d, implicit killed %r12q, implicit-def %r12q

[jonpa]

Ah, I see.

Your test seemed to work fine (without the machine verifier) :-)

[MatzeB]

LGTM

[jonpa]

r302877

lib/CodeGen/ExpandPostRAPseudos.cpp

if (MI->allDefsAreDead()) {
     MI->setDesc(TII->get(TargetOpcode::KILL));
     DEBUG(dbgs() << "replaced by: " << *MI);
     return true;
   }
 
   MachineOperand &DstMO = MI->getOperand(0);
   MachineOperand &SrcMO = MI->getOperand(1);
 
-  if (SrcMO.getReg() == DstMO.getReg()) {
-    DEBUG(dbgs() << "identity copy: " << *MI);
+  bool IdentityCopy = (SrcMO.getReg() == DstMO.getReg());
+  if (IdentityCopy || SrcMO.isUndef()) {
+    DEBUG(dbgs() << (IdentityCopy ? "identity copy: " : "undef copy:    ") << *MI);
     // No need to insert an identity copy instruction, but replace with a KILL
     // if liveness is changed.
     if (SrcMO.isUndef() || MI->getNumOperands() > 2) {
       // We must make sure the super-register gets killed. Replace the
       // instruction with KILL.
       MI->setDesc(TII->get(TargetOpcode::KILL));
       DEBUG(dbgs() << "replaced by:   " << *MI);
       return true;

test/CodeGen/SystemZ/lower-copy-undef-src.mir

+# RUN: llc -mtriple=s390x-linux-gnu -mcpu=z13 -run-pass=postrapseudos -o - %s | FileCheck %s
+#
+# Test that a COPY with an undef source operand gets handled like an identity
+# copy rather than lowered into a target instruction with the undef flag
+# dropped.
+---
+# CHECK-LABEL: name: undef_copy
+# CHECK: %r13d = KILL undef %r0d, implicit killed %r12q, implicit-def %r12q
+name: undef_copy
+tracksRegLiveness: true
+body: |
+  bb.0:
+    liveins: %r12q
+    %r13d = COPY undef %r0d, implicit killed %r12q, implicit-def %r12q