Skip to content

Accumulators as functions base - #22345

Draft
IBBXEF wants to merge 85 commits into
rocq-prover:masterfrom
IBBXEF:accumulators_as_functions_base
Draft

Accumulators as functions base#22345
IBBXEF wants to merge 85 commits into
rocq-prover:masterfrom
IBBXEF:accumulators_as_functions_base

Conversation

@IBBXEF

@IBBXEF IBBXEF commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Hi, this is the branch with the change in representation of accumulators

@IBBXEF
IBBXEF requested review from a team as code owners August 14, 2026 14:47
@coqbot-app

coqbot-app Bot commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

I am not triggering a CI run on this PR because the CI configuration has been modified. CI can be triggered manually by an authorized contributor.

@coqbot-app coqbot-app Bot added the needs: full CI The latest GitLab pipeline that ran was a light CI. Say "@coqbot run full ci" to get a full CI. label Aug 14, 2026
@ppedrot
ppedrot marked this pull request as draft August 14, 2026 14:49
@ppedrot

ppedrot commented Aug 14, 2026

Copy link
Copy Markdown
Member

@coqbot run full ci

@coqbot-app coqbot-app Bot removed the needs: full CI The latest GitLab pipeline that ran was a light CI. Say "@coqbot run full ci" to get a full CI. label Aug 14, 2026
@ppedrot

ppedrot commented Aug 14, 2026

Copy link
Copy Markdown
Member

@coqbot bench

Comment thread kernel/uint63_63.ml
(* Compiles an unsigned int to malfunction code *)
let compile_mlf i =
if i >= 0 then Printf.sprintf "(apply (global $Uint63 $of_int) %i)" i
else Printf.sprintf "(apply (global $Uint63 $of_int) (neg %i))" (-i)

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.

Does this code work properly for minint?

Comment thread kernel/nativevalues.ml
(** differentiates an accumulator from a closure. Should only be used on memory blocks. *)
and is_accu v =
let reference = Obj.repr @@ build_accu (Obj.magic 0) in (* we assume Ocaml will build all accumulators similarly *)
Obj.size v = Obj.size reference && Obj.field v 0 == Obj.field reference 0 (* we check the equality of the function pointer *)

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.

This seems quite fragile to me. Even with inline never, there is no guarantee that the function will be uniquely defined. (In fact, it is not uncommon for OCaml functions to be duplicated, so as to support both fast, direct calls and calls through first-order modules.) I don't know of a way to ensure that only a single occurrence exists. Perhaps the build_accu function should also be part of a once generated malfunction code?

Comment thread kernel/float64_common.ml
if f < 0. then Printf.sprintf "(apply (global $Float64 $of_float) neg_infinity)"
else Printf.sprintf "(apply (global $Float64 $of_float) infinity)"
end else if f < 0. then Printf.sprintf "(apply (global $Float64 $of_float) (neg.f64 %.17e))" (-. f) (* malfunction supports scientific notation *)
else Printf.sprintf "(apply (global $Float64 $of_float) %.17e)" f

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.

There have been bugs in the past when using the decimal scientific notation to perform a roundtrip (i.e., you would get a close but different number, especially for subnormal floating-point numbers). I don't know how safe it is nowadays. Does the malfunction compiler not support the hexadecimal notation?

@github-actions github-actions Bot added the needs: rebase Should be rebased on the latest master to solve conflicts or have a newer CI run. label Aug 16, 2026
IBBXEF and others added 26 commits August 19, 2026 12:45
… in a different function to stay compatible with extraction
…ger needed, and removed the useless import of Constructs
…ns that would break our accumulator detection
@IBBXEF
IBBXEF force-pushed the accumulators_as_functions_base branch from 6257002 to 516eeae Compare August 19, 2026 10:51
@coqbot-app

coqbot-app Bot commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

I am not triggering a CI run on this PR because the CI configuration has been modified. CI can be triggered manually by an authorized contributor.

@coqbot-app coqbot-app Bot added needs: full CI The latest GitLab pipeline that ran was a light CI. Say "@coqbot run full ci" to get a full CI. and removed needs: rebase Should be rebased on the latest master to solve conflicts or have a newer CI run. labels Aug 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs: full CI The latest GitLab pipeline that ran was a light CI. Say "@coqbot run full ci" to get a full CI.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants