Skip to content

WIP - Implement Generalizer #1022

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 57 commits into from

Conversation

oluwatimilehin
Copy link
Collaborator

No description provided.

import SSA.Experimental.Bits.Fast.Defs
import SSA.Experimental.Bits.Fast.FiniteStateMachine
import SSA.Experimental.Bits.Fast.Attr
import SSA.Experimental.Bits.Fast.Decide
Copy link
Collaborator

Choose a reason for hiding this comment

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

This tactic should not depend on Mathlib or SSA, no?

(fun ix => inputs.map ⟨fun input => Inputs.mk ix input, by intros a b; simp⟩)
⟨out, by
⟨out, by
Copy link
Collaborator

Choose a reason for hiding this comment

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

These changes seem unrelated, no?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

They are unrelated. My editor keeps removing these trailing whitespaces, which seems to be according to the setting here, but I'm not sure why they were present before.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It might be worth to commit these whitespace separately, so this diff is clean and our codebase is also cleaner.

BVExpr.shiftRight (substituteBVExpr lhs) (substituteBVExpr rhs)
| .arithShiftRight lhs rhs =>
BVExpr.arithShiftRight (substituteBVExpr lhs) (substituteBVExpr rhs)
| _ => bvExpr --TODO: handle other constructors?
Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, you need to handle all constructors which contain expressions

@oluwatimilehin oluwatimilehin changed the title WIP - Exists/For-all Tactic WIP - Generalize Impl Mar 31, 2025
@oluwatimilehin oluwatimilehin changed the title WIP - Generalize Impl WIP - Implement Generalizer Mar 31, 2025

-- x + c1 == c1
let lhs := BVExpr.bin x BVBinOp.add c1
BoolExpr.literal (BVPred.bin lhs BVBinPred.eq x)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Isn't it x + c1 == x?

@bollu
Copy link
Collaborator

bollu commented Apr 27, 2025

I'd suggest the following course of action:

  1. We update the branch to the lastest master
  2. We merge the branch as-is, since it lives in Experimental.
  3. I will try and reduce divergence with upstream, since IIRC, quite a few of the functions here are modifications of internals of bv_decide. If we reused these functions, then we should try to depend on the ones from bv_decide itself, and if we extended them, then we should split these functions into a separate Lean module for easy upstreaming.

@bollu
Copy link
Collaborator

bollu commented Apr 27, 2025

@oluwatimilehin could you please merge main into this branch so we can pull the branch in?

@oluwatimilehin oluwatimilehin marked this pull request as ready for review April 28, 2025 10:31
@bollu
Copy link
Collaborator

bollu commented Apr 28, 2025

@oluwatimilehin It seems the CI fails, and I conjecture it's because it's not setup to work well with forks. Could you close this PR, and reopen it as a branch against lean-mlir?

@oluwatimilehin
Copy link
Collaborator Author

Created #1153

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.

4 participants