diff --git a/hphp/runtime/base/stats.h b/hphp/runtime/base/stats.h index 6ad906503..b34460ba2 100644 --- a/hphp/runtime/base/stats.h +++ b/hphp/runtime/base/stats.h @@ -217,8 +217,10 @@ inline void inc(StatCounter stat, int n = 1) { } } +static_assert(static_cast(OpLowInvalid) == 0, + "stats.h assumes OpLowInvalid == 0"); + inline StatCounter opcodeToStatCounter(Op opc) { - assert(static_cast(OpLowInvalid) == 0); return StatCounter(Instr_InterpBBLowInvalid + STATS_PER_OPCODE * uint8_t(opc)); } @@ -228,19 +230,16 @@ inline void incOp(Op opc) { } inline StatCounter opcodeToTranslStatCounter(Op opc) { - assert(OpLowInvalid == 0); return StatCounter(Instr_TranslLowInvalid + STATS_PER_OPCODE * uint8_t(opc)); } inline StatCounter opcodeToIRPreStatCounter(Op opc) { - assert(OpLowInvalid == 0); return StatCounter(Instr_TranslIRPreLowInvalid + STATS_PER_OPCODE * uint8_t(opc)); } inline StatCounter opcodeToIRPostStatCounter(Op opc) { - assert(OpLowInvalid == 0); return StatCounter(Instr_TranslIRPostLowInvalid + STATS_PER_OPCODE * uint8_t(opc)); } diff --git a/hphp/runtime/debugger/cmd/cmd_next.cpp b/hphp/runtime/debugger/cmd/cmd_next.cpp index f2c0b8aac..56aa6a9cf 100644 --- a/hphp/runtime/debugger/cmd/cmd_next.cpp +++ b/hphp/runtime/debugger/cmd/cmd_next.cpp @@ -177,15 +177,15 @@ void CmdNext::stepCurrentLine(CmdInterrupt& interrupt, ActRec* fp, PC pc) { // is driven directly from PHP (i.e., a loop calling send($foo)) then we'll // land back at the callsite of send(). For returns from generators, we follow // the execution stack for now, and end up at the caller of ASIO or send(). + auto op = toOp(*pc); if (fp->m_func->isGenerator() && - ((*pc == OpContSuspend) || - (*pc == OpContSuspendK) || - (*pc == OpContRetC))) { + (op == OpContSuspend || op == OpContSuspendK || op == OpContRetC)) { TRACE(2, "CmdNext: encountered yield or return from generator\n"); // Patch the projected return point(s) in both cases, to catch if we exit // the the asio iterator or if we are being iterated directly by PHP. setupStepOuts(); - if ((*pc == OpContSuspend) || (*pc == OpContSuspendK)) { + op = toOp(*pc); + if (op == OpContSuspend || op == OpContSuspendK) { // Patch the next normal execution point so we can pickup the stepping // from there if the caller is C++. setupStepCont(fp, pc); @@ -214,10 +214,11 @@ bool CmdNext::atStepContOffset(Unit* unit, Offset o) { // simplicity of the exceptional path. void CmdNext::setupStepCont(ActRec* fp, PC pc) { // One byte + one byte argument - assert((*pc == OpContSuspend) || (*pc == OpContSuspendK)); - assert(*(pc+2) == OpNull); // One byte - assert(*(pc+3) == OpThrow); // One byte - assert(*(pc+4) == OpNull); // One byte + auto ops = reinterpret_cast(pc); + assert(ops[0] == OpContSuspend || ops[0] == OpContSuspendK); + assert(ops[2] == OpNull); // One byte + assert(ops[3] == OpThrow); // One byte + assert(ops[4] == OpNull); // One byte Offset nextInst = fp->m_func->unit()->offsetOf(pc + 5); m_stepContUnit = fp->m_func->unit(); m_stepContOffset = nextInst; diff --git a/hphp/runtime/vm/bytecode.cpp b/hphp/runtime/vm/bytecode.cpp index 0e553c1a3..7370875c1 100644 --- a/hphp/runtime/vm/bytecode.cpp +++ b/hphp/runtime/vm/bytecode.cpp @@ -2053,7 +2053,7 @@ Array VMExecutionContext::debugBacktrace(bool skip /* = false */, // already do the right thing. The emitter associates object access with // the subsequent expression and this would be difficult to modify. auto const opAtPrevPc = - *reinterpret_cast(prevUnit->at(prevPc)); + toOp(*reinterpret_cast(prevUnit->at(prevPc))); Offset pcAdjust = 0; if (opAtPrevPc == OpPopR || opAtPrevPc == OpUnboxR) { pcAdjust = 1; @@ -7055,7 +7055,7 @@ void VMExecutionContext::op##name() { \ condStackTraceSep("op"#name" "); \ COND_STACKTRACE("op"#name" pre: "); \ PC pc = m_pc; \ - assert(*pc == Op##name); \ + assert(toOp(*pc) == Op##name); \ ONTRACE(1, \ int offset = m_fp->m_func->unit()->offsetOf(pc); \ Trace::trace("op"#name" offset: %d\n", offset)); \ diff --git a/hphp/runtime/vm/hhbc.h b/hphp/runtime/vm/hhbc.h index 7884f2b8a..fcebf9177 100644 --- a/hphp/runtime/vm/hhbc.h +++ b/hphp/runtime/vm/hhbc.h @@ -604,11 +604,6 @@ inline Op toOp(Opcode o) { return op; } -inline bool operator==(Op a, Opcode b) { return a == toOp(b); } -inline bool operator==(Opcode a, Op b) { return b == a; } -inline bool operator!=(Op a, Opcode b) { return !(a == b); } -inline bool operator!=(Opcode a, Op b) { return b != a; } - const MInstrInfo& getMInstrInfo(Op op); enum AstubsOp { @@ -808,7 +803,14 @@ inline bool isFPush(Op opcode) { } inline bool isFCallStar(Op opcode) { - return opcode == OpFCall || opcode == OpFCallArray; + switch (opcode) { + case OpFCall: + case OpFCallArray: + return true; + + default: + return false; + } } inline bool isFPassStar(Op opcode) { @@ -841,7 +843,7 @@ inline bool isSwitch(Opcode op) { template void foreachSwitchTarget(Op* op, L func) { assert(isSwitch(*op)); - bool isStr = readData(op) == OpSSwitch; + bool isStr = readData(op) == OpSSwitch; int32_t size = readData(op); for (int i = 0; i < size; ++i) { if (isStr) readData(op); @@ -851,7 +853,7 @@ void foreachSwitchTarget(Op* op, L func) { template void foreachSwitchString(Opcode* op, L func) { - assert(*op == OpSSwitch); + assert(toOp(*op) == OpSSwitch); readData(op); int32_t size = readData(op) - 1; // the last item is the default for (int i = 0; i < size; ++i) { diff --git a/hphp/runtime/vm/jit/targetcache.cpp b/hphp/runtime/vm/jit/targetcache.cpp index dbef19a34..2815005d0 100644 --- a/hphp/runtime/vm/jit/targetcache.cpp +++ b/hphp/runtime/vm/jit/targetcache.cpp @@ -1094,7 +1094,7 @@ StaticMethodCache::lookup(Handle handle, const NamedEntity *ne, assert(res != LookupResult::MethodFoundWithThis); // Not possible: no this. // We've already sync'ed regs; this is some hard case, we might as well // just let the interpreter handle this entirely. - assert(*vmpc() == OpFPushClsMethodD); + assert(toOp(*vmpc()) == OpFPushClsMethodD); Stats::inc(Stats::Instr_InterpOneFPushClsMethodD); Stats::inc(Stats::Instr_TC, -1); ec->opFPushClsMethodD(); diff --git a/hphp/runtime/vm/jit/translator-instrs.h b/hphp/runtime/vm/jit/translator-instrs.h index 0585c295e..aa81249be 100644 --- a/hphp/runtime/vm/jit/translator-instrs.h +++ b/hphp/runtime/vm/jit/translator-instrs.h @@ -181,56 +181,56 @@ // PSEUDOINSTR_DISPATCH is a switch() fragment that routes opcodes to their // shared handlers, as per the PSEUDOINSTRS macro. -#define PSEUDOINSTR_DISPATCH(func) \ - case OpBitAnd: \ - case OpBitOr: \ - case OpBitXor: \ - case OpSub: \ - case OpMul: \ - func(BinaryArithOp, i) \ - case OpSame: \ - case OpNSame: \ - func(SameOp, i) \ - case OpEq: \ - case OpNeq: \ - func(EqOp, i) \ - case OpLt: \ - case OpLte: \ - case OpGt: \ - case OpGte: \ - func(LtGtOp, i) \ - case OpEmptyL: \ - case OpCastBool: \ - func(UnaryBooleanOp, i) \ - case OpJmpZ: \ - case OpJmpNZ: \ - func(BranchOp, i) \ - case OpSetL: \ - case OpBindL: \ - func(AssignToLocalOp, i) \ - case OpFPassC: \ - case OpFPassCW: \ - case OpFPassCE: \ - func(FPassCOp, i) \ - case OpFPushCuf: \ - case OpFPushCufF: \ - case OpFPushCufSafe: \ - func(FPushCufOp, i) \ - case OpIssetL: \ - case OpIsNullL: \ - case OpIsStringL: \ - case OpIsArrayL: \ - case OpIsIntL: \ - case OpIsObjectL: \ - case OpIsBoolL: \ - case OpIsDoubleL: \ - case OpIsNullC: \ - case OpIsStringC: \ - case OpIsArrayC: \ - case OpIsIntC: \ - case OpIsObjectC: \ - case OpIsBoolC: \ - case OpIsDoubleC: \ +#define PSEUDOINSTR_DISPATCH(func) \ + case Op::BitAnd: \ + case Op::BitOr: \ + case Op::BitXor: \ + case Op::Sub: \ + case Op::Mul: \ + func(BinaryArithOp, i) \ + case Op::Same: \ + case Op::NSame: \ + func(SameOp, i) \ + case Op::Eq: \ + case Op::Neq: \ + func(EqOp, i) \ + case Op::Lt: \ + case Op::Lte: \ + case Op::Gt: \ + case Op::Gte: \ + func(LtGtOp, i) \ + case Op::EmptyL: \ + case Op::CastBool: \ + func(UnaryBooleanOp, i) \ + case Op::JmpZ: \ + case Op::JmpNZ: \ + func(BranchOp, i) \ + case Op::SetL: \ + case Op::BindL: \ + func(AssignToLocalOp, i) \ + case Op::FPassC: \ + case Op::FPassCW: \ + case Op::FPassCE: \ + func(FPassCOp, i) \ + case Op::FPushCuf: \ + case Op::FPushCufF: \ + case Op::FPushCufSafe: \ + func(FPushCufOp, i) \ + case Op::IssetL: \ + case Op::IsNullL: \ + case Op::IsStringL: \ + case Op::IsArrayL: \ + case Op::IsIntL: \ + case Op::IsObjectL: \ + case Op::IsBoolL: \ + case Op::IsDoubleL: \ + case Op::IsNullC: \ + case Op::IsStringC: \ + case Op::IsArrayC: \ + case Op::IsIntC: \ + case Op::IsObjectC: \ + case Op::IsBoolC: \ + case Op::IsDoubleC: \ func(CheckTypeOp, i) #endif diff --git a/hphp/runtime/vm/jit/translator-x64.cpp b/hphp/runtime/vm/jit/translator-x64.cpp index 8fec7ca1d..568cabd41 100644 --- a/hphp/runtime/vm/jit/translator-x64.cpp +++ b/hphp/runtime/vm/jit/translator-x64.cpp @@ -1782,7 +1782,7 @@ int32_t TranslatorX64::emitNativeImpl(const Func* func, */ assert(func->numIterators() == 0 && func->isBuiltin()); assert(func->numLocals() == func->numParams()); - assert(*func->getEntry() == OpNativeImpl); + assert(toOp(*func->getEntry()) == OpNativeImpl); assert(instrLen((Op*)func->getEntry()) == func->past() - func->base()); Offset pcOffset = 0; // NativeImpl is the only instruction in the func Offset stackOff = func->numLocals(); // Builtin stubs have no diff --git a/hphp/runtime/vm/jit/translator.cpp b/hphp/runtime/vm/jit/translator.cpp index de6d6c7e3..8d23154ac 100644 --- a/hphp/runtime/vm/jit/translator.cpp +++ b/hphp/runtime/vm/jit/translator.cpp @@ -2446,7 +2446,7 @@ void Translator::postAnalyze(NormalizedInstruction* ni, SrcKey& sk, SrcKey src = sk; const Unit* unit = ni->m_unit; src.advance(unit); - Opcode next = *unit->at(src.offset()); + Op next = toOp(*unit->at(src.offset())); if (next == OpInstanceOfD || next == OpIsNullC) { ni->outStack->rtt = RuntimeType(KindOfObject); } @@ -3326,13 +3326,14 @@ std::unique_ptr Translator::analyze(SrcKey sk, assert(fpi); Offset fpushOff = fpi->m_fpushOff; PC fpushPc = curUnit()->at(fpushOff); - if (*fpushPc == OpFPushFunc) { + auto const op = toOp(*fpushPc); + if (op == OpFPushFunc) { doVarEnvTaint = true; - } else if (*fpushPc == OpFPushFuncD) { + } else if (op == OpFPushFuncD) { StringData *funcName = curUnit()->lookupLitstrId(getImm((Op*)fpushPc, 1).u_SA); doVarEnvTaint = checkTaintFuncs(funcName); - } else if (*fpushPc == OpFPushFuncU) { + } else if (op == OpFPushFuncU) { StringData *fallbackName = curUnit()->lookupLitstrId(getImm((Op*)fpushPc, 2).u_SA); doVarEnvTaint = checkTaintFuncs(fallbackName); @@ -3413,7 +3414,7 @@ std::unique_ptr Translator::analyze(SrcKey sk, // If we've gotten this far, it mostly boils down to control-flow // instructions. However, we'll trace through a few unconditional jmps. if (ni->op() == OpJmp && - ni->imm[0].u_IA > 0 && + ni->imm[0].u_BA > 0 && tas.m_numJmps < MaxJmpsTracedThrough) { // Continue tracing through jumps. To prevent pathologies, only trace // through a finite number of forward jumps. @@ -3709,7 +3710,7 @@ bool Translator::instrMustInterp(const NormalizedInstruction& inst) { switch (inst.op()) { // Generate a case for each instruction we support at least partially. -# define CASE(name) case Op##name: +# define CASE(name) case Op::name: INSTRS # undef CASE # define NOTHING(...) // PSEUDOINSTR_DISPATCH has the cases in it diff --git a/hphp/runtime/vm/type_profile.cpp b/hphp/runtime/vm/type_profile.cpp index ea0cddfdb..6b1fe638f 100644 --- a/hphp/runtime/vm/type_profile.cpp +++ b/hphp/runtime/vm/type_profile.cpp @@ -295,7 +295,8 @@ PredVal predictType(TypeProfileKey key) { } bool isProfileOpcode(const PC& pc) { - return *pc == OpRetC || *pc == OpCGetM; + auto const op = toOp(*pc); + return op == OpRetC || op == OpCGetM; } } diff --git a/hphp/runtime/vm/unit.cpp b/hphp/runtime/vm/unit.cpp index b2fb3c416..3a50550a3 100644 --- a/hphp/runtime/vm/unit.cpp +++ b/hphp/runtime/vm/unit.cpp @@ -448,7 +448,7 @@ bool Unit::compileTimeFatal(const StringData*& msg, int& line) const { const Opcode* pc = entry; // String ; Fatal; // ^^^^^^ - if (*pc != OpString) { + if (toOp(*pc) != OpString) { return false; } pc++; @@ -458,7 +458,7 @@ bool Unit::compileTimeFatal(const StringData*& msg, int& line) const { pc += sizeof(Id); // String ; Fatal; // ^^^^^ - if (*pc != OpFatal) { + if (toOp(*pc) != OpFatal) { return false; } msg = lookupLitstrId(id); @@ -1581,7 +1581,8 @@ void Unit::prettyPrint(std::ostream& out, PrintOpts opts) const { switch (info.m_kind) { case Unit::MetaInfo::Kind::DataTypeInferred: case Unit::MetaInfo::Kind::DataTypePredicted: - out << " i" << argKind << arg << ":t=" << (int)info.m_data; + out << " i" << argKind << arg + << ":t=" << tname(DataType(info.m_data)); if (info.m_kind == Unit::MetaInfo::Kind::DataTypePredicted) { out << "*"; } diff --git a/hphp/runtime/vm/unwind.cpp b/hphp/runtime/vm/unwind.cpp index 0926d85a4..8914a0727 100644 --- a/hphp/runtime/vm/unwind.cpp +++ b/hphp/runtime/vm/unwind.cpp @@ -146,7 +146,7 @@ UnwindAction checkHandlers(const EHEnt* eh, void tearDownFrame(ActRec*& fp, Stack& stack, PC& pc, Offset& faultOffset) { auto const func = fp->m_func; - auto const curOp = *reinterpret_cast(pc); + auto const curOp = *reinterpret_cast(pc); auto const unwindingGeneratorFrame = func->isGenerator(); auto const unwindingReturningFrame = curOp == OpRetC || curOp == OpRetV; auto const prevFp = fp->arGetSfp(); diff --git a/hphp/runtime/vm/verifier/check_func.cpp b/hphp/runtime/vm/verifier/check_func.cpp index 1ac47950a..2ca6bd890 100644 --- a/hphp/runtime/vm/verifier/check_func.cpp +++ b/hphp/runtime/vm/verifier/check_func.cpp @@ -251,7 +251,7 @@ bool FuncChecker::checkSection(bool is_main, const char* name, Offset base, m_instrs.set(offset(pc) - m_func->base()); if (isSwitch(toOp(*pc)) || instrJumpTarget((Op*)bc, offset(pc)) != InvalidAbsoluteOffset) { - if (*pc == OpSwitch && getImm((Op*)pc, 2).u_IVA != 0) { + if (toOp(*pc) == OpSwitch && getImm((Op*)pc, 2).u_IVA != 0) { int64_t switchBase = getImm((Op*)pc, 1).u_I64A; int32_t len = getImmVector((Op*)pc).size(); if (len <= 2) { @@ -261,7 +261,7 @@ bool FuncChecker::checkSection(bool is_main, const char* name, Offset base, if (switchBase > std::numeric_limits::max() - len + 2) { error("Overflow in Switch bounds [%d:%d]\n", base, past); } - } else if (*pc == OpSSwitch) { + } else if (toOp(*pc) == OpSSwitch) { foreachSwitchString((Opcode*)pc, [&](Id& id) { ok &= checkString(pc, id); });