Add FIsParamByRefCufIter opcode

Summary: Add FIsParamByRefCufIter opcode that fetches Func from a CufIter rather than a pre-live ActRec. Use it in array_map() et al. Reviewed By: alexeyt Differential Revision: D8379435 fbshipit-source-id: c7e7f98d8a30a1ebd4c4d9f9f7d806d9901e27c8
facebook · Jun 26, 2018 · 45ca481a56e94c12a085eeeb7ac62d56761319c9 · 45ca481
1 parent dc7a6c9
commit 45ca481a56e94c12a085eeeb7ac62d56761319c9
diff --git a/hphp/doc/bytecode.specification b/hphp/doc/bytecode.specification
@@ -2232,13 +2232,24 @@ FPushCufIter <num params> <iterator id>   []  ->  []

  FPI push the result of a previous DecodeCufIter. No warning is raised.

-FIsParamByRef <param id> <reffiness hint>   []  ->  [C:Bool]
+FIsParamByRef* opcodes
+----------------------

-  Push a bool indicating whether parameter %1 is pass by ref. If the reffiness
+  FIsParamByRef* opcodes push a bool indicating whether parameter %1 of
+  a function determined by the specific opcode is pass by ref. If the reffiness
  hint %2, which may be 'Any', 'Cell', or 'Ref', is not 'Any' and does not
  match the reffiness of parameter %1 the runtime may raise a warning or throw
  an exception.

+FIsParamByRef <param id> <reffiness hint>   []  ->  [C:Bool]
+
+  Uses function associated with the current FPI.
+
+FIsParamByRefCufIter <param id> <reffiness hint> <iterator id>  []  ->  [C:Bool]
+
+  Uses function stored inside iterator %3 by a previous DecodeCufIter.
+
+
 FThrowOnRefMismatch <reffiness bool vector>   []  ->  []

  Check whether reffiness of parameters 0..N-1 of the function associated with
diff --git a/hphp/hack/src/hhbc/Hhas_parser_actions.ml b/hphp/hack/src/hhbc/Hhas_parser_actions.ml
@@ -1096,6 +1096,8 @@ let maketernaryinst s arg1 arg2 arg3 =
                                (intofiarg arg1, method_id_of_iarg arg2, class_id_of_iarg arg3))
 | "FPushClsMethodSD" -> ICall(FPushClsMethodSD
                                 (intofiarg arg1, specialclsrefofiarg arg2, method_id_of_iarg arg3))
+ | "FIsParamByRefCufIter" ->
+    ICall (FIsParamByRefCufIter (intofiarg arg1, fpasshintof arg2, iterofiarg arg3))
 | "FCallD" ->
    ICall(FCallD (intofiarg arg1,
      class_id_of_iarg arg2, function_id_of_iarg arg3))
diff --git a/hphp/hack/src/hhbc/hhbc_ast.ml b/hphp/hack/src/hhbc/hhbc_ast.ml
@@ -411,6 +411,7 @@ type instruct_call =
  | DecodeCufIter of Iterator.t * Label.t
  | FPushCufIter of num_params * Iterator.t
  | FIsParamByRef of param_num * fpass_hint
+  | FIsParamByRefCufIter of param_num * fpass_hint * Iterator.t
  | FThrowOnRefMismatch of bool list
  | FHandleRefMismatch of param_num * fpass_hint * string
  | FCall of num_params
diff --git a/hphp/hack/src/hhbc/hhbc_hhas.ml b/hphp/hack/src/hhbc/hhbc_hhas.ml
@@ -491,6 +491,9 @@ let string_of_call instruction =
    sep ["FPushCufIter"; string_of_int n; string_of_iterator_id id]
  | FIsParamByRef (i, h) ->
    sep ["FIsParamByRef"; string_of_param_num i; string_of_fpasshint h]
+  | FIsParamByRefCufIter (i, h, id) ->
+    sep ["FIsParamByRefCufIter"; string_of_param_num i; string_of_fpasshint h;
+         string_of_iterator_id id]
  | FThrowOnRefMismatch l ->
    sep ["FThrowOnRefMismatch"; string_of_list_of_bools l]
  | FHandleRefMismatch (i, h, f) ->
diff --git a/hphp/hack/src/hhbc/instruction_sequence.ml b/hphp/hack/src/hhbc/instruction_sequence.ml
@@ -772,7 +772,7 @@ let get_input_output_count i =
    | FPushFuncD _ | FThrowOnRefMismatch _ | FHandleRefMismatch _ -> (0, 0)
    | FPushObjMethod _ -> (2, 0)
    | FPushCtor _ | FPushCtorD _ | FPushCtorI _ | FPushCtorS _
-    | FIsParamByRef _ -> (0, 1)
+    | FIsParamByRef _ | FIsParamByRefCufIter _ -> (0, 1)
    | FCall n | FCallD (n, _, _) | FCallAwait (n, _, _)| FCallUnpack n
    | FCallBuiltin (n, _, _) -> (n, 1) | FCallM (n1, n2) -> (n1, n2)
    | FCallDM (n1, n2, _, _) -> (n1, n2) | FCallUnpackM (n1, n2) -> (n1, n2)
diff --git a/hphp/hhbbc/interp.cpp b/hphp/hhbbc/interp.cpp
@@ -3155,6 +3155,13 @@ void in(ISS& env, const bc::FIsParamByRef& op) {
  }
 }

+void in(ISS& env, const bc::FIsParamByRefCufIter& op) {
+  if (op.subop2 == FPassHint::Any) {
+    nothrow(env);
+  }
+  return push(env, TBool);
+}
+
 void in(ISS& env, const bc::FThrowOnRefMismatch& op) {
  auto& ar = fpiTop(env);
  if (!ar.func || ar.fallbackFunc) return;
diff --git a/hphp/runtime/ext/collections/ext_collections-map.php b/hphp/runtime/ext/collections/ext_collections-map.php
@@ -302,7 +302,7 @@ public function map(<<__OnlyRxIfRxFunc>> mixed $callback): object {
           IterInitK 1 endloop $v $k
  .try 1 {
    loop:   FPushCufIter 1 0
-            FIsParamByRef 0 Any
+            FIsParamByRefCufIter 0 Any 0
            JmpNZ by_ref
            CGetL $v
            Jmp call
@@ -362,13 +362,13 @@ public function mapWithKey(<<__OnlyRxIfRxFunc>> mixed $callback): object {
           IterInitK 1 endloop $v $k
  .try 1 {
    loop:   FPushCufIter 2 0
-            FIsParamByRef 0 Any
+            FIsParamByRefCufIter 0 Any 0
            JmpNZ by_ref_k
            CGetL $k
            Jmp pass_v
    by_ref_k:
            VGetL $k
-    pass_v: FIsParamByRef 1 Any
+    pass_v: FIsParamByRefCufIter 1 Any 0
            JmpNZ by_ref_v
            CGetL $v
            Jmp call
@@ -430,7 +430,7 @@ public function filter(<<__OnlyRxIfRxFunc>> mixed $callback): object {
           IterInitK 1 endloop $v $k
  .try 1 {
    loop:   FPushCufIter 1 0
-            FIsParamByRef 0 Any
+            FIsParamByRefCufIter 0 Any 0
            JmpNZ by_ref
            CGetL $v
            Jmp call
@@ -493,13 +493,13 @@ public function filterWithKey(<<__OnlyRxIfRxFunc>> mixed $callback): object {
           IterInitK 1 endloop $v $k
  .try 1 {
    loop:   FPushCufIter 2 0
-            FIsParamByRef 0 Any
+            FIsParamByRefCufIter 0 Any 0
            JmpNZ by_ref_k
            CGetL $k
            Jmp pass_v
    by_ref_k:
            VGetL $k
-    pass_v: FIsParamByRef 1 Any
+    pass_v: FIsParamByRefCufIter 1 Any 0
            JmpNZ by_ref_v
            CGetL $v
            Jmp call
@@ -595,7 +595,7 @@ public function takeWhile(<<__OnlyRxIfRxFunc>> mixed $callback): object {
           IterInitK 1 endloop $v $k
  .try 1 {
    loop:   FPushCufIter 1 0
-            FIsParamByRef 0 Any
+            FIsParamByRefCufIter 0 Any 0
            JmpNZ by_ref
            CGetL $v
            Jmp call
@@ -943,7 +943,7 @@ public function map(<<__OnlyRxIfRxFunc>> mixed $callback): object {
           IterInitK 1 endloop $v $k
  .try 1 {
    loop:   FPushCufIter 1 0
-            FIsParamByRef 0 Any
+            FIsParamByRefCufIter 0 Any 0
            JmpNZ by_ref
            CGetL $v
            Jmp call
@@ -1008,13 +1008,13 @@ public function mapWithKey(<<__OnlyRxIfRxFunc>> mixed $callback): object {
           IterInitK 1 endloop $v $k
  .try 1 {
    loop:   FPushCufIter 2 0
-            FIsParamByRef 0 Any
+            FIsParamByRefCufIter 0 Any 0
            JmpNZ by_ref_k
            CGetL $k
            Jmp pass_v
    by_ref_k:
            VGetL $k
-    pass_v: FIsParamByRef 1 Any
+    pass_v: FIsParamByRefCufIter 1 Any 0
            JmpNZ by_ref_v
            CGetL $v
            Jmp call
@@ -1079,7 +1079,7 @@ public function filter(<<__OnlyRxIfRxFunc>> mixed $callback): object {
           IterInitK 1 endloop $v $k
  .try 1 {
    loop:   FPushCufIter 1 0
-            FIsParamByRef 0 Any
+            FIsParamByRefCufIter 0 Any 0
            JmpNZ by_ref
            CGetL $v
            Jmp call
@@ -1145,13 +1145,13 @@ public function filterWithKey(<<__OnlyRxIfRxFunc>> mixed $callback): object {
           IterInitK 1 endloop $v $k
  .try 1 {
    loop:   FPushCufIter 2 0
-            FIsParamByRef 0 Any
+            FIsParamByRefCufIter 0 Any 0
            JmpNZ by_ref_k
            CGetL $k
            Jmp pass_v
    by_ref_k:
            VGetL $k
-    pass_v: FIsParamByRef 1 Any
+    pass_v: FIsParamByRefCufIter 1 Any 0
            JmpNZ by_ref_v
            CGetL $v
            Jmp call
@@ -1234,7 +1234,7 @@ public function takeWhile(<<__OnlyRxIfRxFunc>> mixed $callback): object {
           IterInitK 1 endloop $v $k
  .try 1 {
    loop:   FPushCufIter 1 0
-            FIsParamByRef 0 Any
+            FIsParamByRefCufIter 0 Any 0
            JmpNZ by_ref
            CGetL $v
            Jmp call
diff --git a/hphp/runtime/ext/collections/ext_collections-map.tmpl b/hphp/runtime/ext/collections/ext_collections-map.tmpl
@@ -45,13 +45,13 @@ PART2
 ;
  if ($withKey) {
    echo "    loop:   FPushCufIter 2 0\n";
-    echo "            FIsParamByRef 0 Any\n";
+    echo "            FIsParamByRefCufIter 0 Any 0\n";
    echo "            JmpNZ by_ref_k\n";
    echo "            CGetL \$k\n";
    echo "            Jmp pass_v\n";
    echo "    by_ref_k:\n";
    echo "            VGetL \$k\n";
-    echo "    pass_v: FIsParamByRef 1 Any\n";
+    echo "    pass_v: FIsParamByRefCufIter 1 Any 0\n";
    echo "            JmpNZ by_ref_v\n";
    echo "            CGetL \$v\n";
    echo "            Jmp call\n";
@@ -60,7 +60,7 @@ PART2
    echo "    call:   FCall 2\n";
  } else {
    echo "    loop:   FPushCufIter 1 0\n";
-    echo "            FIsParamByRef 0 Any\n";
+    echo "            FIsParamByRefCufIter 0 Any 0\n";
    echo "            JmpNZ by_ref\n";
    echo "            CGetL \$v\n";
    echo "            Jmp call\n";
diff --git a/hphp/runtime/vm/bytecode.cpp b/hphp/runtime/vm/bytecode.cpp
@@ -5207,21 +5207,35 @@ OPTBLD_INLINE void iopFPushCufIter(uint32_t numArgs, Iter* it) {
  }
 }

-OPTBLD_INLINE void iopFIsParamByRef(ActRec* ar, uint32_t paramId,
-                                    FPassHint hint) {
-  assertx(paramId < ar->numArgs());
-  auto const byRef = ar->func()->byRef(paramId);
+namespace {
+
+OPTBLD_INLINE void implFIsParamByRef(const Func* func, uint32_t paramId,
+                                     FPassHint hint) {
+  auto const byRef = func->byRef(paramId);

  if (RuntimeOption::EvalThrowOnCallByRefAnnotationMismatch ||
      RuntimeOption::EvalWarnOnCallByRefAnnotationMismatch) {
    if (hint == (byRef ? FPassHint::Cell : FPassHint::Ref)) {
-      raiseParamRefMismatchForFunc(ar->func(), paramId);
+      raiseParamRefMismatchForFunc(func, paramId);
    }
  }

  vmStack().pushBool(byRef);
 }

+}
+
+OPTBLD_INLINE void iopFIsParamByRef(ActRec* ar, uint32_t paramId,
+                                    FPassHint hint) {
+  assertx(paramId < ar->numArgs());
+  implFIsParamByRef(ar->func(), paramId, hint);
+}
+
+OPTBLD_INLINE void iopFIsParamByRefCufIter(uint32_t paramId, FPassHint hint,
+                                           Iter* it) {
+  implFIsParamByRef(it->cuf().func(), paramId, hint);
+}
+
 OPTBLD_INLINE void iopFThrowOnRefMismatch(ActRec* ar, imm_array<bool> byRefs) {
  assertx(byRefs.size <= ar->numArgs());
  auto const func = ar->func();
diff --git a/hphp/runtime/vm/hhbc.h b/hphp/runtime/vm/hhbc.h
@@ -583,6 +583,8 @@ constexpr uint32_t kMaxConcatN = 4;
  O(FPushCufIter,    TWO(IVA,IA),      NOV,             NOV,        PF) \
  O(FIsParamByRef,   TWO(IVA,OA(FPassHint)),                            \
                                       NOV,             ONE(CV),    FF) \
+  O(FIsParamByRefCufIter, THREE(IVA,OA(FPassHint),IA),                  \
+                                       NOV,             ONE(CV),    NF) \
  O(FThrowOnRefMismatch, ONE(BLLA),    NOV,             NOV,        FF) \
  O(FHandleRefMismatch, THREE(IVA,OA(FPassHint),SA),                    \
                                       NOV,             NOV,        NF) \
diff --git a/hphp/runtime/vm/jit/irgen-call.cpp b/hphp/runtime/vm/jit/irgen-call.cpp
@@ -1303,10 +1303,8 @@ void handleRefMismatch(IRGS& env, SSATmp* func, uint32_t paramId) {
  gen(env, RaiseParamRefMismatchForFunc, ParamData { (int32_t)paramId }, func);
 }

-}
-
-void emitFIsParamByRef(IRGS& env, uint32_t paramId, FPassHint hint) {
-  auto const func = ldPreLiveFunc(env);
+void implFIsParamByRef(IRGS& env, SSATmp* func, uint32_t paramId,
+                       FPassHint hint) {
  if (func->hasConstVal(TFunc)) {
    auto const byRef = func->funcVal()->byRef(paramId);
    if (hint == (byRef ? FPassHint::Cell : FPassHint::Ref)) {
@@ -1339,6 +1337,18 @@ void emitFIsParamByRef(IRGS& env, uint32_t paramId, FPassHint hint) {
  ));
 }

+}
+
+void emitFIsParamByRef(IRGS& env, uint32_t paramId, FPassHint hint) {
+  implFIsParamByRef(env, ldPreLiveFunc(env), paramId, hint);
+}
+
+void emitFIsParamByRefCufIter(IRGS& env, uint32_t paramId, FPassHint hint,
+                              int32_t itId) {
+  auto const func = gen(env, LdCufIterFunc, TFunc, IterId(itId), fp(env));
+  implFIsParamByRef(env, func, paramId, hint);
+}
+
 void emitFThrowOnRefMismatch(IRGS& env, const ImmVector& immVec) {
  if (!immVec.size()) return;

diff --git a/hphp/runtime/vm/jit/translator.cpp b/hphp/runtime/vm/jit/translator.cpp
@@ -318,6 +318,8 @@ static const struct {
  { OpFPushCufIter,{None,             FStack,       OutFDesc        }},
  { OpFIsParamByRef,
                   {None,             Stack1,       OutBoolean      }},
+  { OpFIsParamByRefCufIter,
+                   {None,             Stack1,       OutBoolean      }},
  { OpFThrowOnRefMismatch,
                   {None,             None,         OutNone         }},
  { OpFHandleRefMismatch,
@@ -879,6 +881,7 @@ bool dontGuardAnyInputs(const NormalizedInstruction& ni) {
  case Op::FCallAwait:
  case Op::ClsCnsD:
  case Op::FIsParamByRef:
+  case Op::FIsParamByRefCufIter:
  case Op::FThrowOnRefMismatch:
  case Op::FCallBuiltin:
  case Op::NewStructArray:
diff --git a/hphp/system/php/array_filter.hhas b/hphp/system/php/array_filter.hhas
@@ -48,7 +48,7 @@ start:          IsTypeL $arr ArrLike
                JmpNZ loop_key

    loop_val:   FPushCufIter 1 0
-                FIsParamByRef 0 Any
+                FIsParamByRefCufIter 0 Any 0
                JmpNZ by_ref_val
                CGetL $v
                Jmp call_val
@@ -62,13 +62,13 @@ start:          IsTypeL $arr ArrLike
                Jmp endloop_a

    loop_both:  FPushCufIter 2 0
-                FIsParamByRef 0 Any
+                FIsParamByRefCufIter 0 Any 0
                JmpNZ by_ref_v_both
                CGetL $v
                Jmp pass_k_both
    by_ref_v_both:
                VGetL $v
-    pass_k_both:FIsParamByRef 1 Any
+    pass_k_both:FIsParamByRefCufIter 1 Any 0
                JmpNZ by_ref_k_both
                CGetL $k
                Jmp call_both
@@ -83,7 +83,7 @@ start:          IsTypeL $arr ArrLike
                Jmp endloop_a

    loop_key:   FPushCufIter 1 0
-                FIsParamByRef 0 Any
+                FIsParamByRefCufIter 0 Any 0
                JmpNZ by_ref_key
                CGetL $k
                Jmp call_key
@@ -151,7 +151,7 @@ start_kt:         IssetL $func
                  JmpNZ loop_key_kt

    loop_val_kt:  FPushCufIter 1 0
-                  FIsParamByRef 0 Any
+                  FIsParamByRefCufIter 0 Any 0
                  JmpNZ by_ref_val_kt
                  CGetL $v
                  Jmp call_val_kt
@@ -167,14 +167,14 @@ start_kt:         IssetL $func
                  Jmp endloop_a_kt

    loop_both_kt: FPushCufIter 2 0
-                  FIsParamByRef 0 Any
+                  FIsParamByRefCufIter 0 Any 0
                  JmpNZ by_ref_v_both_kt
                  CGetL $v
                  Jmp pass_k_both_kt
    by_ref_v_both_kt:
                  VGetL $v
    pass_k_both_kt:
-                  FIsParamByRef 1 Any
+                  FIsParamByRefCufIter 1 Any 0
                  JmpNZ by_ref_k_both_kt
                  CGetL $k
                  Jmp call_both_kt
@@ -191,7 +191,7 @@ start_kt:         IssetL $func
                  Jmp endloop_a_kt

    loop_key_kt:  FPushCufIter 1 0
-                  FIsParamByRef 0 Any
+                  FIsParamByRefCufIter 0 Any 0
                  JmpNZ by_ref_key_kt
                  CGetL $k
                  Jmp call_key_kt