Remove BeginInlinedFunction and EndInlinedFunction in trivial cases

Summary:
When a function is just returning a constant, for example, we
can get rid of the shadow frame bookkeeping. It is not needed for deopt
or otherwise reifying frames. Such a function will likely not even show
up in profiles, so don't bother keeping a shadow frame around for it.

Reviewed By: swtaarrs

Differential Revision: D35374703

fbshipit-source-id: 3806bab

Jit/compiler.cpp

@@ -103,6 +103,7 @@ void Compiler::runPasses(
   if (_PyJIT_IsHIRInlinerEnabled()) {
     runPass<jit::hir::InlineFunctionCalls>(irfunc, callback);
     runPass<jit::hir::Simplify>(irfunc, callback);
+    runPass<jit::hir::BeginInlinedFunctionElimination>(irfunc, callback);
   }
   runPass<jit::hir::DeadCodeElimination>(irfunc, callback);
   runPass<jit::hir::RefcountInsertion>(irfunc, callback);

Jit/hir/hir.h

@@ -2080,14 +2080,20 @@ class INSTR_CLASS(BeginInlinedFunction, (), Operands<0>), public InlineBase {
 
 class INSTR_CLASS(EndInlinedFunction, (), Operands<0>), public InlineBase {
  public:
-  EndInlinedFunction(int inline_depth) : InstrT(), inline_depth(inline_depth) {}
+  EndInlinedFunction(BeginInlinedFunction * begin)
+      : InstrT(), begin_(begin), inline_depth_(begin->inlineDepth()) {}
+
+  BeginInlinedFunction* matchingBegin() const {
+    return begin_;
+  }
 
   int inlineDepth() const {
-    return inline_depth;
+    return inline_depth_;
   }
 
  private:
-  int inline_depth{-1};
+  BeginInlinedFunction* begin_{nullptr};
+  int inline_depth_{-1};
 };
 
 enum class PrimitiveUnaryOpKind {

Jit/hir/optimization.cpp

@@ -40,6 +40,7 @@ PassRegistry::PassRegistry() {
   addPass(Simplify::Factory);
   addPass(DeadCodeElimination::Factory);
   addPass(GuardTypeRemoval::Factory);
+  addPass(BeginInlinedFunctionElimination::Factory);
 }
 
 std::unique_ptr<Pass> PassRegistry::MakePass(const std::string& name) {
@@ -898,8 +899,7 @@ void inlineFunctionCall(Function& caller, AbstractCall* call_instr) {
   } else {
     call_instr->instr->ExpandInto({begin_inlined_function, callee_branch});
   }
-  tail->push_front(
-      EndInlinedFunction::create(begin_inlined_function->inlineDepth()));
+  tail->push_front(EndInlinedFunction::create(begin_inlined_function));
 
   // Transform LoadArg into Assign
   for (auto it = result.entry->begin(); it != result.entry->end();) {
@@ -982,5 +982,49 @@ void InlineFunctionCalls::Run(Function& irfunc) {
   CleanCFG{}.Run(irfunc);
 }
 
+static void tryEliminateBeginEnd(EndInlinedFunction* end) {
+  BeginInlinedFunction* begin = end->matchingBegin();
+  if (begin->block() != end->block()) {
+    // TODO(emacs): Support elimination across basic blocks
+    return;
+  }
+  auto it = begin->block()->iterator_to(*begin);
+  it++;
+  std::vector<Instr*> to_delete{begin, end};
+  for (; &*it != end; it++) {
+    // Snapshots reference the FrameState owned by BeginInlinedFunction and, if
+    // not removed, will contain bad pointers.
+    if (it->IsSnapshot()) {
+      to_delete.push_back(&*it);
+      continue;
+    }
+    // Instructions that either deopt or otherwise materialize a PyFrameObject
+    // need the shadow frames to exist. Everything that materializes a
+    // PyFrameObject should also be marked as deopting.
+    if (dynamic_cast<DeoptBase*>(&*it)) {
+      return;
+    }
+  }
+  for (Instr* instr : to_delete) {
+    instr->unlink();
+    delete instr;
+  }
+}
+
+void BeginInlinedFunctionElimination::Run(Function& irfunc) {
+  std::vector<EndInlinedFunction*> ends;
+  for (auto& block : irfunc.cfg.blocks) {
+    for (auto& instr : block) {
+      if (!instr.IsEndInlinedFunction()) {
+        continue;
+      }
+      ends.push_back(static_cast<EndInlinedFunction*>(&instr));
+    }
+  }
+  for (EndInlinedFunction* end : ends) {
+    tryEliminateBeginEnd(end);
+  }
+}
+
 } // namespace hir
 } // namespace jit

Jit/hir/optimization.h

@@ -188,6 +188,17 @@ class InlineFunctionCalls : public Pass {
   }
 };
 
+class BeginInlinedFunctionElimination : public Pass {
+ public:
+  BeginInlinedFunctionElimination() : Pass("BeginInlinedFunctionElimination") {}
+
+  void Run(Function& irfunc) override;
+
+  static std::unique_ptr<BeginInlinedFunctionElimination> Factory() {
+    return std::make_unique<BeginInlinedFunctionElimination>();
+  }
+};
+
 class PassRegistry {
  public:
   PassRegistry();

RuntimeTests/hir_tests/inliner_elimination_static_test.txt

@@ -0,0 +1,107 @@
+InlinerEliminationStaticTest
+---
+InlineFunctionCalls
+Simplify
+BeginInlinedFunctionElimination
+---
+Simple
+---
+def foo():
+    # Chosen by fair dice roll. Guaranteed to be random.
+    return 4
+
+def test():
+    return foo()
+---
+fun jittestmodule:test {
+  bb 0 {
+    v5:MortalLongExact[4] = LoadConst<MortalLongExact[4]>
+    Return v5
+  }
+}
+---
+InlineMultipleFunctions
+---
+def foo():
+    return 3
+
+def bar():
+    return 4
+
+def test():
+    return foo() + bar()
+---
+fun jittestmodule:test {
+  bb 0 {
+    v10:MortalLongExact[3] = LoadConst<MortalLongExact[3]>
+    v14:MortalLongExact[4] = LoadConst<MortalLongExact[4]>
+    UseType<LongExact> v10
+    UseType<LongExact> v14
+    v16:LongExact = LongBinaryOp<Add> v10 v14 {
+      FrameState {
+        NextInstrOffset 12
+      }
+    }
+    Return v16
+  }
+}
+---
+BeginEndWithMemoryEffectsNotRemoved
+---
+def add(x, y):
+  return x + y
+
+def test():
+    return add(3, 4)
+---
+fun jittestmodule:test {
+  bb 0 {
+    v4:MortalLongExact[3] = LoadConst<MortalLongExact[3]>
+    v5:MortalLongExact[4] = LoadConst<MortalLongExact[4]>
+    BeginInlinedFunction<jittestmodule:add> {
+      NextInstrOffset 10
+    }
+    UseType<LongExact> v4
+    UseType<LongExact> v5
+    v17:LongExact = LongBinaryOp<Add> v4 v5 {
+      FrameState {
+        NextInstrOffset 8
+        Locals<2> v4 v5
+      }
+    }
+    EndInlinedFunction
+    Return v17
+  }
+}
+---
+InlinedStoreFieldLoadFieldIsEliminated
+---
+from __static__ import int8
+
+class C:
+  def __init__(self):
+    self.foo: int8 = 4
+
+  def getfoo(self) -> int8:
+    return self.foo
+
+def test() -> int8:
+    return C().getfoo()
+---
+fun jittestmodule:test {
+  bb 0 {
+    v5:ObjectUser[C:Exact] = TpAlloc<C> {
+      FrameState {
+        NextInstrOffset 4
+      }
+    }
+    v15:CInt8[4] = LoadConst<CInt8[4]>
+    v17:Nullptr = LoadConst<Nullptr>
+    UseType<CInt8> v15
+    StoreField<foo@16> v5 v15 v17
+    v19:NoneType = LoadConst<NoneType>
+    v26:CInt8 = LoadField<foo@16, CInt8, borrowed> v5
+    Return<CInt8> v26
+  }
+}
+---

RuntimeTests/hir_tests/inliner_elimination_test.txt

@@ -0,0 +1,104 @@
+InlinerEliminationTest
+---
+InlineFunctionCalls
+Simplify
+BeginInlinedFunctionElimination
+---
+Simple
+---
+def foo():
+    # Chosen by fair dice roll. Guaranteed to be random.
+    return 4
+
+def test():
+    return foo()
+---
+fun jittestmodule:test {
+  bb 0 {
+    v2:OptObject = LoadGlobalCached<0; "foo">
+    v3:MortalFunc[function:0xdeadbeef] = GuardIs<0xdeadbeef> v2 {
+      Descr 'LOAD_GLOBAL: foo'
+    }
+    v9:Object = LoadField<func_code@16, Object, borrowed> v3
+    v10:MortalCode["foo"] = GuardIs<0xdeadbeef> v9 {
+    }
+    v7:MortalLongExact[4] = LoadConst<MortalLongExact[4]>
+    Return v7
+  }
+}
+---
+InlineMultipleFunctions
+---
+def foo():
+    return 3
+
+def bar():
+    return 4
+
+def test():
+    return foo() + bar()
+---
+fun jittestmodule:test {
+  bb 0 {
+    v5:OptObject = LoadGlobalCached<0; "foo">
+    v6:MortalFunc[function:0xdeadbeef] = GuardIs<0xdeadbeef> v5 {
+      Descr 'LOAD_GLOBAL: foo'
+    }
+    v16:Object = LoadField<func_code@16, Object, borrowed> v6
+    v17:MortalCode["foo"] = GuardIs<0xdeadbeef> v16 {
+    }
+    v14:MortalLongExact[3] = LoadConst<MortalLongExact[3]>
+    v8:OptObject = LoadGlobalCached<1; "bar">
+    v9:MortalFunc[function:0xdeadbeef] = GuardIs<0xdeadbeef> v8 {
+      Descr 'LOAD_GLOBAL: bar'
+    }
+    v22:Object = LoadField<func_code@16, Object, borrowed> v9
+    v23:MortalCode["bar"] = GuardIs<0xdeadbeef> v22 {
+    }
+    v20:MortalLongExact[4] = LoadConst<MortalLongExact[4]>
+    UseType<LongExact> v14
+    UseType<LongExact> v20
+    v24:LongExact = LongBinaryOp<Add> v14 v20 {
+      FrameState {
+        NextInstrOffset 10
+      }
+    }
+    Return v24
+  }
+}
+---
+BeginEndWithMemoryEffectsNotRemoved
+---
+def add(x, y):
+  return x + y
+
+def test():
+    return add(3, 4)
+---
+fun jittestmodule:test {
+  bb 0 {
+    v4:OptObject = LoadGlobalCached<0; "add">
+    v5:MortalFunc[function:0xdeadbeef] = GuardIs<0xdeadbeef> v4 {
+      Descr 'LOAD_GLOBAL: add'
+    }
+    v6:MortalLongExact[3] = LoadConst<MortalLongExact[3]>
+    v7:MortalLongExact[4] = LoadConst<MortalLongExact[4]>
+    v19:Object = LoadField<func_code@16, Object, borrowed> v5
+    v20:MortalCode["add"] = GuardIs<0xdeadbeef> v19 {
+    }
+    BeginInlinedFunction<jittestmodule:add> {
+      NextInstrOffset 8
+    }
+    UseType<LongExact> v6
+    UseType<LongExact> v7
+    v21:LongExact = LongBinaryOp<Add> v6 v7 {
+      FrameState {
+        NextInstrOffset 6
+        Locals<2> v6 v7
+      }
+    }
+    EndInlinedFunction
+    Return v21
+  }
+}
+---

RuntimeTests/main.cpp

@@ -108,6 +108,10 @@ int main(int argc, char* argv[]) {
   register_test(
       "RuntimeTests/hir_tests/inliner_static_test.txt",
       HIRTest::kCompileStatic);
+  register_test("RuntimeTests/hir_tests/inliner_elimination_test.txt");
+  register_test(
+      "RuntimeTests/hir_tests/inliner_elimination_static_test.txt",
+      HIRTest::kCompileStatic);
   register_test("RuntimeTests/hir_tests/phi_elimination_test.txt");
   register_test("RuntimeTests/hir_tests/refcount_insertion_test.txt");
   register_test(