Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Make separate IR instructions for guard type checks
LdLoc and LdStack, when provided a label, also do the type check, and
that's how guards were being implemented.  Although that has the nice
effect of hoisting the loads early in the trace, it's bad because the
loads effectively end up as part of the guards and they execute
redundantly when guards fail.

This diff splits the checks into their own instructions, but also
loads all the dependencies right after the guards to preserve the
hoisting effect.
  • Loading branch information
ottoni authored and sgolemon committed Dec 18, 2012
1 parent f9f407d commit a474a67
Show file tree
Hide file tree
Showing 13 changed files with 207 additions and 55 deletions.
121 changes: 83 additions & 38 deletions src/runtime/vm/translator/hopt/codegen.cpp
Expand Up @@ -3223,29 +3223,8 @@ Address CodeGenerator::cgLoadTypedValue(Type::Tag type,
}

// Check type if needed
if (label && type != Type::Gen) {
ConditionCode cc;
switch (type) {
case Type::Cell : {
m_as.cmp_imm32_disp_reg32(HPHP::KindOfRef, off + TVOFF(m_type), base);
cc = CC_GE;
break;
}
case Type::Uncounted : {
m_as.cmp_imm32_disp_reg32(HPHP::KindOfStaticString,
off + TVOFF(m_type), base);
cc = CC_G;
break;
}
case Type::UncountedInit : {
m_as.test_imm32_disp_reg32(HPHP::KindOfUncountedInitBit,
off + TVOFF(m_type), base);
cc = CC_Z;
break;
}
default : not_reached();
}
emitGuardOrFwdJcc(inst, cc, label);
if (label) {
cgGuardType(type, base, off, label, inst);
}

// Load type if it's not dead
Expand Down Expand Up @@ -3371,18 +3350,7 @@ Address CodeGenerator::cgLoad(Type::Tag type,
return cgLoadTypedValue(type, dst, base, off, label, inst);
}
if (label != NULL && type != Type::Home) {
// generate a guard for the type
// see Translator.cpp checkType
DataType dataType = Type::toDataType(type);
ConditionCode cc;
if (IS_STRING_TYPE(dataType)) {
m_as.test_imm32_disp_reg32(KindOfStringBit, off + TVOFF(m_type), base);
cc = CC_Z;
} else {
m_as.cmp_imm32_disp_reg32(dataType, off + TVOFF(m_type), base);
cc = CC_NE;
}
emitGuardOrFwdJcc(inst, cc, label);
cgGuardType(type, base, off, label, inst);
}
if (type == Type::Uninit || type == Type::Null) {
return start; // these are constants
Expand Down Expand Up @@ -3473,9 +3441,9 @@ static void getLocalRegOffset(SSATmp* src, PhysReg& reg, int64& off) {

Address CodeGenerator::cgLdLoc(IRInstruction* inst) {
Address start = m_as.code.frontier;
Type::Tag type = inst->getType();
SSATmp* dst = inst->getDst();
LabelInstruction* label = inst->getLabel();
Type::Tag type = inst->getType();
SSATmp* dst = inst->getDst();
LabelInstruction* label = inst->getLabel();

PhysReg fpReg;
int64 offset;
Expand Down Expand Up @@ -3520,6 +3488,83 @@ Address CodeGenerator::cgLdStack(IRInstruction* inst) {
inst);
}

Address CodeGenerator::cgGuardType(Type::Tag type,
PhysReg baseReg,
int64_t offset,
LabelInstruction* label,
IRInstruction* inst) {
ASSERT(label);
Address start = m_as.code.frontier;
int64_t typeOffset = offset + TVOFF(m_type);
ConditionCode cc;

switch (type) {
case Type::StaticStr :
case Type::Str : {
m_as.test_imm32_disp_reg32(KindOfStringBit, typeOffset, baseReg);
cc = CC_Z;
break;
}
case Type::UncountedInit : {
m_as.test_imm32_disp_reg32(KindOfUncountedInitBit, typeOffset, baseReg);
cc = CC_Z;
break;
}
case Type::Uncounted : {
m_as.cmp_imm32_disp_reg32(KindOfRefCountThreshold, typeOffset, baseReg);
cc = CC_G;
break;
}
case Type::Cell : {
m_as.cmp_imm32_disp_reg32(KindOfRef, typeOffset, baseReg);
cc = CC_GE;
break;
}
case Type::Gen : {
return start; // nothing to check
}
default: {
DataType dataType = Type::toDataType(type);
ASSERT(dataType >= KindOfUninit);
m_as.cmp_imm32_disp_reg32(dataType, typeOffset, baseReg);
cc = CC_NZ;
break;
}
}

emitGuardOrFwdJcc(inst, cc, label);

return start;
}

Address CodeGenerator::cgGuardStk(IRInstruction* inst) {
Type::Tag type = inst->getType();
SSATmp* sp = inst->getSrc(0);
SSATmp* index = inst->getSrc(1);
LabelInstruction* label = inst->getLabel();

ASSERT(index->isConst());

return cgGuardType(type,
sp->getReg(0),
index->getConstValAsInt() * sizeof(Cell),
label,
inst);
}

Address CodeGenerator::cgGuardLoc(IRInstruction* inst) {
Type::Tag type = inst->getType();
SSATmp* index = inst->getSrc(0);
LabelInstruction* label = inst->getLabel();
PhysReg fpReg;
int64 offset;
getLocalRegOffset(index, fpReg, offset);
ASSERT(fpReg == HPHP::VM::Transl::reg::rbp);

return cgGuardType(type, fpReg, offset, label, inst);
}


Address CodeGenerator::cgGuardType(IRInstruction* inst) {
Address start = m_as.code.frontier;
UNUSED Type::Tag type = inst->getType();
Expand Down
6 changes: 6 additions & 0 deletions src/runtime/vm/translator/hopt/codegen.h
Expand Up @@ -102,6 +102,12 @@ class CodeGenerator {
LabelInstruction* label,
IRInstruction* inst = NULL);

Address cgGuardType(Type::Tag type,
PhysReg baseReg,
int64_t offset,
LabelInstruction* label,
IRInstruction* instr);

Address cgStMemWork(IRInstruction* inst, bool genStoreType);
Address cgStRefWork(IRInstruction* inst, bool genStoreType);
Address cgStLocWork(IRInstruction* inst, bool genStoreType);
Expand Down
10 changes: 7 additions & 3 deletions src/runtime/vm/translator/hopt/dce.cpp
Expand Up @@ -409,15 +409,19 @@ void eliminateDeadCode(Trace* trace, IRFactory* irFactory) {
}
}

// If main trace starts with guards, have them generate a patchable jump to the anchor trace
// If main trace starts with guards, have them generate a patchable jump
// to the anchor trace
if (RuntimeOption::EvalHHIRDirectExit) {
LabelInstruction* guardLabel = NULL;
IRInstruction::List& instList = trace->getInstructionList();
// Check the beginning of the trace for guards
for (IRInstruction::Iterator it = instList.begin(); it != instList.end(); ++it) {
for (IRInstruction::Iterator it = instList.begin(); it != instList.end();
++it) {
IRInstruction* inst = *it;
Opcode opc = inst->getOpcode();
if ((opc == LdLoc || opc == LdStack) && inst->getLabel()) {
if (inst->getLabel() &&
(opc == LdLoc || opc == LdStack ||
opc == GuardLoc || opc == GuardStk)) {
LabelInstruction* exitLabel = inst->getLabel();
// Find the GuardFailure's label and confirm this branches there
if (guardLabel == NULL) {
Expand Down
28 changes: 26 additions & 2 deletions src/runtime/vm/translator/hopt/hhbctranslator.cpp
Expand Up @@ -1425,7 +1425,8 @@ Trace* HhbcTranslator::guardTypeLocal(uint32 localIndex,
if (nextTrace == NULL) {
nextTrace = getGuardExit();
}
m_tb.genLdLoc(localIndex, type, nextTrace);
m_tb.genGuardLoc(localIndex, type, nextTrace);
m_typeGuards.push_back(TypeGuard(TypeGuard::Local, localIndex, type));
return nextTrace;
}

Expand All @@ -1438,6 +1439,8 @@ void HhbcTranslator::checkTypeLocal(uint32 localIndex, Type::Tag type) {
}

void HhbcTranslator::assertTypeLocal(uint32 localIndex, Type::Tag type) {
// Type assertions are currently implemented as loads. If the value doesn't
// get used, DCE gets rid of it.
m_tb.genLdLoc(localIndex, type, NULL);
}

Expand All @@ -1462,7 +1465,9 @@ Trace* HhbcTranslator::guardTypeStack(uint32 stackIndex,
if (nextTrace == NULL) {
nextTrace = getGuardExit();
}
checkTypeStackAux(stackIndex, type, nextTrace);
m_tb.genGuardStk(stackIndex, type, nextTrace);
m_typeGuards.push_back(TypeGuard(TypeGuard::Stack, stackIndex, type));

return nextTrace;
}

Expand All @@ -1474,12 +1479,31 @@ void HhbcTranslator::checkTypeStack(uint32 stackIndex,
}

void HhbcTranslator::assertTypeStack(uint32 stackIndex, Type::Tag type) {
// Type assertions are currently implemented as loads. If the value doesn't
// get used, DCE gets rid of it.
loadStack(stackIndex, type);
}

void HhbcTranslator::loadStack(uint32 stackIndex, Type::Tag type) {
ASSERT(stackIndex != (uint32)-1);
// top() generates the LdStack if necessary, and sets 'type' accordingly
SSATmp* tmp = top(type, stackIndex);
replace(stackIndex, tmp);
}

void HhbcTranslator::emitLoadDeps() {
for (auto guard : m_typeGuards) {
uint32 index = guard.getIndex();
Type::Tag type = guard.getType();

if (guard.getKind() == TypeGuard::Local) {
m_tb.genLdLoc(index, type, NULL);
} else {
loadStack(index, type);
}
}
}

Trace* HhbcTranslator::guardRefs(int64 entryArDelta,
const vector<bool>& mask,
const vector<bool>& vals,
Expand Down
50 changes: 39 additions & 11 deletions src/runtime/vm/translator/hopt/hhbctranslator.h
Expand Up @@ -81,6 +81,29 @@ class FpiStack {
std::stack<SSATmp*> stack;
};

class TypeGuard {
public:
enum Kind {
Local,
Stack
};

TypeGuard(Kind kind, uint32 index, Type::Tag type)
: m_kind(kind)
, m_index(index)
, m_type(type) {
}

Kind getKind() const { return m_kind; }
uint32 getIndex() const { return m_index; }
Type::Tag getType() const { return m_type; }

private:
Kind m_kind;
uint32 m_index;
Type::Tag m_type;
};

class HhbcTranslator {
public:
HhbcTranslator(TraceBuilder& builder, const Func* func)
Expand Down Expand Up @@ -316,6 +339,8 @@ class HhbcTranslator {

void setThisAvailable();

void emitLoadDeps();

private:

/*
Expand Down Expand Up @@ -345,6 +370,8 @@ class HhbcTranslator {
void emitInterpOneOrPunt(Type::Tag type, Trace* target = NULL);
void emitBinaryArith(Opcode);
void checkTypeStackAux(uint32 stackIndex, Type::Tag type, Trace* nextTrace);
void loadStack(uint32 stackIndex, Type::Tag type);


/*
* Accessors for the current function being compiled and its
Expand Down Expand Up @@ -387,20 +414,21 @@ class HhbcTranslator {
/*
* Fields
*/
TraceBuilder& m_tb;
const Func* m_curFunc;
uint32 m_bcOff;
uint32 m_bcOffNextTrace;
bool m_firstBcOff;
bool m_lastBcOff;
bool m_hasRet;
TraceBuilder& m_tb;
const Func* m_curFunc;
uint32 m_bcOff;
uint32 m_bcOffNextTrace;
bool m_firstBcOff;
bool m_lastBcOff;
bool m_hasRet;
// if set, then generate unbox instructions for memory accesses (Get
// and Set bytecodes). Otherwise, memory accesses will bail the trace
// on an access to a boxed value.
bool m_unboxPtrs;
uint32 m_stackDeficit; // offset of virtual sp from physical sp
EvalStack m_evalStack;
FpiStack m_fpiStack;
bool m_unboxPtrs;
uint32 m_stackDeficit; // offset of virtual sp from physical sp
EvalStack m_evalStack;
FpiStack m_fpiStack;
vector<TypeGuard> m_typeGuards;
};

}}} // namespace HPHP::VM::JIT
Expand Down
3 changes: 3 additions & 0 deletions src/runtime/vm/translator/hopt/ir.cpp
Expand Up @@ -235,6 +235,9 @@ void IRInstruction::setExtendedSrc(uint32 i, SSATmp* newSrc) {

void IRInstruction::printOpcode(std::ostream& ostream) {
ostream << opcodeName(m_op);
if (m_op == GuardLoc || m_op == GuardStk) {
ostream << "<" << Type::Strings[m_type] << ">";
}
}

void IRInstruction::printDst(std::ostream& ostream) {
Expand Down
4 changes: 3 additions & 1 deletion src/runtime/vm/translator/hopt/ir.h
Expand Up @@ -113,7 +113,9 @@ static const TCA kIRDirectGuardActive = (TCA)0x03;
// error (has an implicit exit edge)
#define IR_OPCODES \
/* checks */ \
OPC(GuardType, 1, 1, 1, 0, 0, 0, 0, 0, 0, 0) \
OPC(GuardType, 1, 1, 1, 0, 0, 0, 0, 0, 0, 0) \
OPC(GuardLoc, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0) \
OPC(GuardStk, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0) \
OPC(GuardRefs, 0, 1, 1, 0, 0, 0, 0, 0, 0, 0)/* XXX validate */ \
\
/* arith ops (integer) */ \
Expand Down
6 changes: 6 additions & 0 deletions src/runtime/vm/translator/hopt/irtranslator.cpp
Expand Up @@ -179,6 +179,12 @@ TranslatorX64::irCheckType(X64Assembler& a,
return;
}

void
TranslatorX64::irEmitLoadDeps() {
ASSERT(m_useHHIR);
m_hhbcTrans->emitLoadDeps();
}


void
TranslatorX64::irTranslateBinaryArithOp(const Tracelet& t,
Expand Down
2 changes: 2 additions & 0 deletions src/runtime/vm/translator/hopt/simplifier.cpp
Expand Up @@ -207,6 +207,8 @@ SSATmp* Simplifier::simplifyInst(Opcode opc,
case DecRef:
case DecRefNZ:
case GuardType:
case GuardLoc:
case GuardStk:
case LdThis:
case LdLoc:
case LdMemNR:
Expand Down

0 comments on commit a474a67

Please sign in to comment.