Skip to content

only lookup __[set/get/del]slice__ for slice AST nodes #1280

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Jul 8, 2016
Merged

only lookup __[set/get/del]slice__ for slice AST nodes #1280

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2edfc49
fix crash in setitem rewrite path
undingen Jul 3, 2016
e4938dd
only lookup __[set/get/del]slice__ for slice AST nodes
undingen Jul 3, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 28 additions & 30 deletions src/asm_writing/rewriter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -287,24 +287,29 @@ void RewriterVar::addGuard(uint64_t val) {
rewriter->addAction([=]() { rewriter->_addGuard(this, val_var); }, { this, val_var }, ActionType::GUARD);
}

void Rewriter::_nextSlotJump(bool condition_eq) {
void Rewriter::_nextSlotJump(assembler::ConditionCode condition) {
// If a jump offset is larger then 0x80 the instruction encoding requires 6bytes instead of 2bytes.
// This adds up quickly, thats why we will try to find another jump to the slowpath with the same condition with a
// smaller offset and jump to it / use it as a trampoline.
// The benchmark show that this increases the performance slightly even though it introduces additional jumps.
int& last_jmp_offset = condition_eq ? offset_eq_jmp_next_slot : offset_ne_jmp_next_slot;
auto condition = condition_eq ? assembler::COND_EQUAL : assembler::COND_NOT_EQUAL;
int last_jmp_offset = -1;
for (auto it = next_slot_jmps.rbegin(), it_end = next_slot_jmps.rend(); it != it_end; ++it) {
if (std::get<2>(*it) == condition) {
last_jmp_offset = std::get<0>(*it);
break;
}
}

if (last_jmp_offset != -1 && assembler->bytesWritten() - last_jmp_offset < 0x80) {
assembler->jmp_cond(assembler::JumpDestination::fromStart(last_jmp_offset), condition);
} else {
last_jmp_offset = assembler->bytesWritten();
int last_jmp_offset = assembler->bytesWritten();
assembler->jmp_cond(assembler::JumpDestination::fromStart(rewrite->getSlotSize()), condition);
next_slot_jmps.emplace_back(last_jmp_offset, assembler->bytesWritten(), condition);
}
}

void Rewriter::_addGuard(RewriterVar* var, RewriterVar* val_constant) {
void Rewriter::_addGuard(RewriterVar* var, RewriterVar* val_constant, bool negate) {
if (LOG_IC_ASSEMBLY)
assembler->comment("_addGuard");

Expand All @@ -316,12 +321,15 @@ void Rewriter::_addGuard(RewriterVar* var, RewriterVar* val_constant) {
assembler::Register reg = val_constant->getInReg(Location::any(), true, /* otherThan */ var_reg);
assembler->cmp(var_reg, reg);
} else {
assembler->cmp(var_reg, assembler::Immediate(val));
if (val == 0)
assembler->test(var_reg, var_reg);
else
assembler->cmp(var_reg, assembler::Immediate(val));
}

restoreArgs(); // can only do movs, doesn't affect flags, so it's safe
assertArgsInPlace();
_nextSlotJump(false /*= not equal jmp */);
_nextSlotJump(negate ? assembler::COND_EQUAL : assembler::COND_NOT_EQUAL);

var->bumpUse();
val_constant->bumpUse();
Expand All @@ -333,32 +341,24 @@ void RewriterVar::addGuardNotEq(uint64_t val) {
STAT_TIMER(t0, "us_timer_rewriter", 10);

RewriterVar* val_var = rewriter->loadConst(val);
rewriter->addAction([=]() { rewriter->_addGuardNotEq(this, val_var); }, { this, val_var }, ActionType::GUARD);
rewriter->addAction([=]() { rewriter->_addGuard(this, val_var, true /* negate */); }, { this, val_var },
ActionType::GUARD);
}

void Rewriter::_addGuardNotEq(RewriterVar* var, RewriterVar* val_constant) {
if (LOG_IC_ASSEMBLY)
assembler->comment("_addGuardNotEq");
void RewriterVar::addGuardNotLt0() {
rewriter->addAction([=]() {
assembler::Register var_reg = this->getInReg();
rewriter->assembler->test(var_reg, var_reg);

assert(val_constant->is_constant);
uint64_t val = val_constant->constant_value;
rewriter->restoreArgs(); // can only do movs, doesn't affect flags, so it's safe
rewriter->assertArgsInPlace();

assembler::Register var_reg = var->getInReg();
if (isLargeConstant(val)) {
assembler::Register reg = val_constant->getInReg(Location::any(), true, /* otherThan */ var_reg);
assembler->cmp(var_reg, reg);
} else {
assembler->cmp(var_reg, assembler::Immediate(val));
}

restoreArgs(); // can only do movs, doesn't affect flags, so it's safe
assertArgsInPlace();
_nextSlotJump(true /*= equal jmp */);
rewriter->_nextSlotJump(assembler::COND_SIGN);

var->bumpUse();
val_constant->bumpUse();
bumpUse();
rewriter->assertConsistent();

assertConsistent();
}, { this }, ActionType::GUARD);
}

void RewriterVar::addAttrGuard(int offset, uint64_t val, bool negate) {
Expand Down Expand Up @@ -405,7 +405,7 @@ void Rewriter::_addAttrGuard(RewriterVar* var, int offset, RewriterVar* val_cons

restoreArgs(); // can only do movs, doesn't affect flags, so it's safe
assertArgsInPlace();
_nextSlotJump(negate);
_nextSlotJump(negate ? assembler::COND_EQUAL : assembler::COND_NOT_EQUAL);

var->bumpUse();
val_constant->bumpUse();
Expand Down Expand Up @@ -2220,8 +2220,6 @@ Rewriter::Rewriter(std::unique_ptr<ICSlotRewrite> rewrite, int num_args, const L
marked_inside_ic(false),
done_guarding(false),
last_guard_action(-1),
offset_eq_jmp_next_slot(-1),
offset_ne_jmp_next_slot(-1),
allocatable_regs(std_allocatable_regs) {
initPhaseCollecting();

Expand Down
8 changes: 3 additions & 5 deletions src/asm_writing/rewriter.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ class RewriterVar {

void addGuard(uint64_t val);
void addGuardNotEq(uint64_t val);
void addGuardNotLt0();
void addAttrGuard(int offset, uint64_t val, bool negate = false);
RewriterVar* getAttr(int offset, Location loc = Location::any(), assembler::MovType type = assembler::MovType::Q);
// getAttrFloat casts to double (maybe I should make that separate?)
Expand Down Expand Up @@ -502,8 +503,6 @@ class Rewriter : public ICSlotRewrite::CommitHook {
}

int last_guard_action;
int offset_eq_jmp_next_slot;
int offset_ne_jmp_next_slot;

// keeps track of all jumps to the next slot so we can patch them if the size of the current slot changes
std::vector<NextSlotJumpInfo> next_slot_jmps;
Expand Down Expand Up @@ -540,7 +539,7 @@ class Rewriter : public ICSlotRewrite::CommitHook {

bool finishAssembly(int continue_offset, bool& should_fill_with_nops, bool& variable_size_slots) override;

void _nextSlotJump(bool condition_eq);
void _nextSlotJump(assembler::ConditionCode condition);
void _trap();
void _loadConst(RewriterVar* result, int64_t val);
void _setupCall(bool has_side_effects, llvm::ArrayRef<RewriterVar*> args = {},
Expand All @@ -557,8 +556,7 @@ class Rewriter : public ICSlotRewrite::CommitHook {
void _checkAndThrowCAPIException(RewriterVar* r, int64_t exc_val, assembler::MovType size);

// The public versions of these are in RewriterVar
void _addGuard(RewriterVar* var, RewriterVar* val_constant);
void _addGuardNotEq(RewriterVar* var, RewriterVar* val_constant);
void _addGuard(RewriterVar* var, RewriterVar* val_constant, bool negate = false);
void _addAttrGuard(RewriterVar* var, int offset, RewriterVar* val_constant, bool negate = false);
void _getAttr(RewriterVar* result, RewriterVar* var, int offset, Location loc = Location::any(),
assembler::MovType type = assembler::MovType::Q);
Expand Down
2 changes: 1 addition & 1 deletion src/capi/typeobject.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1242,7 +1242,7 @@ static int slot_tp_descr_set(PyObject* self, PyObject* target, PyObject* value)

PyObject* slot_sq_item(PyObject* self, Py_ssize_t i) noexcept {
STAT_TIMER(t0, "us_timer_slot_sqitem", SLOT_AVOIDABILITY(self));
return getitemInternal<CAPI>(self, autoDecref(boxInt(i)));
return PyObject_GetItem(self, autoDecref(boxInt(i)));
}

/* Pyston change: static */ Py_ssize_t slot_sq_length(PyObject* self) noexcept {
Expand Down
77 changes: 64 additions & 13 deletions src/codegen/ast_interpreter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -554,12 +554,27 @@ void ASTInterpreter::doStore(AST_expr* node, STOLEN(Value) value) {
Value target = visit_expr(subscript->value);
AUTO_DECREF(target.o);

Value slice = visit_slice(subscript->slice);
AUTO_DECREF(slice.o);
bool is_slice = (subscript->slice->type == AST_TYPE::Slice) && (((AST_Slice*)subscript->slice)->step == NULL);
if (is_slice) {
AST_Slice* slice = (AST_Slice*)subscript->slice;
Value lower = slice->lower ? visit_expr(slice->lower) : Value();
AUTO_XDECREF(lower.o);
Value upper = slice->upper ? visit_expr(slice->upper) : Value();
AUTO_XDECREF(upper.o);
assert(slice->step == NULL);

if (jit)
jit->emitSetItem(target, slice, value);
setitem(target.o, slice.o, value.o);
if (jit)
jit->emitAssignSlice(target, lower, upper, value);
assignSlice(target.o, lower.o, upper.o, value.o);
} else {
Value slice = visit_slice(subscript->slice);

AUTO_DECREF(slice.o);

if (jit)
jit->emitSetItem(target, slice, value);
setitem(target.o, slice.o, value.o);
}
} else {
RELEASE_ASSERT(0, "not implemented");
}
Expand Down Expand Up @@ -1321,11 +1336,26 @@ Value ASTInterpreter::visit_delete(AST_Delete* node) {
AST_Subscript* sub = (AST_Subscript*)target_;
Value value = visit_expr(sub->value);
AUTO_DECREF(value.o);
Value slice = visit_slice(sub->slice);
AUTO_DECREF(slice.o);
if (jit)
jit->emitDelItem(value, slice);
delitem(value.o, slice.o);

bool is_slice = (sub->slice->type == AST_TYPE::Slice) && (((AST_Slice*)sub->slice)->step == NULL);
if (is_slice) {
AST_Slice* slice = (AST_Slice*)sub->slice;
Value lower = slice->lower ? visit_expr(slice->lower) : Value();
AUTO_XDECREF(lower.o);
Value upper = slice->upper ? visit_expr(slice->upper) : Value();
AUTO_XDECREF(upper.o);
assert(slice->step == NULL);

if (jit)
jit->emitAssignSlice(value, lower, upper, jit->imm(0ul));
assignSlice(value.o, lower.o, upper.o, NULL);
} else {
Value slice = visit_slice(sub->slice);
AUTO_DECREF(slice.o);
if (jit)
jit->emitDelItem(value, slice);
delitem(value.o, slice.o);
}
break;
}
case AST_TYPE::Attribute: {
Expand Down Expand Up @@ -1735,10 +1765,31 @@ Value ASTInterpreter::visit_name(AST_Name* node) {
Value ASTInterpreter::visit_subscript(AST_Subscript* node) {
Value value = visit_expr(node->value);
AUTO_DECREF(value.o);
Value slice = visit_slice(node->slice);
AUTO_DECREF(slice.o);

return Value(getitem(value.o, slice.o), jit ? jit->emitGetItem(node, value, slice) : NULL);
bool is_slice = (node->slice->type == AST_TYPE::Slice) && (((AST_Slice*)node->slice)->step == NULL);
if (is_slice) {
AST_Slice* slice = (AST_Slice*)node->slice;
Value lower = slice->lower ? visit_expr(slice->lower) : Value();
AUTO_XDECREF(lower.o);
Value upper = slice->upper ? visit_expr(slice->upper) : Value();
AUTO_XDECREF(upper.o);
assert(slice->step == NULL);

Value v;
if (jit)
v.var = jit->emitApplySlice(value, lower, upper);
v.o = applySlice(value.o, lower.o, upper.o);
return v;
} else {
Value slice = visit_slice(node->slice);
AUTO_DECREF(slice.o);

Value v;
if (jit)
v.var = jit->emitGetItem(node, value, slice);
v.o = getitem(value.o, slice.o);
return v;
}
}

Value ASTInterpreter::visit_list(AST_List* node) {
Expand Down
17 changes: 17 additions & 0 deletions src/codegen/baseline_jit.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,14 @@ RewriterVar* JitFragmentWriter::emitAugbinop(AST_expr* node, RewriterVar* lhs, R
return emitPPCall((void*)augbinop, { lhs, rhs, imm(op_type) }, 2 * 320, node).first->setType(RefType::OWNED);
}

RewriterVar* JitFragmentWriter::emitApplySlice(RewriterVar* target, RewriterVar* lower, RewriterVar* upper) {
if (!lower)
lower = imm(0ul);
if (!upper)
upper = imm(0ul);
return emitPPCall((void*)applySlice, { target, lower, upper }, 256).first->setType(RefType::OWNED);
}

RewriterVar* JitFragmentWriter::emitBinop(AST_expr* node, RewriterVar* lhs, RewriterVar* rhs, int op_type) {
return emitPPCall((void*)binop, { lhs, rhs, imm(op_type) }, 2 * 240, node).first->setType(RefType::OWNED);
}
Expand Down Expand Up @@ -553,6 +561,15 @@ RewriterVar* JitFragmentWriter::emitYield(RewriterVar* v) {
return rtn;
}

void JitFragmentWriter::emitAssignSlice(RewriterVar* target, RewriterVar* lower, RewriterVar* upper,
RewriterVar* value) {
if (!lower)
lower = imm(0ul);
if (!upper)
upper = imm(0ul);
emitPPCall((void*)assignSlice, { target, lower, upper, value }, 256).first;
}

void JitFragmentWriter::emitDelAttr(RewriterVar* target, BoxedString* attr) {
emitPPCall((void*)delattr, { target, imm(attr) }, 144).first;
}
Expand Down
2 changes: 2 additions & 0 deletions src/codegen/baseline_jit.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,7 @@ class JitFragmentWriter : public Rewriter {
RewriterVar* imm(void* val);

RewriterVar* emitAugbinop(AST_expr* node, RewriterVar* lhs, RewriterVar* rhs, int op_type);
RewriterVar* emitApplySlice(RewriterVar* target, RewriterVar* lower, RewriterVar* upper);
RewriterVar* emitBinop(AST_expr* node, RewriterVar* lhs, RewriterVar* rhs, int op_type);
RewriterVar* emitCallattr(AST_expr* node, RewriterVar* obj, BoxedString* attr, CallattrFlags flags,
const llvm::ArrayRef<RewriterVar*> args, std::vector<BoxedString*>* keyword_names);
Expand Down Expand Up @@ -275,6 +276,7 @@ class JitFragmentWriter : public Rewriter {
std::vector<RewriterVar*> emitUnpackIntoArray(RewriterVar* v, uint64_t num);
RewriterVar* emitYield(RewriterVar* v);

void emitAssignSlice(RewriterVar* target, RewriterVar* lower, RewriterVar* upper, RewriterVar* value);
void emitDelAttr(RewriterVar* target, BoxedString* attr);
void emitDelGlobal(BoxedString* name);
void emitDelItem(RewriterVar* target, RewriterVar* slice);
Expand Down
41 changes: 25 additions & 16 deletions src/codegen/compvars.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,9 @@ class UnknownType : public ConcreteCompilerType {

CompilerVariable* getitem(IREmitter& emitter, const OpInfo& info, ConcreteCompilerVariable* var,
CompilerVariable* slice) override {
ExceptionStyle target_exception_style = info.preferredExceptionStyle();
bool do_patchpoint = ENABLE_ICGETITEMS;

if (slice->getType() == UNBOXED_SLICE) {
UnboxedSlice slice_val = extractSlice(slice);

Expand All @@ -340,31 +343,37 @@ class UnknownType : public ConcreteCompilerType {
= var->getType()->getattrType(attr, true)->callType(ArgPassSpec(1), { SLICE }, NULL);
assert(return_type->getConcreteType() == return_type);

if (return_type != UNDEF) {
llvm::Value* cstart, *cstop;
cstart = slice_val.start ? slice_val.start->makeConverted(emitter, UNKNOWN)->getValue()
: emitter.setType(getNullPtr(g.llvm_value_type_ptr), RefType::BORROWED);
cstop = slice_val.stop ? slice_val.stop->makeConverted(emitter, UNKNOWN)->getValue()
: emitter.setType(getNullPtr(g.llvm_value_type_ptr), RefType::BORROWED);

llvm::Value* r = emitter.createCall3(info.unw_info, g.funcs.apply_slice, var->getValue(), cstart,
cstop, CAPI, getNullPtr(g.llvm_value_type_ptr));
emitter.setType(r, RefType::OWNED);
llvm::Value* cstart, *cstop;
cstart = slice_val.start ? slice_val.start->makeConverted(emitter, UNKNOWN)->getValue()
: emitter.setType(getNullPtr(g.llvm_value_type_ptr), RefType::BORROWED);
cstop = slice_val.stop ? slice_val.stop->makeConverted(emitter, UNKNOWN)->getValue()
: emitter.setType(getNullPtr(g.llvm_value_type_ptr), RefType::BORROWED);
llvm::Value* r = NULL;
if (do_patchpoint) {
ICSetupInfo* pp = createGetitemIC(info.getTypeRecorder(), info.getBJitICInfo());
llvm::Instruction* uncasted = emitter.createIC(
pp, (void*)(target_exception_style == CAPI ? pyston::apply_slice : pyston::applySlice),
{ var->getValue(), cstart, cstop }, info.unw_info, target_exception_style,
getNullPtr(g.llvm_value_type_ptr));
r = createAfter<llvm::IntToPtrInst>(uncasted, uncasted, g.llvm_value_type_ptr, "");
} else {
r = emitter.createCall3(
info.unw_info, target_exception_style == CAPI ? g.funcs.apply_slice : g.funcs.applySlice,
var->getValue(), cstart, cstop, target_exception_style, getNullPtr(g.llvm_value_type_ptr));
}
emitter.setType(r, RefType::OWNED);
if (target_exception_style == CAPI)
emitter.setNullable(r, true);

if (return_type != UNDEF)
return new ConcreteCompilerVariable(static_cast<ConcreteCompilerType*>(return_type), r);
} else {
// TODO: we could directly emit an exception if we know getitem is undefined but for now let it just
// call the normal getitem which will raise the exception
}
return new ConcreteCompilerVariable(UNKNOWN, r);
}
}

ConcreteCompilerVariable* converted_slice = slice->makeConverted(emitter, slice->getBoxType());

ExceptionStyle target_exception_style = info.preferredExceptionStyle();

bool do_patchpoint = ENABLE_ICGETITEMS;
llvm::Value* rtn;
if (do_patchpoint) {
ICSetupInfo* pp = createGetitemIC(info.getTypeRecorder(), info.getBJitICInfo());
Expand Down
Loading