From 5371743d9d5ee7424ee020fb0b1975bae1dd4dba Mon Sep 17 00:00:00 2001 From: Jordan DeLong Date: Wed, 26 Jun 2013 22:09:17 -0700 Subject: [PATCH] Convert tv_comparisons/tv_conversions by-pointer TypedValue/Cells to by value If we're not going to mutate the Cell, it might make sense to pass it by value rather than pointer to const. Do folks like this better? I can see a couple arguments various ways. But it does seem like even if we want to pass it by pointer at the hardware level we would ideally passing by const reference at the language level, so this choice would be transparent at callsite code. This diff doesn't change anything in tv_helpers.h for now. --- .../expression/binary_op_expression.cpp | 4 +- hphp/runtime/base/array/array_data.cpp | 4 +- hphp/runtime/base/comparisons.h | 74 +++--- hphp/runtime/base/tv_arith.cpp | 4 +- hphp/runtime/base/tv_comparisons-inl.h | 6 +- hphp/runtime/base/tv_comparisons.cpp | 246 +++++++++--------- hphp/runtime/base/tv_comparisons.h | 62 ++--- hphp/runtime/base/tv_conversions-inl.h | 34 +-- hphp/runtime/base/tv_conversions.h | 4 +- hphp/runtime/base/type_array.cpp | 4 +- hphp/runtime/vm/bytecode.cpp | 16 +- hphp/runtime/vm/jit/codegen.cpp | 2 +- hphp/runtime/vm/jit/translator-runtime.cpp | 2 +- hphp/runtime/vm/member_operations.h | 2 +- 14 files changed, 230 insertions(+), 234 deletions(-) diff --git a/hphp/compiler/expression/binary_op_expression.cpp b/hphp/compiler/expression/binary_op_expression.cpp index 189870c5c..7749412ff 100644 --- a/hphp/compiler/expression/binary_op_expression.cpp +++ b/hphp/compiler/expression/binary_op_expression.cpp @@ -544,11 +544,11 @@ ExpressionPtr BinaryOpExpression::foldConst(AnalysisResultConstPtr ar) { case '<': result = less(v1, v2); break; case T_IS_SMALLER_OR_EQUAL: - result = cellLessOrEqual(v1.asCell(), v2.asCell()); break; + result = cellLessOrEqual(*v1.asCell(), *v2.asCell()); break; case '>': result = more(v1, v2); break; case T_IS_GREATER_OR_EQUAL: - result = cellGreaterOrEqual(v1.asCell(), v2.asCell()); break; + result = cellGreaterOrEqual(*v1.asCell(), *v2.asCell()); break; case '+': *result.asCell() = cellAdd(*v1.asCell(), *v2.asCell()); break; diff --git a/hphp/runtime/base/array/array_data.cpp b/hphp/runtime/base/array/array_data.cpp index 5f987b0ae..c5e312bb7 100644 --- a/hphp/runtime/base/array/array_data.cpp +++ b/hphp/runtime/base/array/array_data.cpp @@ -184,8 +184,8 @@ bool ArrayData::equal(const ArrayData *v2, bool strict) const { for (ArrayIter iter(this); iter; ++iter) { Variant key(iter.first()); if (!v2->exists(key)) return false; - if (!tvEqual(iter.second().asTypedValue(), - v2->get(key).asTypedValue())) { + if (!tvEqual(*iter.second().asTypedValue(), + *v2->get(key).asTypedValue())) { return false; } } diff --git a/hphp/runtime/base/comparisons.h b/hphp/runtime/base/comparisons.h index 4c91fbfc0..a161f9544 100644 --- a/hphp/runtime/base/comparisons.h +++ b/hphp/runtime/base/comparisons.h @@ -36,95 +36,95 @@ bool same(CVarRef v1, litstr v2); bool same(CVarRef v1, CArrRef v2); bool same(CVarRef v1, CObjRef v2); inline bool same(CVarRef v1, CVarRef v2) { - return tvSame(v1.asTypedValue(), v2.asTypedValue()); + return tvSame(*v1.asTypedValue(), *v2.asTypedValue()); } -inline bool equal(CVarRef v1, bool v2) { return cellEqual(v1.asCell(), v2); } -inline bool equal(CVarRef v1, int v2) { return cellEqual(v1.asCell(), v2); } -inline bool equal(CVarRef v1, int64_t v2) { return cellEqual(v1.asCell(), v2);} -inline bool equal(CVarRef v1, double v2) { return cellEqual(v1.asCell(), v2);} +inline bool equal(CVarRef v1, bool v2) { return cellEqual(*v1.asCell(), v2); } +inline bool equal(CVarRef v1, int v2) { return cellEqual(*v1.asCell(), v2); } +inline bool equal(CVarRef v1, int64_t v2) { return cellEqual(*v1.asCell(), v2);} +inline bool equal(CVarRef v1, double v2) { return cellEqual(*v1.asCell(), v2);} inline bool equal(CVarRef v1, const StringData* v2) { - return cellEqual(v1.asCell(), v2); + return cellEqual(*v1.asCell(), v2); } inline bool equal(CVarRef v1, CStrRef v2) { - if (!v2.get()) return cellEqual(v1.asCell(), false); - return cellEqual(v1.asCell(), v2.get()); + if (!v2.get()) return cellEqual(*v1.asCell(), false); + return cellEqual(*v1.asCell(), v2.get()); } inline bool equal(CVarRef v1, litstr v2) = delete; inline bool equal(CVarRef v1, CArrRef v2) { - if (!v2.get()) return cellEqual(v1.asCell(), false); - return cellEqual(v1.asCell(), v2.get()); + if (!v2.get()) return cellEqual(*v1.asCell(), false); + return cellEqual(*v1.asCell(), v2.get()); } inline bool equal(CVarRef v1, CObjRef v2) { - if (!v2.get()) return cellEqual(v1.asCell(), false); - return cellEqual(v1.asCell(), v2.get()); + if (!v2.get()) return cellEqual(*v1.asCell(), false); + return cellEqual(*v1.asCell(), v2.get()); } inline bool equal(CVarRef v1, CVarRef v2) { - return tvEqual(v1.asTypedValue(), v2.asTypedValue()); + return tvEqual(*v1.asTypedValue(), *v2.asTypedValue()); } inline bool less(CVarRef v1, bool v2) { - return cellLess(v1.asCell(), v2); + return cellLess(*v1.asCell(), v2); } inline bool less(CVarRef v1, int v2) { - return cellLess(v1.asCell(), v2); + return cellLess(*v1.asCell(), v2); } inline bool less(CVarRef v1, int64_t v2) { - return cellLess(v1.asCell(), v2); + return cellLess(*v1.asCell(), v2); } inline bool less(CVarRef v1, double v2) { - return cellLess(v1.asCell(), v2); + return cellLess(*v1.asCell(), v2); } inline bool less(CVarRef v1, const StringData* v2) { - return cellLess(v1.asCell(), v2); + return cellLess(*v1.asCell(), v2); } inline bool less(CVarRef v1, CStrRef v2) { - if (!v2.get()) return cellLess(v1.asCell(), false); - return cellLess(v1.asCell(), v2.get()); + if (!v2.get()) return cellLess(*v1.asCell(), false); + return cellLess(*v1.asCell(), v2.get()); } inline bool less(CVarRef v1, litstr v2) = delete; inline bool less(CVarRef v1, CArrRef v2) { - if (!v2.get()) return cellLess(v1.asCell(), false); - return cellLess(v1.asCell(), v2.get()); + if (!v2.get()) return cellLess(*v1.asCell(), false); + return cellLess(*v1.asCell(), v2.get()); } inline bool less(CVarRef v1, CObjRef v2) { - if (!v2.get()) return cellLess(v1.asCell(), false); - return cellLess(v1.asCell(), v2.get()); + if (!v2.get()) return cellLess(*v1.asCell(), false); + return cellLess(*v1.asCell(), v2.get()); } inline bool less(CVarRef v1, CVarRef v2) { - return tvLess(v1.asTypedValue(), v2.asTypedValue()); + return tvLess(*v1.asTypedValue(), *v2.asTypedValue()); } inline bool more(CVarRef v1, bool v2) { - return cellGreater(v1.asCell(), v2); + return cellGreater(*v1.asCell(), v2); } inline bool more(CVarRef v1, int v2) { - return cellGreater(v1.asCell(), v2); + return cellGreater(*v1.asCell(), v2); } inline bool more(CVarRef v1, int64_t v2) { - return cellGreater(v1.asCell(), v2); + return cellGreater(*v1.asCell(), v2); } inline bool more(CVarRef v1, double v2) { - return cellGreater(v1.asCell(), v2); + return cellGreater(*v1.asCell(), v2); } inline bool more(CVarRef v1, const StringData* v2) { - return cellGreater(v1.asCell(), v2); + return cellGreater(*v1.asCell(), v2); } inline bool more(CVarRef v1, CStrRef v2) { - if (!v2.get()) return cellGreater(v1.asCell(), false); - return cellGreater(v1.asCell(), v2.get()); + if (!v2.get()) return cellGreater(*v1.asCell(), false); + return cellGreater(*v1.asCell(), v2.get()); } inline bool more(CVarRef v1, litstr v2) = delete; inline bool more(CVarRef v1, CArrRef v2) { - if (!v2.get()) return cellGreater(v1.asCell(), false); - return cellGreater(v1.asCell(), v2.get()); + if (!v2.get()) return cellGreater(*v1.asCell(), false); + return cellGreater(*v1.asCell(), v2.get()); } inline bool more(CVarRef v1, CObjRef v2) { - if (!v2.get()) return cellGreater(v1.asCell(), false); - return cellGreater(v1.asCell(), v2.get()); + if (!v2.get()) return cellGreater(*v1.asCell(), false); + return cellGreater(*v1.asCell(), v2.get()); } inline bool more(CVarRef v1, CVarRef v2) { - return tvGreater(v1.asTypedValue(), v2.asTypedValue()); + return tvGreater(*v1.asTypedValue(), *v2.asTypedValue()); } /////////////////////////////////////////////////////////////////////////////// diff --git a/hphp/runtime/base/tv_arith.cpp b/hphp/runtime/base/tv_arith.cpp index 22fb2707e..410b40673 100644 --- a/hphp/runtime/base/tv_arith.cpp +++ b/hphp/runtime/base/tv_arith.cpp @@ -172,8 +172,8 @@ Cell cellDiv(Cell c1, Cell c2) { } Cell cellMod(Cell c1, Cell c2) { - auto const i1 = cellToInt(&c1); - auto const i2 = cellToInt(&c2); + auto const i1 = cellToInt(c1); + auto const i2 = cellToInt(c2); if (UNLIKELY(i2 == 0)) { raise_warning(Strings::DIVISION_BY_ZERO); return make_tv(false); diff --git a/hphp/runtime/base/tv_comparisons-inl.h b/hphp/runtime/base/tv_comparisons-inl.h index 5b0e24fae..7d2cf2561 100644 --- a/hphp/runtime/base/tv_comparisons-inl.h +++ b/hphp/runtime/base/tv_comparisons-inl.h @@ -18,15 +18,15 @@ namespace HPHP { ////////////////////////////////////////////////////////////////////// -inline bool cellEqual(const Cell* cell, int ival) { +inline bool cellEqual(Cell cell, int ival) { return cellEqual(cell, static_cast(ival)); } -inline bool cellLess(const Cell* cell, int ival) { +inline bool cellLess(Cell cell, int ival) { return cellLess(cell, static_cast(ival)); } -inline bool cellGreater(const Cell* cell, int ival) { +inline bool cellGreater(Cell cell, int ival) { return cellGreater(cell, static_cast(ival)); } diff --git a/hphp/runtime/base/tv_comparisons.cpp b/hphp/runtime/base/tv_comparisons.cpp index 586b4e3fb..6169977db 100644 --- a/hphp/runtime/base/tv_comparisons.cpp +++ b/hphp/runtime/base/tv_comparisons.cpp @@ -46,31 +46,31 @@ namespace { */ template -bool cellRelOp(Op op, const Cell* cell, bool val) { +bool cellRelOp(Op op, Cell cell, bool val) { return op(cellToBool(cell), val); } template -bool cellRelOp(Op op, const Cell* cell, int64_t val) { - assert(cellIsPlausible(cell)); +bool cellRelOp(Op op, Cell cell, int64_t val) { + assert(cellIsPlausible(&cell)); - switch (cell->m_type) { + switch (cell.m_type) { case KindOfUninit: case KindOfNull: return op(false, !!val); - case KindOfBoolean: return op(!!cell->m_data.num, val != 0); - case KindOfInt64: return op(cell->m_data.num, val); - case KindOfDouble: return op(cell->m_data.dbl, val); + case KindOfBoolean: return op(!!cell.m_data.num, val != 0); + case KindOfInt64: return op(cell.m_data.num, val); + case KindOfDouble: return op(cell.m_data.dbl, val); case KindOfArray: return op(true, false); case KindOfObject: - return cell->m_data.pobj->isCollection() + return cell.m_data.pobj->isCollection() ? op.collectionVsNonObj() - : op(cell->m_data.pobj->o_toInt64(), val); + : op(cell.m_data.pobj->o_toInt64(), val); case KindOfStaticString: case KindOfString: { - auto const sdata = cell->m_data.pstr; + auto const sdata = cell.m_data.pstr; int64_t ival; double dval; auto const dt = sdata->isNumericWithVal(ival, dval, @@ -87,25 +87,25 @@ bool cellRelOp(Op op, const Cell* cell, int64_t val) { } template -bool cellRelOp(Op op, const Cell* cell, double val) { - assert(cellIsPlausible(cell)); +bool cellRelOp(Op op, Cell cell, double val) { + assert(cellIsPlausible(&cell)); - switch (cell->m_type) { + switch (cell.m_type) { case KindOfUninit: case KindOfNull: return op(false, val != 0); - case KindOfBoolean: return op(!!cell->m_data.num, val != 0); - case KindOfInt64: return op(cell->m_data.num, val); - case KindOfDouble: return op(cell->m_data.dbl, val); + case KindOfBoolean: return op(!!cell.m_data.num, val != 0); + case KindOfInt64: return op(cell.m_data.num, val); + case KindOfDouble: return op(cell.m_data.dbl, val); case KindOfArray: return op(true, false); case KindOfObject: - return cell->m_data.pobj->isCollection() + return cell.m_data.pobj->isCollection() ? op.collectionVsNonObj() - : op(cell->m_data.pobj->o_toDouble(), val); + : op(cell.m_data.pobj->o_toDouble(), val); case KindOfStaticString: case KindOfString: - return op(toDouble(cell->m_data.pstr), val); + return op(toDouble(cell.m_data.pstr), val); default: break; @@ -114,17 +114,17 @@ bool cellRelOp(Op op, const Cell* cell, double val) { } template -bool cellRelOp(Op op, const Cell* cell, const StringData* val) { - assert(cellIsPlausible(cell)); +bool cellRelOp(Op op, Cell cell, const StringData* val) { + assert(cellIsPlausible(&cell)); - switch (cell->m_type) { + switch (cell.m_type) { case KindOfUninit: case KindOfNull: return op(empty_string.get(), val); - case KindOfBoolean: return op(!!cell->m_data.num, toBoolean(val)); - case KindOfDouble: return op(cell->m_data.dbl, val->toDouble()); + case KindOfBoolean: return op(!!cell.m_data.num, toBoolean(val)); + case KindOfDouble: return op(cell.m_data.dbl, val->toDouble()); case KindOfArray: return op(true, false); case KindOfString: - case KindOfStaticString: return op(cell->m_data.pstr, val); + case KindOfStaticString: return op(cell.m_data.pstr, val); case KindOfInt64: { @@ -132,15 +132,15 @@ bool cellRelOp(Op op, const Cell* cell, const StringData* val) { double dval; auto const dt = val->isNumericWithVal(ival, dval, /* allow_error */ true); - return dt == KindOfInt64 ? op(cell->m_data.num, ival) : - dt == KindOfDouble ? op(cell->m_data.num, dval) : - op(cell->m_data.num, 0); + return dt == KindOfInt64 ? op(cell.m_data.num, ival) : + dt == KindOfDouble ? op(cell.m_data.num, dval) : + op(cell.m_data.num, 0); } case KindOfObject: { - auto const od = cell->m_data.pobj; - if (od->isResource()) return op(od->o_toDouble(), val->toDouble()); + auto const od = cell.m_data.pobj; + if (od->isResource()) return op(od->o_toDouble(), val->toDouble()); if (od->isCollection()) return op.collectionVsNonObj(); try { String str(const_cast(od)->t___tostring()); @@ -157,21 +157,21 @@ bool cellRelOp(Op op, const Cell* cell, const StringData* val) { } template -bool cellRelOp(Op op, const Cell* cell, const ArrayData* ad) { - assert(cellIsPlausible(cell)); +bool cellRelOp(Op op, Cell cell, const ArrayData* ad) { + assert(cellIsPlausible(&cell)); - switch (cell->m_type) { + switch (cell.m_type) { case KindOfUninit: case KindOfNull: return op(false, !ad->empty()); - case KindOfBoolean: return op(cell->m_data.num, !ad->empty()); + case KindOfBoolean: return op(cell.m_data.num, !ad->empty()); case KindOfInt64: return op(false, true); case KindOfDouble: return op(false, true); - case KindOfArray: return op(cell->m_data.parr, ad); + case KindOfArray: return op(cell.m_data.parr, ad); case KindOfStaticString: case KindOfString: return op(false, true); case KindOfObject: { - auto const od = cell->m_data.pobj; + auto const od = cell.m_data.pobj; if (od->isResource()) return op(false, true); return od->isCollection() ? op.collectionVsNonObj() @@ -185,40 +185,36 @@ bool cellRelOp(Op op, const Cell* cell, const ArrayData* ad) { } template -bool cellRelOp(Op op, const Cell* cell, const ObjectData* od) { - assert(cellIsPlausible(cell)); +bool cellRelOp(Op op, Cell cell, const ObjectData* od) { + assert(cellIsPlausible(&cell)); - switch (cell->m_type) { + switch (cell.m_type) { case KindOfUninit: case KindOfNull: // TODO: should use o_toBoolean return op(false, true); - case KindOfBoolean: return op(!!cell->m_data.num, true); + case KindOfBoolean: return op(!!cell.m_data.num, true); case KindOfInt64: return od->isCollection() ? op.collectionVsNonObj() - : op(cell->m_data.num, od->o_toInt64()); + : op(cell.m_data.num, od->o_toInt64()); case KindOfDouble: return od->isCollection() ? op.collectionVsNonObj() - : op(cell->m_data.dbl, od->o_toDouble()); + : op(cell.m_data.dbl, od->o_toDouble()); case KindOfArray: if (od->isResource()) return op(true, false); return od->isCollection() ? op.collectionVsNonObj() : op(false, true); case KindOfString: case KindOfStaticString: - { - auto const str = cell->m_data.pstr; - if (od->isResource()) return op(str->toDouble(), od->o_toDouble()); - - if (od->isCollection()) return op.collectionVsNonObj(); - try { - String str(const_cast(od)->t___tostring()); - return op(cell->m_data.pstr, str.get()); - } catch (BadTypeConversionException&) { - return op(false, true); - } + if (od->isResource()) return op(cell.m_data.pstr->toDouble(), + od->o_toDouble()); + if (od->isCollection()) return op.collectionVsNonObj(); + try { + String str(const_cast(od)->t___tostring()); + return op(cell.m_data.pstr, str.get()); + } catch (BadTypeConversionException&) { + return op(false, true); } case KindOfObject: - return op(cell->m_data.pobj, od); - + return op(cell.m_data.pobj, od); default: break; } @@ -227,23 +223,23 @@ bool cellRelOp(Op op, const Cell* cell, const ObjectData* od) { } template -bool cellRelOp(Op op, const Cell* c1, const Cell* c2) { - assert(cellIsPlausible(c1)); - assert(cellIsPlausible(c2)); +bool cellRelOp(Op op, Cell c1, Cell c2) { + assert(cellIsPlausible(&c1)); + assert(cellIsPlausible(&c2)); - switch (c2->m_type) { + switch (c2.m_type) { case KindOfUninit: case KindOfNull: - return IS_STRING_TYPE(c1->m_type) - ? op(c1->m_data.pstr, empty_string.get()) + return IS_STRING_TYPE(c1.m_type) + ? op(c1.m_data.pstr, empty_string.get()) : cellRelOp(op, c1, false); - case KindOfInt64: return cellRelOp(op, c1, c2->m_data.num); - case KindOfBoolean: return cellRelOp(op, c1, !!c2->m_data.num); - case KindOfDouble: return cellRelOp(op, c1, c2->m_data.dbl); + case KindOfInt64: return cellRelOp(op, c1, c2.m_data.num); + case KindOfBoolean: return cellRelOp(op, c1, !!c2.m_data.num); + case KindOfDouble: return cellRelOp(op, c1, c2.m_data.dbl); case KindOfStaticString: - case KindOfString: return cellRelOp(op, c1, c2->m_data.pstr); - case KindOfArray: return cellRelOp(op, c1, c2->m_data.parr); - case KindOfObject: return cellRelOp(op, c1, c2->m_data.pobj); + case KindOfString: return cellRelOp(op, c1, c2.m_data.pstr); + case KindOfArray: return cellRelOp(op, c1, c2.m_data.parr); + case KindOfObject: return cellRelOp(op, c1, c2.m_data.pobj); default: break; } @@ -251,10 +247,10 @@ bool cellRelOp(Op op, const Cell* c1, const Cell* c2) { } template -bool tvRelOp(Op op, const TypedValue* tv1, const TypedValue* tv2) { - assert(tvIsPlausible(tv1)); - assert(tvIsPlausible(tv2)); - return cellRelOp(op, tvToCell(tv1), tvToCell(tv2)); +bool tvRelOp(Op op, TypedValue tv1, TypedValue tv2) { + assert(tvIsPlausible(&tv1)); + assert(tvIsPlausible(&tv2)); + return cellRelOp(op, *tvToCell(&tv1), *tvToCell(&tv2)); } /* @@ -372,36 +368,36 @@ struct Gt { } -bool cellSame(const Cell* c1, const Cell* c2) { - assert(cellIsPlausible(c1)); - assert(cellIsPlausible(c2)); +bool cellSame(Cell c1, Cell c2) { + assert(cellIsPlausible(&c1)); + assert(cellIsPlausible(&c2)); - bool const null1 = IS_NULL_TYPE(c1->m_type); - bool const null2 = IS_NULL_TYPE(c2->m_type); + bool const null1 = IS_NULL_TYPE(c1.m_type); + bool const null2 = IS_NULL_TYPE(c2.m_type); if (null1 && null2) return true; if (null1 || null2) return false; - switch (c1->m_type) { + switch (c1.m_type) { case KindOfInt64: case KindOfBoolean: - if (c2->m_type != c1->m_type) return false; - return c1->m_data.num == c2->m_data.num; + if (c2.m_type != c1.m_type) return false; + return c1.m_data.num == c2.m_data.num; case KindOfDouble: - if (c2->m_type != c1->m_type) return false; - return c1->m_data.dbl == c2->m_data.dbl; + if (c2.m_type != c1.m_type) return false; + return c1.m_data.dbl == c2.m_data.dbl; case KindOfStaticString: case KindOfString: - if (!IS_STRING_TYPE(c2->m_type)) return false; - return c1->m_data.pstr->same(c2->m_data.pstr); + if (!IS_STRING_TYPE(c2.m_type)) return false; + return c1.m_data.pstr->same(c2.m_data.pstr); case KindOfArray: - if (c2->m_type != KindOfArray) return false; - return c1->m_data.parr->equal(c2->m_data.parr, true); + if (c2.m_type != KindOfArray) return false; + return c1.m_data.parr->equal(c2.m_data.parr, true); case KindOfObject: - return c2->m_type == KindOfObject && - c1->m_data.pobj == c2->m_data.pobj; + return c2.m_type == KindOfObject && + c1.m_data.pobj == c2.m_data.pobj; default: break; @@ -409,10 +405,10 @@ bool cellSame(const Cell* c1, const Cell* c2) { not_reached(); } -bool tvSame(const TypedValue* tv1, const TypedValue* tv2) { - assert(tvIsPlausible(tv1)); - assert(tvIsPlausible(tv2)); - return cellSame(tvToCell(tv1), tvToCell(tv2)); +bool tvSame(TypedValue tv1, TypedValue tv2) { + assert(tvIsPlausible(&tv1)); + assert(tvIsPlausible(&tv2)); + return cellSame(*tvToCell(&tv1), *tvToCell(&tv2)); } ////////////////////////////////////////////////////////////////////// @@ -422,125 +418,125 @@ bool tvSame(const TypedValue* tv1, const TypedValue* tv2) { * in the old code ... we should probably re-evaluate this. */ -bool cellEqual(const Cell* cell, bool val) { +bool cellEqual(Cell cell, bool val) { return cellRelOp(Eq(), cell, val); } HOT_FUNC -bool cellEqual(const Cell* cell, int64_t val) { +bool cellEqual(Cell cell, int64_t val) { return cellRelOp(Eq(), cell, val); } -bool cellEqual(const Cell* cell, double val) { +bool cellEqual(Cell cell, double val) { return cellRelOp(Eq(), cell, val); } -bool cellEqual(const Cell* cell, const StringData* val) { +bool cellEqual(Cell cell, const StringData* val) { return cellRelOp(Eq(), cell, val); } -bool cellEqual(const Cell* cell, const ArrayData* val) { +bool cellEqual(Cell cell, const ArrayData* val) { return cellRelOp(Eq(), cell, val); } -bool cellEqual(const Cell* cell, const ObjectData* val) { +bool cellEqual(Cell cell, const ObjectData* val) { return cellRelOp(Eq(), cell, val); } -bool cellEqual(const Cell* c1, const Cell* c2) { +bool cellEqual(Cell c1, Cell c2) { return cellRelOp(Eq(), c1, c2); } HOT_FUNC -bool tvEqual(const TypedValue* tv1, const TypedValue* tv2) { +bool tvEqual(TypedValue tv1, TypedValue tv2) { return tvRelOp(Eq(), tv1, tv2); } -bool cellLess(const Cell* cell, bool val) { +bool cellLess(Cell cell, bool val) { return cellRelOp(Lt(), cell, val); } -bool cellLess(const Cell* cell, int64_t val) { +bool cellLess(Cell cell, int64_t val) { return cellRelOp(Lt(), cell, val); } -bool cellLess(const Cell* cell, double val) { +bool cellLess(Cell cell, double val) { return cellRelOp(Lt(), cell, val); } -bool cellLess(const Cell* cell, const StringData* val) { +bool cellLess(Cell cell, const StringData* val) { return cellRelOp(Lt(), cell, val); } -bool cellLess(const Cell* cell, const ArrayData* val) { +bool cellLess(Cell cell, const ArrayData* val) { return cellRelOp(Lt(), cell, val); } -bool cellLess(const Cell* cell, const ObjectData* val) { +bool cellLess(Cell cell, const ObjectData* val) { return cellRelOp(Lt(), cell, val); } -bool cellLess(const Cell* c1, const Cell* c2) { +bool cellLess(Cell c1, Cell c2) { return cellRelOp(Lt(), c1, c2); } HOT_FUNC -bool tvLess(const TypedValue* tv1, const TypedValue* tv2) { +bool tvLess(TypedValue tv1, TypedValue tv2) { return tvRelOp(Lt(), tv1, tv2); } -bool cellGreater(const Cell* cell, bool val) { +bool cellGreater(Cell cell, bool val) { return cellRelOp(Gt(), cell, val); } //NB: was HOT_FUNC in old code ... dunno if this makes sense anymore. -bool cellGreater(const Cell* cell, int64_t val) { +bool cellGreater(Cell cell, int64_t val) { return cellRelOp(Gt(), cell, val); } -bool cellGreater(const Cell* cell, double val) { +bool cellGreater(Cell cell, double val) { return cellRelOp(Gt(), cell, val); } -bool cellGreater(const Cell* cell, const StringData* val) { +bool cellGreater(Cell cell, const StringData* val) { return cellRelOp(Gt(), cell, val); } -bool cellGreater(const Cell* cell, const ArrayData* val) { +bool cellGreater(Cell cell, const ArrayData* val) { return cellRelOp(Gt(), cell, val); } -bool cellGreater(const Cell* cell, const ObjectData* val) { +bool cellGreater(Cell cell, const ObjectData* val) { return cellRelOp(Gt(), cell, val); } -bool cellGreater(const Cell* c1, const Cell* c2) { +bool cellGreater(Cell c1, Cell c2) { return cellRelOp(Gt(), c1, c2); } -bool tvGreater(const TypedValue* tv1, const TypedValue* tv2) { +bool tvGreater(TypedValue tv1, TypedValue tv2) { return tvRelOp(Gt(), tv1, tv2); } ////////////////////////////////////////////////////////////////////// -bool cellLessOrEqual(const Cell* c1, const Cell* c2) { - assert(cellIsPlausible(c1)); - assert(cellIsPlausible(c2)); +bool cellLessOrEqual(Cell c1, Cell c2) { + assert(cellIsPlausible(&c1)); + assert(cellIsPlausible(&c2)); - if ((c1->m_type == KindOfArray && c2->m_type == KindOfArray) || - (c1->m_type == KindOfObject && c2->m_type == KindOfObject)) { + if ((c1.m_type == KindOfArray && c2.m_type == KindOfArray) || + (c1.m_type == KindOfObject && c2.m_type == KindOfObject)) { return cellLess(c1, c2) || cellEqual(c1, c2); } return !cellGreater(c1, c2); } -bool cellGreaterOrEqual(const Cell* c1, const Cell* c2) { - assert(cellIsPlausible(c1)); - assert(cellIsPlausible(c2)); +bool cellGreaterOrEqual(Cell c1, Cell c2) { + assert(cellIsPlausible(&c1)); + assert(cellIsPlausible(&c2)); - if ((c1->m_type == KindOfArray && c2->m_type == KindOfArray) || - (c1->m_type == KindOfObject && c2->m_type == KindOfObject)) { + if ((c1.m_type == KindOfArray && c2.m_type == KindOfArray) || + (c1.m_type == KindOfObject && c2.m_type == KindOfObject)) { return cellGreater(c1, c2) || cellEqual(c1, c2); } return !cellLess(c1, c2); diff --git a/hphp/runtime/base/tv_comparisons.h b/hphp/runtime/base/tv_comparisons.h index 977cf2bf8..c505536d4 100644 --- a/hphp/runtime/base/tv_comparisons.h +++ b/hphp/runtime/base/tv_comparisons.h @@ -27,13 +27,13 @@ namespace HPHP { * Returns whether two Cells have the same value, in the sense of * php's === operator. */ -bool cellSame(const Cell*, const Cell*); +bool cellSame(Cell, Cell); /* * Returns whether two TypedValues have the same value, in sense of * php's === operator. */ -bool tvSame(const TypedValue*, const TypedValue*); +bool tvSame(TypedValue, TypedValue); ////////////////////////////////////////////////////////////////////// // Php's operator == @@ -42,25 +42,25 @@ bool tvSame(const TypedValue*, const TypedValue*); * Returns whether a Cell has the same value as an unpackaged type, in * the sense of php's == operator. */ -bool cellEqual(const Cell*, bool); -bool cellEqual(const Cell*, int); -bool cellEqual(const Cell*, int64_t); -bool cellEqual(const Cell*, double); -bool cellEqual(const Cell*, const StringData*); -bool cellEqual(const Cell*, const ArrayData*); -bool cellEqual(const Cell*, const ObjectData*); +bool cellEqual(Cell, bool); +bool cellEqual(Cell, int); +bool cellEqual(Cell, int64_t); +bool cellEqual(Cell, double); +bool cellEqual(Cell, const StringData*); +bool cellEqual(Cell, const ArrayData*); +bool cellEqual(Cell, const ObjectData*); /* * Returns whether two Cells have the same value, in the same of php's * == operator. */ -bool cellEqual(const Cell*, const Cell*); +bool cellEqual(Cell, Cell); /* * Returns whether two TypedValues have the same value, in the sense * of php's == operator. */ -bool tvEqual(const TypedValue*, const TypedValue*); +bool tvEqual(TypedValue, TypedValue); ////////////////////////////////////////////////////////////////////// // Php's operator < @@ -69,24 +69,24 @@ bool tvEqual(const TypedValue*, const TypedValue*); * Returns whether a Cell is less than an unpackaged type, in the * sense of php's < operator. */ -bool cellLess(const Cell*, bool); -bool cellLess(const Cell*, int); -bool cellLess(const Cell*, int64_t); -bool cellLess(const Cell*, double); -bool cellLess(const Cell*, const StringData*); -bool cellLess(const Cell*, const ArrayData*); -bool cellLess(const Cell*, const ObjectData*); +bool cellLess(Cell, bool); +bool cellLess(Cell, int); +bool cellLess(Cell, int64_t); +bool cellLess(Cell, double); +bool cellLess(Cell, const StringData*); +bool cellLess(Cell, const ArrayData*); +bool cellLess(Cell, const ObjectData*); /* * Returns whether a Cell is greater than another Cell, in the sense * of php's < operator. */ -bool cellLess(const Cell*, const Cell*); +bool cellLess(Cell, Cell); /* * Returns whether tv1 is less than tv2, as in php's < operator. */ -bool tvLess(const TypedValue*, const TypedValue*); +bool tvLess(TypedValue, TypedValue); ////////////////////////////////////////////////////////////////////// // Php's operator > @@ -95,24 +95,24 @@ bool tvLess(const TypedValue*, const TypedValue*); * Returns whether a Cell is greater than an unpackaged type, in the * sense of php's > operator. */ -bool cellGreater(const Cell*, bool); -bool cellGreater(const Cell*, int); -bool cellGreater(const Cell*, int64_t); -bool cellGreater(const Cell*, double); -bool cellGreater(const Cell*, const StringData*); -bool cellGreater(const Cell*, const ArrayData*); -bool cellGreater(const Cell*, const ObjectData*); +bool cellGreater(Cell, bool); +bool cellGreater(Cell, int); +bool cellGreater(Cell, int64_t); +bool cellGreater(Cell, double); +bool cellGreater(Cell, const StringData*); +bool cellGreater(Cell, const ArrayData*); +bool cellGreater(Cell, const ObjectData*); /* * Returns whether a Cell is greater than another Cell, in the sense * of php's > operator. */ -bool cellGreater(const Cell*, const Cell*); +bool cellGreater(Cell, Cell); /* * Returns whether tv1 is greather than tv2, as in php's > operator. */ -bool tvGreater(const TypedValue*, const TypedValue*); +bool tvGreater(TypedValue, TypedValue); ////////////////////////////////////////////////////////////////////// @@ -124,8 +124,8 @@ bool tvGreater(const TypedValue*, const TypedValue*); * * These functions are necessary to handle those cases specially. */ -bool cellLessOrEqual(const Cell*, const Cell*); -bool cellGreaterOrEqual(const Cell*, const Cell*); +bool cellLessOrEqual(Cell, Cell); +bool cellGreaterOrEqual(Cell, Cell); ////////////////////////////////////////////////////////////////////// diff --git a/hphp/runtime/base/tv_conversions-inl.h b/hphp/runtime/base/tv_conversions-inl.h index aaa857173..8eb687369 100644 --- a/hphp/runtime/base/tv_conversions-inl.h +++ b/hphp/runtime/base/tv_conversions-inl.h @@ -20,18 +20,18 @@ namespace HPHP { ////////////////////////////////////////////////////////////////////// -inline bool cellToBool(const Cell* cell) { - assert(cellIsPlausible(cell)); +inline bool cellToBool(Cell cell) { + assert(cellIsPlausible(&cell)); - switch (cell->m_type) { + switch (cell.m_type) { case KindOfUninit: case KindOfNull: return false; - case KindOfInt64: return cell->m_data.num != 0; - case KindOfBoolean: return cell->m_data.num; - case KindOfDouble: return cell->m_data.dbl != 0; + case KindOfInt64: return cell.m_data.num != 0; + case KindOfBoolean: return cell.m_data.num; + case KindOfDouble: return cell.m_data.dbl != 0; case KindOfStaticString: - case KindOfString: return cell->m_data.pstr->toBoolean(); - case KindOfArray: return !cell->m_data.parr->empty(); + case KindOfString: return cell.m_data.pstr->toBoolean(); + case KindOfArray: return !cell.m_data.parr->empty(); case KindOfObject: // TODO: should handle o_toBoolean? return true; default: break; @@ -39,17 +39,17 @@ inline bool cellToBool(const Cell* cell) { not_reached(); } -inline int64_t cellToInt(const Cell* cell) { - assert(cellIsPlausible(cell)); +inline int64_t cellToInt(Cell cell) { + assert(cellIsPlausible(&cell)); - switch (cell->m_type) { - case KindOfInt64: return cell->m_data.num; - case KindOfDouble: return toInt64(cell->m_data.dbl); + switch (cell.m_type) { + case KindOfInt64: return cell.m_data.num; + case KindOfDouble: return toInt64(cell.m_data.dbl); case KindOfString: - case KindOfStaticString: return cell->m_data.pstr->toInt64(10); - case KindOfArray: return cell->m_data.parr->empty() ? 0 : 1; - case KindOfObject: return cell->m_data.pobj->o_toInt64(); - case KindOfBoolean: return cell->m_data.num; + case KindOfStaticString: return cell.m_data.pstr->toInt64(10); + case KindOfArray: return cell.m_data.parr->empty() ? 0 : 1; + case KindOfObject: return cell.m_data.pobj->o_toInt64(); + case KindOfBoolean: return cell.m_data.num; case KindOfUninit: case KindOfNull: return 0; default: break; diff --git a/hphp/runtime/base/tv_conversions.h b/hphp/runtime/base/tv_conversions.h index ce9228d97..4d8fc73e1 100644 --- a/hphp/runtime/base/tv_conversions.h +++ b/hphp/runtime/base/tv_conversions.h @@ -25,8 +25,8 @@ namespace HPHP { /* * Convert a cell to various types, without changing the Cell. */ -bool cellToBool(const Cell*); -int64_t cellToInt(const Cell*); +bool cellToBool(Cell); +int64_t cellToInt(Cell); /* * Convert a string to a TypedNum following php semantics, allowing diff --git a/hphp/runtime/base/type_array.cpp b/hphp/runtime/base/type_array.cpp index e7df977d5..2b3a89b88 100644 --- a/hphp/runtime/base/type_array.cpp +++ b/hphp/runtime/base/type_array.cpp @@ -1007,12 +1007,12 @@ bool Array::MultiSort(std::vector &data, bool renumber) { int Array::SortRegularAscending(CVarRef v1, CVarRef v2, const void *data) { if (HPHP::less(v1, v2)) return -1; - if (tvEqual(v1.asTypedValue(), v2.asTypedValue())) return 0; + if (tvEqual(*v1.asTypedValue(), *v2.asTypedValue())) return 0; return 1; } int Array::SortRegularDescending(CVarRef v1, CVarRef v2, const void *data) { if (HPHP::less(v1, v2)) return 1; - if (tvEqual(v1.asTypedValue(), v2.asTypedValue())) return 0; + if (tvEqual(*v1.asTypedValue(), *v2.asTypedValue())) return 0; return -1; } diff --git a/hphp/runtime/vm/bytecode.cpp b/hphp/runtime/vm/bytecode.cpp index 8e1b92672..4b4a9dbb5 100644 --- a/hphp/runtime/vm/bytecode.cpp +++ b/hphp/runtime/vm/bytecode.cpp @@ -3712,14 +3712,14 @@ void OPTBLD_INLINE VMExecutionContext::implCellBinOpBool(PC& pc, Op op) { NEXT(); auto const c1 = m_stack.topC(); auto const c2 = m_stack.indC(1); - bool const result = op(c2, c1); + bool const result = op(*c2, *c1); tvRefcountedDecRefCell(c2); *c2 = make_tv(result); m_stack.popC(); } inline void OPTBLD_INLINE VMExecutionContext::iopXor(PC& pc) { - implCellBinOpBool(pc, [&] (const Cell* c1, const Cell* c2) -> bool { + implCellBinOpBool(pc, [&] (Cell c1, Cell c2) -> bool { return cellToBool(c1) ^ cellToBool(c2); }); } @@ -3729,25 +3729,25 @@ inline void OPTBLD_INLINE VMExecutionContext::iopSame(PC& pc) { } inline void OPTBLD_INLINE VMExecutionContext::iopNSame(PC& pc) { - implCellBinOpBool(pc, [&] (const Cell* c1, const Cell* c2) { + implCellBinOpBool(pc, [&] (Cell c1, Cell c2) { return !cellSame(c1, c2); }); } inline void OPTBLD_INLINE VMExecutionContext::iopEq(PC& pc) { - implCellBinOpBool(pc, [&] (const Cell* c1, const Cell* c2) { + implCellBinOpBool(pc, [&] (Cell c1, Cell c2) { return cellEqual(c1, c2); }); } inline void OPTBLD_INLINE VMExecutionContext::iopNeq(PC& pc) { - implCellBinOpBool(pc, [&] (const Cell* c1, const Cell* c2) { + implCellBinOpBool(pc, [&] (Cell c1, Cell c2) { return !cellEqual(c1, c2); }); } inline void OPTBLD_INLINE VMExecutionContext::iopLt(PC& pc) { - implCellBinOpBool(pc, [&] (const Cell* c1, const Cell* c2) { + implCellBinOpBool(pc, [&] (Cell c1, Cell c2) { return cellLess(c1, c2); }); } @@ -3757,7 +3757,7 @@ inline void OPTBLD_INLINE VMExecutionContext::iopLte(PC& pc) { } inline void OPTBLD_INLINE VMExecutionContext::iopGt(PC& pc) { - implCellBinOpBool(pc, [&] (const Cell* c1, const Cell* c2) { + implCellBinOpBool(pc, [&] (Cell c1, Cell c2) { return cellGreater(c1, c2); }); } @@ -4169,7 +4169,7 @@ inline void OPTBLD_INLINE VMExecutionContext::iopSSwitch(PC& pc) { for (i = 0; i < cases; ++i) { auto& item = jmptab[i]; const StringData* str = u->lookupLitstrId(item.str); - if (cellEqual(val, str)) { + if (cellEqual(*val, str)) { pc = origPC + item.dest; break; } diff --git a/hphp/runtime/vm/jit/codegen.cpp b/hphp/runtime/vm/jit/codegen.cpp index 5918a03cb..874d7a134 100644 --- a/hphp/runtime/vm/jit/codegen.cpp +++ b/hphp/runtime/vm/jit/codegen.cpp @@ -2494,7 +2494,7 @@ static TCA sswitchHelperSlow(TypedValue typedVal, TCA* jmptab) { Cell* cell = tvToCell(&typedVal); for (int i = 0; i < numStrs; ++i) { - if (cellEqual(cell, strs[i])) return jmptab[i]; + if (cellEqual(*cell, strs[i])) return jmptab[i]; } return jmptab[numStrs]; // default case } diff --git a/hphp/runtime/vm/jit/translator-runtime.cpp b/hphp/runtime/vm/jit/translator-runtime.cpp index 16377c024..bc4828bbd 100644 --- a/hphp/runtime/vm/jit/translator-runtime.cpp +++ b/hphp/runtime/vm/jit/translator-runtime.cpp @@ -159,7 +159,7 @@ int64_t convStrToIntHelper(const StringData* s) { int64_t convCellToIntHelper(TypedValue tv) { // TODO call cellToInt directly from the TC. - return cellToInt(&tv); + return cellToInt(tv); } ObjectData* convCellToObjHelper(TypedValue tv) { diff --git a/hphp/runtime/vm/member_operations.h b/hphp/runtime/vm/member_operations.h index 70864706e..4ddf94463 100644 --- a/hphp/runtime/vm/member_operations.h +++ b/hphp/runtime/vm/member_operations.h @@ -643,7 +643,7 @@ template inline int64_t castKeyToInt(TypedValue* key) { TypedValue scratch; initScratchKey(scratch, key); - return cellToInt(tvToCell(key)); + return cellToInt(*tvToCell(key)); } template<>