Skip to content
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

Memory stack #288

Merged
merged 1 commit into from
Sep 26, 2024
Merged

Memory stack #288

merged 1 commit into from
Sep 26, 2024

Conversation

zherczeg
Copy link
Collaborator

This is not the only option to do this. If we could store the instance in a register (my long term plan), we could cache the memory in the instance. Then saving/restoring would be unnecessary.

@zherczeg zherczeg force-pushed the memory_stack branch 3 times, most recently from d6d7fc2 to 9883c3a Compare September 23, 2024 13:42
@zherczeg zherczeg marked this pull request as ready for review September 23, 2024 13:43
Create the cache only when necessary.

Signed-off-by: Zoltan Herczeg [email protected]
@zherczeg
Copy link
Collaborator Author

Patch is ready for review

@clover2123
Copy link
Collaborator

Although this patch is not relevant to it, but I wonder that if we are going to support multiple memories this caching strategy would be valid or easily updated for it?

@zherczeg
Copy link
Collaborator Author

To tell you the truth, I plan to rework this whole caching after #289, where I will remove most of these changes. However, I cannot do that in one step. The caching should be done by the Instance after that point. This will work well for multi memory, and the caching overhead will be smaller. Using the instance pointer, I can directly access the cache.

Copy link
Collaborator

@clover2123 clover2123 left a comment

Choose a reason for hiding this comment

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

LGTM

@clover2123 clover2123 merged commit ba0ad52 into Samsung:main Sep 26, 2024
14 checks passed
@zherczeg zherczeg deleted the memory_stack branch September 26, 2024 10:02
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.

2 participants