-
Notifications
You must be signed in to change notification settings - Fork 12
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
Hint IO in the SDK #631
base: master
Are you sure you want to change the base?
Hint IO in the SDK #631
Conversation
ceno_host/src/lib.rs
Outdated
let (prefix, lengths, suffix) = unsafe { buf.align_to::<u32>() }; | ||
assert!(prefix.is_empty()); | ||
assert!(suffix.is_empty()); | ||
let mut iter = lengths.iter().map(|len| { |
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 would be part of our library, and not visible in normal guest code.
ceno_host/src/lib.rs
Outdated
Ok(()) | ||
} | ||
|
||
pub fn finalise(&self) -> SerialisedCenoStdin { |
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.
Sorry, this is a bit of a mess. I think it might be comparatively easier to understand the memory format by looking at the readers first, and then you can try to decipher this function. (Or you can wait for the cleanup up version. :)
The main complexity here comes from me having mixed all the padding logic throughout. What we are actually doing ain't very complicated conceptually.
ceno_host/src/lib.rs
Outdated
impl<'b> SerialisedCenoStdinIter<'b> { | ||
pub fn read<'a, T>(&'a mut self) -> &'b T | ||
where | ||
T: Portable + for<'c> CheckBytes<HighValidator<'c, Error>>, |
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.
In the guest we would change Error
to Failure
here. rancor::Error
keeps track of error messages, which is useful for the prototype and in the host. rancor::Failure
only keeps track of whether we have an error or not, so that's simpler and faster for the guest.
ceno_host/src/lib.rs
Outdated
println!("buf.len() after offset padding: {}", buf.len()); | ||
for (offset, item) in izip!(offsets, &self.items) { | ||
buf.extend_from_slice(item); | ||
buf.extend_from_slice(&vec![0; buf.len().next_multiple_of(16) - buf.len()]); |
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 guess it will be more neat if just call extend_from_slice once directly to buf.len().next_multiple_of(16) then use copy from slice to fill the prefix data.
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.
Thanks for the suggestion, let me look into that.
ceno_host/src/lib.rs
Outdated
} | ||
|
||
pub fn finalise(&self) -> SerialisedCenoStdin { | ||
// TODO: perhaps don't hardcode 16 here. |
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.
IIUC 16 means align with 16 "byte", which means 4 u32 word, I guess it's for rkyv format. But I am wondering does rkyv support configurable 4 bytes, i.e. 1 word? in this case all memory access still align u32 so we can use "LW" instruction to do everything and overall format still more compact
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.
Rkyv specifically wants 16 bytes (unless you use the unaligned
feature), which is 4 words on u32.
No worries, the alignment to 16 bytes is only a requirement for the backing storage. Internally rkyv only uses one eg 4 bytes to store a u32
in a struct or array somewhere, they don't blow it up to 16 bytes for every single element.
ceno_host/src/lib.rs
Outdated
where | ||
T: Portable + for<'c> CheckBytes<HighValidator<'c, Error>>, | ||
{ | ||
rkyv::access::<T, Error>(self.read_slice()).unwrap() |
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.
if I remember correctly, from your previous example, we can throw the whole deliberate slice into rkyv and iterating through it by rkyv iterator implementation, which means we dont need to implementing a wrapping iterator by ourself?
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.
Yes, in principle. But that only works, if we make sure that the end of the mapped data always shows up at a consistent fixed memory address. rkyv
finds its data from the end.
Since we decided that our VM can only put the start of our mapped data at a fixed consistent address, we would need to eg store the length of the mapped data at the fixed start of the region.
But once we are mucking around with bytes ourselves, we might as well do the whole outer layer by ourselves, since it's very simple.
There's some additional complexity required for directly storing rkyv-serialised data inside of rkyv in such a way that you can zero-copy deserialise both layers. The extra complexity is mostly around the 16 byte alignment requirement. That's why my original prototype used the unaligned
feature.
It's totally doable, but then probably not really simpler than coding up this outer layer ourselves.
However, I'm open to using rkyv for the outer layer as well, if you think that's cleaner. The code in here is just a prototype.
f0a4ce9
to
b789f43
Compare
Could you add some example that showcases how |
Yes, I already added them. Have a look at the PR description for the link. I can add some nob-standalone examples, too. |
Here we are implementing hints for the SDK. See this example Ceno application for how to use the new functionality.
See #607
Closes #639
Works towards #179