[PATCH] D106408: Allow rematerialization of virtual reg uses

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec added a comment.

In D106408#3020762 <https://reviews.llvm.org/D106408#3020762>, @wxiao3 wrote:

>> Obviously 0.01 there is a pure guesstimate.
>
> To fix one of my local performance regressions, I need to set it to 0.1 at least.
> But tuning the value seems to be meaningless. Because:
> Without your patch, //isReallyTriviallyReMaterializableGeneric return true// means that the VR is **definitely** ReMaterializable.
> With your patch,// isReallyTriviallyReMaterializableGeneric return true// means that the VR is **probably** ReMaterializable.
> I don't think the change is consistent with this function's orignal design goal.
> Moreover,  your patch **conflicts** with our target implementation: X86InstrInfo::isReallyTriviallyReMaterializable (llvm/lib/Target/X86/X86InstrInfo.cpp).  E.g., for most LEA instructions, our implementation will return false. While your patch will always return true if LEA is using virtual registers. Your patch makes our target implementation useless anymore.
>
>> it seems to reduce spilling (as expected) or be neutral
>
> I don't observe any data to prove it's good for performance in our internal performance track system running on various hardware. Instead, your patch brings -1.5% drop for CPU2017rerf/557.xz on server side and -2% drop for coremark-pro/core on desktop side. In scenario with big register pressure such as loops with a lot of LiveIntervals, optimistically assuming critical VR (LiveInterval) to be ReMaterializable (but it's not in reality) will make RA tend to spill the critical VR, which is definitely bad for performance.
>
> Could you please revert this change or at least make it not impact our X86 CPU BE? I don't observe the conflicted change bring any benefit but It results at least 2 regressions for X86 CPU BE.

Since this is second perf regression report I will revert it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106408