moved array metadata to be behind a single pointer refrence #1550
Conversation
TomerStarkware
requested review from
Oppen,
azteca1998,
edg-l,
entropidelic,
gabrielbosio,
igaray,
jrchatruc and
pefontana
as code owners
|
@TomerStarkware there are some merge conflicts, could you please solve them? Thanks! |
TomerStarkware
force-pushed
the
tomer/move_array_metadata
branch
from
eac395d to
e0f8e0b
Compare
TomerStarkware
left a comment
TomerStarkware
left a comment
There was a problem hiding this comment.
done
@TomerStarkware made 1 comment.
Reviewable status: 0 of 5 files reviewed, all discussions resolved (waiting on @azteca1998, @edg-l, @entropidelic, @gabrielbosio, @igaray, @jrchatruc, @Oppen, and @pefontana).
|
@TomerStarkware There are some failing CI jobs you have to fix |
JulianGCalderon
left a comment
JulianGCalderon
left a comment
There was a problem hiding this comment.
Hey @TomerStarkware! I left some comments.
There was a problem hiding this comment.
Please update this file's top-level documentation with the updated layout.
| // Load max_len from metadata at offset 4 (after refcount) | ||
| let max_len_ptr = block.gep( | ||
| context, | ||
| location, | ||
| array_ptr, | ||
| &[GepIndex::Const( | ||
| -((refcount_offset - size_of::<u32>()) as i32), | ||
| )], | ||
| metadata_ptr, | ||
| &[GepIndex::Const(size_of::<u32>() as i32)], | ||
| IntegerType::new(context, 8).into(), | ||
| )?; |
There was a problem hiding this comment.
Instead of calculating max_len_ptr manually, we can let LLVM do that by declaring a type !llvm.struct<i32, i32, i32, !llvm.ptr> and specifying the field index. Note that gep assumes that the type is an array, so you should specify (0, 1) as the index.
| &[GepIndex::Const(-(refcount_offset as i32))], | ||
| IntegerType::new(context, 8).into(), | ||
| )?; | ||
| // Metadata struct: { refcount: u32, max_len: u32, capacity: u32, data_ptr: *mut u8 } |
There was a problem hiding this comment.
The comment includes capacity as a field, but the ArrayMetadata struct does not. What is the difference between max_len and capacity?
| struct ArrayMetadata<T> { | ||
| refcount: u32, | ||
| max_len: u32, | ||
| data_ptr: *mut T, | ||
| } |
There was a problem hiding this comment.
Can we reuse ArrayMetadata from types::arrays?
3 participants
Title
Closes #NA
Introduces Breaking Changes?
No
starknet-blocks.ymlworkflow to use these PRs.These PRs should be merged after this one right away, in that order.
Checklist
This change is