diff --git a/hphp/runtime/vm/translator/hopt/codegen.cpp b/hphp/runtime/vm/translator/hopt/codegen.cpp index debcbdd46..9a5587371 100644 --- a/hphp/runtime/vm/translator/hopt/codegen.cpp +++ b/hphp/runtime/vm/translator/hopt/codegen.cpp @@ -681,7 +681,7 @@ void CodeGenerator::cgCallNative(IRInstruction* inst) { argGroup.ssa(src); break; case TV: - argGroup.valueType(src); + argGroup.typedValue(src); break; case VecKeyS: argGroup.vectorKeyS(src); @@ -703,39 +703,44 @@ void CodeGenerator::cgCallNative(IRInstruction* inst) { } cgCallHelper(m_as, addr, - info.dest ? inst->getDst() : nullptr, + info.dest != DestType::None ? inst->getDst() : nullptr, info.sync, - argGroup); + argGroup, + info.dest); } void CodeGenerator::cgCallHelper(Asm& a, TCA addr, SSATmp* dst, SyncOptions sync, - ArgGroup& args) { + ArgGroup& args, + DestType destType) { PhysReg dstReg0 = InvalidReg; PhysReg dstReg1 = InvalidReg; if (dst) { dstReg0 = dst->getReg(0); dstReg1 = dst->getReg(1); } - return cgCallHelper(a, Transl::Call(addr), dstReg0, dstReg1, sync, args); + return cgCallHelper(a, Transl::Call(addr), dstReg0, dstReg1, sync, args, + destType); } void CodeGenerator::cgCallHelper(Asm& a, TCA addr, PhysReg dstReg, SyncOptions sync, - ArgGroup& args) { - cgCallHelper(a, Transl::Call(addr), dstReg, InvalidReg, sync, args); + ArgGroup& args, + DestType destType) { + cgCallHelper(a, Transl::Call(addr), dstReg, InvalidReg, sync, args, destType); } void CodeGenerator::cgCallHelper(Asm& a, const Transl::Call& call, PhysReg dstReg, SyncOptions sync, - ArgGroup& args) { - cgCallHelper(a, call, dstReg, InvalidReg, sync, args); + ArgGroup& args, + DestType destType) { + cgCallHelper(a, call, dstReg, InvalidReg, sync, args, destType); } void CodeGenerator::cgCallHelper(Asm& a, @@ -743,7 +748,8 @@ void CodeGenerator::cgCallHelper(Asm& a, PhysReg dstReg0, PhysReg dstReg1, SyncOptions sync, - ArgGroup& args) { + ArgGroup& args, + DestType destType) { assert(int(args.size()) <= kNumRegisterArgs); assert(m_curInst->isNative()); @@ -771,17 +777,20 @@ void CodeGenerator::cgCallHelper(Asm& a, recordSyncPoint(a, sync); } - // assume if dstReg1 is needed that the result is a TypedValue passed - // by value. In that case we need to rightshift the packed m_type - // enum to occupy the low 32bits of the dest register. - if (dstReg1 != InvalidReg) { - // dstReg1 contains m_type and m_aux, but we're expecting just the - // type in the lower 32 bits, so shift the 2nd result register. - a. shrq (kTypeShiftBits, reg::rdx); + // copy the call result to the destination register(s) + if (destType == DestType::TV) { + // rdx contains m_type and m_aux; we need to put just the type in the + // lower bits, so right-shift. rax contains m_data. + if (kTypeShiftBits > 0) a.shrq(kTypeShiftBits, reg::rdx); + shuffle2(a, reg::rax, reg::rdx, dstReg0, dstReg1); + } else if (destType == DestType::SSA) { + // copy the single-register result to dstReg0 + assert(dstReg1 == InvalidReg); + if (dstReg0 != InvalidReg) emitMovRegReg(a, reg::rax, dstReg0); + } else { + // void return type, no registers have values + assert(dstReg0 == InvalidReg && dstReg1 == InvalidReg); } - - // safely copy the return value (rax:rdx) to (dstReg0:dstReg1) - shuffle2(a, reg::rax, reg::rdx, dstReg0, dstReg1); } void CodeGenerator::cgMov(IRInstruction* inst) { @@ -1657,19 +1666,21 @@ void CodeGenerator::cgConv(IRInstruction* inst) { } else { Transl::Call helper(nullptr); ArgGroup args; - args.ssa(src); if (fromType == Type::Cell) { // Cell -> Bool - args.type(src); + args.typedValue(src); helper = Transl::Call((TCA)cellToBoolHelper); } else if (fromType.isString()) { // Str -> Bool + args.ssa(src); helper = Transl::Call(getMethodPtr(&StringData::toBoolean)); } else if (fromType.isArray()) { // Arr -> Bool + args.ssa(src); helper = Transl::Call((TCA)arrToBoolHelper); } else if (fromType == Type::Obj) { // Obj -> Bool + args.ssa(src); helper = Transl::Call(getMethodPtr(&ObjectData::o_toBoolean)); } else { // Dbl -> Bool @@ -1704,8 +1715,7 @@ void CodeGenerator::cgConv(IRInstruction* inst) { if (toType.isArray()) { ArgGroup args; - args.ssa(src); - args.type(src); + args.typedValue(src); ArrayData*(*fPtr)(TypedValue) = new_singleton_array_helper; cgCallHelper(m_as, (TCA)fPtr, dst, kNoSyncPoint, args); return; @@ -2030,7 +2040,7 @@ void CodeGenerator::cgLdSSwitchDestSlow(IRInstruction* inst) { inst->getDst(), kSyncPoint, ArgGroup() - .valueType(inst->getSrc(0)) + .typedValue(inst->getSrc(0)) .immPtr(strtab) .imm(data->numCases) .immPtr(jmptab)); @@ -3155,10 +3165,11 @@ void CodeGenerator::cgCallBuiltin(IRInstruction* inst) { if (returnType.subtypeOf(Type::Cell) || returnType.subtypeOf(Type::BoxedCell)) { m_as. loadl (returnBase[returnOffset + TVOFF(m_type)], r32(dstType)); - m_as. loadq (returnBase[returnOffset], dstReg); + m_as. loadq (returnBase[returnOffset + TVOFF(m_data)], dstReg); emitLoadImm(m_as, KindOfNull, rScratch); - static_assert(KindOfUninit == 0, "CallBuiltin needs update for KindOfUninit"); - m_as. testq (dstType, dstType); + static_assert(KindOfUninit == 0, + "CallBuiltin needs update for KindOfUninit"); + m_as. testl (r32(dstType), r32(dstType)); m_as. cmov_reg64_reg64 (CC_Z, rScratch, dstType); return; } @@ -4087,7 +4098,7 @@ void CodeGenerator::cgLookupClsCns(IRInstruction* inst) { .immPtr(cnsName->getValStr()); cgCallHelper(m_as, TCA(TargetCache::lookupClassConstantTv), - inst->getDst(), kSyncPoint, args); + inst->getDst(), kSyncPoint, args, DestType::TV); } HOT_FUNC_VM @@ -4315,7 +4326,8 @@ void CodeGenerator::cgReleaseVVOrExit(IRInstruction* inst) { TCA(static_cast(ExtraArgs::deallocate)), nullptr, kSyncPoint, - ArgGroup().reg(rFp) + ArgGroup().reg(rFp), + DestType::None ); }); } @@ -4386,7 +4398,7 @@ void CodeGenerator::cgConcat(IRInstruction* inst) { CG_PUNT(cgConcat); } cgCallHelper(m_as, (TCA)concat_value, dst, kNoSyncPoint, - ArgGroup().valueType(tl).valueType(tr)); + ArgGroup().typedValue(tl).typedValue(tr)); } } @@ -4461,7 +4473,7 @@ void CodeGenerator::cgDefFunc(IRInstruction* inst) { SSATmp* dst = inst->getDst(); SSATmp* func = inst->getSrc(0); cgCallHelper(m_as, (TCA)defFuncHelper, dst, kSyncPoint, - ArgGroup().ssa(func)); + ArgGroup().ssa(func), DestType::None); } void CodeGenerator::cgFillContThis(IRInstruction* inst) { diff --git a/hphp/runtime/vm/translator/hopt/codegen.h b/hphp/runtime/vm/translator/hopt/codegen.h index 7718d1090..bd683aec9 100644 --- a/hphp/runtime/vm/translator/hopt/codegen.h +++ b/hphp/runtime/vm/translator/hopt/codegen.h @@ -38,6 +38,14 @@ class FailedCodeGen : public std::exception { struct ArgGroup; +// DestType describes the return type of native helper calls, particularly +// register assignments +enum class DestType : unsigned { + None, // return void (no valid registers) + SSA, // return a single-register value + TV // return a TypedValue packed in two registers +}; + enum SyncOptions { kNoSyncPoint, kSyncPoint, @@ -121,23 +129,27 @@ private: TCA addr, SSATmp* dst, SyncOptions sync, - ArgGroup& args); + ArgGroup& args, + DestType destType = DestType::SSA); void cgCallHelper(Asm& a, TCA addr, PhysReg dstReg, SyncOptions sync, - ArgGroup& args); + ArgGroup& args, + DestType destType = DestType::SSA); void cgCallHelper(Asm& a, const Transl::Call& call, PhysReg dstReg, SyncOptions sync, - ArgGroup& args); + ArgGroup& args, + DestType destType = DestType::SSA); void cgCallHelper(Asm& a, const Transl::Call& call, PhysReg dstReg0, PhysReg dstReg1, SyncOptions sync, - ArgGroup& args); + ArgGroup& args, + DestType destType = DestType::SSA); void cgStore(PhysReg base, int64_t off, @@ -416,7 +428,7 @@ struct ArgGroup { /* * Pass tmp as a TypedValue passed by value. */ - ArgGroup& valueType(SSATmp* tmp) { + ArgGroup& typedValue(SSATmp* tmp) { return ssa(tmp).type(tmp); } @@ -436,11 +448,9 @@ struct ArgGroup { private: ArgGroup& vectorKeyImpl(SSATmp* key, bool allowInt) { if (key->isString() || (allowInt && key->isA(Type::Int))) { - ssa(key).none(); - } else { - valueType(key); + return ssa(key).none(); } - return *this; + return typedValue(key); } std::vector m_args; diff --git a/hphp/runtime/vm/translator/hopt/nativecalls.cpp b/hphp/runtime/vm/translator/hopt/nativecalls.cpp index afa3dedd0..037ec61f9 100644 --- a/hphp/runtime/vm/translator/hopt/nativecalls.cpp +++ b/hphp/runtime/vm/translator/hopt/nativecalls.cpp @@ -40,8 +40,9 @@ static const SyncOptions SSyncAdj1 = kSyncPointAdjustOne; * (TCA) - Raw function pointer * {FSSA, idx} - Use a const TCA from inst->getSrc(idx) * Dest - * DSSA - The helper returns a value to be assigned to inst->getDst() - * DNone - The helper does not return a value + * DestType::SSA - The helper returns a single-register value + * DestType::TV - The helper returns a TypedValue in two registers + * DestType::None - The helper does not return a value * SyncPoint * SNone - The helper does not need a sync point * SSync - The helper needs a normal sync point @@ -57,63 +58,66 @@ static const SyncOptions SSyncAdj1 = kSyncPointAdjustOne; */ static CallMap s_callMap({ /* Opcode, Func, Dest, SyncPoint, Args */ - {AddElemStrKey, (TCA)addElemStringKeyHelper, DSSA, SNone, + {AddElemStrKey, (TCA)addElemStringKeyHelper, DestType::SSA, SNone, {{SSA, 0}, {SSA, 1}, {TV, 2}}}, - {AddElemIntKey, (TCA)addElemIntKeyHelper, DSSA, SNone, + {AddElemIntKey, (TCA)addElemIntKeyHelper, DestType::SSA, SNone, {{SSA, 0}, {SSA, 1}, {TV, 2}}}, - {AddNewElem, (TCA)&HphpArray::AddNewElemC, DSSA, SNone, + {AddNewElem, (TCA)&HphpArray::AddNewElemC, DestType::SSA, SNone, {{SSA, 0}, {TV, 1}}}, - {ArrayAdd, (TCA)array_add, DSSA, SNone, {{SSA, 0}, {SSA, 1}}}, - {Box, (TCA)box_value, DSSA, SNone, {{TV, 0}}}, - {LdObjMethod, (TCA)TargetCache::MethodCache::lookup, - DSSA, SSync, {{SSA, 0}, {SSA, 2}, {SSA, 1}}}, - {NewArray, (TCA)new_array, DSSA, SNone, {{SSA, 0}}}, - {NewTuple, (TCA)new_tuple, DSSA, SNone, + {ArrayAdd, (TCA)array_add, DestType::SSA, SNone, {{SSA, 0}, {SSA, 1}}}, - {PrintStr, (TCA)print_string, DNone, SNone, {{SSA, 0}}}, - {PrintInt, (TCA)print_int, DNone, SNone, {{SSA, 0}}}, - {PrintBool, (TCA)print_boolean, DNone, SNone, {{SSA, 0}}}, - {RaiseUninitLoc, (TCA)raiseUndefVariable, DNone, SSync, {{SSA, 0}}}, + {Box, (TCA)box_value, DestType::SSA, SNone, {{TV, 0}}}, + {LdObjMethod, (TCA)TargetCache::MethodCache::lookup, + DestType::SSA, SSync, + {{SSA, 0}, {SSA, 2}, {SSA, 1}}}, + {NewArray, (TCA)new_array, DestType::SSA, SNone, {{SSA, 0}}}, + {NewTuple, (TCA)new_tuple, DestType::SSA, SNone, + {{SSA, 0}, {SSA, 1}}}, + {PrintStr, (TCA)print_string, DestType::None, SNone, {{SSA, 0}}}, + {PrintInt, (TCA)print_int, DestType::None, SNone, {{SSA, 0}}}, + {PrintBool, (TCA)print_boolean, DestType::None, SNone, {{SSA, 0}}}, + {RaiseUninitLoc, (TCA)raiseUndefVariable, DestType::None, SSync, + {{SSA, 0}}}, /* Switch helpers */ - {LdSwitchDblIndex, (TCA)switchDoubleHelper, DSSA, SSync, + {LdSwitchDblIndex, (TCA)switchDoubleHelper, DestType::SSA, SSync, {{SSA, 0}, {SSA, 1}, {SSA, 2}}}, - {LdSwitchStrIndex, (TCA)switchStringHelper, DSSA, SSync, + {LdSwitchStrIndex, (TCA)switchStringHelper, DestType::SSA, SSync, {{SSA, 0}, {SSA, 1}, {SSA, 2}}}, - {LdSwitchObjIndex, (TCA)switchObjHelper, DSSA, SSync, + {LdSwitchObjIndex, (TCA)switchObjHelper, DestType::SSA, SSync, {{SSA, 0}, {SSA, 1}, {SSA, 2}}}, /* Continuation support helpers */ - {CreateCont, {FSSA, 0}, DSSA, SNone, + {CreateCont, {FSSA, 0}, DestType::SSA, SNone, {{SSA, 0}, {SSA, 1}, {SSA, 2}, {SSA, 3}, {SSA, 4}}}, - {FillContLocals, - (TCA)&VMExecutionContext::fillContinuationVars, - DNone, SNone, {{SSA, 0}, {SSA, 1}, {SSA, 2}, {SSA, 3}, {SSA, 4}}}, + {FillContLocals, (TCA)&VMExecutionContext::fillContinuationVars, + DestType::None, SNone, + {{SSA, 0}, {SSA, 1}, {SSA, 2}, {SSA, 3}, {SSA, 4}}}, /* VectorTranslator helpers */ - {BaseG, {FSSA, 0}, DSSA, SSync, {{TV, 1}, {SSA, 2}}}, - {PropX, {FSSA, 0}, DSSA, SSync, + {BaseG, {FSSA, 0}, DestType::SSA, SSync, {{TV, 1}, {SSA, 2}}}, + {PropX, {FSSA, 0}, DestType::SSA, SSync, {{SSA, 1}, {SSA, 2}, {VecKeyS, 3}, {SSA, 4}}}, - {CGetProp, {FSSA, 0}, DSSA, SSync, + {CGetProp, {FSSA, 0}, DestType::TV, SSync, {{SSA, 1}, {SSA, 2}, {VecKeyS, 3}, {SSA, 4}}}, - {SetProp, {FSSA, 0}, DSSA, SSync, + {SetProp, {FSSA, 0}, DestType::TV, SSync, {{SSA, 1}, {SSA, 2}, {VecKeyS, 3}, {TV, 4}}}, - {ElemX, {FSSA, 0}, DSSA, SSync, + {ElemX, {FSSA, 0}, DestType::SSA, SSync, {{SSA, 1}, {VecKeyIS, 2}, {SSA, 3}}}, - {ElemDX, {FSSA, 0}, DSSA, SSync, + {ElemDX, {FSSA, 0}, DestType::SSA, SSync, {{SSA, 1}, {VecKeyIS, 2}, {SSA, 3}}}, - {CGetElem, {FSSA, 0}, DSSA, SSync, + {CGetElem, {FSSA, 0}, DestType::TV, SSync, {{SSA, 1}, {VecKeyIS, 2}, {SSA, 3}}}, - {SetElem, {FSSA, 0}, DSSA, SSync, + {SetElem, {FSSA, 0}, DestType::TV, SSync, {{SSA, 1}, {VecKeyIS, 2}, {TV, 3}}}, - {SetNewElem, (TCA)setNewElem, DSSA, SSync, {{SSA, 0}, {TV, 1}}}, - {IssetElem,{FSSA, 0}, DSSA, SSync, + {SetNewElem, (TCA)setNewElem, DestType::TV, SSync, {{SSA, 0}, {TV, 1}}}, + {IssetElem,{FSSA, 0}, DestType::SSA, SSync, {{SSA, 1}, {VecKeyIS, 2}, {SSA, 3}}}, - {EmptyElem,{FSSA, 0}, DSSA, SSync, + {EmptyElem,{FSSA, 0}, DestType::SSA, SSync, {{SSA, 1}, {VecKeyIS, 2}, {SSA, 3}}}, /* debug assert helpers */ - {DbgAssertPtr, (TCA)assertTv, DNone, SNone, {{SSA, 0}}}, + {DbgAssertPtr, (TCA)assertTv, DestType::None, SNone, {{SSA, 0}}}, }); CallMap::CallMap(CallInfoList infos) { diff --git a/hphp/runtime/vm/translator/hopt/nativecalls.h b/hphp/runtime/vm/translator/hopt/nativecalls.h index fcb9b376c..d47becf72 100644 --- a/hphp/runtime/vm/translator/hopt/nativecalls.h +++ b/hphp/runtime/vm/translator/hopt/nativecalls.h @@ -33,6 +33,7 @@ enum FuncType : unsigned { FPtr, FSSA, }; + struct FuncPtr { FuncPtr() {} FuncPtr(TCA f) : type(FPtr), ptr(f) {} @@ -45,11 +46,6 @@ struct FuncPtr { }; }; -enum DestType : unsigned { - DNone, - DSSA, -}; - enum ArgType : unsigned { SSA, TV,