-
Notifications
You must be signed in to change notification settings - Fork 55
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
Split large memory accesses in stack alloc #741
base: main
Are you sure you want to change the base?
Conversation
opam strikes back. You used a feature (symmetry of disequality) that is not available before Coq 8.18 (cf. coq/coq#17025), and opam tests 8.16. The stupid fix is to call a lemma instead of symmetry, but maybe it is time to increase the lowest supported version to 8.18? |
I will use the lemma, thanks! |
6f36509
to
8585a1c
Compare
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 strongly suspect that we will have to wrote the same code for risc-V.
So maybe we should take the time to make the code a little bit more modular.
8585a1c
to
d6f0923
Compare
compiler/src/compile.ml
Outdated
then Some (UseLea, "LEA instruction is used") | ||
else None | ||
| Split_memory_access -> | ||
let msg = |
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.
Can you add an option like it is done with Glob_options.lea, to say if we print the warning or not ?
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.
Would this flag control the warning only or whether we split accesses as well?
d6f0923
to
8b37b53
Compare
8b37b53
to
3a4a8b4
Compare
When stack allocation replaces
x = e
by[rsp + off] = e
, the offset may be too large. I introduced a check at everyCopn
that splits this memory access into several instructions in ARM. I also added a warning. Note that whenx
is an array set, this can be fixed manually withreg ptr
s, but if it's a variable there's no way of doing so (but this scenario is probably far-fetched).