-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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
Spill fewer temps on iv writes #9974
Conversation
I tried this code: class Foo
def initialize
@a = 1
@b = 1
@c = 1
end
end
def foo
50.times { Foo.new }
end
foo
puts RubyVM::YJIT.disasm(Foo.instance_method(:initialize)) Disasm before this patch:
Disasm after this patch:
The code to store temps is eliminated. |
4d4a5db
to
c08f9ad
Compare
Not all IV writes require calling a C function. If we don't need to execute a write barrier (IOW the written value is an immediate), and we don't need to expand the object to accommodate a new IV, we won't need to make a C call and we can avoid spilling temps.
c08f9ad
to
d7e6273
Compare
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 ended up rewriting the whole patch, but this looks good
This does skip spilling temps when ivar_index is known and the written value is immediate. This seems like the only obvious improvement on the current setivar.
@k0kubun lol thank you! |
As Maxime shared, this was a nice change for Optcarrot.
|
Railsbench is also 75% faster on speed.yjit.org now. Liquid-render 2.56x, pretty impressive! Though Alan's work on nibbling at exits may have something to do with that too :) |
Not all IV writes require calling a C function. If we don't need to execute a write barrier (IOW the written value is an immediate), and we don't need to expand the object to accommodate a new IV, we won't need to make a C call and we can avoid spilling temps.