[PATCH] D37467: Add a new pass to speculate around PHI nodes with constant (integer) operands when profitable.

Chandler Carruth via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Sep 8 08:29:23 PDT 2017


chandlerc added a comment.

So, I'm still parsing your first comment Danny, but seeing the second, I think there may be some confusion here...

I'm not actually modeling any simplifications at all, and I'm not really trying to. While that is interesting, I completely agree that this is the wrong place to do it and doesn't have the machinery you would need to do a good job. I think something based around a GVN-like analysis makes way more sense.

The "cost model" is trying to handle one fairly narrow and specific case: when the target's cost for materializing N constants along N incoming edges is equal to  the cost of duplicating the user instructions of the incoming constant (and those users' dependencies) along the N incoming edges and letting the constant get materialized as part of the instruction.

I don't actually expect this to ever be profitable for large numbers of duplicated instructions... That typically doesn't make sense. The example I added to the top-level comment on the pass is really the case I care about. The theory is that all simplification-based, or non-duplicating variants of this transform, if advisable, would have *already happend*, likely by NewGVN or similar. This pass is just trying to handle the case where there are target-specific costs associated with materializing constants that don't occur when the constant is an operand of an instruction.

Anyways, hopefully this clarified some of the intent behind the cost modeling. Like I said, I'm still going throuh the larger comment around safety checks and complexity. I actually had another data structure there and I suspect that Danny is correct that removing it does make this quadratic. And quite possibly there is a better way to avoid that, but yeah, need to go through that more to be confident of anything.


https://reviews.llvm.org/D37467





More information about the llvm-commits mailing list