[llvm] ConstraintElim: add dry-run routine to fail early (PR #99670)

[Florian Hahn via llvm-commits]

fhahn wrote:


> My gut reaction is that this diff will not be helpful: we will get a lot of asserts in the case of binary changes, and these asserts will not have information to meaningfully update the estimates. The dry-run estimation can't be used for a logically precise condition: it is a cheap estimation that "works in practice". I wish we had some kind of formula for the upper-bound we could just lift out from a paper (as is the case for many algorithms like FM), but this is not possible for ConstraintElimination.
> 
> > Presumably some of those issues would go away with a larger default for MaxColumns?
> 
> My intuition from writing the dry-run estimation routine is that the 1.4 factor is the more problematic issue, and not MaxColumns. We should bump the factor to 1.6, and try again: the reason I used 1.4 is because I wanted a fine upper bound so we don't lose the benefits of the patch; 1.6 could work just as well, but a factor that's too high might not yield such great overall results. I _think_ the row-estimation is off by a constant factor (modulo minor error), so multiplying a constant factor should be _possible_.
> 
> I've observed that the column-estimation is pretty accurate, although it is possible that MaxColumns would need to be bumped up to 64 if the row-estimation times the constant factor is correct.

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.

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)
*  skip adding new facts when doing so would exceed to available columns in the matrix
* resize the matrix data structure on demand, even if costly, if the cost would be amortized by more efficient ops on average.


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.


https://github.com/llvm/llvm-project/pull/99670