Walking the use list of a Constant (particularly, ConstantData)
is not scalable, since a given constant may be used by many
instructinos in many functions in many modules.
Hi, thanks for the review.
Instances of ConstantData are unique for the entire an LLVMContext, so its uses can be spread across multiple functions and modules.
Here are some links for more info about it:
Adding some reviewers that have touched the SLPVectorizer recently to check this. (I have context on the motivation for the change and planned changes to the IR, but I don't know this pass well.)
It's possible that rejecting all Constants makes sense here, depending on whether this code needs to walk through GlobalValue-derived constants. I suspect it doesn't and we can just do Constant.
I think this comment will be more clear if it highlights the semantic problem. I suggest something like:
// Since this is a function pass, it doesn't make semantic sense to walk the // users of a subclass of Constant. The users could be in another function, // or even another module that happens to be in the same LLVMContext.
That sounds right to me - this pass is only concerned with "basic" integer/FP constants. For example, we have allConstant() around line 219:
/// \returns True if all of the values in \p VL are constants (but not /// globals/constant expressions).
So I think it's safe to ignore any Constant here.
Hi, well, not really, after ConstantData use-list removed, we definitely cannot walk its use-list.
We could change the check from isa to calling hasUseList(), but I'm not sure if it would be more readable here.