Miscompile after changes to MachineCopyPropagation in commit cae46f6210293ba4d3568eb2

[bjope]

We started to see miscompiles downstream after the update to MachineCopyPropagation here: cae46f6

I managed to reduce one failure and convert it into this (just picking AMDGCPU as target to get something with register tuples):

# RUN: llc -mtriple=amdgcn-- -mcpu=gfx900 -run-pass machine-cp -o - %s

---
name:            test1
body:             |
  bb.0:
    liveins: $sgpr4_sgpr5, $sgpr6_sgpr7

    $sgpr2_sgpr3 = COPY $sgpr6_sgpr7
    $sgpr0 = COPY $sgpr3
    S_NOP 0, implicit-def $sgpr0
    $sgpr3 = COPY killed $sgpr5
    $sgpr2_sgpr3 = COPY $sgpr6_sgpr7
    S_NOP 0, implicit $sgpr2_sgpr3
...

Without the changes to MCP:

> llc -mtriple=amdgcn-- -mcpu=gfx900 -o - -run-pass machine-cp mcp.mir

    $sgpr2_sgpr3 = COPY $sgpr6_sgpr7
    S_NOP 0, implicit-def $sgpr0
    $sgpr2_sgpr3 = COPY $sgpr6_sgpr7
    S_NOP 0, implicit $sgpr2_sgpr3

After cae46f6:

> llc -mtriple=amdgcn-- -mcpu=gfx900 -o - -run-pass machine-cp mcp.mir

    $sgpr2_sgpr3 = COPY $sgpr6_sgpr7
    S_NOP 0, implicit-def $sgpr0
    $sgpr3 = COPY killed $sgpr5                       <-- clobbers $sgpr3 but this is a dead def in the test case
    S_NOP 0, implicit $sgpr2_sgpr3                   <- this is incorrect, $sgpr3 should have the value of $sgrp7

[bjope]

I reverted the offending commit.

[LWenH]

Sorry for the late reply. I would like to ask if there are a lot of other miscompiles in the downstream？
If so, could you provide some more test samples（one or two are fine）, so that I can address the root cause and
make further changes? Thank you.

[bjope]

Sorry for the late reply. I would like to ask if there are a lot of other miscompiles in the downstream？ If so, could you provide some more test samples（one or two are fine）, so that I can address the root cause and make further changes? Thank you.

I don't have any other reproducers that I can share (and that would be meaningful). Took me quite some time just to reduce the test case for the downstream target. And then I needed to figure out how to convert that into something showing the problem for an in-tree target.
Hopefully it's just one main problem, and if the reproducer from this ticket pass then I can try it out on the larger downstream test cases.

With that said, my first suspicion here was around implicit operands. Not quite sure if that is the case here (i.e. those S_NOP:s with implicit operands can probaby be any instruction with non-implicit defs/uses, if you just know how to write AMDPPU mir).

Anyway, afaict lots of code in MachineCopyPropagation is inspecting all operands (including implicit operands). But then there are several places where things like CopyOperands->Destination->getReg() and CopyOperands->Source->getReg() is used. And then I wonder what happens if the COPY instruction also has implicit operands. Maybe that is uncommon, but at least downstream we sometimes need to add implicit defs operands on COPY instruction to model that writing to a subreg might have side-effect on other parts of the register. No idea if this cause problems in reality. It just made me suspicious about the pass in general since I don't know exactly how it works :-)

[LWenH]

Yeah, I'll try to debug the pass and figure out the root cause for this test case later.

Essentially, when we encounter:
$sgpr3 = COPY killed $sgpr5,
the preceding instruction:
$sgpr2_sgpr3 = COPY $sgpr6_sgpr7,
will become unpropagatable.

Consequently, it prevents the removal of the subsequent instruction during the basic block scanning procedure.
However, it's possible that this behavior is also influenced by the implicit source operand, necessitating deeper investigation to
pinpoint the exact cause. Hopefully it's just the only special test case here :-)

Thank you for taking the time to investigate this issue.

[dtcxzyw]

Fixed by #74239.