Skip to content

Commit 9c8f88f

Browse files
committed
callattr: don't generate arg guards if we know that the classes can't change
1 parent fd85b56 commit 9c8f88f

File tree

8 files changed

+38
-20
lines changed

8 files changed

+38
-20
lines changed

src/asm_writing/icinfo.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,7 @@ ICInfo::ICInfo(void* start_addr, void* slowpath_rtn_addr, void* continue_addr, S
236236
retry_in(0),
237237
retry_backoff(1),
238238
times_rewritten(0),
239+
has_const_arg_classes(false),
239240
start_addr(start_addr),
240241
slowpath_rtn_addr(slowpath_rtn_addr),
241242
continue_addr(continue_addr) {

src/asm_writing/icinfo.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@ class ICSlotRewrite {
103103

104104
typedef BitSet<16> LiveOutSet;
105105

106+
class BoxedClass;
106107
class ICInfo {
107108
private:
108109
std::vector<ICSlotInfo> slots;
@@ -122,6 +123,7 @@ class ICInfo {
122123
TypeRecorder* const type_recorder;
123124
int retry_in, retry_backoff;
124125
int times_rewritten;
126+
bool has_const_arg_classes;
125127

126128
// for ICSlotRewrite:
127129
ICSlotInfo* pickEntryForRewrite(const char* debug_name);
@@ -149,6 +151,9 @@ class ICInfo {
149151
int percentBackedoff() const { return retry_backoff; }
150152
int timesRewritten() const { return times_rewritten; }
151153

154+
void setHasConstArgClasses(bool b = true) { has_const_arg_classes = b; }
155+
bool hasConstArgClasses() const { return has_const_arg_classes; }
156+
152157
friend class ICSlotRewrite;
153158
static void visitGCReferences(gc::GCVisitor* visitor);
154159

src/asm_writing/rewriter.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1986,9 +1986,9 @@ Rewriter* Rewriter::createRewriter(void* rtn_addr, int num_args, const char* deb
19861986
// Horrible non-robust optimization: addresses below this address are probably in the binary (ex the interpreter),
19871987
// so don't do the more-expensive hash table lookup to find it.
19881988
if (rtn_addr > (void*)0x1000000) {
1989-
ic = getICInfo(rtn_addr);
1989+
ic = pyston::getICInfo(rtn_addr);
19901990
} else {
1991-
ASSERT(!getICInfo(rtn_addr), "%p", rtn_addr);
1991+
ASSERT(!pyston::getICInfo(rtn_addr), "%p", rtn_addr);
19921992
}
19931993

19941994
log_ic_attempts(debug_name);

src/asm_writing/rewriter.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -555,6 +555,8 @@ class Rewriter : public ICSlotRewrite::CommitHook, public gc::GCVisitable {
555555

556556
TypeRecorder* getTypeRecorder();
557557

558+
const ICInfo* getICInfo() { return rewrite->getICInfo(); }
559+
558560
const char* debugName() { return rewrite->debugName(); }
559561

560562
// Register that this rewrite will embed a reference to a particular gc object.

src/codegen/compvars.cpp

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -580,12 +580,16 @@ static ConcreteCompilerVariable* _call(IREmitter& emitter, const OpInfo& info, l
580580
bool pass_keyword_names = (keyword_names != nullptr);
581581
assert(pass_keyword_names == (argspec.num_keywords > 0));
582582

583+
bool has_const_arg_classes = true;
583584
std::vector<BoxedClass*> guaranteed_classes;
584585
std::vector<ConcreteCompilerVariable*> converted_args;
585586
for (int i = 0; i < args.size(); i++) {
586587
assert(args[i]);
587588
converted_args.push_back(args[i]->makeConverted(emitter, args[i]->getBoxType()));
588-
guaranteed_classes.push_back(converted_args.back()->guaranteedClass());
589+
BoxedClass* guaranteed_class = converted_args.back()->guaranteedClass();
590+
guaranteed_classes.push_back(guaranteed_class);
591+
if (guaranteed_class == NULL)
592+
has_const_arg_classes = false;
589593
}
590594

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

653-
ICSetupInfo* pp = createCallsiteIC(info.getTypeRecorder(), args.size(), info.getBJitICInfo());
657+
ICSetupInfo* pp
658+
= createCallsiteIC(info.getTypeRecorder(), args.size(), info.getBJitICInfo(), has_const_arg_classes);
654659

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

src/codegen/patchpoints.cpp

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,9 @@ int ICSetupInfo::totalSize() const {
4545
static std::vector<std::pair<PatchpointInfo*, void* /* addr of func to call */>> new_patchpoints;
4646

4747
ICSetupInfo* ICSetupInfo::initialize(bool has_return_value, int num_slots, int slot_size, ICType type,
48-
TypeRecorder* type_recorder) {
49-
ICSetupInfo* rtn = new ICSetupInfo(type, num_slots, slot_size, has_return_value, type_recorder);
48+
TypeRecorder* type_recorder, bool has_const_arg_classes) {
49+
ICSetupInfo* rtn
50+
= new ICSetupInfo(type, num_slots, slot_size, has_return_value, type_recorder, has_const_arg_classes);
5051

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

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

333-
ICSetupInfo* createCallsiteIC(TypeRecorder* type_recorder, int num_args, ICInfo* bjit_ic_info) {
334-
return ICSetupInfo::initialize(true, numSlots(bjit_ic_info, 4), 640 + 48 * num_args, ICSetupInfo::Callsite,
335-
type_recorder);
335+
ICSetupInfo* createCallsiteIC(TypeRecorder* type_recorder, int num_args, ICInfo* bjit_ic_info, bool const_arg_classes) {
336+
return ICSetupInfo::initialize(true, const_arg_classes ? 1 : numSlots(bjit_ic_info, 4), 640 + 48 * num_args,
337+
ICSetupInfo::Callsite, type_recorder, const_arg_classes);
336338
}
337339

338340
ICSetupInfo* createGetGlobalIC(TypeRecorder* type_recorder) {

src/codegen/patchpoints.h

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,7 @@ struct PatchpointInfo {
100100
static void* getSlowpathAddr(unsigned int pp_id);
101101
};
102102

103+
class BoxedClass;
103104
class ICSetupInfo {
104105
public:
105106
enum ICType {
@@ -118,18 +119,21 @@ class ICSetupInfo {
118119
};
119120

120121
private:
121-
ICSetupInfo(ICType type, int num_slots, int slot_size, bool has_return_value, TypeRecorder* type_recorder)
122+
ICSetupInfo(ICType type, int num_slots, int slot_size, bool has_return_value, TypeRecorder* type_recorder,
123+
bool has_const_arg_classes = false)
122124
: type(type),
123125
num_slots(num_slots),
124126
slot_size(slot_size),
125127
has_return_value(has_return_value),
128+
has_const_arg_classes(has_const_arg_classes),
126129
type_recorder(type_recorder) {}
127130

128131
public:
129132
const ICType type;
130133

131134
const int num_slots, slot_size;
132135
const bool has_return_value;
136+
const bool has_const_arg_classes;
133137
TypeRecorder* const type_recorder;
134138

135139
int totalSize() const;
@@ -149,12 +153,12 @@ class ICSetupInfo {
149153
}
150154

151155
static ICSetupInfo* initialize(bool has_return_value, int num_slots, int slot_size, ICType type,
152-
TypeRecorder* type_recorder);
156+
TypeRecorder* type_recorder, bool has_const_arg_classes = false);
153157
};
154158

155159
class ICInfo;
156160
ICSetupInfo* createGenericIC(TypeRecorder* type_recorder, bool has_return_value, int size);
157-
ICSetupInfo* createCallsiteIC(TypeRecorder* type_recorder, int num_args, ICInfo* bjit_ic_info);
161+
ICSetupInfo* createCallsiteIC(TypeRecorder* type_recorder, int num_args, ICInfo* bjit_ic_info, bool all_known);
158162
ICSetupInfo* createGetGlobalIC(TypeRecorder* type_recorder);
159163
ICSetupInfo* createGetattrIC(TypeRecorder* type_recorder, ICInfo* bjit_ic_info);
160164
ICSetupInfo* createSetattrIC(TypeRecorder* type_recorder, ICInfo* bjit_ic_info);

src/runtime/objmodel.cpp

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3136,11 +3136,11 @@ Box* _callattrEntry(Box* obj, BoxedString* attr, CallattrFlags flags, Box* arg1,
31363136
}
31373137

31383138
if (rewriter.get()) {
3139-
// TODO feel weird about doing this; it either isn't necessary
3140-
// or this kind of thing is necessary in a lot more places
3141-
// rewriter->getArg(3).addGuard(npassed_args);
3142-
31433139
CallattrRewriteArgs rewrite_args(rewriter.get(), rewriter->getArg(0), rewriter->getReturnDestination());
3140+
3141+
if (rewriter->getICInfo()->hasConstArgClasses())
3142+
rewrite_args.args_guarded = true;
3143+
31443144
if (npassed_args >= 1)
31453145
rewrite_args.arg1 = rewriter->getArg(3);
31463146
if (npassed_args >= 2)
@@ -4423,11 +4423,10 @@ static Box* runtimeCallEntry(Box* obj, ArgPassSpec argspec, Box* arg1, Box* arg2
44234423
#endif
44244424

44254425
if (rewriter.get()) {
4426-
// TODO feel weird about doing this; it either isn't necessary
4427-
// or this kind of thing is necessary in a lot more places
4428-
// rewriter->getArg(1).addGuard(npassed_args);
4429-
44304426
CallRewriteArgs rewrite_args(rewriter.get(), rewriter->getArg(0), rewriter->getReturnDestination());
4427+
if (rewriter->getICInfo()->hasConstArgClasses())
4428+
rewrite_args.args_guarded = true;
4429+
44314430
if (npassed_args >= 1)
44324431
rewrite_args.arg1 = rewriter->getArg(2);
44334432
if (npassed_args >= 2)

0 commit comments

Comments
 (0)