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

send-pop optimisation #2100

Open
wants to merge 20 commits into
base: master
from
Open
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

@@ -409,6 +409,30 @@ class String; def =~ other; true; end; end
'true' =~ /true/
},

[ 'opt_RubyVM_return_value_is_used_', <<~'},', ], # {
def used
used = yield
return used == true
end

used do
next RubyVM.return_value_is_used? # <- HERE
end
},

[ 'opt_RubyVM_return_value_is_used_', <<~'},', ], # {
def notused
yield
rescue
return true
else
return false
end

notused do
raise unless RubyVM.return_value_is_used? # <- HERE
end
},
[ 'opt_call_c_function', 'Struct.new(:x).new.x = true', ],
]

@@ -417,6 +441,8 @@ class String; def =~ other; true; end; end

# with trace
tests.compact.each {|(insn, expr, *a)|
# opt_RubyVM_return_value_is_used_ is always true when traced
next if insn == 'opt_RubyVM_return_value_is_used_'
progn = "set_trace_func(proc{})\n" + expr
assert_equal 'true', progn, 'trace_' + insn, *a
}
293 compile.c
@@ -959,7 +959,7 @@ FIRST_ELEMENT(const LINK_ANCHOR *const anchor)
}

static LINK_ELEMENT *
LAST_ELEMENT(LINK_ANCHOR *const anchor)
LAST_ELEMENT(const LINK_ANCHOR *const anchor)
{
return anchor->last;
}
@@ -1152,6 +1152,7 @@ new_callinfo(rb_iseq_t *iseq, ID mid, int argc, unsigned int flag, struct rb_cal
ci->mid = mid;
ci->flag = flag;
ci->orig_argc = argc;
ci->compiled_frame_bits = 0;

if (kw_arg) {
ci->flag |= VM_CALL_KWARG;
@@ -2223,6 +2224,9 @@ iseq_set_sequence(rb_iseq_t *iseq, LINK_ANCHOR *const anchor)
"unknown operand type: %c", type);
return COMPILE_NG;
}
if (IS_INSN_ID(iobj, opt_bailout)) {
generated_iseq[code_index + 1] = sp;
}
}
if (add_insn_info(insns_info, positions, insns_info_index, code_index, iobj)) insns_info_index++;
code_index += len;
@@ -3253,6 +3257,10 @@ iseq_specialized_instruction(rb_iseq_t *iseq, INSN *iobj)
case idEmptyP: SP_INSN(empty_p);return COMPILE_OK;
case idSucc: SP_INSN(succ); return COMPILE_OK;
case idNot: SP_INSN(not); return COMPILE_OK;
default:
if (ci->mid == rb_intern("return_value_is_used?")) {
return SP_INSN(RubyVM_return_value_is_used_);
}
}
break;
case 1:
@@ -3309,6 +3317,282 @@ tailcallable_p(rb_iseq_t *iseq)
}
}

static bool
is_the_insn_sendish(const INSN *i)
{
const char *t = insn_op_types(INSN_OF(i));

for (int j = 0; t[j]; j++) {
if (t[j] == TS_CALLINFO) {
return true;
}
}
return false;
}

static enum rb_insn_purity
calc_purity(enum rb_insn_purity p, const INSN *i)
{
switch (INSN_OF(i)) {
case BIN(leave):
case BIN(jump):
case BIN(branchif):
case BIN(branchunless):
case BIN(branchnil):
/* bailout not possible for above insns */
return rb_insn_is_not_pure;

case BIN(opt_setinlinecache):
/* When we see setinlinecache that should be a part of
* getconstant-getconstant chain. We should not bail out in
* middle of that. */
return rb_insn_is_not_pure;

default:
if (is_the_insn_sendish(i)) {
/* CALL_CACHE is not filled yet; cannot determine purity. */
return rb_insn_is_not_pure;
}
else {
return purity_merge(p,
insn_purity_dispatch_casted(i->insn_id, i->operands));
}
}
}

static const INSN *
get_strictly_next_insn(const LINK_ELEMENT *e)
{
for (const LINK_ELEMENT *i = e->next; i; i = i->next) {
if (IS_INSN(i)) {
return (const INSN *)i;
}
}
return NULL;
}

static const LINK_ELEMENT *
fixup_bailout_point(const LINK_ELEMENT *e)
{
const INSN *i = IS_INSN(e) ? (const INSN *)e : NULL;
const INSN *j = e ? get_strictly_next_insn(e) : NULL;
const INSN *k = j ? get_strictly_next_insn(&j->link) : NULL;

/* We are going to bail out between `i` and `j`. Is that
* appropriate? Let's check. Returns NULL if we don't need to
* bailout. Otherwise returns a link element where do so.
*/

if (! j) {
/* There is no `j`... This is the very end of the iseq. */
return NULL;
}
else switch (INSN_OF(j)) {
case BIN(leave):
/* The point is immediately before leave; bailing out here
* makes no sense. Just leave normally. */
return NULL;

case BIN(nop):
case BIN(pop):
case BIN(putiseq):
case BIN(putnil):
case BIN(putobject):
case BIN(putself):
case BIN(putspecialobject):
case BIN(putstring):
if (IS_INSN_ID(k, leave)) {
/* We assume these sequences are super-lightweight that
* are not worth optimising. Note that `nop; leave;` can
* happen inside of ensure. */
return NULL;
}
else {
/* FALLTHROUGH */
}

default: ;
const LINK_ELEMENT *ret;

if (! i) {
ret = e; /* (non-insn) [HERE] */
}
else if (! is_the_insn_sendish(i)) {
ret = &i->link; /* (non-send) [HERE] */
}
else if (j && ! IS_INSN_ID(j, pop)) {
ret = &i->link; /* send [HERR] -> (non-pop) */
}
else if (k && ! IS_INSN_ID(k, leave)) {
ret = fixup_bailout_point(&j->link); /* send -> pop [HERE] -> (non-leave) */
}
else {
return NULL; /* send -> pop -> leave */
}
if (ret && ret->next && IS_LABEL(ret->next)) {
/* We should not bail out right before a jump destination. */
return ret->next;
}
else {
return ret;
}
}
}

static void
iseq_insert_bailouts(rb_iseq_t *iseq, const LINK_ANCHOR *anchor)
{
enum rb_insn_purity p = rb_insn_is_pure;
const INSN *leave = NULL;

for (const LINK_ELEMENT *e = LAST_ELEMENT(anchor); e; e = e->prev) {
if (IS_INSN(e) && IS_INSN_ID(e, opt_bailout)) {
/* This happens during rb_iseq_build_from_ary()
* we should not modify the sequence here. */
return;
}
}

for (const LINK_ELEMENT *e = LAST_ELEMENT(anchor); e; e = e->prev) {
INSN *i = (INSN *)e;
const LINK_ELEMENT *j;

if (! IS_INSN(e)) {
continue;
}
else if (! leave) {
goto prev_insn;
}
else if ((p = calc_purity(p, i)) == rb_insn_is_pure) {
continue; /* Still subject to skip */
}
else if ((j = fixup_bailout_point(&i->link)) != NULL) {
/* Got it; we can bail out here */
if (IS_INSN(j)) {
i = (INSN *)j;
}
else {
i = (INSN *)get_strictly_next_insn(j->prev);
}
int line = i->insn_info.line_no;
INSERT_AFTER_INSN1(i, line, opt_bailout, INT2FIX(0));
}

prev_insn:
/* Go find another leave insn, until we hit the first
* element. */
leave = i && IS_INSN_ID(i, leave) ? i : NULL;
p = rb_insn_is_pure;
}

if (leave) {
/* bail out candidate reached the entry point */
const LINK_ELEMENT *i = fixup_bailout_point(FIRST_ELEMENT(anchor));

if (i) {
INSN *j = (INSN *)get_strictly_next_insn(i);
int line = j->insn_info.line_no;
INSERT_BEFORE_INSN1(j, line, opt_bailout, INT2FIX(0));
}
}
}

static struct rb_call_info *
elem2ci(const LINK_ELEMENT *e)
{
/* There are situations when the instruction have multiple call
* info. We are interested in the last one here. */
struct rb_call_info *ci = NULL;

if (IS_INSN(e)) {
const char *t = insn_op_types(INSN_OF(e));

for (int j = 0; t[j]; j++) {
if (t[j] == TS_CALLINFO) {
ci = (struct rb_call_info *)OPERAND_AT(e, j);
}
}
}

return ci;
}

static void
iseq_sendpop_optimization_phase1(const LINK_ANCHOR *anchor)
{
for (const LINK_ELEMENT *e = FIRST_ELEMENT(anchor); e->next; e = e->next) {
struct rb_call_info *ci = elem2ci(e);

if (! ci) {
continue;
}
else if (! IS_INSN(e->next)) {
continue;
}
else if (! IS_INSN_ID(e->next, pop)) {
/* We are going to optimize only if send and pop are
* directly adjacent i.e. no jump labels are between. If
* the pop is a jump destination that shall not be
* optimized out. */
continue;
}
else {
ci->compiled_frame_bits |= VM_FRAME_FLAG_POPIT;
}
}
}

static void
iseq_sendpop_optimization_phase3(const LINK_ANCHOR *anchor, int do_si)
{
for (const LINK_ELEMENT *e = FIRST_ELEMENT(anchor); e->next; e = e->next) {
struct rb_call_info *ci = elem2ci(e);

if (! ci) {
continue;
}
else if (! (ci->compiled_frame_bits & VM_FRAME_FLAG_POPIT)) {
continue;
}
else if (! IS_INSN(e->next)) {
continue;
}
else if (! LIKELY(do_si)) {
/* We don't optimize. Be tidy. */
ci->compiled_frame_bits &= ~VM_FRAME_FLAG_POPIT;
}
else if (! LIKELY(IS_INSN_ID(e->next, pop))) {
/* Don't know if we reach here but the flag shall be
* wiped because we do use the return value this case.
*/
ci->compiled_frame_bits &= ~VM_FRAME_FLAG_POPIT;
}
else {
ELEM_REMOVE(e->next);
}
}
}

static void
iseq_construct_compiled_frame_bits(const LINK_ANCHOR *anchor)
{
for (const LINK_ELEMENT *e = FIRST_ELEMENT(anchor); e->next; e = e->next) {
struct rb_call_info *ci = elem2ci(e);

if (ci) {
const LINK_ELEMENT *i = get_next_insn((INSN *)e);

if (i && IS_INSN_ID(i, pop)) {
ci->compiled_frame_bits |= VM_FRAME_FLAG_POPPED;
}
else if (ci->compiled_frame_bits & VM_FRAME_FLAG_POPIT) {
/* POPIT implies POPED */
ci->compiled_frame_bits |= VM_FRAME_FLAG_POPPED;
}
}
}
}

static int
iseq_optimize(rb_iseq_t *iseq, LINK_ANCHOR *const anchor)
{
@@ -3322,6 +3606,7 @@ iseq_optimize(rb_iseq_t *iseq, LINK_ANCHOR *const anchor)
int tailcallopt = do_tailcallopt;

list = FIRST_ELEMENT(anchor);
iseq_sendpop_optimization_phase1(anchor);

while (list) {
if (IS_INSN(list)) {
@@ -3348,6 +3633,10 @@ iseq_optimize(rb_iseq_t *iseq, LINK_ANCHOR *const anchor)
}
list = list->next;
}

iseq_sendpop_optimization_phase3(anchor, do_si);
iseq_insert_bailouts(iseq, anchor);
iseq_construct_compiled_frame_bits(anchor);
return COMPILE_OK;
}

@@ -7680,6 +7969,8 @@ insn_data_to_s_detail(INSN *iobj)
struct rb_call_info *ci = (struct rb_call_info *)OPERAND_AT(iobj, j);
rb_str_cat2(str, "<callinfo:");
if (ci->mid) rb_str_catf(str, "%"PRIsVALUE, rb_id2str(ci->mid));
if (ci->compiled_frame_bits)
rb_str_catf(str, "(%#x)", ci->compiled_frame_bits);
rb_str_catf(str, ", %d>", ci->orig_argc);
break;
}
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.