actually call o_toBoolean when casting to a bool - take 2

Redoing the diff. The original was breaking perflab, I want to see why (it worked for me, but without the `movb` change, maybe that was it).

hphp/doc/ir.specification

@@ -509,6 +509,7 @@ D:Bool = ConvArrToBool S0:Arr
 D:Bool = ConvDblToBool S0:Dbl
 D:Bool = ConvIntToBool S0:Int
 D:Bool = ConvStrToBool S0:Str
+D:Bool = ConvObjToBool S0:Obj
 D:Bool = ConvCellToBool S0:Cell
 
 D:Dbl  = ConvArrToDbl S0:Arr

hphp/runtime/base/tv_comparisons.cpp

@@ -184,9 +184,8 @@ bool cellRelOp(Op op, Cell cell, const ObjectData* od) {
 
   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 KindOfNull:        return op(false, true);
+  case KindOfBoolean:     return op(!!cell.m_data.num, od->o_toBoolean());
   case KindOfInt64:
     return od->isCollection() ? op.collectionVsNonObj()
                               : op(cell.m_data.num, od->o_toInt64());
@@ -224,9 +223,9 @@ bool cellRelOp(Op op, Cell c1, Cell c2) {
   switch (c2.m_type) {
   case KindOfUninit:
   case KindOfNull:
-    return IS_STRING_TYPE(c1.m_type)
-      ? op(c1.m_data.pstr, empty_string.get())
-      : cellRelOp(op, c1, false);
+    return IS_STRING_TYPE(c1.m_type) ? op(c1.m_data.pstr, empty_string.get()) :
+           c1.m_type == KindOfObject ? op(true, false) :
+           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);

hphp/runtime/base/tv_conversions-inl.h

@@ -33,8 +33,7 @@ inline bool cellToBool(Cell cell) {
   case KindOfStaticString:
   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;
+  case KindOfObject:        return cell.m_data.pobj->o_toBoolean();
   default:                  break;
   }
   not_reached();

hphp/runtime/base/tv_helpers.cpp

@@ -104,7 +104,9 @@ void tvCastToBooleanInPlace(TypedValue* tv) {
   case KindOfStaticString: b = tv->m_data.pstr->toBoolean(); break;
   case KindOfString:  b = tv->m_data.pstr->toBoolean(); tvDecRefStr(tv); break;
   case KindOfArray:   b = (!tv->m_data.parr->empty()); tvDecRefArr(tv); break;
-  case KindOfObject:  b = (tv->m_data.pobj != nullptr); tvDecRefObj(tv); break;
+  case KindOfObject:  b = tv->m_data.pobj->o_toBoolean();
+                      tvDecRefObj(tv);
+                      break;
   default:            assert(false); b = false; break;
   }
   tv->m_data.num = b;

hphp/runtime/vm/jit/code-gen.cpp

@@ -2195,6 +2195,27 @@ void CodeGenerator::cgConvIntToBool(IRInstruction* inst) {
   }
 }
 
+void CodeGenerator::cgConvObjToBool(IRInstruction* inst) {
+  SSATmp* dst = inst->dst();
+  auto dstReg = m_regs[dst].reg();
+  assert(dstReg != InvalidReg);
+  SSATmp* src = inst->src(0);
+  auto srcReg = m_regs[src].reg();
+
+  m_as.testw   (ObjectData::CallToImpl, srcReg[ObjectData::attributeOff()]);
+  unlikelyIfThenElse(CC_NZ, [&] (Asm& a) {
+    cgCallHelper(
+      a,
+      (TCA)convObjToBoolHelper,
+      dst,
+      SyncOptions::kSyncPoint,
+      ArgGroup(m_regs)
+        .ssa(src));
+  }, [&] (Asm& a) {
+    a.movb(1, rbyte(dstReg));
+  });
+}
+
 void CodeGenerator::emitConvBoolOrIntToDbl(IRInstruction* inst) {
   SSATmp* src = inst->src(0);
   SSATmp* dst = inst->dst();

hphp/runtime/vm/jit/code-gen.h

@@ -367,6 +367,30 @@ private:
     asm_label(m_as, done);
   }
 
+  /*
+   * Same as ifThenElse except the first block is off in astubs
+   */
+  template <class Then, class Else>
+  void unlikelyIfThenElse(ConditionCode cc, Then unlikely, Else elseBlock) {
+    if (&m_as == &m_astubs) {
+      Label elseLabel, done;
+      m_as.jcc8(ccNegate(cc), elseLabel);
+      unlikely(m_as);
+      m_as.jmp8(done);
+      asm_label(m_as, elseLabel);
+      elseBlock(m_as);
+      asm_label(m_as, done);
+    } else {
+      Label unlikelyLabel, done;
+      m_as.jcc(cc, unlikelyLabel);
+      elseBlock(m_as);
+      asm_label(m_astubs, unlikelyLabel);
+      unlikely(m_astubs);
+      m_astubs.jmp(done);
+      asm_label(m_as, done);
+    }
+  }
+
   // This is for printing partially-generated traces when debugging
   void print() const;
 

hphp/runtime/vm/jit/ir.h

@@ -233,6 +233,7 @@ O(ConvArrToBool,               D(Bool), S(Arr),                          C|N) \
 O(ConvDblToBool,               D(Bool), S(Dbl),                            C) \
 O(ConvIntToBool,               D(Bool), S(Int),                            C) \
 O(ConvStrToBool,               D(Bool), S(Str),                            N) \
+O(ConvObjToBool,               D(Bool), S(Obj),                            N) \
 O(ConvCellToBool,              D(Bool), S(Cell),                           N) \
                                                                               \
 O(ConvArrToDbl,                 D(Dbl), S(Arr),                          C|N) \

hphp/runtime/vm/jit/simplifier.cpp

@@ -1211,11 +1211,16 @@ SSATmp* Simplifier::simplifyCmp(Opcode opName, IRInstruction* inst,
     return newInst(commuteQueryOp(opName), src2, src1);
   }
 
-  // case 1: null cmp string. Convert null to ""
+  // case 1a: null cmp string. Convert null to ""
   if (src1->type().isString() && src2->type().isNull()) {
     return newInst(opName, src1, cns(StringData::GetStaticString("")));
   }
 
+  // case 1b: null cmp object. Convert null to false and the object to true
+  if (src1->type().isObj() && src2->type().isNull()) {
+    return newInst(opName, cns(true), cns(false));
+  }
+
   // case 2a: null cmp anything. Convert null to false
   if (src2->type().isNull()) {
     return newInst(opName, src1, cns(false));
@@ -1538,7 +1543,7 @@ SSATmp* Simplifier::simplifyConvCellToBool(IRInstruction* inst) {
   if (srcType.isDbl())    return gen(ConvDblToBool, src);
   if (srcType.isInt())    return gen(ConvIntToBool, src);
   if (srcType.isString()) return gen(ConvStrToBool, src);
-  if (srcType.isObj())    return cns(true);
+  if (srcType.isObj())    return gen(ConvObjToBool, src);
 
   return nullptr;
 }

hphp/runtime/vm/jit/translator-runtime.cpp

@@ -113,6 +113,10 @@ int64_t convStrToBoolHelper(const StringData* s) {
   return s->toBoolean();
 }
 
+int64_t convObjToBoolHelper(const ObjectData* o) {
+  return o->o_toBoolean();
+}
+
 int64_t convCellToBoolHelper(TypedValue tv) {
   // Cannot call tvCastToBooleanInPlace here because some of the
   // call sites will not be increasing the ref count on tv before
@@ -127,7 +131,7 @@ int64_t convCellToBoolHelper(TypedValue tv) {
     case KindOfStaticString:
     case KindOfString:  return tv.m_data.pstr->toBoolean();
     case KindOfArray:   return !tv.m_data.parr->empty();
-    case KindOfObject:  return tv.m_data.pobj != nullptr;
+    case KindOfObject:  return tv.m_data.pobj->o_toBoolean();
     default:            not_reached();
   }
 }

hphp/runtime/vm/jit/translator-runtime.h

@@ -83,6 +83,7 @@ inline double reinterpretIntAsDbl(int64_t i) {
 
 int64_t convArrToBoolHelper(const ArrayData* a);
 int64_t convStrToBoolHelper(const StringData* s);
+int64_t convObjToBoolHelper(const ObjectData* o);
 int64_t convCellToBoolHelper(TypedValue tv);
 int64_t convArrToDblHelper(ArrayData* a);
 int64_t convStrToDblHelper(const StringData* s);

hphp/test/slow/simple_xml/empty_node.php

@@ -0,0 +1,14 @@
+<?php
+
+function main() {
+  $a = simplexml_load_string("<root />");
+  var_dump($a->unknown);
+  var_dump((bool) $a->unknown);
+  var_dump((int) $a->unknown);
+  var_dump((string) $a->unknown);
+  var_dump((double) $a->unknown);
+  var_dump((array) $a->unknown);
+  var_dump($a->unknown == null);
+  var_dump(null == $a->unknown);
+}
+main();

hphp/test/slow/simple_xml/empty_node.php.expect

@@ -0,0 +1,10 @@
+object(SimpleXMLElement)#2 (0) {
+}
+bool(false)
+int(0)
+string(0) ""
+float(0)
+array(0) {
+}
+bool(false)
+bool(false)