Skip to content

Commit

Permalink
callattr runtimeCall: don't generate arg guards if we know that the c…
Browse files Browse the repository at this point in the history
…lasses can't change
  • Loading branch information
undingen committed Oct 27, 2015
1 parent fd85b56 commit 329d617
Show file tree
Hide file tree
Showing 8 changed files with 36 additions and 20 deletions.
1 change: 1 addition & 0 deletions src/asm_writing/icinfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,7 @@ ICInfo::ICInfo(void* start_addr, void* slowpath_rtn_addr, void* continue_addr, S
retry_in(0),
retry_backoff(1),
times_rewritten(0),
has_const_arg_classes(false),
start_addr(start_addr),
slowpath_rtn_addr(slowpath_rtn_addr),
continue_addr(continue_addr) {
Expand Down
4 changes: 4 additions & 0 deletions src/asm_writing/icinfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ class ICInfo {
TypeRecorder* const type_recorder;
int retry_in, retry_backoff;
int times_rewritten;
bool has_const_arg_classes;

// for ICSlotRewrite:
ICSlotInfo* pickEntryForRewrite(const char* debug_name);
Expand Down Expand Up @@ -149,6 +150,9 @@ class ICInfo {
int percentBackedoff() const { return retry_backoff; }
int timesRewritten() const { return times_rewritten; }

void setHasConstArgClasses(bool b = true) { has_const_arg_classes = b; }
bool hasConstArgClasses() const { return has_const_arg_classes; }

friend class ICSlotRewrite;
static void visitGCReferences(gc::GCVisitor* visitor);

Expand Down
4 changes: 2 additions & 2 deletions src/asm_writing/rewriter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1986,9 +1986,9 @@ Rewriter* Rewriter::createRewriter(void* rtn_addr, int num_args, const char* deb
// Horrible non-robust optimization: addresses below this address are probably in the binary (ex the interpreter),
// so don't do the more-expensive hash table lookup to find it.
if (rtn_addr > (void*)0x1000000) {
ic = getICInfo(rtn_addr);
ic = pyston::getICInfo(rtn_addr);
} else {
ASSERT(!getICInfo(rtn_addr), "%p", rtn_addr);
ASSERT(!pyston::getICInfo(rtn_addr), "%p", rtn_addr);
}

log_ic_attempts(debug_name);
Expand Down
2 changes: 2 additions & 0 deletions src/asm_writing/rewriter.h
Original file line number Diff line number Diff line change
Expand Up @@ -555,6 +555,8 @@ class Rewriter : public ICSlotRewrite::CommitHook, public gc::GCVisitable {

TypeRecorder* getTypeRecorder();

const ICInfo* getICInfo() { return rewrite->getICInfo(); }

const char* debugName() { return rewrite->debugName(); }

// Register that this rewrite will embed a reference to a particular gc object.
Expand Down
9 changes: 7 additions & 2 deletions src/codegen/compvars.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -580,12 +580,16 @@ static ConcreteCompilerVariable* _call(IREmitter& emitter, const OpInfo& info, l
bool pass_keyword_names = (keyword_names != nullptr);
assert(pass_keyword_names == (argspec.num_keywords > 0));

bool has_const_arg_classes = true;
std::vector<BoxedClass*> guaranteed_classes;
std::vector<ConcreteCompilerVariable*> converted_args;
for (int i = 0; i < args.size(); i++) {
assert(args[i]);
converted_args.push_back(args[i]->makeConverted(emitter, args[i]->getBoxType()));
guaranteed_classes.push_back(converted_args.back()->guaranteedClass());
BoxedClass* guaranteed_class = converted_args.back()->guaranteedClass();
guaranteed_classes.push_back(guaranteed_class);
if (guaranteed_class == NULL)
has_const_arg_classes = false;
}

std::vector<llvm::Value*> llvm_args;
Expand Down Expand Up @@ -650,7 +654,8 @@ static ConcreteCompilerVariable* _call(IREmitter& emitter, const OpInfo& info, l
if (do_patchpoint) {
assert(func_addr);

ICSetupInfo* pp = createCallsiteIC(info.getTypeRecorder(), args.size(), info.getBJitICInfo());
ICSetupInfo* pp
= createCallsiteIC(info.getTypeRecorder(), args.size(), info.getBJitICInfo(), has_const_arg_classes);

llvm::Value* uncasted = emitter.createIC(pp, func_addr, llvm_args, info.unw_info, target_exception_style);

Expand Down
12 changes: 7 additions & 5 deletions src/codegen/patchpoints.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,9 @@ int ICSetupInfo::totalSize() const {
static std::vector<std::pair<PatchpointInfo*, void* /* addr of func to call */>> new_patchpoints;

ICSetupInfo* ICSetupInfo::initialize(bool has_return_value, int num_slots, int slot_size, ICType type,
TypeRecorder* type_recorder) {
ICSetupInfo* rtn = new ICSetupInfo(type, num_slots, slot_size, has_return_value, type_recorder);
TypeRecorder* type_recorder, bool has_const_arg_classes) {
ICSetupInfo* rtn
= new ICSetupInfo(type, num_slots, slot_size, has_return_value, type_recorder, has_const_arg_classes);

// We use size == CALL_ONLY_SIZE to imply that the call isn't patchable
assert(rtn->totalSize() > CALL_ONLY_SIZE);
Expand Down Expand Up @@ -255,6 +256,7 @@ void processStackmap(CompiledFunction* cf, StackMap* stackmap) {
start_addr, initialization_info.slowpath_start, initialization_info.continue_addr,
initialization_info.slowpath_rtn_addr, ic, StackInfo(scratch_size, scratch_rsp_offset),
std::move(initialization_info.live_outs));
icinfo->setHasConstArgClasses(ic->has_const_arg_classes);

assert(cf);
// TODO: unsafe. hard to use a unique_ptr here though.
Expand Down Expand Up @@ -330,9 +332,9 @@ ICSetupInfo* createDelattrIC(TypeRecorder* type_recorder) {
return ICSetupInfo::initialize(false, 1, 144, ICSetupInfo::Delattr, type_recorder);
}

ICSetupInfo* createCallsiteIC(TypeRecorder* type_recorder, int num_args, ICInfo* bjit_ic_info) {
return ICSetupInfo::initialize(true, numSlots(bjit_ic_info, 4), 640 + 48 * num_args, ICSetupInfo::Callsite,
type_recorder);
ICSetupInfo* createCallsiteIC(TypeRecorder* type_recorder, int num_args, ICInfo* bjit_ic_info, bool const_arg_classes) {
return ICSetupInfo::initialize(true, const_arg_classes ? 1 : numSlots(bjit_ic_info, 4), 640 + 48 * num_args,
ICSetupInfo::Callsite, type_recorder, const_arg_classes);
}

ICSetupInfo* createGetGlobalIC(TypeRecorder* type_recorder) {
Expand Down
10 changes: 7 additions & 3 deletions src/codegen/patchpoints.h
Original file line number Diff line number Diff line change
Expand Up @@ -118,18 +118,21 @@ class ICSetupInfo {
};

private:
ICSetupInfo(ICType type, int num_slots, int slot_size, bool has_return_value, TypeRecorder* type_recorder)
ICSetupInfo(ICType type, int num_slots, int slot_size, bool has_return_value, TypeRecorder* type_recorder,
bool has_const_arg_classes = false)
: type(type),
num_slots(num_slots),
slot_size(slot_size),
has_return_value(has_return_value),
has_const_arg_classes(has_const_arg_classes),
type_recorder(type_recorder) {}

public:
const ICType type;

const int num_slots, slot_size;
const bool has_return_value;
const bool has_const_arg_classes;
TypeRecorder* const type_recorder;

int totalSize() const;
Expand All @@ -149,12 +152,13 @@ class ICSetupInfo {
}

static ICSetupInfo* initialize(bool has_return_value, int num_slots, int slot_size, ICType type,
TypeRecorder* type_recorder);
TypeRecorder* type_recorder, bool has_const_arg_classes = false);
};

class ICInfo;
ICSetupInfo* createGenericIC(TypeRecorder* type_recorder, bool has_return_value, int size);
ICSetupInfo* createCallsiteIC(TypeRecorder* type_recorder, int num_args, ICInfo* bjit_ic_info);
ICSetupInfo* createCallsiteIC(TypeRecorder* type_recorder, int num_args, ICInfo* bjit_ic_info,
bool has_const_arg_classes);
ICSetupInfo* createGetGlobalIC(TypeRecorder* type_recorder);
ICSetupInfo* createGetattrIC(TypeRecorder* type_recorder, ICInfo* bjit_ic_info);
ICSetupInfo* createSetattrIC(TypeRecorder* type_recorder, ICInfo* bjit_ic_info);
Expand Down
14 changes: 6 additions & 8 deletions src/runtime/objmodel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3136,11 +3136,10 @@ Box* _callattrEntry(Box* obj, BoxedString* attr, CallattrFlags flags, Box* arg1,
}

if (rewriter.get()) {
// TODO feel weird about doing this; it either isn't necessary
// or this kind of thing is necessary in a lot more places
// rewriter->getArg(3).addGuard(npassed_args);

CallattrRewriteArgs rewrite_args(rewriter.get(), rewriter->getArg(0), rewriter->getReturnDestination());
if (rewriter->getICInfo()->hasConstArgClasses())
rewrite_args.args_guarded = true;

if (npassed_args >= 1)
rewrite_args.arg1 = rewriter->getArg(3);
if (npassed_args >= 2)
Expand Down Expand Up @@ -4423,11 +4422,10 @@ static Box* runtimeCallEntry(Box* obj, ArgPassSpec argspec, Box* arg1, Box* arg2
#endif

if (rewriter.get()) {
// TODO feel weird about doing this; it either isn't necessary
// or this kind of thing is necessary in a lot more places
// rewriter->getArg(1).addGuard(npassed_args);

CallRewriteArgs rewrite_args(rewriter.get(), rewriter->getArg(0), rewriter->getReturnDestination());
if (rewriter->getICInfo()->hasConstArgClasses())
rewrite_args.args_guarded = true;

if (npassed_args >= 1)
rewrite_args.arg1 = rewriter->getArg(2);
if (npassed_args >= 2)
Expand Down

0 comments on commit 329d617

Please sign in to comment.