-
Notifications
You must be signed in to change notification settings - Fork 93
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
Ltac2 local2ptree #712
base: master
Are you sure you want to change the base?
Ltac2 local2ptree #712
Conversation
One note: I am using |
I suggest: Add a brief comment just before the Std.lazy line (if that's the right place?), saying (* use lazy instead of cbv because Qed can be faster *) |
I suggest, at line 71 augment your comment with, "Normally this makes a significant but small improvement in performance; but when there are many local variables (e.g. 60) it makes a huge difference." |
I don't understand this comment. The difference between |
Message.Format.kfprintf (fun x => Control.throw (Invalid_argument (Some x))) fmt. | ||
Ltac2 Notation "throw_invalid_argument" fmt(format) := throw_invalid_argument fmt. | ||
|
||
(* If one works with computed symbol lists, one can't use the tactic notation for evaluation functions (which construct such records in a more concise way) *) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couldn't you do something like Ltac2 adjust_redflags_delta (syms : Std.reference list) (redflags : Std.red_flags) : Std.red_flags := { redflags with Std.rDelta := false; Std.rConst := syms }
and then make a notation so you can write adjust_redflags_delta [id] syms
or adjust_redflags_delta (beta iota) syms
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or { redflags with Std.rConst := List.app redflags.rConst syms }
and then you could do reduction_strategy:([<list of things>])
below and pass that around instead of starting with a list of references. Might be slightly more concise
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have to think about it. I tried a few things with notations, but none of this did work well in case one has mixed references, say local and global, manually specified and computed lists. In the end it was more effort and harder to understand than doing it low level. But meanwhile I am a bit smarter and can reconsider it.
Std.rConst := syms | ||
}. | ||
|
||
Ltac2 reference_of_constr(ref : constr) : Std.reference := |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems worth upstreaming into the Coq standard library, I've needed something like this a couple of times too
Regarding Anyway at some point I started to prefer lazy, but I can't exclude that this was caused by doing computations directly in hypothesis, which (afaik) doesn't leave an annotation. |
If such an example exists, it's a bug in Coq. The difference between lazy and cbv is evaluation ordering and sharing of reductions, but so long as Coq reduction is confluent (which it is widely believed to be, I think), they cannot give different results. |
Isn't it so that the argument of confluence only applies to full evaluation? We are doing partial evaluation here (with restricted delta lists). I mean I tried to make some trivial examples, but failed spending just a few minutes. If you believe my understanding of this is substantially flawed, I will spend some more time trying to understand this in more detail. |
Imagine wrapping the entire environment in a lambda abstracting over the constants to be left behind. If full reduction is confluent even under lambdas it should be confluent even with restricted delta lists. Less sure about leaving out the other reductions, though I think most of them can be modeled by replacing certain constructs with opaque applications.
You might be missing the fact that once there are no more redexes, |
(If you're interested in reading more, there's coq/coq#17503 (comment), but most of that is not relevant here) |
Also not sure if this is the difference of understanding, but in case it is:
|
And I guess a more accurate characterization here is that |
Regarding the
I guess this is so because cbv would compute in the complete compspecs, while with lazy evaluation the compsepcs is not evaluated since it is not required to get the representation type of void. So the comment is wrong, but I there are still good reasons in to prefer lazy over cbv in VST. |
Just out of curiosity, I examined this question: are the compspecs fully evaluated already? That is, would doing one extra "eval compute" in the make_compspecs tactic change anything? And the answer is, basically no. In a typical case, "progress compute" will fail to progress. |
@andrew-appel : yes, this matches what I believe is happening. lazy looks only at what it "needs" to compute the final result, while cbv goes at least once through everything. So especially in structures which are a mix of data and proof terms, lazy should be of advantage. |
This PR replaces local2ptree with a version written in Ltac2 which is much faster (in measurements about 100x for a case with about 60 local variables).
The difference in
make test
is anyway quite small (1..2%) - the time of local2ptree is more visible in developments with many local variables.Here are the
make test
times for the old and new version.