-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Inverted marking of vector tuple type #15244
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -305,9 +305,13 @@ static Value *emit_unbox(Type *to, const jl_cgval_t &x, jl_value_t *jt) | |
if (jt == (jl_value_t*)jl_bool_type) | ||
return builder.CreateZExt(builder.CreateTrunc(builder.CreateLoad(p), T_int1), to); | ||
|
||
if (!x.isboxed) // stack has default alignment | ||
if (x.isboxed) | ||
return builder.CreateAlignedLoad(p, 16); // julia's gc gives 16-byte aligned addresses | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. did @jrevels' recent PR change this? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The PR only changes the array alignment and the GC only gives 16-byte aligned addresses for large enough objects. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is the alignment here still too large for small objects on 32bits? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, the alignment is too large if GC is not guaranteeing 16-byte alignment for small objects. For that matter, providing an alignment bigger than the object seems unlikely to provide any benefit, except for a few highly contrived cases. (Exercise left to the student :-). Can someone provide a concise summary of the GC alignment guarantees? E.g., a function that maps "object size" x "object LLVM alignment" -> "min guaranteed alignment in Julia heap"? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. On 64bit I believe we garentee 16-byte alignment for GC allocated memory for all objects except singleton (although you don't really need to load from it...) On 32bits, we garentee at least 4-byte alignment for objects smaller than 8 bytes, for anything larger than or equal to 8 bytes, we should have 16-byte alignment. Object with a non-zero size no larger than 4 bytes should also always be 8-byte aligned..... |
||
else if (jt) | ||
return build_load(p, jt); | ||
else | ||
// stack has default alignment | ||
return builder.CreateLoad(p); | ||
return builder.CreateAlignedLoad(p, 16); // julia's gc gives 16-byte aligned addresses | ||
} | ||
|
||
// unbox, trying to determine correct bitstype automatically | ||
|
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 branch doesn't make sense to me. since
ispointer
is set, the value is expected to be aT*
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.
ah, nevermind. because it's a VecElement, we know that idx is 0 (it has one element), in which case we would be emitting
gep V, 0, 0
, and can simply drop it as a no-op