merge revision(s) 43ff0c2c488c80aaf83b486d45bcd4a92ebe3848: [Backport #19299]

YJIT: Fix `yield` into block with >=30 locals on ARM

	It's a register spill issue. Fix by moving the Qnil fill snippet to
	after registers are released.

	[Bug #19299]
	---
	 test/ruby/test_yjit.rb | 14 ++++++++++++++
	 yjit/src/codegen.rs    | 29 ++++++++++++++---------------
	 2 files changed, 28 insertions(+), 15 deletions(-)
This commit is contained in:
NARUSE, Yui 2023-01-18 17:18:44 +09:00
Родитель 52ea5ea990
Коммит 97c32b49e2
3 изменённых файлов: 29 добавлений и 16 удалений

Просмотреть файл

@ -1028,6 +1028,20 @@ class TestYJIT < Test::Unit::TestCase
RUBY
end
def test_invokeblock_many_locals
# [Bug #19299]
assert_compiles(<<~'RUBY', result: :ok)
def foo
yield
end
foo do
a1=a2=a3=a4=a5=a6=a7=a8=a9=a10=a11=a12=a13=a14=a15=a16=a17=a18=a19=a20=a21=a22=a23=a24=a25=a26=a27=a28=a29=a30 = :ok
a30
end
RUBY
end
private
def code_gc_helpers

Просмотреть файл

@ -11,7 +11,7 @@
# define RUBY_VERSION_MINOR RUBY_API_VERSION_MINOR
#define RUBY_VERSION_TEENY 0
#define RUBY_RELEASE_DATE RUBY_RELEASE_YEAR_STR"-"RUBY_RELEASE_MONTH_STR"-"RUBY_RELEASE_DAY_STR
#define RUBY_PATCHLEVEL 6
#define RUBY_PATCHLEVEL 7
#include "ruby/version.h"
#include "ruby/internal/abi.h"

Просмотреть файл

@ -4455,19 +4455,6 @@ fn gen_push_frame(
};
asm.store(Opnd::mem(64, sp, SIZEOF_VALUE_I32 * -2), specval);
// Arm requires another register to load the immediate value of Qnil before storing it.
// So doing this after releasing the register for specval to avoid register spill.
let num_locals = frame.local_size;
if num_locals > 0 {
asm.comment("initialize locals");
// Initialize local variables to Qnil
for i in 0..num_locals {
let offs = (SIZEOF_VALUE as i32) * (i - num_locals - 3);
asm.store(Opnd::mem(64, sp, offs), Qnil.into());
}
}
// Write env flags at sp[-1]
// sp[-1] = frame_type;
asm.store(Opnd::mem(64, sp, SIZEOF_VALUE_I32 * -1), frame.frame_type.into());
@ -4505,6 +4492,20 @@ fn gen_push_frame(
asm.mov(cfp_opnd(RUBY_OFFSET_CFP_SELF), frame.recv);
asm.mov(cfp_opnd(RUBY_OFFSET_CFP_BLOCK_CODE), 0.into());
// This Qnil fill snippet potentially requires 2 more registers on Arm, one for Qnil and
// another for calculating the address in case there are a lot of local variables. So doing
// this after releasing the register for specval and the receiver to avoid register spill.
let num_locals = frame.local_size;
if num_locals > 0 {
asm.comment("initialize locals");
// Initialize local variables to Qnil
for i in 0..num_locals {
let offs = (SIZEOF_VALUE as i32) * (i - num_locals - 3);
asm.store(Opnd::mem(64, sp, offs), Qnil.into());
}
}
if set_sp_cfp {
// Saving SP before calculating ep avoids a dependency on a register
// However this must be done after referencing frame.recv, which may be SP-relative
@ -4778,8 +4779,6 @@ fn gen_return_branch(
}
}
/// Pushes arguments from an array to the stack that are passed with a splat (i.e. *args)
/// It optimistically compiles to a static size that is the exact number of arguments
/// needed for the function.