Move the PC regardless of the leaf flag (#8232)
Co-authored-by: Alan Wu <alansi.xingwu@shopify.com>
This commit is contained in:
parent
5bb9462285
commit
e210b899dc
Notes:
git
2023-08-17 03:28:54 +00:00
Merged-By: k0kubun <takashikkbn@gmail.com>
@ -10217,12 +10217,6 @@ dump_disasm_list_with_cursor(const LINK_ELEMENT *link, const LINK_ELEMENT *curr,
|
|||||||
fflush(stdout);
|
fflush(stdout);
|
||||||
}
|
}
|
||||||
|
|
||||||
bool
|
|
||||||
rb_insns_leaf_p(int i)
|
|
||||||
{
|
|
||||||
return insn_leaf_p(i);
|
|
||||||
}
|
|
||||||
|
|
||||||
int
|
int
|
||||||
rb_insn_len(VALUE insn)
|
rb_insn_len(VALUE insn)
|
||||||
{
|
{
|
||||||
|
35
gc.c
35
gc.c
@ -2423,42 +2423,7 @@ static void
|
|||||||
gc_event_hook_body(rb_execution_context_t *ec, rb_objspace_t *objspace, const rb_event_flag_t event, VALUE data)
|
gc_event_hook_body(rb_execution_context_t *ec, rb_objspace_t *objspace, const rb_event_flag_t event, VALUE data)
|
||||||
{
|
{
|
||||||
if (UNLIKELY(!ec->cfp)) return;
|
if (UNLIKELY(!ec->cfp)) return;
|
||||||
const VALUE *pc = ec->cfp->pc;
|
|
||||||
if (pc && VM_FRAME_RUBYFRAME_P(ec->cfp)) {
|
|
||||||
int prev_opcode = rb_vm_insn_addr2opcode((void *)*ec->cfp->iseq->body->iseq_encoded);
|
|
||||||
for (const VALUE *insn = ec->cfp->iseq->body->iseq_encoded; insn < pc; insn += rb_insn_len(prev_opcode)) {
|
|
||||||
prev_opcode = rb_vm_insn_addr2opcode((void *)*insn);
|
|
||||||
}
|
|
||||||
|
|
||||||
/* If the previous instruction is a leaf instruction, then the PC is
|
|
||||||
* the currently executing instruction. We should increment the PC
|
|
||||||
* because the source line is calculated with PC-1 in calc_pos.
|
|
||||||
*
|
|
||||||
* If the previous instruction is not a leaf instruction and the
|
|
||||||
* current instruction is not a leaf instruction, then the PC was
|
|
||||||
* incremented before the instruction was ran (meaning the currently
|
|
||||||
* executing instruction is actually the previous instruction), so we
|
|
||||||
* should not increment the PC otherwise we will calculate the source
|
|
||||||
* line for the next instruction.
|
|
||||||
*
|
|
||||||
* However, this implementation still has a bug. Consider the
|
|
||||||
* following situation:
|
|
||||||
*
|
|
||||||
* non-leaf
|
|
||||||
* leaf <-
|
|
||||||
*
|
|
||||||
* Where the PC currently points to a leaf instruction. We don't know
|
|
||||||
* which instruction we really are at since we could be at the non-leaf
|
|
||||||
* instruction (since it incremented the PC before executing the
|
|
||||||
* instruction). We could also be at the leaf instruction since the PC
|
|
||||||
* doesn't get incremented until the instruction finishes.
|
|
||||||
*/
|
|
||||||
if (rb_insns_leaf_p(prev_opcode)) {
|
|
||||||
ec->cfp->pc++;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
EXEC_EVENT_HOOK(ec, event, ec->cfp->self, 0, 0, 0, data);
|
EXEC_EVENT_HOOK(ec, event, ec->cfp->self, 0, 0, 0, data);
|
||||||
ec->cfp->pc = pc;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
#define gc_event_hook_available_p(objspace) ((objspace)->flags.has_hook)
|
#define gc_event_hook_available_p(objspace) ((objspace)->flags.has_hook)
|
||||||
|
@ -17,7 +17,6 @@ struct rb_iseq_struct; /* in vm_core.h */
|
|||||||
/* compile.c */
|
/* compile.c */
|
||||||
int rb_dvar_defined(ID, const struct rb_iseq_struct *);
|
int rb_dvar_defined(ID, const struct rb_iseq_struct *);
|
||||||
int rb_local_defined(ID, const struct rb_iseq_struct *);
|
int rb_local_defined(ID, const struct rb_iseq_struct *);
|
||||||
bool rb_insns_leaf_p(int i);
|
|
||||||
int rb_insn_len(VALUE insn);
|
int rb_insn_len(VALUE insn);
|
||||||
const char *rb_insns_name(int i);
|
const char *rb_insns_name(int i);
|
||||||
VALUE rb_insns_name_array(void);
|
VALUE rb_insns_name_array(void);
|
||||||
|
@ -225,6 +225,13 @@ class TestObjSpace < Test::Unit::TestCase
|
|||||||
assert_equal(__FILE__, ObjectSpace.allocation_sourcefile(o4))
|
assert_equal(__FILE__, ObjectSpace.allocation_sourcefile(o4))
|
||||||
assert_equal(line4, ObjectSpace.allocation_sourceline(o4))
|
assert_equal(line4, ObjectSpace.allocation_sourceline(o4))
|
||||||
|
|
||||||
|
# The line number should be based on newarray instead of getinstancevariable.
|
||||||
|
line5 = __LINE__; o5 = [ # newarray (leaf)
|
||||||
|
@ivar, # getinstancevariable (not leaf)
|
||||||
|
]
|
||||||
|
assert_equal(__FILE__, ObjectSpace.allocation_sourcefile(o5))
|
||||||
|
assert_equal(line5, ObjectSpace.allocation_sourceline(o5))
|
||||||
|
|
||||||
# [Bug #19482]
|
# [Bug #19482]
|
||||||
EnvUtil.under_gc_stress do
|
EnvUtil.under_gc_stress do
|
||||||
100.times do
|
100.times do
|
||||||
|
@ -24,7 +24,7 @@ INSN_ENTRY(<%= insn.name %>)
|
|||||||
<%= ope[:decl] %> = (<%= ope[:type] %>)GET_OPERAND(<%= i + 1 %>);
|
<%= ope[:decl] %> = (<%= ope[:type] %>)GET_OPERAND(<%= i + 1 %>);
|
||||||
% end
|
% end
|
||||||
# define INSN_ATTR(x) <%= insn.call_attribute(' ## x ## ') %>
|
# define INSN_ATTR(x) <%= insn.call_attribute(' ## x ## ') %>
|
||||||
const bool leaf = INSN_ATTR(leaf);
|
const bool MAYBE_UNUSED(leaf) = INSN_ATTR(leaf);
|
||||||
% insn.pops.reverse_each.with_index.reverse_each do |pop, i|
|
% insn.pops.reverse_each.with_index.reverse_each do |pop, i|
|
||||||
<%= pop[:decl] %> = <%= insn.cast_from_VALUE pop, "TOPN(#{i})"%>;
|
<%= pop[:decl] %> = <%= insn.cast_from_VALUE pop, "TOPN(#{i})"%>;
|
||||||
% end
|
% end
|
||||||
@ -35,7 +35,7 @@ INSN_ENTRY(<%= insn.name %>)
|
|||||||
% end
|
% end
|
||||||
|
|
||||||
/* ### Instruction preambles. ### */
|
/* ### Instruction preambles. ### */
|
||||||
if (! leaf) ADD_PC(INSN_ATTR(width));
|
ADD_PC(INSN_ATTR(width));
|
||||||
% if insn.handles_sp?
|
% if insn.handles_sp?
|
||||||
POPN(INSN_ATTR(popn));
|
POPN(INSN_ATTR(popn));
|
||||||
% end
|
% end
|
||||||
@ -68,7 +68,6 @@ INSN_ENTRY(<%= insn.name %>)
|
|||||||
VM_ASSERT(!RB_TYPE_P(TOPN(<%= i %>), T_MOVED));
|
VM_ASSERT(!RB_TYPE_P(TOPN(<%= i %>), T_MOVED));
|
||||||
% end
|
% end
|
||||||
% end
|
% end
|
||||||
if (leaf) ADD_PC(INSN_ATTR(width));
|
|
||||||
# undef INSN_ATTR
|
# undef INSN_ATTR
|
||||||
|
|
||||||
/* ### Leave the instruction. ### */
|
/* ### Leave the instruction. ### */
|
||||||
|
@ -10,31 +10,6 @@
|
|||||||
|
|
||||||
#include "iseq.h"
|
#include "iseq.h"
|
||||||
|
|
||||||
extern const bool rb_vm_insn_leaf_p[];
|
|
||||||
|
|
||||||
#ifdef RUBY_VM_INSNS_INFO
|
|
||||||
const bool rb_vm_insn_leaf_p[] = {
|
|
||||||
% RubyVM::Instructions.each_slice(20) do |insns|
|
|
||||||
<%= insns.map do |insn|
|
|
||||||
if insn.is_a?(RubyVM::BareInstructions)
|
|
||||||
insn.always_leaf? ? '1' : '0'
|
|
||||||
else
|
|
||||||
'0'
|
|
||||||
end
|
|
||||||
end.join(', ')
|
|
||||||
%>,
|
|
||||||
% end
|
|
||||||
};
|
|
||||||
#endif
|
|
||||||
|
|
||||||
CONSTFUNC(MAYBE_UNUSED(static bool insn_leaf_p(VALUE insn)));
|
|
||||||
|
|
||||||
bool
|
|
||||||
insn_leaf_p(VALUE insn)
|
|
||||||
{
|
|
||||||
return rb_vm_insn_leaf_p[insn];
|
|
||||||
}
|
|
||||||
|
|
||||||
// This is used to tell RJIT that this insn would be leaf if CHECK_INTS didn't exist.
|
// This is used to tell RJIT that this insn would be leaf if CHECK_INTS didn't exist.
|
||||||
// It should be used only when RUBY_VM_CHECK_INTS is directly written in insns.def.
|
// It should be used only when RUBY_VM_CHECK_INTS is directly written in insns.def.
|
||||||
static bool leafness_of_check_ints = false;
|
static bool leafness_of_check_ints = false;
|
||||||
|
@ -181,10 +181,8 @@ CC_SET_FASTPATH(const struct rb_callcache *cc, vm_call_handler func, bool enable
|
|||||||
/**********************************************************/
|
/**********************************************************/
|
||||||
|
|
||||||
#define CALL_SIMPLE_METHOD() do { \
|
#define CALL_SIMPLE_METHOD() do { \
|
||||||
rb_snum_t x = leaf ? INSN_ATTR(width) : 0; \
|
rb_snum_t insn_width = attr_width_opt_send_without_block(0); \
|
||||||
rb_snum_t y = attr_width_opt_send_without_block(0); \
|
ADD_PC(-insn_width); \
|
||||||
rb_snum_t z = x - y; \
|
|
||||||
ADD_PC(z); \
|
|
||||||
DISPATCH_ORIGINAL_INSN(opt_send_without_block); \
|
DISPATCH_ORIGINAL_INSN(opt_send_without_block); \
|
||||||
} while (0)
|
} while (0)
|
||||||
|
|
||||||
|
Loading…
x
Reference in New Issue
Block a user