[PATCH] D58233: Allow replacing intrinsic operands with variables

[Eli Friedman via Phabricator via llvm-commits]

efriedma added a comment.

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.)


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

https://reviews.llvm.org/D58233