[PATCH] D76248: Fix a bug in the inliner that causes subsequent double inlining

[David Li via Phabricator via llvm-commits]

davidxl added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/CloneFunction.cpp:361
+          // away later on.
+          if (!dyn_cast<CallBase>(NewInst) || !dyn_cast<CallBase>(V)) {
+            VMap[&*II] = V;
----------------
hoyFB wrote:
> davidxl wrote:
> > hoyFB wrote:
> > > davidxl wrote:
> > > > Is the first check needed (!dyn_cast<CallBase>(NewInst)) ?
> > > It's needed when the inlined instruction is not a call but promoted to a call during the simplification. For example, some code on arm32/arm64 target could be promoted to a call or an intrinsic call. In this case we'd like to keep this simplification since the it will not cause double inlining given that the original instruction is not a call.
> > Ok. Perhaps add a comment in the code?
> Sounds good.
It should be ok to do the simplification if V has no side effects .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76248