Fix propagation of predicted function-return types

Tx64 had an optimization in planPop() to avoid dec-refing a function
return value that's predicted by the front-end to be of a
non-ref-counted type.  This optimization was clearing the
outputPredicted flag in the NormalizedInstruction, thus preventing
HHIR from getting those predictions. So, I got rid of the optimization
in Tx64 to allow HHIR to get those predictions.

hphp/doc/ir.specification

@@ -337,7 +337,7 @@ GuardLoc<T,localId> S0:FramePtr
   the type T; if not, make a fallback jump.  (A jump to a service
   request that chains to a retranslation for this tracelet.)
 
-CheckLoc<T,localId> S0:FramePtr
+CheckLoc<T,localId> S0:FramePtr -> L
 
   Check that type of the given localId on the frame S0 is T; if not,
   branch to the label L.

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

@@ -91,8 +91,8 @@ void HhbcTranslator::refineType(SSATmp* tmp, Type type) {
       // delete label on these instructions if this is due to an
       // assertType and also handled LdClsCns.
       // TODO(#2035446): fix this for LdClsCns
-      assert(opc == LdLoc || opc == LdStack  ||
-             opc == LdMem || opc == LdProp ||
+      assert(opc == LdLoc || opc == LdStack ||
+             opc == LdMem || opc == LdProp  ||
              opc == LdRef);
       inst->setTypeParam(type);
       tmp->setType(type);

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

@@ -1567,6 +1567,10 @@ TranslatorX64::irTranslateInstrWork(const Tracelet& t,
   }
 }
 
+static bool isPop(Opcode opc) {
+  return opc == OpPopC || opc == OpPopR;
+}
+
 void
 TranslatorX64::irPassPredictedAndInferredTypes(const NormalizedInstruction& i) {
   assert(m_useHHIR);
@@ -1574,16 +1578,27 @@ TranslatorX64::irPassPredictedAndInferredTypes(const NormalizedInstruction& i) {
   if (!i.outStack || i.breaksTracelet) return;
 
   NormalizedInstruction::OutputUse u = i.getOutputUsage(i.outStack);
+  JIT::Type jitType = JIT::Type::fromRuntimeType(i.outStack->rtt);
 
-  if ((u == NormalizedInstruction::OutputUsed && i.outputPredicted) ||
-      (u == NormalizedInstruction::OutputInferred)) {
-    JIT::Type jitType = JIT::Type::fromRuntimeType(i.outStack->rtt);
-    if (u == NormalizedInstruction::OutputInferred) {
-      TRACE(1, "HHIR: irPassPredictedAndInferredTypes: output inferred as %s\n",
+  if (u == NormalizedInstruction::OutputInferred) {
+    TRACE(1, "irPassPredictedAndInferredTypes: output inferred as %s\n",
+          jitType.toString().c_str());
+    m_hhbcTrans->assertTypeStack(0, jitType);
+
+  } else if ((u == NormalizedInstruction::OutputUsed && i.outputPredicted)) {
+    // If the value was predicted statically by the front-end, it
+    // means that it's either the predicted type or null.  In this
+    // case, if the predicted value is not ref-counted and it's simply
+    // going to be popped, then pass the information as an assertion
+    // that the type is not ref-counted.  This avoid both generating a
+    // type check and dec-refing the value.
+    if (i.outputPredictionStatic && isPop(i.next->op()) &&
+        !jitType.isCounted()) {
+      TRACE(1, "irPassPredictedAndInferredTypes: output inferred as %s\n",
             jitType.toString().c_str());
-      m_hhbcTrans->assertTypeStack(0, jitType);
+      m_hhbcTrans->assertTypeStack(0, JIT::Type::Uncounted);
     } else {
-      TRACE(1, "HHIR: irPassPredictedAndInferredTypes: output predicted as %s\n",
+      TRACE(1, "irPassPredictedAndInferredTypes: output predicted as %s\n",
             jitType.toString().c_str());
       m_hhbcTrans->checkTypeTopOfStack(jitType, i.next->offset());
     }

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

@@ -531,27 +531,29 @@ SSATmp* Simplifier::simplifyCheckType(IRInstruction* inst) {
     if (hoistGuardToLoad(src, type)) {
       return src;
     }
-  } else {
-    if (type.equals(Type::Str) && srcType.maybe(Type::Str)) {
-      // If we're guarding against Str and srcType has StaticStr or CountedStr
-      // in it, refine the output type. This can happen when we have a
-      // KindOfString guard from Translator but internally we know a more
-      // specific subtype of Str.
-      FTRACE(1, "Guarding {} to {}\n", srcType.toString(), type.toString());
-      inst->setTypeParam(type & srcType);
-    } else {
-      /*
-       * incompatible types!  We should just generate a jump here and
-       * return null.
-       *
-       * For now, this case should currently be impossible, but it may
-       * come up later due to other optimizations.  The assert is so
-       * we'll remember this spot ...
-       */
-      not_implemented();
-    }
+    return nullptr;
   }
-  return nullptr;
+  if (type.equals(Type::Str) && srcType.maybe(Type::Str)) {
+    /*
+     * If we're guarding against Str and srcType has StaticStr or CountedStr
+     * in it, refine the output type. This can happen when we have a
+     * KindOfString guard from Translator but internally we know a more
+     * specific subtype of Str.
+     */
+    FTRACE(1, "CheckType: refining {} to {}\n", srcType.toString(),
+           type.toString());
+    inst->setTypeParam(type & srcType);
+    return nullptr;
+  }
+
+  /*
+   * We got a predicted type that is wrong -- it's incompatible with
+   * the tracked type.  So throw the prediction away, since it would
+   * always fail.
+   */
+  FTRACE(1, "WARNING: CheckType: removed incorrect prediction that {} is {}\n",
+         srcType.toString(), type.toString());
+  return src;
 }
 
 SSATmp* Simplifier::simplifyQueryJmp(IRInstruction* inst) {
@@ -1701,6 +1703,10 @@ SSATmp* Simplifier::simplifyLdStackAddr(IRInstruction* inst) {
 }
 
 SSATmp* Simplifier::simplifyDecRefStack(IRInstruction* inst) {
+  if (inst->getTypeParam().notCounted()) {
+    inst->convertToNop();
+    return nullptr;
+  }
   auto const info = getStackValue(inst->getSrc(0),
                                   inst->getExtra<StackOffset>()->offset);
   if (info.value && !info.spansCall) {

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

@@ -4903,21 +4903,9 @@ TranslatorX64::translateAssignToLocalOp(const Tracelet& t,
   }
 }
 
-
 static void
 planPop(NormalizedInstruction& i) {
   DataType type = i.inputs[0]->outerType();
-  // Avoid type-prediction guard simply for popping the value out of the stack.
-  if (i.prev && i.prev->outputPredicted) {
-    i.prev->outputPredicted = false;
-    // If the prediction is based on static analysis, the type is either 'type'
-    // or null. So if 'type' is not ref-counted, keeping it avoids the dynamic
-    // check for decref.
-    if (!(i.prev->outputPredictionStatic) || IS_REFCOUNTED_TYPE(type)) {
-      i.inputs[0]->rtt = RuntimeType(KindOfInvalid);
-      type = KindOfInvalid;
-    }
-  }
   i.m_txFlags =
     (type == KindOfInvalid || IS_REFCOUNTED_TYPE(type)) ? Supported : Native;
   i.manuallyAllocInputs = true;