Omit stack overflow checks on leaf functions

I did an experiment turning off all stack checks, and it
looks like it does cost something.  This seemed like an easy way to
turn it off some of the time, but maybe we should resurrect the SEGV
handler.

hphp/compiler/analysis/emitter.cpp

@@ -275,6 +275,7 @@ static int32_t countStackValues(const std::vector<uchar>& immVec) {
     getUnitEmitter().recordSourceLocation(m_tempLoc ? m_tempLoc.get() : \
                                           m_node->getLocation().get(), curPos); \
     if (flags & TF) getEmitterVisitor().restoreJumpTargetEvalStack(); \
+    if (isFCallStar(opcode)) getEmitterVisitor().recordCall(); \
     getEmitterVisitor().setPrevOpcode(opcode); \
   }
 
@@ -1398,6 +1399,10 @@ void EmitterVisitor::restoreJumpTargetEvalStack() {
   m_evalStack = it->second;
 }
 
+void EmitterVisitor::recordCall() {
+  m_curFunc->setContainsCalls();
+}
+
 bool EmitterVisitor::isJumpTarget(Offset target) {
   // Returns true iff one of the following conditions is true:
   //   1) We have seen an instruction that jumps to the specified offset
@@ -6666,8 +6671,13 @@ void EmitterVisitor::copyOverFPIRegions(FuncEmitter* fe) {
 }
 
 void EmitterVisitor::saveMaxStackCells(FuncEmitter* fe) {
-  fe->setMaxStackCells(m_actualStackHighWater + kNumActRecCells
-                       + m_fdescHighWater);
+  // Max stack cells is used for stack overflow checks.  We need to
+  // count all the locals, and all cells due to ActRecs.  We don't
+  // need to count this function's own ActRec because whoever called
+  // it already included it in its "max stack cells".
+  fe->setMaxStackCells(m_actualStackHighWater +
+                       fe->numLocals() +
+                       m_fdescHighWater);
   m_actualStackHighWater = 0;
   m_fdescHighWater = 0;
 }

hphp/compiler/analysis/emitter.h

@@ -359,6 +359,7 @@ public:
     recordJumpTarget(target, m_evalStack);
   }
   void restoreJumpTargetEvalStack();
+  void recordCall();
   bool isJumpTarget(Offset target);
   void setPrevOpcode(Opcode op) { m_prevOpcode = op; }
   Opcode getPrevOpcode() const { return m_prevOpcode; }

hphp/runtime/vm/bytecode.cpp

@@ -1652,7 +1652,7 @@ static inline void checkStack(Stack& stk, const Func* f) {
   // Check whether func's maximum stack usage would overflow the stack.
   // Both native and VM stack overflows are independently possible.
   if (!stack_in_bounds(info) ||
-      stk.wouldOverflow(f->maxStackCells() + kMaxJITInlineStackCells)) {
+      stk.wouldOverflow(f->maxStackCells() + kStackCheckPadding)) {
     TRACE(1, "Maximum VM stack depth exceeded.\n");
     raise_error("Stack overflow");
   }

hphp/runtime/vm/bytecode.h

@@ -419,11 +419,15 @@ constexpr size_t kNumIterCells = sizeof(Iter) / sizeof(Cell);
 constexpr size_t kNumActRecCells = sizeof(ActRec) / sizeof(Cell);
 
 /*
- * We pad all stack overflow checks by a small amount to allow for
- * inlining functions without having to either do another stack check
- * or chase down prologues to smash.
+ * We pad all stack overflow checks by a small amount to allow for two
+ * things:
+ *
+ *   - inlining functions without having to either do another stack
+ *     check (or chase down prologues to smash checks to be bigger).
+ *
+ *   - omitting stack overflow checks on leaf functions
  */
-constexpr int kMaxJITInlineStackCells = 4 + kNumActRecCells;
+constexpr int kStackCheckPadding = 20;
 
 struct Fault {
   enum Type : int16_t {

hphp/runtime/vm/class.cpp

@@ -13,6 +13,7 @@
    | license@php.net so we can mail you a copy immediately.               |
    +----------------------------------------------------------------------+
 */
+#include "runtime/vm/class.h"
 
 #include "runtime/vm/class.h"
 #include "runtime/base/base_includes.h"
@@ -1305,7 +1306,7 @@ void Class::setSpecial() {
   // Use 86ctor(), since no program-supplied constructor exists
   m_ctor = findSpecialMethod(this, sd86ctor);
   assert(m_ctor && "class had no user-defined constructor or 86ctor");
-  assert(m_ctor->attrs() == (AttrPublic|AttrNoInjection));
+  assert(m_ctor->attrs() == (AttrPublic|AttrNoInjection|AttrPhpLeafFn));
 }
 
 void Class::applyTraitPrecRule(const PreClass::TraitPrecRule& rule) {

hphp/runtime/vm/core_types.h

@@ -72,25 +72,32 @@ constexpr Offset kInvalidOffset = std::numeric_limits<Offset>::max();
 typedef uint32_t Slot;
 const Slot kInvalidSlot = Slot(-1);
 
-// Special types that are not relevant to the runtime as a whole.
-// The order for public/protected/private matters in numerous places.
-//
-// Attr unions are directly stored as integers in .hhbc repositories, so
-// incompatible changes here require a schema version bump.
-//
-// AttrTrait on a method means that the method is NOT a constructor,
-// even though it may look like one
-//
-// AttrNoOverride (WholeProgram only) on a class means its not extended
-// and on a method means that no extending class defines the method.
-//
-// AttrVariadicByRef indicates a function is a builtin that takes
-// variadic arguments, where the arguments are either by ref or
-// optionally by ref.  (It is equivalent to having ClassInfo's
-// (RefVariableArguments | MixedVariableArguments).)
-//
-// AttrMayUseVV indicates that a function may need to use a VarEnv or
-// varargs (aka extraArgs) at run time.
+/*
+ * Special types that are not relevant to the runtime as a whole.
+ * The order for public/protected/private matters in numerous places.
+ *
+ * Attr unions are directly stored as integers in .hhbc repositories, so
+ * incompatible changes here require a schema version bump.
+ *
+ * AttrTrait on a method means that the method is NOT a constructor,
+ * even though it may look like one
+ *
+ * AttrNoOverride (WholeProgram only) on a class means its not extended
+ * and on a method means that no extending class defines the method.
+ *
+ * AttrVariadicByRef indicates a function is a builtin that takes
+ * variadic arguments, where the arguments are either by ref or
+ * optionally by ref.  (It is equivalent to having ClassInfo's
+ * (RefVariableArguments | MixedVariableArguments).)
+ *
+ * AttrMayUseVV indicates that a function may need to use a VarEnv or
+ * varargs (aka extraArgs) at run time.
+ *
+ * AttrPhpLeafFn indicates a function does not make any explicit calls
+ * to other php functions.  It may still call other user-level
+ * functions via re-entry (e.g. for destructors or autoload), and it
+ * may make calls to builtins using FCallBuiltin.
+ */
 enum Attr {
   AttrNone      = 0,             // class  property  method  //
   AttrReference = (1 << 0),      //                     X    //
@@ -101,6 +108,7 @@ enum Attr {
   AttrAbstract  = (1 << 5),      //    X                X    //
   AttrFinal     = (1 << 6),      //    X                X    //
   AttrInterface = (1 << 7),      //    X                     //
+  AttrPhpLeafFn = (1 << 7),      //                     X    //
   AttrTrait     = (1 << 8),      //    X                X    //
   AttrNoInjection = (1 << 9),    //                     X    //
   AttrUnique    = (1 << 10),     //    X                X    //

hphp/runtime/vm/func.cpp

@@ -13,6 +13,7 @@
    | license@php.net so we can mail you a copy immediately.               |
    +----------------------------------------------------------------------+
 */
+#include "runtime/vm/func.h"
 
 #include <iostream>
 #include <boost/scoped_ptr.hpp>
@@ -23,7 +24,6 @@
 #include "util/debug.h"
 #include "runtime/base/strings.h"
 #include "runtime/vm/core_types.h"
-#include "runtime/vm/func.h"
 #include "runtime/vm/runtime.h"
 #include "runtime/vm/repo.h"
 #include "runtime/vm/translator/targetcache.h"
@@ -459,6 +459,7 @@ void Func::prettyPrint(std::ostream& out) const {
     if (m_attrs & AttrPrivate) { out << "private "; }
     if (m_attrs & AttrAbstract) { out << "abstract "; }
     if (m_attrs & AttrFinal) { out << "final "; }
+    if (m_attrs & AttrPhpLeafFn) { out << "(leaf) "; }
     out << preClass()->name()->data() << "::" << m_name->data();
   } else {
     out << "Function " << m_name->data();
@@ -674,23 +675,50 @@ const Func* Func::getGeneratorBody(const StringData* name) const {
 // FuncEmitter.
 
 FuncEmitter::FuncEmitter(UnitEmitter& ue, int sn, Id id, const StringData* n)
-  : m_ue(ue), m_pce(nullptr), m_sn(sn), m_id(id), m_name(n), m_numLocals(0),
-    m_numUnnamedLocals(0), m_activeUnnamedLocals(0), m_numIterators(0),
-    m_nextFreeIterator(0), m_retTypeConstraint(nullptr),
-    m_returnType(KindOfInvalid), m_top(false), m_isClosureBody(false),
-    m_isGenerator(false), m_isGeneratorFromClosure(false),
-    m_hasGeneratorAsBody(false), m_info(nullptr), m_builtinFuncPtr(nullptr) {
-}
+  : m_ue(ue)
+  , m_pce(nullptr)
+  , m_sn(sn)
+  , m_id(id)
+  , m_name(n)
+  , m_numLocals(0)
+  , m_numUnnamedLocals(0)
+  , m_activeUnnamedLocals(0)
+  , m_numIterators(0)
+  , m_nextFreeIterator(0)
+  , m_retTypeConstraint(nullptr)
+  , m_returnType(KindOfInvalid)
+  , m_top(false)
+  , m_isClosureBody(false)
+  , m_isGenerator(false)
+  , m_isGeneratorFromClosure(false)
+  , m_hasGeneratorAsBody(false)
+  , m_containsCalls(false)
+  , m_info(nullptr)
+  , m_builtinFuncPtr(nullptr)
+{}
 
 FuncEmitter::FuncEmitter(UnitEmitter& ue, int sn, const StringData* n,
                          PreClassEmitter* pce)
-  : m_ue(ue), m_pce(pce), m_sn(sn), m_name(n), m_numLocals(0),
-    m_numUnnamedLocals(0), m_activeUnnamedLocals(0), m_numIterators(0),
-    m_nextFreeIterator(0), m_retTypeConstraint(nullptr),
-    m_returnType(KindOfInvalid), m_top(false), m_isClosureBody(false),
-    m_isGenerator(false), m_isGeneratorFromClosure(false),
-    m_hasGeneratorAsBody(false), m_info(nullptr), m_builtinFuncPtr(nullptr) {
-}
+  : m_ue(ue)
+  , m_pce(pce)
+  , m_sn(sn)
+  , m_name(n)
+  , m_numLocals(0)
+  , m_numUnnamedLocals(0)
+  , m_activeUnnamedLocals(0)
+  , m_numIterators(0)
+  , m_nextFreeIterator(0)
+  , m_retTypeConstraint(nullptr)
+  , m_returnType(KindOfInvalid)
+  , m_top(false)
+  , m_isClosureBody(false)
+  , m_isGenerator(false)
+  , m_isGeneratorFromClosure(false)
+  , m_hasGeneratorAsBody(false)
+  , m_containsCalls(false)
+  , m_info(nullptr)
+  , m_builtinFuncPtr(nullptr)
+{}
 
 FuncEmitter::~FuncEmitter() {
 }
@@ -880,6 +908,8 @@ Func* FuncEmitter::create(Unit& unit, PreClass* preClass /* = NULL */) const {
     attrs = Attr(attrs & ~AttrPersistent);
   }
 
+  if (!m_containsCalls) attrs = Attr(attrs | AttrPhpLeafFn);
+
   Func* f = (m_pce == nullptr)
     ? m_ue.newFunc(this, unit, m_id, m_line1, m_line2, m_base,
                    m_past, m_name, attrs, m_top, m_docComment,
@@ -997,6 +1027,7 @@ void FuncEmitter::serdeMetaData(SerDe& sd) {
     (m_isGenerator)
     (m_isGeneratorFromClosure)
     (m_hasGeneratorAsBody)
+    (m_containsCalls)
 
     (m_params)
     (m_localNames)

hphp/runtime/vm/func.h

@@ -638,6 +638,8 @@ public:
   void setHasGeneratorAsBody(bool b) { m_hasGeneratorAsBody = b; }
   bool hasGeneratorAsBody() const { return m_hasGeneratorAsBody; }
 
+  void setContainsCalls() { m_containsCalls = true; }
+
   void addUserAttribute(const StringData* name, TypedValue tv);
 
   void commit(RepoTxn& txn) const;
@@ -683,6 +685,7 @@ private:
   bool m_isGenerator;
   bool m_isGeneratorFromClosure;
   bool m_hasGeneratorAsBody;
+  bool m_containsCalls;
 
   Func::UserAttributeMap m_userAttributes;
 
@@ -695,7 +698,7 @@ class FuncRepoProxy : public RepoProxy {
   friend class Func;
   friend class FuncEmitter;
 public:
-  FuncRepoProxy(Repo& repo);
+  explicit FuncRepoProxy(Repo& repo);
   ~FuncRepoProxy();
   void createSchema(int repoId, RepoTxn& txn);
 

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

@@ -1132,10 +1132,8 @@ static bool shouldIRInline(const Func* curFunc,
   if (func->numIterators() != 0) {
     return refuse("iterators");
   }
-  if (func->maxStackCells() >= kMaxJITInlineStackCells) {
-    FTRACE(1, "{} >= {}\n",
-           func->maxStackCells(),
-           kMaxJITInlineStackCells);
+  if (func->maxStackCells() >= kStackCheckPadding) {
+    FTRACE(1, "{} >= {}\n", func->maxStackCells(), kStackCheckPadding);
     return refuse("too many stack cells");
   }
 

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

@@ -1555,7 +1555,7 @@ void TranslatorX64::unprotectCode() {
 
 void
 TranslatorX64::emitStackCheck(int funcDepth, Offset pc) {
-  funcDepth += kMaxJITInlineStackCells * sizeof(Cell);
+  funcDepth += kStackCheckPadding * sizeof(Cell);
 
   uint64_t stackMask = cellsToBytes(RuntimeOption::EvalVMStackElms) - 1;
   a.    mov_reg64_reg64(rVmSp, rScratch); // copy to destroy
@@ -2127,8 +2127,18 @@ TranslatorX64::funcPrologue(Func* func, int nPassed, ActRec* ar) {
     emitPopRetIntoActRec(a);
     // entry point for magic methods comes later
     emitRB(a, RBTypeFuncEntry, func->fullName()->data());
-    // Guard: we have stack enough stack space to complete this function.
-    emitStackCheck(cellsToBytes(func->maxStackCells()), func->base());
+
+    /*
+     * Guard: we have stack enough stack space to complete this
+     * function.  We omit overflow checks if it is a leaf function
+     * that can't use more than kStackCheckPadding cells.
+     */
+    auto const needStackCheck =
+      !(func->attrs() & AttrPhpLeafFn) ||
+      func->maxStackCells() >= kStackCheckPadding;
+    if (needStackCheck) {
+      emitStackCheck(cellsToBytes(func->maxStackCells()), func->base());
+    }
   }
 
   SrcKey skFuncBody = emitPrologue(func, nPassed);

hphp/test/quick/exceptions2.php.expectf

@@ -1235,4 +1235,3 @@ C::__destruct
 C::__destruct
 C::__destruct
 C::__destruct
-C::__destruct

hphp/test/quick/exceptions3.php.expectf

@@ -1460,5 +1460,3 @@ C::__destruct
 C::__destruct
 C::__destruct
 C::__destruct
-C::__destruct
-C::__destruct

hphp/test/quick/exceptions5.php.expectf

@@ -1 +1 @@
-HipHop Fatal error: Stack overflow in %s on line 21
+HipHop Fatal error: Stack overflow in %s on line 18

hphp/test/quick/exceptions6.php.expectf

@@ -1 +1 @@
-HipHop Fatal error: Stack overflow in %s on line 26
+HipHop Fatal error: Stack overflow in %s on line 21