This is an archive of the discontinued LLVM Phabricator instance.

[Polly][ScopInfo] Remove RunTimeChecksMaxAccessDisjuncts bail-out condition.
AbandonedPublic

Authored by Meinersbur on Jan 3 2018, 2:43 AM.

Details

Summary

The condition was introduced in r267142 to mitigate a long compile-time case. In r306087, a max-computation limit was introduced that should handle the same case while leaving the max disjuncts heuristic it should have replaced intact.

Today, the max disjuncts bail-out causes problems in that it prematurely stops SCoPs from being detected, e.g. in SPEC's lbm. This would hit less like if isl_set_coalesce would be called after isl_set_remove_divs (which makes more basic_set likely to be coalescable) instead of before.

This patch removes max disjuncts bail-out as having been superseded by the max-computations limit.

Diff Detail

Event Timeline

Meinersbur created this revision.Jan 3 2018, 2:43 AM

Hi Michael,

as far as I remember I left this check in originally to save compile time, as we would bail out before having spent a lot of time in exploding lexmin/lexmax computations. Would calling isl_set_remove_divs at the right place not fix this issue?

Best,
Tobias

I'd rather remove the number of magic constants that make impactful decisions. If the goal is to reduce the computational overhead, I'd recommend to take the convex hull before lexmin/lexmin, not bail-out.

I am afraid taking a convex hull might also be costly (we could take a simple/affine hull instead).

I feel that in general, we should still decide to bail out early (before potentially costly functions) rather than waiting for compute outs to trigger. So I don't see how we can get rid of magic constants. However, we can potentially just take a hull rather than erroring out here.

Ping, as emails got lost at some point earlier in time.

Meinersbur abandoned this revision.Apr 20 2018, 12:04 PM

Deprecated in favor of D45066.