Commit c387acda authored by Kevin Modzelewski's avatar Kevin Modzelewski

Get assembly comments working for the bjit

First issue is that the assembly logging previously worked
on the assembler buffer, which is reasonable but has issues with the bjit
which will update the assembly post-relocation (ie by initializing the patchpoints).

Also add some extra flags to separate commenting for rewriter-level comments
("this is the beginning of a setattr") and the new bjit ones ("this is
a set local").
parent a01643bc
......@@ -96,16 +96,16 @@ public:
#ifndef NDEBUG
inline void comment(const llvm::Twine& msg) {
if (ASSEMBLY_LOGGING) {
logger.log_comment(msg, addr - start_addr);
}
logger.log_comment(msg, addr - start_addr);
}
inline std::string dump() {
if (ASSEMBLY_LOGGING) {
return logger.finalize_log(start_addr, addr);
} else {
return "";
}
return logger.finalize_log(start_addr, addr);
}
// Dumps the assembly but with the start address overridden, so
// that we can dump the assembly (with comments) after the code has
// been relocated.
inline std::string dump(uint8_t* from_addr) {
return logger.finalize_log(from_addr, from_addr + (addr - start_addr));
}
#else
inline void comment(const llvm::Twine& msg) {}
......
......@@ -93,6 +93,7 @@ std::string AssemblyLogger::finalize_log(uint8_t const* start_addr, uint8_t cons
int AsmPrinterVariant = MAI->getAssemblerDialect(); // 0 is ATT, 1 is Intel
IP = target->createMCInstPrinter(AsmPrinterVariant, *MAI, *MII, *MRI, *STI);
assert(IP);
IP->setPrintImmHex(true);
llvm::MCObjectFileInfo* MOFI = new llvm::MCObjectFileInfo();
assert(MOFI);
llvm::MCContext* Ctx = new llvm::MCContext(MAI, MRI, MOFI);
......@@ -107,20 +108,39 @@ std::string AssemblyLogger::finalize_log(uint8_t const* start_addr, uint8_t cons
size_t pos = 0;
append_comments(stream, pos);
int nnops = 0;
auto print_nops = [&]() {
if (nnops == 1)
stream << start_addr + pos - nnops << " nop\n";
else if (nnops)
stream << start_addr + pos - nnops << " nop * " << nnops << '\n';
nnops = 0;
};
while (pos < (end_addr - start_addr)) {
llvm::MCInst inst;
uint64_t size;
if (*(start_addr + pos) == 0x90) {
nnops++;
pos++;
append_comments(stream, pos);
continue;
} else {
print_nops();
}
llvm::MCDisassembler::DecodeStatus s = DisAsm->getInstruction(
inst /* out */, size /* out */, llvm::ArrayRef<uint8_t>(start_addr + pos, end_addr), 0, llvm::nulls(),
llvm::nulls());
assert(s == llvm::MCDisassembler::Success);
stream << start_addr + pos;
IP->printInst(&inst, stream, "");
stream << "\n";
pos += size;
append_comments(stream, pos);
}
print_nops();
return stream.str();
}
......
......@@ -299,7 +299,7 @@ void Rewriter::_slowpathJump(bool condition_eq) {
}
void Rewriter::_addGuard(RewriterVar* var, RewriterVar* val_constant) {
assembler->comment("_addGuard");
if (LOG_IC_ASSEMBLY) assembler->comment("_addGuard");
assert(val_constant->is_constant);
uint64_t val = val_constant->constant_value;
......@@ -332,7 +332,7 @@ void RewriterVar::addGuardNotEq(uint64_t val) {
}
void Rewriter::_addGuardNotEq(RewriterVar* var, RewriterVar* val_constant) {
assembler->comment("_addGuardNotEq");
if (LOG_IC_ASSEMBLY) assembler->comment("_addGuardNotEq");
assert(val_constant->is_constant);
uint64_t val = val_constant->constant_value;
......@@ -369,7 +369,7 @@ void RewriterVar::addAttrGuard(int offset, uint64_t val, bool negate) {
}
void Rewriter::_addAttrGuard(RewriterVar* var, int offset, RewriterVar* val_constant, bool negate) {
assembler->comment("_addAttrGuard");
if (LOG_IC_ASSEMBLY) assembler->comment("_addAttrGuard");
assert(val_constant->is_constant);
uint64_t val = val_constant->constant_value;
......@@ -419,7 +419,7 @@ RewriterVar* RewriterVar::getAttr(int offset, Location dest, assembler::MovType
}
void Rewriter::_getAttr(RewriterVar* result, RewriterVar* ptr, int offset, Location dest, assembler::MovType type) {
assembler->comment("_getAttr");
if (LOG_IC_ASSEMBLY) assembler->comment("_getAttr");
// TODO if var is a constant, we will end up emitting something like
// mov $0x123, %rax
......@@ -450,7 +450,7 @@ RewriterVar* RewriterVar::getAttrDouble(int offset, Location dest) {
}
void Rewriter::_getAttrDouble(RewriterVar* result, RewriterVar* ptr, int offset, Location dest) {
assembler->comment("_getAttrDouble");
if (LOG_IC_ASSEMBLY) assembler->comment("_getAttrDouble");
assembler::Register ptr_reg = ptr->getInReg();
......@@ -474,7 +474,7 @@ RewriterVar* RewriterVar::getAttrFloat(int offset, Location dest) {
}
void Rewriter::_getAttrFloat(RewriterVar* result, RewriterVar* ptr, int offset, Location dest) {
assembler->comment("_getAttrFloat");
if (LOG_IC_ASSEMBLY) assembler->comment("_getAttrFloat");
assembler::Register ptr_reg = ptr->getInReg();
......@@ -594,7 +594,7 @@ RewriterVar* RewriterVar::cmp(AST_TYPE::AST_TYPE cmp_type, RewriterVar* other, L
}
void Rewriter::_cmp(RewriterVar* result, RewriterVar* v1, AST_TYPE::AST_TYPE cmp_type, RewriterVar* v2, Location dest) {
assembler->comment("_cmp");
if (LOG_IC_ASSEMBLY) assembler->comment("_cmp");
assembler::Register v1_reg = v1->getInReg();
assembler::Register v2_reg = v2->getInReg();
......@@ -630,7 +630,7 @@ RewriterVar* RewriterVar::toBool(Location dest) {
}
void Rewriter::_toBool(RewriterVar* result, RewriterVar* var, Location dest) {
assembler->comment("_toBool");
if (LOG_IC_ASSEMBLY) assembler->comment("_toBool");
assembler::Register this_reg = var->getInReg();
......@@ -656,7 +656,7 @@ void RewriterVar::setAttr(int offset, RewriterVar* val) {
}
void Rewriter::_setAttr(RewriterVar* ptr, int offset, RewriterVar* val) {
assembler->comment("_setAttr");
if (LOG_IC_ASSEMBLY) assembler->comment("_setAttr");
assembler::Register ptr_reg = ptr->getInReg();
......@@ -702,6 +702,12 @@ assembler::Immediate RewriterVar::tryGetAsImmediate(bool* is_immediate) {
}
}
#ifndef NDEBUG
void Rewriter::comment(const llvm::Twine& msg) {
addAction([=]() { this->assembler->comment(msg); }, {}, ActionType::NORMAL);
}
#endif
assembler::Register RewriterVar::getInReg(Location dest, bool allow_constant_in_reg, Location otherThan) {
assert(dest.type == Location::Register || dest.type == Location::AnyReg);
......@@ -1121,7 +1127,7 @@ void Rewriter::_setupCall(bool has_side_effects, llvm::ArrayRef<RewriterVar*> ar
void Rewriter::_call(RewriterVar* result, bool has_side_effects, void* func_addr, llvm::ArrayRef<RewriterVar*> args,
llvm::ArrayRef<RewriterVar*> args_xmm) {
assembler->comment("_call");
if (LOG_IC_ASSEMBLY) assembler->comment("_call");
// RewriterVarUsage scratch = createNewVar(Location::any());
assembler::Register r = allocReg(assembler::R11);
......@@ -1331,7 +1337,7 @@ void Rewriter::commit() {
}
if (marked_inside_ic) {
assembler->comment("mark inside ic");
if (LOG_IC_ASSEMBLY) assembler->comment("mark inside ic");
ASSERT(this->needs_invalidation_support, "why did we mark ourselves as inside this?");
......@@ -1345,7 +1351,7 @@ void Rewriter::commit() {
}
}
assembler->comment("live outs");
if (LOG_IC_ASSEMBLY) assembler->comment("live outs");
// Make sure that we have been calling bumpUse correctly.
// All uses should have been accounted for, other than the live outs
......@@ -1464,7 +1470,7 @@ void Rewriter::commit() {
uint64_t asm_size_bytes = assembler->bytesWritten();
#ifndef NDEBUG
std::string asm_dump;
if (ASSEMBLY_LOGGING) {
if (LOG_IC_ASSEMBLY) {
assembler->comment("size in bytes: " + std::to_string(asm_size_bytes));
asm_dump = assembler->dump();
}
......@@ -1480,7 +1486,7 @@ void Rewriter::commit() {
finished = true;
#ifndef NDEBUG
if (ASSEMBLY_LOGGING) {
if (LOG_IC_ASSEMBLY) {
fprintf(stderr, "%s\n\n", asm_dump.c_str());
}
#endif
......@@ -1509,7 +1515,7 @@ void Rewriter::commitReturning(RewriterVar* var) {
var->stealRef();
addAction([=]() {
assembler->comment("commitReturning");
if (LOG_IC_ASSEMBLY) assembler->comment("commitReturning");
var->getInReg(getReturnDestination(), true /* allow_constant_in_reg */);
var->bumpUse();
}, { var }, ActionType::NORMAL);
......@@ -1546,7 +1552,7 @@ RewriterVar* Rewriter::add(RewriterVar* a, int64_t b, Location dest) {
}
void Rewriter::_add(RewriterVar* result, RewriterVar* a, int64_t b, Location dest) {
assembler->comment("_add");
if (LOG_IC_ASSEMBLY) assembler->comment("_add");
// TODO better reg alloc (e.g., mov `a` directly to the dest reg)
......@@ -1578,7 +1584,7 @@ RewriterVar* Rewriter::allocate(int n) {
}
int Rewriter::_allocate(RewriterVar* result, int n) {
assembler->comment("_allocate");
if (LOG_IC_ASSEMBLY) assembler->comment("_allocate");
assert(n >= 1);
......@@ -1630,7 +1636,7 @@ RewriterVar* Rewriter::allocateAndCopy(RewriterVar* array_ptr, int n) {
}
void Rewriter::_allocateAndCopy(RewriterVar* result, RewriterVar* array_ptr, int n) {
assembler->comment("_allocateAndCopy");
if (LOG_IC_ASSEMBLY) assembler->comment("_allocateAndCopy");
// TODO smart register allocation
......@@ -1670,7 +1676,7 @@ RewriterVar* Rewriter::allocateAndCopyPlus1(RewriterVar* first_elem, RewriterVar
}
void Rewriter::_allocateAndCopyPlus1(RewriterVar* result, RewriterVar* first_elem, RewriterVar* rest_ptr, int n_rest) {
assembler->comment("_allocateAndCopyPlus1");
if (LOG_IC_ASSEMBLY) assembler->comment("_allocateAndCopyPlus1");
int offset = _allocate(result, n_rest + 1);
......@@ -1703,7 +1709,7 @@ void Rewriter::checkAndThrowCAPIException(RewriterVar* r, int64_t exc_val) {
}
void Rewriter::_checkAndThrowCAPIException(RewriterVar* r, int64_t exc_val) {
assembler->comment("_checkAndThrowCAPIException");
if (LOG_IC_ASSEMBLY) assembler->comment("_checkAndThrowCAPIException");
assembler::Register var_reg = r->getInReg();
if (exc_val == 0)
......
......@@ -685,6 +685,12 @@ public:
// infer these from loadConst calls.
void addGCReference(void* obj);
#ifndef NDEBUG
void comment(const llvm::Twine& msg);
#else
void comment(const llvm::Twine& msg) {}
#endif
void trap();
RewriterVar* loadConst(int64_t val, Location loc = Location::any());
// has_side_effects: whether this call could have "side effects". the exact side effects we've
......
......@@ -501,7 +501,7 @@ void ASTInterpreter::doStore(AST_Name* node, STOLEN(Value) value) {
}
}
void ASTInterpreter::doStore(AST_expr* node, Value value) {
void ASTInterpreter::doStore(AST_expr* node, STOLEN(Value) value) {
if (node->type == AST_TYPE::Name) {
AST_Name* name = (AST_Name*)node;
doStore(name, value);
......
......@@ -141,6 +141,7 @@ JitFragmentWriter::JitFragmentWriter(CFGBlock* block, std::unique_ptr<ICInfo> ic
code_block(code_block),
interp(0),
ic_info(std::move(ic_info)) {
if (LOG_BJIT_ASSEMBLY) comment("BJIT: JitFragmentWriter() start");
interp = createNewVar();
addLocationToVar(interp, assembler::R12);
interp->setAttr(ASTInterpreterJitInterface::getCurrentBlockOffset(), imm(block));
......@@ -148,6 +149,7 @@ JitFragmentWriter::JitFragmentWriter(CFGBlock* block, std::unique_ptr<ICInfo> ic
vregs_array = createNewVar();
addLocationToVar(vregs_array, assembler::R14);
addAction([=]() { vregs_array->bumpUse(); }, vregs_array, ActionType::NORMAL);
if (LOG_BJIT_ASSEMBLY) comment("BJIT: JitFragmentWriter() end");
}
RewriterVar* JitFragmentWriter::getInterp() {
......@@ -167,7 +169,6 @@ RewriterVar* JitFragmentWriter::emitAugbinop(AST_expr* node, RewriterVar* lhs, R
}
RewriterVar* JitFragmentWriter::emitBinop(AST_expr* node, RewriterVar* lhs, RewriterVar* rhs, int op_type) {
printf("lhs is %p\n", lhs);
/// XXX increase this too much for testing
return emitPPCall((void*)binop, { lhs, rhs, imm(op_type) }, 2, 640, node)->setType(RefType::OWNED);
}
......@@ -339,11 +340,13 @@ RewriterVar* JitFragmentWriter::emitGetItem(AST_expr* node, RewriterVar* value,
}
RewriterVar* JitFragmentWriter::emitGetLocal(InternedString s, int vreg) {
if (LOG_BJIT_ASSEMBLY) comment("BJIT: emitGetLocal start");
assert(vreg >= 0);
// TODO Can we use BORROWED here? Not sure if there are cases when we can't rely on borrowing the ref
// from the vregs array. Safer like this.
RewriterVar* val_var = vregs_array->getAttr(vreg * 8)->setType(RefType::OWNED);
addAction([=]() { _emitGetLocal(val_var, s.c_str()); }, { val_var }, ActionType::NORMAL);
if (LOG_BJIT_ASSEMBLY) comment("BJIT: emitGetLocal end");
return val_var;
}
......@@ -478,6 +481,7 @@ void JitFragmentWriter::emitExec(RewriterVar* code, RewriterVar* globals, Rewrit
}
void JitFragmentWriter::emitJump(CFGBlock* b) {
if (LOG_BJIT_ASSEMBLY) comment("BJIT: emitJump() start");
for (auto v : local_syms) {
if (v.second)
v.second->decvref(); // xdecref?
......@@ -485,12 +489,15 @@ void JitFragmentWriter::emitJump(CFGBlock* b) {
RewriterVar* next = imm(b);
addAction([=]() { _emitJump(b, next, exit_info); }, { next }, ActionType::NORMAL);
if (LOG_BJIT_ASSEMBLY) comment("BJIT: emitJump() end");
}
void JitFragmentWriter::emitOSRPoint(AST_Jump* node) {
if (LOG_BJIT_ASSEMBLY) comment("BJIT: emitOSRPoint() start");
RewriterVar* node_var = imm(node);
RewriterVar* result = createNewVar();
addAction([=]() { _emitOSRPoint(result, node_var); }, { result, node_var, getInterp() }, ActionType::NORMAL);
if (LOG_BJIT_ASSEMBLY) comment("BJIT: emitOSRPoint() end");
}
void JitFragmentWriter::emitPrint(RewriterVar* dest, RewriterVar* var, bool nl) {
......@@ -524,13 +531,15 @@ void JitFragmentWriter::emitSetAttr(AST_expr* node, RewriterVar* obj, BoxedStrin
emitPPCall((void*)setattr, { obj, imm(s), attr }, 2, 512, node)->setType(RefType::OWNED);
}
void JitFragmentWriter::emitSetBlockLocal(InternedString s, RewriterVar* v) {
void JitFragmentWriter::emitSetBlockLocal(InternedString s, STOLEN(RewriterVar*) v) {
if (LOG_BJIT_ASSEMBLY) comment("BJIT: emitSetBlockLocal() start");
RewriterVar* prev = local_syms[s];
local_syms[s] = v;
if (prev) {
// TODO: xdecref?
prev->decvref();
}
if (LOG_BJIT_ASSEMBLY) comment("BJIT: emitSetBlockLocal() end");
}
void JitFragmentWriter::emitSetCurrentInst(AST_stmt* node) {
......@@ -554,6 +563,7 @@ void JitFragmentWriter::emitSetItemName(BoxedString* s, RewriterVar* v) {
}
void JitFragmentWriter::emitSetLocal(InternedString s, int vreg, bool set_closure, STOLEN(RewriterVar*) v) {
if (LOG_BJIT_ASSEMBLY) comment("BJIT: emitSetLocal() start");
assert(vreg >= 0);
if (set_closure) {
assert(0 && "check refcounting");
......@@ -575,9 +585,11 @@ void JitFragmentWriter::emitSetLocal(InternedString s, int vreg, bool set_closur
prev->decvref();
//prev->xdecvref();
}
if (LOG_BJIT_ASSEMBLY) comment("BJIT: emitSetLocal() end");
}
void JitFragmentWriter::emitSideExit(RewriterVar* v, Box* cmp_value, CFGBlock* next_block) {
if (LOG_BJIT_ASSEMBLY) comment("BJIT: emitSideExit start");
RewriterVar* var = imm(cmp_value);
RewriterVar* next_block_var = imm(next_block);
......@@ -592,6 +604,7 @@ void JitFragmentWriter::emitSideExit(RewriterVar* v, Box* cmp_value, CFGBlock* n
addAction([=]() { _emitSideExit(v, var, next_block, next_block_var); }, { v, var, next_block_var },
ActionType::NORMAL);
if (LOG_BJIT_ASSEMBLY) comment("BJIT: emitSideExit end");
}
void JitFragmentWriter::emitUncacheExcInfo() {
......@@ -637,6 +650,12 @@ int JitFragmentWriter::finishCompilation() {
block->code = (void*)((uint64_t)entry_code + code_offset);
block->entry_code = (decltype(block->entry_code))entry_code;
if (LOG_BJIT_ASSEMBLY) {
printf("\n");
printf("Successfully bjit'd code for cfg block %d\n", block->idx);
printf("Code goes from %p-%p\n", block->code, (char*)block->code + assembler->bytesWritten());
}
// if any side exits point to this block patch them to a direct jump to this block
auto it = block_patch_locations.find(block);
if (it != block_patch_locations.end()) {
......@@ -676,6 +695,11 @@ int JitFragmentWriter::finishCompilation() {
pp.release();
}
if (LOG_BJIT_ASSEMBLY) {
auto s = assembler->dump((uint8_t*)block->code/*, (uint8_t*)block->code + assembler->bytesWritten()*/);
printf("%s\n", s.c_str());
}
void* next_fragment_start = (uint8_t*)block->code + assembler->bytesWritten();
if (exit_info.num_bytes)
ASSERT(assembler->curInstPointer() == (uint8_t*)exit_info.exit_start + exit_info.num_bytes,
......@@ -732,6 +756,7 @@ uint64_t JitFragmentWriter::asUInt(InternedString s) {
RewriterVar* JitFragmentWriter::emitPPCall(void* func_addr, llvm::ArrayRef<RewriterVar*> args, int num_slots,
int slot_size, AST* ast_node, TypeRecorder* type_recorder) {
if (LOG_BJIT_ASSEMBLY) comment("BJIT: emitPPCall() start");
RewriterVar::SmallVector args_vec(args.begin(), args.end());
#if ENABLE_BASELINEJIT_ICS
RewriterVar* result = createNewVar();
......@@ -752,10 +777,13 @@ RewriterVar* JitFragmentWriter::emitPPCall(void* func_addr, llvm::ArrayRef<Rewri
ActionType::NORMAL);
return result;
}
if (LOG_BJIT_ASSEMBLY) comment("BJIT: emitPPCall() end");
return result;
#else
assert(args_vec.size() < 7);
return call(false, func_addr, args_vec);
auto result = call(false, func_addr, args_vec);
if (LOG_BJIT_ASSEMBLY) comment("BJIT: emitPPCall() end");
return result;
#endif
}
......
......@@ -24,7 +24,9 @@ int PYSTON_VERSION_MICRO = 0;
int MAX_OPT_ITERATIONS = 1;
bool ASSEMBLY_LOGGING = false;
bool LOG_IC_ASSEMBLY = false;
bool LOG_BJIT_ASSEMBLY = true;
bool CONTINUE_AFTER_FATAL = false;
bool FORCE_INTERPRETER = true;
bool FORCE_OPTIMIZE = false;
......
......@@ -37,8 +37,9 @@ extern int MAX_OBJECT_CACHE_ENTRIES;
extern bool SHOW_DISASM, FORCE_INTERPRETER, FORCE_OPTIMIZE, PROFILE, DUMPJIT, TRAP, USE_STRIPPED_STDLIB,
CONTINUE_AFTER_FATAL, ENABLE_INTERPRETER, ENABLE_BASELINEJIT, ENABLE_PYPA_PARSER, ENABLE_CPYTHON_PARSER,
USE_REGALLOC_BASIC, PAUSE_AT_ABORT, ENABLE_TRACEBACKS, ASSEMBLY_LOGGING, FORCE_LLVM_CAPI_CALLS,
FORCE_LLVM_CAPI_THROWS;
USE_REGALLOC_BASIC, PAUSE_AT_ABORT, ENABLE_TRACEBACKS, FORCE_LLVM_CAPI_CALLS, FORCE_LLVM_CAPI_THROWS;
extern bool LOG_IC_ASSEMBLY, LOG_BJIT_ASSEMBLY;
extern bool ENABLE_ICS, ENABLE_ICGENERICS, ENABLE_ICGETITEMS, ENABLE_ICSETITEMS, ENABLE_ICDELITEMS, ENABLE_ICBINEXPS,
ENABLE_ICNONZEROS, ENABLE_ICCALLSITES, ENABLE_ICSETATTRS, ENABLE_ICGETATTRS, ENALBE_ICDELATTRS, ENABLE_ICGETGLOBALS,
......
......@@ -200,7 +200,7 @@ int handleArg(char code) {
} else if (code == 'n') {
ENABLE_INTERPRETER = false;
} else if (code == 'a') {
ASSEMBLY_LOGGING = true;
LOG_IC_ASSEMBLY = true;
} else if (code == 'p') {
PROFILE = true;
} else if (code == 'j') {
......@@ -379,9 +379,11 @@ static int main(int argc, char** argv) noexcept {
setvbuf(stderr, (char*)NULL, _IONBF, BUFSIZ);
}
if (ASSEMBLY_LOGGING) {
#ifndef NDEBUG
if (LOG_IC_ASSEMBLY || LOG_BJIT_ASSEMBLY) {
assembler::disassemblyInitialize();
}
#endif
{
Timer _t("for Py_Initialize");
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment