Skip to content

feat(bv_decide): veir_bv_decide_upto tactic#912

Open
tobiasgrosser wants to merge 1 commit into
mainfrom
tobias/bv_upto
Open

feat(bv_decide): veir_bv_decide_upto tactic#912
tobiasgrosser wants to merge 1 commit into
mainfrom
tobias/bv_upto

Conversation

@tobiasgrosser

Copy link
Copy Markdown
Collaborator

No description provided.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VeIR Benchmarks

Details
Benchmark suite Current: 7d91079 Previous: 4368f8c Ratio
add-fold-worklist/create 2330000 ns (± 50446) 2256000 ns (± 63926) 1.03
add-fold-worklist/rewrite 4078000 ns (± 94942) 3950000 ns (± 29970) 1.03
add-fold-worklist-local/create 2335000 ns (± 80789) 2218000 ns (± 83244) 1.05
add-fold-worklist-local/rewrite 3469000 ns (± 76998) 3343000 ns (± 96842) 1.04
add-zero-worklist/create 2324000 ns (± 102100) 2208000 ns (± 108954) 1.05
add-zero-worklist/rewrite 2565000 ns (± 81705) 2549000 ns (± 69170) 1.01
add-zero-reuse-worklist/create 1963500 ns (± 114288) 1975000 ns (± 79912) 0.99
add-zero-reuse-worklist/rewrite 2173500 ns (± 147103) 2121000 ns (± 42346) 1.02
mul-two-worklist/create 2289000 ns (± 258589) 2238000 ns (± 140133) 1.02
mul-two-worklist/rewrite 5699500 ns (± 188141) 5600000 ns (± 413824) 1.02
add-fold-forwards/create 2300500 ns (± 110718) 2208000 ns (± 71293) 1.04
add-fold-forwards/rewrite 3101500 ns (± 41299) 3090000 ns (± 56043) 1.00
add-zero-forwards/create 2298000 ns (± 53190) 2212000 ns (± 39927) 1.04
add-zero-forwards/rewrite 2031000 ns (± 31467) 2029000 ns (± 23864) 1.00
add-zero-reuse-forwards/create 1952000 ns (± 56344) 1889000 ns (± 75381) 1.03
add-zero-reuse-forwards/rewrite 1583000 ns (± 38499) 1592000 ns (± 20611) 0.99
mul-two-forwards/create 2245000 ns (± 21183) 2242000 ns (± 69816) 1.00
mul-two-forwards/rewrite 3755000 ns (± 82612) 3790000 ns (± 75939) 0.99
add-zero-reuse-first/create 1939000 ns (± 95894) 1819000 ns (± 79615) 1.07
add-zero-reuse-first/rewrite 8000 ns (± 1675) 8000 ns (± 378) 1
add-zero-lots-of-reuse-first/create 1927000 ns (± 94309) 1857000 ns (± 66017) 1.04
add-zero-lots-of-reuse-first/rewrite 827000 ns (± 38492) 781000 ns (± 17225) 1.06

This comment was automatically generated by workflow using github-action-benchmark.

@tobiasgrosser tobiasgrosser changed the title fix(bv_decide): veir_bv_decide_upto tactic feat(bv_decide): veir_bv_decide_upto tactic Jun 23, 2026

@regehr regehr left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand the top part of this but not the bottom, but LGTM!

@math-fehr math-fehr left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, I just added a few comments but after that we can merge it

Comment thread Veir/Meta/BVDecide.lean
Comment on lines +89 to +92
if solverRes.isSome then
remainingGoals := remainingGoals ++ []
else
remainingGoals := remainingGoals ++ [subgoal.mvarId]

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if solverRes.isSome then
remainingGoals := remainingGoals ++ []
else
remainingGoals := remainingGoals ++ [subgoal.mvarId]
if solverRes = none then
remainingGoals := remainingGoals ++ [subgoal.mvarId]

As the then branch is a no-op

Comment thread Veir/Meta/BVDecide.lean
remainingGoals := remainingGoals ++ unclosed
return remainingGoals
else
-- If upper bound is zero (contradiction case), let omega clean it up

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the case of a non-literal symbol, rather than the 0 case?

Comment thread Veir/Meta/BVDecide.lean
Comment on lines +77 to +79
-- FIX: We no longer run the solver on the abstract parent goal here.
-- This prevents the SMT/SAT solver from running on generic, non-instantiated bitwidths
-- and avoids the "potentially spurious counterexample" error entirely.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this comment for? Is it for an issue that we should add, or should the comment be removed?

@math-fehr math-fehr self-requested a review June 28, 2026 19:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants