Added test case that copy has side effect.

Updated the patch. Currently we don't handle non-trivial copy which has more than 2 operands(Cuz I notice current copy forward propagation algorithm considers copy's implicit operands).

Rebased.

Pre-committed machine-backward-cp.mir, including edge cases as far as I can imagine. Some test cases remain unchanged, reviewers might have to read whole diff of machine-backward-cp.mir to see unchanged cases.

@gribozavr Please let me know if the latest patch resolves the issue.

When replacing copy's src with copy's def, I missed a point that copy's def is clobbered by selected SrcMI and Tracker should be informed.

Hi @gribozavr , thanks for the affirmative action. Could you provide me your command to generate the executable and the target platform you compile to?

Check if SrcOp is tied or an implicit-def.

Added reserved register check on SrcOp.getReg().

Hi @craig.topper , would you mind having a look at changes of X86 test cases?

Added comment for bool MachineCopyPropagation::eraseIfRedundant(MachineInstr &Copy).

Rebased and updated tests.

Gentle ping.

ping.

Since this patch has long been not updated, I'll close it and plan another patch that also works for PowerPC.

Hi @RKSimon , currently I'm not working on it. I should have abandoned this patch. I might have another patch which also works for PowerPC.

Updated the patch and summary.

Updated patch to remove redundant copies by implementing backward copy propagation in machine-cp.

//   $reg0 = OP ....
// ...
//   $reg1 = COPY $reg0

If we can be sure that $reg0 is renamable, and no use of $reg0 or $reg1 in between (and of course there might be other restrictions, like ReservedReg etc),
we should be able to remove the COPY here.

// `$reg1` = OP ..
// ...

One concern here, there might be uses of $reg0 in succ BBs. To overcome it, we might constraint this opt in terminal BBs considering current machine-cp working on single BB. To summarize, without complex uses and defs(thus we don't need to do complex replace-uses-with after RA), in a terminal BB we should have

$reg0 = OP ...
... <<< No uses of $reg0 and $reg1
$reg1 = COPY $reg0 <<< $reg0 is killed
...
<RET>

In D65267#1625969, @jsji wrote:

Have you considered extend MachineCopyPropagation to cover this? Looks like to me that this is a 'backward' COPY propagation.

ping

In D65267#1610661, @hfinkel wrote:

Is this something that could be fixed in TailDuplication?

Updated patch to use llvm::Register::isVirtualRegister.

Ping

FYI, I should have make getDefMIPostRA public. So I committed another NFC patch to make getDefMIPostRA public. https://reviews.llvm.org/rG985e52a4c1c6

Hi @xbolva00 , I have some concern that @anton-afanasyev might have plan to revert some patches related to these files. If not so, I'll backport it to 9.0.

Use hasMinSize to check if optimized for size.

In D64394#1592621, @anton-afanasyev wrote:

Btw, this change breaks multiple (more than two) hoisting to common dominator. I've tested this patch for the original test case taken from here: https://bugs.llvm.org/show_bug.cgi?id=38917. There are several comparisons giving 96 > 40 + 10, 96 > 29 + 10, 96 > 18 + 10 (so no hoisting at all), meanwhile 96 < 97 = 40 + 29 + 18 + 10.
However I do not see easy solution for this issue.

Updated patch following @dmgreen 's suggestion.

Ping

Address @jsji 's comments and added new test.

Updated the patch, using MachineBlockFrequency as metric to check if CMBB is appropriate to hoist into.

@hfinkel , thanks for review.