[llvm] ConstraintElim: add dry-run routine to fail early (PR #99670)
Ramkumar Ramachandra via llvm-commits
llvm-commits at lists.llvm.org
Mon Jul 29 06:51:05 PDT 2024
artagnon wrote:
> Hm I think we should aim for a path forward without missing any simplification cases if possible. It also changes the behavior of the max column limit IIRC, as previously we just skipped in the case of hitting the limit until we removed rows again, then we continue.
>
> The current patch means we don't even try to simplify anything if the max is going to be above the threshold, which doesn't seem ideal.
I agree with your reasoning fully, since it is impossible to know if there will be a case where fewer simplifications are made. Even if we manage to get SPEC 2017 to not have binary changes, there could be degenerate cases out there that will miss some simplifications because of the dry-run-cutoff. There are, however, fine trade-offs we need to make, which I will explain shortly.
> Thinking also in the context of going forward with the new matrix data structure, some other alternatives that come to mind
>
> * using the estimate from the dry run for the initial matrix size, without the tight upper bound (possibly with one that is much larger to avoid degenerate cases)
Okay, so the current patch is a strict over-estimate. It doesn't under-estimate under any circumstance. The only reason for MaxColumns to exist is for the test.
> I don't think we need to aim for the current patch having a positive compile-time impact on its own if it comes at the cost of lost simplifications; having a positive impact in combination with the new matrix data type should be sufficient IMO.
The crux of the issue here is that the dry-run estimation adds a very small overhead, and the Matrix data structure is just a zero-cost abstraction (such a small benefit, that the benchmarks classify it as noise). We would therefore see a small negative impact overall after applying both patches. Although I'm not sure, I think removing the dry-run routine, and column-resizing the Matrix at runtime would make it a non-zero-cost abstraction.
The reason I developed the Matrix data structure is so that we can take slices of the Matrix, and operate on that slice, as if it were first-class. This would allow us to parse dependency information into a larger Matrix, and pass a sub-Matrix to the solver: dependency information has a "domain" and "range" which are parsed into separate column ranges, but solvers cannot operate on "mappings"; you need to extract a "set" out of the mapping, and operate on that. The way Presburger solves this problem is by copying pieces of "mappings" into "sets", which is far from ideal. Another benefit is that it would allow us to experiment with implementing different ILP algorithms like Simplex-based methods, which are supposedly faster (although we'd have to experiment to find out if it really is faster in our case). Although it is technically possible to use vector-of-vectors to implement Simplex-based methods, the most natural data structure is a "tableau" (or Matrix).
Now, it wouldn't be ideal to land a patch that has negative impact overall, with the promise that it _could_ be addressed in the future. The current patch is in its current form because that was the trade-off calculation I performed in my head.
https://github.com/llvm/llvm-project/pull/99670
More information about the llvm-commits
mailing list