Refactor to prepare for inline IRUnit stitching

Summary:
Currently, we generate the callee `IRUnit` when calculating the cost of inlining for the first time (which then gets cached). This diff refactors to return back to `irGenTryInlineFCall` the callee's `IRUnit` whenever `Cfg::HHIR::EnableInlingPass` is true or we need to calculate cost for the first time.

In order to support that, I had to move the generating `IRUnit` out of `computeTranslationCostSlow` into a new function `computeInlineIRUnit` which returns whether to inline the callee region and also returns back the unit.

Reviewed By: paulbiss

Differential Revision: D70131260

fbshipit-source-id: 41b3e30d5f4100cb8ed4e88ea322ea58e738082a

Author: jaewie

hphp/doc/configs.specification

@@ -1546,6 +1546,7 @@ The format can be found in hphp/tools/configs/generate_configs.rs
 - bool HHIR.GenOpts = true, UNKNOWN
 - bool HHIR.RefcountOpts = true, UNKNOWN
 - bool HHIR.EnableGenTimeInlining = true, UNKNOWN
+- bool HHIR.EnableInliningPass = false, UNKNOWN
 - uint32_t HHIR.InliningCostFactorMain = 100, UNKNOWN
 - uint32_t HHIR.InliningCostFactorCold = 32, UNKNOWN
 - uint32_t HHIR.InliningCostFactorFrozen = 10, UNKNOWN

hphp/runtime/vm/jit/inlining-decider.cpp

@@ -24,6 +24,7 @@
 #include "hphp/runtime/vm/func.h"
 #include "hphp/runtime/vm/hhbc.h"
 #include "hphp/runtime/vm/jit/irgen.h"
+#include "hphp/runtime/vm/jit/irgen-inlining.h"
 #include "hphp/runtime/vm/jit/location.h"
 #include "hphp/runtime/vm/jit/irlower.h"
 #include "hphp/runtime/vm/jit/mcgen.h"
@@ -299,43 +300,30 @@ using InlineCostCache = folly::ConcurrentHashMap<
 >;
 
 Vcost computeTranslationCostSlow(SrcKey at,
-                                 const RegionDesc& region,
+                                 const irgen::RegionAndLazyUnit& regionAndLazyUnit,
                                  AnnotationData* annotationData) {
-  TransContext ctx {
-    TransIDSet{},
-    0,  // optIndex
-    TransKind::Optimize,
-    at,
-    &region,
-    at.packageInfo(),
-    PrologueID(),
-  };
-
-  tracing::Block _{"compute-inline-cost", [&] { return traceProps(ctx); }};
-
-  rqtrace::DisableTracing notrace;
-  auto const unbumper = mcgen::unbumpFunctions();
-
-  auto const unit = irGenInlineRegion(ctx, region);
+  auto const unit = regionAndLazyUnit.unit();
   if (!unit) return {0, true};
 
   // TODO(T52856776) - annotations should be copied from unit into outer unit
   // via annotationData
-
+  rqtrace::DisableTracing notrace;
+  auto const unbumper = mcgen::unbumpFunctions();
   SCOPE_ASSERT_DETAIL("Inline-IRUnit") { return show(*unit); };
   return irlower::computeIRUnitCost(*unit);
 }
 
 InlineCostCache s_inlCostCache;
 
 int computeTranslationCost(SrcKey at,
-                           const RegionDesc& region,
+                           const irgen::RegionAndLazyUnit& regionAndLazyUnit,
                            AnnotationData* annotationData) {
+  auto const region = *regionAndLazyUnit.region();
   InlineRegionKey irk{region};
   auto f = s_inlCostCache.find(irk);
   if (f != s_inlCostCache.end()) return f->second;
 
-  auto const info = computeTranslationCostSlow(at, region, annotationData);
+  auto const info = computeTranslationCostSlow(at, regionAndLazyUnit, annotationData);
   auto cost = info.cost;
 
   // We normally store the computed cost into the cache.  However, if the region
@@ -420,7 +408,7 @@ uint64_t adjustedMaxVasmCost(const irgen::IRGS& env,
  */
 int costOfInlining(SrcKey callerSk,
                    const Func* callee,
-                   const RegionDesc& region,
+                   const irgen::RegionAndLazyUnit& regionAndLazyUnit,
                    AnnotationData* annotationData) {
   auto const alwaysInl =
     (!Cfg::HHIR::InliningIgnoreHints &&
@@ -430,7 +418,7 @@ int costOfInlining(SrcKey callerSk,
   // Functions marked as always inline don't contribute to overall cost
   return alwaysInl ?
     0 :
-    computeTranslationCost(callerSk, region, annotationData);
+    computeTranslationCost(callerSk, regionAndLazyUnit, annotationData);
 }
 
 bool isCoeffectsBackdoor(SrcKey callerSk, const Func* callee) {
@@ -463,8 +451,9 @@ bool isCoeffectsBackdoor(SrcKey callerSk, const Func* callee) {
 bool shouldInline(const irgen::IRGS& irgs,
                   SrcKey callerSk,
                   const Func* callee,
-                  const RegionDesc& region,
+                  const irgen::RegionAndLazyUnit& regionAndUnit,
                   int& cost) {
+  auto const region = *regionAndUnit.region();
   auto sk = region.empty() ? SrcKey() : region.start();
   assertx(callee);
   assertx(sk.func() == callee);
@@ -581,7 +570,7 @@ bool shouldInline(const irgen::IRGS& irgs,
     // In debug builds compute the cost anyway to catch bugs in the inlining
     // machinery. Many inlining tests utilize the __ALWAYS_INLINE attribute.
     if (debug) {
-      computeTranslationCost(callerSk, region, annotationsPtr);
+      computeTranslationCost(callerSk, regionAndUnit, annotationsPtr);
     }
     return accept("callee marked as __ALWAYS_INLINE");
   }
@@ -590,7 +579,7 @@ bool shouldInline(const irgen::IRGS& irgs,
   // We measure the cost of inlining each callstack and stop when it exceeds a
   // certain threshold.  (Note that we do not measure the total cost of all the
   // inlined calls for a given caller---just the cost of each nested stack.)
-  cost = costOfInlining(callerSk, callee, region, annotationsPtr);
+  cost = costOfInlining(callerSk, callee, regionAndUnit, annotationsPtr);
   if (cost <= Cfg::HHIR::AlwaysInlineVasmCostLimit) {
     return accept(folly::sformat("cost={} within always-inline limit", cost));
   }
@@ -744,10 +733,10 @@ RegionDescPtr selectCalleeCFG(SrcKey callerSk, SrcKey entry,
 }
 }
 
-RegionDescPtr selectCalleeRegion(const irgen::IRGS& irgs,
-                                 SrcKey entry,
-                                 Type ctxType,
-                                 SrcKey callerSk) {
+irgen::RegionAndLazyUnit selectCalleeRegion(const irgen::IRGS& irgs,
+                                            SrcKey entry,
+                                            Type ctxType,
+                                            SrcKey callerSk) {
   assertx(entry.funcEntry());
   auto const callee = entry.func();
 
@@ -761,7 +750,7 @@ RegionDescPtr selectCalleeRegion(const irgen::IRGS& irgs,
 
   if (ctxType == TBottom) {
     traceRefusal(callerSk, callee, "ctx is TBottom", annotationsPtr);
-    return nullptr;
+    return {callerSk, nullptr};
   }
   if (callee->isClosureBody()) {
     if (!callee->cls()) {
@@ -778,21 +767,21 @@ RegionDescPtr selectCalleeRegion(const irgen::IRGS& irgs,
          (!callerSk.hasThis() && isFCallClsMethod(callerSk.op())))) {
       traceRefusal(callerSk, callee, "calling static method with an object",
                    annotationsPtr);
-      return nullptr;
+      return {callerSk, nullptr};
     }
   }
 
   if (callee->cls()) {
     if (callee->isStatic() && !ctxType.maybe(TCls)) {
       traceRefusal(callerSk, callee, "calling a static method with an instance",
                    annotationsPtr);
-      return nullptr;
+      return {callerSk, nullptr};
     }
     if (!callee->isStatic() && !ctxType.maybe(TObj)) {
       traceRefusal(callerSk, callee,
                    "calling an instance method without an instance",
                    annotationsPtr);
-      return nullptr;
+      return {callerSk, nullptr};
     }
   }
 
@@ -808,7 +797,7 @@ RegionDescPtr selectCalleeRegion(const irgen::IRGS& irgs,
 
     // If we don't have sufficient type information to inline the region return
     // early
-    if (type == TBottom) return nullptr;
+    if (type == TBottom) return {callerSk, nullptr};
     FTRACE(2, "input {}: {}\n", i + 1, type);
     inputTypes.push_back(type);
   }
@@ -823,21 +812,22 @@ RegionDescPtr selectCalleeRegion(const irgen::IRGS& irgs,
   if (profData()) {
     auto region = selectCalleeCFG(callerSk, entry, ctxType, inputTypes,
                                   Cfg::Jit::MaxInlineRegionInstrs, annotationsPtr);
-    if (region) return region;
+    if (region) return {callerSk, region};
     // Special case: even if we don't have prof data for this func, if
     // it takes no arguments and returns a constant, it might be a
     // trivial function (IE, "return 123;"). Attempt to inline it
     // anyways using the tracelet selector.
-    if (callee->numFuncEntryInputs() > 0) return nullptr;
+    if (callee->numFuncEntryInputs() > 0) return {callerSk, nullptr};
     auto const retType =
       typeFromRAT(callee->repoReturnType(), callerSk.func()->cls());
     // Deliberately using hasConstVal, not admitsSingleVal, since we
     // don't want TInitNull, etc.
-    if (!retType.hasConstVal()) return nullptr;
+    if (!retType.hasConstVal()) return {callerSk, nullptr};
   }
 
-  return selectCalleeTracelet(entry, ctxType, inputTypes,
-                              Cfg::Jit::MaxInlineRegionInstrs);
+  auto region = selectCalleeTracelet(entry, ctxType, inputTypes,
+                                     Cfg::Jit::MaxInlineRegionInstrs);
+  return {callerSk, region};
 }
 
 void setBaseInliningProfCount(uint64_t value) {

hphp/runtime/vm/jit/inlining-decider.h

@@ -19,6 +19,7 @@
 #include "hphp/runtime/vm/hhbc.h"
 #include "hphp/runtime/vm/jit/annotation-data.h"
 #include "hphp/runtime/vm/jit/region-selection.h"
+#include "hphp/runtime/vm/jit/irgen-inlining.h"
 
 #include <vector>
 
@@ -86,23 +87,23 @@ bool canInlineAt(SrcKey callSK,
  * selector /ought/ to do so.
  */
 bool shouldInline(const irgen::IRGS& irgs, SrcKey callerSk, const Func* callee,
-                  const RegionDesc& region, int& cost);
+                  const irgen::RegionAndLazyUnit& regionAndLazyUnit, int& cost);
 
 /*
  * Return the cost of inlining the given callee.
  */
 int costOfInlining(SrcKey callerSk,
                    const Func* callee,
-                   const RegionDesc& region,
+                   const irgen::RegionAndLazyUnit& regionAndLazyUnit,
                    AnnotationData* annotationData);
 
 /*
  * Select an inlining region for the call to `callee' at `sk'.
  */
-RegionDescPtr selectCalleeRegion(const irgen::IRGS& irgs,
-                                 SrcKey entry,
-                                 Type ctxType,
-                                 SrcKey callerSk);
+irgen::RegionAndLazyUnit selectCalleeRegion(const irgen::IRGS& irgs,
+                                            SrcKey entry,
+                                            Type ctxType,
+                                            SrcKey callerSk);
 
 void setBaseInliningProfCount(uint64_t value);
 

hphp/runtime/vm/jit/irgen-inlining.cpp

@@ -192,11 +192,39 @@ Outer:                                 | Inner:
 #include "hphp/runtime/vm/hhbc-codec.h"
 #include "hphp/runtime/vm/resumable.h"
 #include "hphp/runtime/vm/unwind.h"
+#include "hphp/runtime/vm/jit/inlining-decider.h"
+#include "hphp/runtime/vm/jit/mcgen-translate.h"
+#include "hphp/runtime/vm/jit/translate-region.h"
 
 namespace HPHP::jit::irgen {
 
 TRACE_SET_MOD(hhir);  
 
+RegionAndLazyUnit::RegionAndLazyUnit(
+    SrcKey callerSk,
+    RegionDescPtr region
+  ) : m_callerSk(callerSk)
+    , m_region(std::move(region)) {}
+
+IRUnit* RegionAndLazyUnit::unit() const {
+    if (m_unit) return m_unit.get();
+    always_assert(m_region);
+    TransContext ctx {
+      TransIDSet{},
+      0,  // optIndex
+      TransKind::Optimize,
+      m_callerSk,
+      m_region.get(),
+      m_callerSk.packageInfo(),
+      PrologueID(),
+    };
+    tracing::Block _{"compute-inline-cost", [&] { return traceProps(ctx); }};
+    rqtrace::DisableTracing notrace;
+    auto const unbumper = mcgen::unbumpFunctions();
+    m_unit = irGenInlineRegion(ctx, *m_region);
+    return m_unit.get();
+}
+
 bool isInlining(const IRGS& env) {
   return !env.inlineState.frames.empty();
 }
@@ -751,6 +779,16 @@ bool spillInlinedFrames(IRGS& env) {
   return spilled;
 }
 
+void fixCalleeUnit(const IRGS& env, IRUnit& unit) {
+}
+
+
+bool stitchInlinedRegion(irgen::IRGS& irgs, SrcKey callerSk, SrcKey calleeSk,
+                         const RegionDesc& calleeRegion, IRUnit& calleeUnit) {
+    fixCalleeUnit(irgs, calleeUnit);                          
+    return true;
+}
+
 //////////////////////////////////////////////////////////////////////
 
 }

hphp/runtime/vm/jit/irgen-inlining.h

@@ -17,6 +17,7 @@
 
 #include "hphp/runtime/vm/srckey.h"
 #include "hphp/runtime/vm/jit/extra-data.h"
+#include "hphp/runtime/vm/jit/region-selection.h"
 
 namespace HPHP {
 
@@ -28,6 +29,26 @@ struct SSATmp;
 
 namespace irgen {
 
+struct RegionAndLazyUnit {
+  RegionAndLazyUnit(
+    SrcKey callerSk,
+    RegionDescPtr region
+  );
+  ~RegionAndLazyUnit() = default;
+  RegionAndLazyUnit(RegionAndLazyUnit&&) = default;
+  RegionAndLazyUnit& operator=(RegionAndLazyUnit&&) = default;
+  RegionAndLazyUnit(const RegionAndLazyUnit&) = delete;
+  RegionAndLazyUnit& operator=(const RegionAndLazyUnit&) = delete;
+
+  IRUnit* unit() const;
+  RegionDescPtr region() const { return m_region; }
+private:
+  SrcKey m_callerSk;
+  RegionDescPtr m_region;
+  mutable std::unique_ptr<IRUnit> m_unit;
+};
+
+
 struct IRGS;
 
 ///////////////////////////////////////////////////////////////////////////////
@@ -65,8 +86,13 @@ bool spillInlinedFrames(IRGS& env);
  * FCall bytecode is being translated.
  */
 SSATmp* genCalleeFP(IRGS& env, const Func* callee);
+	
+/**
+ * Stitch callee's IRUnit into caller's IRGS.
+ */
+bool stitchInlinedRegion(irgen::IRGS& irgs, SrcKey callerSk, SrcKey calleeSk,
+                         const RegionDesc& calleeRegion, IRUnit& calleeUnit);
 
 ///////////////////////////////////////////////////////////////////////////////
 
 }}}
-