During its work, GenerateReassociationsImpl may end up trying calculating a sum
of a huge amount of huge SCEVs. Attempts to simplify them lead to huge compile
time problem. This patch applies a heuristic that limits simplification recursion depth
depending on number of arguments in this method to get an acceptible compile time
in corner cases while it should not affect most of real situations.
Details
Details
- Reviewers
sanjoy evstupac anna apilipenko reames
Diff Detail
Diff Detail
Event Timeline
lib/Transforms/Scalar/LoopStrengthReduce.cpp | ||
---|---|---|
3497 | This worries me from a design perspective -- the Depth parameter definitely should not be part of the public API of SCEV (I had not noticed that we've added these parameters to the public API). We can't guarantee its meaning or even its presence to passes that use SCEV. Can we instead make SCEV smarter to avoid spending a lot of cycles in pathological cases like this? |
Comment Actions
The other way is limiting SCEV transformation not by depth only, but also by the size of operands list, if it makes sense.
Comment Actions
I think your current approach is fine (penalize depth for high "breadth"), but it just needs to somehow happen within SCEV (so that "depth" remains a SCEV implementation detail).
This worries me from a design perspective -- the Depth parameter definitely should not be part of the public API of SCEV (I had not noticed that we've added these parameters to the public API). We can't guarantee its meaning or even its presence to passes that use SCEV.
Can we instead make SCEV smarter to avoid spending a lot of cycles in pathological cases like this?