From 0aa1e0143d30378a5f616b3774c327e94e5489eb Mon Sep 17 00:00:00 2001
From: bsimmers <bsimmers@fb.com>
Date: Tue, 27 May 2014 16:49:30 -0700
Subject: [PATCH] Don't duplicate the dispatch loop just for RetC profiling

Summary: For the interpreter-based warmup profiling, we just call
shouldProfile() in each bytecode's implementation. I see no reason why RetC
should be any different.

Reviewed By: @edwinsmith

Differential Revision: D1350436
---
 hphp/runtime/base/execution-context.h |  8 ++------
 hphp/runtime/vm/bytecode.cpp          | 35 +++++++++++++----------------------
 2 files changed, 15 insertions(+), 28 deletions(-)

diff --git a/hphp/runtime/base/execution-context.h b/hphp/runtime/base/execution-context.h
index 043ebf2f5ee..e4011f064a4 100644
--- a/hphp/runtime/base/execution-context.h
+++ b/hphp/runtime/base/execution-context.h
@@ -693,14 +693,10 @@ public:
   void op##name();
 OPCODES
 #undef O
-  enum DispatchFlags {
-    BreakOnCtlFlow = 1 << 0,
-    Profile        = 1 << 1
-  };
-  template <int dispatchFlags>
+  template <bool breakOnCtlFlow>
   void dispatchImpl();
   void dispatch();
-  // dispatchBB() tries to run until a control-flow instruction has been run.
+  // dispatchBB() exits if a control-flow instruction has been run.
   void dispatchBB();
 
 public:
diff --git a/hphp/runtime/vm/bytecode.cpp b/hphp/runtime/vm/bytecode.cpp
index 0205390c649..07d202027fe 100644
--- a/hphp/runtime/vm/bytecode.cpp
+++ b/hphp/runtime/vm/bytecode.cpp
@@ -4538,6 +4538,16 @@ OPTBLD_INLINE void ExecutionContext::ret(IOP_ARGS) {
     retval.m_type = KindOfObject;
   }
 
+  if (shouldProfile()) {
+    auto f = const_cast<Func*>(m_fp->func());
+    f->incProfCounter();
+    if (!(f->isPseudoMain() || f->isClosureBody() || f->isMagic() ||
+          Func::isSpecial(f->name()))) {
+      recordType(TypeProfileKey(TypeProfileKey::MethodName, f->name()),
+                 retval.m_type);
+    }
+  }
+
   // Type profile return value.
   if (RuntimeOption::EvalRuntimeTypeProfile) {
     profileOneArgument(retval, -1, m_fp->func());
@@ -7413,19 +7423,8 @@ OPCODES
 #undef DECODE_JMP
 #undef DECODE
 
-static inline void
-profileReturnValue(const DataType dt) {
-  const Func* f = liveFunc();
-  if (f->isPseudoMain() || f->isClosureBody() || f->isMagic() ||
-      Func::isSpecial(f->name()))
-    return;
-  recordType(TypeProfileKey(TypeProfileKey::MethodName, f->name()), dt);
-}
-
-template <int dispatchFlags>
+template <bool breakOnCtlFlow>
 inline void ExecutionContext::dispatchImpl() {
-  static const bool breakOnCtlFlow = dispatchFlags & BreakOnCtlFlow;
-  static const bool profile = dispatchFlags & Profile;
   static const void *optabDirect[] = {
 #define O(name, imm, push, pop, flags) \
     &&Label##name,
@@ -7478,10 +7477,6 @@ inline void ExecutionContext::dispatchImpl() {
     ONTRACE(1,                                                          \
             Trace::trace("dispatch: %d: %s\n", pcOff(this),             \
                          nametab[uint8_t(op)]));                        \
-    if (profile && (op == OpRetC || op == OpRetV)) {                    \
-      const_cast<Func*>(liveFunc())->incProfCounter();                  \
-      profileReturnValue(m_stack.top()->m_type);                        \
-    }                                                                   \
     goto *optab[uint8_t(op)];                                           \
 } while (0)
 
@@ -7521,11 +7516,7 @@ inline void ExecutionContext::dispatchImpl() {
 }
 
 void ExecutionContext::dispatch() {
-  if (shouldProfile()) {
-    dispatchImpl<Profile>();
-  } else {
-    dispatchImpl<0>();
-  }
+  dispatchImpl<false>();
 }
 
 // We are about to go back to translated code, check whether we should
@@ -7545,7 +7536,7 @@ void ExecutionContext::dispatchBB() {
     Stats::incStatGrouped(cat, name, 1);
   }
 
-  dispatchImpl<BreakOnCtlFlow>();
+  dispatchImpl<true>();
   switchModeForDebugger();
 }
 
-- 
2.11.4.GIT