[PATCH] D56772: [MIR] Add simple PRE pass to MachineCSE

Tue Jul 23 08:27:16 PDT 2019

anton-afanasyev marked an inline comment as done.
anton-afanasyev added a comment.

In D56772#1596814 <https://reviews.llvm.org/D56772#1596814>, @LuoYuanke wrote:

> @anton-afanasyev
>  Hi,
>  Do you have any performance data for the patch? I'd like to know what benchmark has performance gain with your patch. https://reviews.llvm.org/D64394 fixed perlbench regression, but I wonder what the performance gain do we achieve with the 2 patch?

Hi @LuoYuanke, I've benchmarked the first patch and posted results in two posts started from here: https://reviews.llvm.org/D56772#1376284.
But you are right: with the second patch the performance gain may be eliminated! I'm to measure second patch effect.

================
Comment at: llvm/trunk/lib/CodeGen/MachineCSE.cpp:807
+      if (BB != nullptr && BB1 != nullptr &&
+          (isPotentiallyReachable(BB1, BB) ||
+           isPotentiallyReachable(BB, BB1))) {
----------------
lkail wrote:
> Hi @anton-afanasyev , I have a concern here that CFG of LLVM IR might not be equivalent to CFG of Machine IR.
Hi @lkail, yes this is possible case. But that is not an issue actually, for this case in place it would be just non-optimization. PRE will create instruction, but if CSE doesn't eliminate original instructions, than "hoisted" instruction will be deleted by Remove Dead Machine Instructions pass later.

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56772/new/

https://reviews.llvm.org/D56772