Add the K flag to ConvObjToArr and ConvGenToArr.

Brett commented on another differential: CRc means the opcode takes ownership of a (probably recently created) reference to its source, either by decreffing it or by storing it into an object property, array, etc... It's true that these are generally used right after an incref, but the design of the opcode shouldn't assume anything about its use, especially for opcodes like this that can be generically useful.

Any opcode that decrefs one of its sources without guaranteeing that the refcount won't go to zero should kill that source. If you can guarantee that these instructions will never destruct their source then they don't have to kill the source.

hphp/doc/ir.specification

@@ -256,9 +256,14 @@ CallsNative
 
 ConsumesRC
 
-  The instruction decrefs its sources.
+  The instruction consumes a reference to one or more of its sources, either by
+  decreasing its refcount or storing the reference to memory.
 
-  TODO: explain better
+KillsSource
+
+  The instruction calls decref on one or more of its sources. Unless a source
+  is known to have a refcount > 1 before the instruction executes, it cannot
+  safely be used after the instruction has executed.
 
 ProducesRC
 
@@ -292,11 +297,6 @@ Passthrough
   variable, or some other property that doesn't affect the value of the
   variable itself.
 
-KillsSource
-
-  The instruction might destroy one or more of its sources, so they
-  cannot be safely used after it has executed.
-
 ModifiesStack
 
   The instruction modifies the in-memory evaluation stack in the process of

hphp/runtime/vm/translator/hopt/codegen.cpp

@@ -1622,7 +1622,7 @@ void CodeGenerator::cgConvStrToArr(IRInstruction* inst) {
 // of just ConvGenToArr? Because we need at least two variants
 // and we don't want to overload opcodes and we want consistency.
 
-ArrayData* new_singleton_array_helper(TypedValue value) {
+ArrayData* convGenToArrHelper(TypedValue value) {
   // Note: the call sites of this function all assume that
   // no user code will run and no recoverable exceptions will
   // occur while running this code. This seems trivially true
@@ -1631,7 +1631,7 @@ ArrayData* new_singleton_array_helper(TypedValue value) {
   // is essentially metadata for the object. If that is not true,
   // you might end up looking at this code in a debugger and now
   // you know why.
-  tvCastToArrayInPlace(&value);
+  tvCastToArrayInPlace(&value); // consumes a ref on counted values
   return value.m_data.parr;
 }
 
@@ -1641,7 +1641,7 @@ void CodeGenerator::cgConvGenToArr(IRInstruction* inst) {
 
   ArgGroup args;
   args.typedValue(src);
-  ArrayData*(*fPtr)(TypedValue) = new_singleton_array_helper;
+  ArrayData*(*fPtr)(TypedValue) = convGenToArrHelper;
   cgCallHelper(m_as, (TCA)fPtr, dst, kNoSyncPoint, args);
 }
 

hphp/runtime/vm/translator/hopt/ir.cpp

@@ -450,16 +450,20 @@ bool IRInstruction::killsSources() const {
 
 bool IRInstruction::killsSource(int idx) const {
   if (!killsSources()) return false;
-
-  if (m_op == DecRef || m_op == DecRefKillThis) {
-    assert(idx == 0);
-    return true;
+  switch (m_op) {
+    case DecRef:
+    case DecRefKillThis:
+    case ConvObjToArr:
+    case ConvGenToArr:
+      assert(idx == 0);
+      return true;
+    case ArraySet:
+    case ArraySetRef:
+      return idx == 1;
+    default:
+      not_reached();
+      break;
   }
-  if (m_op == ArraySet || m_op == ArraySetRef) {
-    // Kills its input array
-    return idx == 1;
-  }
-  not_reached();
 }
 
 bool IRInstruction::modifiesStack() const {

hphp/runtime/vm/translator/hopt/ir.h

@@ -170,9 +170,9 @@ O(OpMul,                        DParam, SNum SNum,                         C) \
 O(ConvBoolToArr,                D(Arr), S(Bool),                         C|N) \
 O(ConvDblToArr,                 D(Arr), S(Dbl),                          C|N) \
 O(ConvIntToArr,                 D(Arr), S(Int),                          C|N) \
-O(ConvObjToArr,                 D(Arr), S(Obj),                        N|CRc) \
+O(ConvObjToArr,                 D(Arr), S(Obj),                      N|CRc|K) \
 O(ConvStrToArr,                 D(Arr), S(Str),                        N|CRc) \
-O(ConvGenToArr,                 D(Arr), S(Gen),                        N|CRc) \
+O(ConvGenToArr,                 D(Arr), S(Gen),                      N|CRc|K) \
 O(ConvToBool,                  D(Bool), S(Gen),                          C|N) \
 O(ConvArrToDbl,                 D(Dbl), S(Arr),                        N|CRc) \
 O(ConvBoolToDbl,                D(Dbl), S(Bool),                         C|N) \
@@ -2393,7 +2393,7 @@ void forEachTraceInst(Trace* main, Body body) {
   });
 }
 
-/* 
+/*
  * Some utilities related to dumping. Rather than file-by-file control, we control
  * most IR logging via the hhir trace module.
  */