[PATCH] D111456: [InlineCost] model calls to llvm.objectsize.*

[Kazu Hirata via Phabricator via llvm-commits]

kazu added a comment.

Hmm.  I may be a bit pedantic, but please bear with me for a moment. Even though I've already LGTMed https://reviews.llvm.org/D111272, and this patch is quite similar in nature -- "speculative" folding, I think we could run into a problem for the following reason. `InlineCost.cpp` performs two tasks -- the legality check ("Can we inline?") and the desirability check ("Should we inline?").  Once we "speculatively fold" these intrinsics and update `SimplifiedValues`, we don't examine those "dead" basic blocks for legality purposes. This can be a problem if those basic blocks come back to life, and they contain un-inlinable instructions.  I suspect even the return value from `@llvm.objectsize` could change from the failure value (like -1) to the actual object size after ThinLTO importing.

I am currently thinking about maintaining a parallel world `SimplifiedValues` and `LikelyValues` for operations that matter to folding conditional branches that check `@llvm.is.constant` and `@llvm.objectsize`.  I would also maintain a set of basic blocks that are "speculatively dead".  If I am scanning a basic block that is marked speculatively dead, then I check legality but do not increment `Cost`.

`SimplifiedValues` appears 40 times in `InlineCost.cpp`, so it would be a nightmare to duplicate each one and maintain the perfect parallel world for little gain.

Thoughts?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111456