Skip to content

Introduce an "Inline IVAR cache" struct #2697

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

Closed
wants to merge 1 commit into from

Conversation

tenderlove
Copy link
Member

This commit introduces an "inline ivar cache" struct. The reason we
need this is so compaction can differentiate from an ivar cache and a
regular inline cache. Regular inline caches contain references to
VALUE and ivar caches just contain references to the ivar index. With
this new struct we can easily update references for inline caches (but
not inline var caches as they just contain an int)

@@ -16,6 +16,7 @@
"CDHASH" => %w[H TS_CDHASH],
"GENTRY" => %w[G TS_GENTRY],
"IC" => %w[K TS_IC],
Copy link
Contributor

Choose a reason for hiding this comment

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

IC should be also renamed.

Copy link
Member Author

Choose a reason for hiding this comment

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

Do you have a suggestion? IC is still used for opt_getinlinecache and opt_setinlinecache

ruby/insns.def

Lines 1038 to 1062 in e530cfd

/* push inline-cached value and go to dst if it is valid */
DEFINE_INSN
opt_getinlinecache
(OFFSET dst, IC ic)
()
(VALUE val)
{
if (vm_ic_hit_p(ic, GET_EP())) {
val = ic->ic_value.value;
JUMP(dst);
}
else {
val = Qnil;
}
}
/* set inline cache */
DEFINE_INSN
opt_setinlinecache
(IC ic)
(VALUE val)
(VALUE val)
{
vm_ic_update(ic, val, GET_EP());
}

I could rename, but IC matches the instruction names.

Copy link
Contributor

Choose a reason for hiding this comment

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

reading related code, we need to refactoring overall. I'll do it so you don't need to change them. thanks.

@ko1
Copy link
Contributor

ko1 commented Nov 27, 2019

ic_cref is not needed for ivar cache.

This commit introduces an "inline ivar cache" struct.  The reason we
need this is so compaction can differentiate from an ivar cache and a
regular inline cache.  Regular inline caches contain references to
`VALUE` and ivar caches just contain references to the ivar index.  With
this new struct we can easily update references for inline caches (but
not inline var caches as they just contain an int)
@tenderlove tenderlove force-pushed the separate-ic-and-ivar branch from 772f882 to 1a7bf3d Compare December 5, 2019 20:50
@tenderlove tenderlove closed this Dec 5, 2019
@tenderlove tenderlove deleted the separate-ic-and-ivar branch December 5, 2019 21:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants