Very similar to https://reviews.llvm.org/D111272. We very often can
evaluate calls to llvm.objectsize.* regardless of inlining. Don't count
calls to llvm.objectsize.* against the InlineCost when we can evaluate
the call to a constant.
@craig.topper also mentioned that "it looks like maybe ObjectSizeOffsetVisitor doesn't know how to look through bitcasts?" on IRC. Let me look into that, because I suspect we should be able to evaluate call i64 @llvm.objectsize.i64.p0i8(i8* bitcast (%struct.nodemask_t* @numa_nodes_parsed to i8*), i1 false, i1 false, i1 false) to not--1.
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.
Why not have a separate SimplifiedValues object when checking for "cost"? There could be a canonical SimplifiedValues when checking legality, but when checking for cost, the only values that modify the canonical SimplifiedValues are those we *know* are correct---i.e., llvm.is.constant evaluates to true, or llvm.objectsize evaluates to a constant value. So when checking for "cost", we make a copy of the canonical version and then proceed as normal.
I like the idea of entering known correct values into SimplifiedValues. Now, when you say a separate SimplifiedValues object, do you mean maintaining something like SimplifiedValuesForCost and SimplifiedValuesForLegality?