[PATCH] D58233: Allow replacing intrinsic operands with variables

Tue Sep 10 11:50:00 PDT 2019

arsenm added a comment.

In D58233#1665072 <https://reviews.llvm.org/D58233#1665072>, @efriedma wrote:

> I don't really have any concerns about this from the perspective of correctness; any breakage will be obvious and easy to fix.
>
> In terms of performance, I'm a little concerned we might not have an appropriate heuristic for profitability for all the callers of canReplaceOperandWithVariable.  Even if it's technically legal, for example, pulling the length out of a memcpy forces it to be lowered to a library call, which could be a lot slower.  (Even for non-intrinsic functions, we've had issues with this in the past; there was a bug reported at one point that we were pulling the shift amount out of an i256 shift, or something like that.)

I could move the intrinsic check into ConstantHoisting for now; I would like canReplaceOperandWithVariable to report what's legal

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

https://reviews.llvm.org/D58233