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

Pin pointer literals in code, and julia values in julia_to_scm #80

Merged
merged 2 commits into from
Dec 18, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/ast.c
Original file line number Diff line number Diff line change
Expand Up @@ -815,6 +815,7 @@ static value_t julia_to_list2_noalloc(fl_context_t *fl_ctx, jl_value_t *a, jl_va

static value_t julia_to_scm_(fl_context_t *fl_ctx, jl_value_t *v, int check_valid)
{
PTR_PIN(v);
value_t retval;
if (julia_to_scm_noalloc1(fl_ctx, v, &retval))
return retval;
Expand Down
1 change: 1 addition & 0 deletions src/cgutils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -578,6 +578,7 @@ static Value *literal_pointer_val(jl_codectx_t &ctx, jl_value_t *p)
{
if (p == NULL)
return Constant::getNullValue(ctx.types().T_pjlvalue);
PTR_PIN(p);
Value *pgv = literal_pointer_val_slot(ctx, p);
jl_aliasinfo_t ai = jl_aliasinfo_t::fromTBAA(ctx, ctx.tbaa().tbaa_const);
auto load = ai.decorateInst(maybe_mark_load_dereferenceable(
Expand Down
1 change: 1 addition & 0 deletions src/jitlayers.h
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,7 @@ void add_named_global(StringRef name, void *addr) JL_NOTSAFEPOINT;

static inline Constant *literal_static_pointer_val(const void *p, Type *T) JL_NOTSAFEPOINT
{
PTR_PIN((void*)p);
// this function will emit a static pointer into the generated code
// the generated code will only be valid during the current session,
// and thus, this should typically be avoided in new API's
Expand Down
2 changes: 2 additions & 0 deletions src/julia.h
Original file line number Diff line number Diff line change
Expand Up @@ -2755,6 +2755,8 @@ extern void mmtk_object_reference_write_slow(void* mutator, const void* parent,
#define MMTK_IMMORTAL_BUMP_ALLOCATOR (0)

// VO bit is required to support conservative stack scanning and moving.
// NB: We have to set VO bit even if this is a non_moving build. Otherwise, assertions in mmtk-core
Copy link
Member

Choose a reason for hiding this comment

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

The reason is that we enable mmtk/is_mmtk_object for all the builds. If the feature mmtk/is_mmtk_object is not enabled, VO bit is not used in MMTk and MMTk will not assert VO bit. We can disable the feature mmtk/is_mmtk_object for a non-moving build, and we will not need VO bit here. But I don't know if that is worth doing.

Copy link
Author

Choose a reason for hiding this comment

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

Did you measure the performance penalty of having the VO bit? If people don't care about fragmentation or if we want to do an apples to apples comparison with stock (both non-moving) I suppose we might want to disable the VO bit depending on whether it's too expensive to maintain it.

Copy link
Member

Choose a reason for hiding this comment

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

I didn't measure it with dev. You posted the results for v1.9.2 before: mmtk/mmtk-julia#158 (comment)

// will complain about seeing objects without VO bit.
#define MMTK_NEEDS_VO_BIT (1)

void mmtk_immortal_post_alloc_fast(MMTkMutatorContext* mutator, void* obj, size_t size);
Expand Down
Loading