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

The instruction cache only supports execution from one segment #1529

Closed
odesenfans opened this issue Dec 27, 2023 · 2 comments
Closed

The instruction cache only supports execution from one segment #1529

odesenfans opened this issue Dec 27, 2023 · 2 comments
Labels
bug Something isn't working

Comments

@odesenfans
Copy link
Contributor

Describe the bug

In an effort to run the Cairo bootloader on top of the Cairo VM, I found out that the instruction cache used in vm_core.rs only considers execution from one segment. This hypothesis is not respected when running the bootloader as we can execute code from different segments.

In this situation, the bootloader and the program(s) it attempts to load pollute each other's instruction cache, which typically ends up with a crash of the VM.

Expected behavior
The cache should either be multi-dimensional or just not get in the way. I see several solutions:

  1. Use a HashMap<Relocatable, Option<Instruction>> instead of the vector used right now.
  2. Use a Vec<Vec<Option<Instruction>>: if solution 1 ends up hitting performance too badly, using a 2-D vector could be a solution.
  3. Limit the use of the cache to segment 0: just add a condition to prevent cache conflicts.

What do you think? There's also the option to introduce a caching policy to keep the current caching solution for most cases and use a more complex one when running the bootloader.

What version/commit are you on?
v0.9.1.

@odesenfans odesenfans added the bug Something isn't working label Dec 27, 2023
@Oppen
Copy link
Contributor

Oppen commented Dec 27, 2023

The cache is already side-stepped in main for non-zero segments. We need to make a new release including support for running programs from other segments.

@odesenfans
Copy link
Contributor Author

You're talking about #1493, right? That should work, I'll test it.

@fmoletta fmoletta closed this as completed Jan 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants