[coz3-deepperf-fix] lp: hoist loop-invariant pivot reads in HNF pivot_column_non_fractional#10073
Merged
Merged
Conversation
In the Bareiss-style fractional-free elimination step, the inner double loop re-reads m[r][r], m[r-1][r-1] and m[r][j] on every iteration. For general_matrix each m[a][b] builds a temporary ref_row and performs two permutation-array indirections (row/column permutation lookups) before the actual vector access, so these repeated reads dominate the loop. Rows <= r are never written by this loop, so those three entries are loop-invariant. Bind m[r][r] and m[r][j] to references (the latter hoisted out of the inner i-loop) and take a pointer to m[r-1][r-1] once, guarding the r == 0 case that skips the division. Also bind m[i][j] to a reference to avoid indexing it three times per iteration. This turns O(n^2) repeated permutation-indexed mpq reads into O(1)/O(n) hoisted reads with no change in the computed result. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR optimizes the Hermite Normal Form (HNF) fraction-free elimination step used in Z3’s integer linear arithmetic path by hoisting loop-invariant matrix element accesses out of the hot nested loops in lp::hnf_calc::pivot_column_non_fractional.
Changes:
- Hoist invariant pivot entries (
m[r][r],m[r-1][r-1]) outside the nested elimination loops. - Hoist
m[r][j]outside the inneriloop and bindm[i][j]once per iteration to reduce repeated permutation-indexed accesses. - Preserve arithmetic behavior while reducing repeated
general_matrixindexing overhead in the O(n²) update.
Contributor
Author
|
@angelica-moreira , @NikolajBjorner : another pleasant surprise from the AI gods. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Hoists loop-invariant matrix reads out of the hot inner loop of
pivot_column_non_fractionalin the Hermite Normal Form (HNF) computation used by z3's linear-arithmetic integer solver. The arithmetic is unchanged; the patch only removes repeated permutation-indexedmpqaccesses from an O(n2) elimination loop.Hotspot
lp::hnf_calc::pivot_column_non_fractional<M>(src/math/lp/hnf.h:130) performs the Bareiss-style fraction-free Gaussian elimination step over a matrixm:For
general_matrix, everym[a][b]builds a temporaryref_rowand performs two permutation-array indirections (row and column permutation lookups insrc/math/lp/general_matrix.h) before the underlying vector access. Inside this double loop the termsm[r][r],m[r-1][r-1]andm[r][j]are re-read on every iteration even though they are invariant, so those redundant indexed reads dominate the loop cost.Change and complexity argument
Rows
<= rare never written by this loop — it only assignsm[i][j]fori > r,j > r— so the three pivot entries in rows<= rare loop-invariant:m[r][r]andm[r-1][r-1]are invariant across both loops → bindm[r][r]to a reference and take a pointer tom[r-1][r-1]once before the outer loop. The pointer isnullptrwhenr == 0, which also encodes the existing "no division" case with a single branch.m[r][j]is invariant across the inneriloop → hoist it to a reference at the top of the outerjloop.m[i][j]is bound to a reference so it is indexed once per iteration instead of three times (two reads + one write).This turns O(n2) repeated permutation-indexed
mpqreads into O(1)/O(n) hoisted reads. The operands, operation order, and division are identical to the original, so the computed matrix is bit-for-bit the same.Measurements
Profiled with callgrind (z3 built with the same configuration,
model_validate=true) on a representative integer-arithmetic problem that exercises the HNF cut generator:Logic class
Integer linear arithmetic — the HNF-based cut generation path in the
lpint-solver.