Track IR function of operand to GetClosureScopeInst

Summary: We have to track the IR function of the closure operand so that we can find the `GetClosureScopeInst` by iterating users of the `Function`. This is useful if we want to update the parent of that function for instance. Reviewed By: avp Differential Revision: D66236886 fbshipit-source-id: 393cdd9ff9e12482f380800ac0c6f04891bbf85d
facebook · Feb 5, 2025 · 289d302 · 289d302
1 parent dbb8ada
commit 289d302
Show file tree

Hide file tree

Showing 8 changed files with 36 additions and 11 deletions.
diff --git a/doc/IR.md b/doc/IR.md
@@ -374,8 +374,8 @@ Effects | Does not read or write to memory.
 GetClosureScopeInst | _
 --- | --- |
 Description | Retrieve the scope from the given closure.
-Example | %0 = GetClosureScopeInst %varScope, %closure
-Arguments | %varScope is the VariableScope that describes the resulting scope. %closure is the closure from which to read the scope.
+Example | %0 = GetClosureScopeInst %varScope, %function, %closure
+Arguments | %varScope is the VariableScope that describes the resulting scope. %function is the IR function that the closure operand is known to refer to. %closure is the closure from which to read the scope.
 Semantics | The instruction returns the scope stored in the given closure.
 Effects | Does not read or write to memory.
 

diff --git a/include/hermes/IR/IRBuilder.h b/include/hermes/IR/IRBuilder.h
@@ -327,9 +327,8 @@ class IRBuilder {
       Instruction *startScope,
       LiteralNumber *numLevels);
 
-  GetClosureScopeInst *createGetClosureScopeInst(
-      VariableScope *scope,
-      Value *closure);
+  GetClosureScopeInst *
+  createGetClosureScopeInst(VariableScope *scope, Function *F, Value *closure);
 
   LoadStackInst *createLoadStackInst(AllocStackInst *ptr);
 

diff --git a/include/hermes/IR/Instrs.h b/include/hermes/IR/Instrs.h
@@ -786,21 +786,28 @@ class GetClosureScopeInst : public BaseScopeInst {
   GetClosureScopeInst(const GetClosureScopeInst &) = delete;
   void operator=(const GetClosureScopeInst &) = delete;
 
-  enum { ClosureIdx = BaseScopeInst::LAST_IDX };
+  enum { FunctionCodeIdx = BaseScopeInst::LAST_IDX, ClosureIdx };
 
  public:
-  explicit GetClosureScopeInst(VariableScope *varScope, Value *closure)
+  explicit GetClosureScopeInst(
+      VariableScope *varScope,
+      Function *F,
+      Value *closure)
       : BaseScopeInst(ValueKind::GetClosureScopeInstKind, varScope) {
+    pushOperand(F);
     pushOperand(closure);
   }
   explicit GetClosureScopeInst(
       const GetClosureScopeInst *src,
       llvh::ArrayRef<Value *> operands)
       : BaseScopeInst(src, operands) {}
 
-  Value *getClosure() {
+  Value *getClosure() const {
     return cast<Instruction>(getOperand(ClosureIdx));
   }
+  Function *getFunctionCode() const {
+    return cast<Function>(getOperand(FunctionCodeIdx));
+  }
 
   SideEffect getSideEffectImpl() const {
     return SideEffect{}.setIdempotent();

diff --git a/lib/IR/IRBuilder.cpp b/lib/IR/IRBuilder.cpp
@@ -409,8 +409,9 @@ LIRResolveScopeInst *IRBuilder::createLIRResolveScopeInst(
 
 GetClosureScopeInst *IRBuilder::createGetClosureScopeInst(
     VariableScope *scope,
+    Function *F,
     Value *closure) {
-  auto *GCSI = new GetClosureScopeInst(scope, closure);
+  auto *GCSI = new GetClosureScopeInst(scope, F, closure);
   insert(GCSI);
   return GCSI;
 }

diff --git a/lib/IR/IRVerifier.cpp b/lib/IR/IRVerifier.cpp
@@ -1389,6 +1389,15 @@ bool Verifier::visitLIRResolveScopeInst(
 }
 bool Verifier::visitGetClosureScopeInst(
     const hermes::GetClosureScopeInst &Inst) {
+  for (auto *U : Inst.getFunctionCode()->getUsers()) {
+    if (auto *BCLI = llvh::dyn_cast<BaseCreateLexicalChildInst>(U)) {
+      AssertIWithMsg(
+          Inst,
+          BCLI->getVarScope() == Inst.getVariableScope(),
+          "Scope result does not match function creation");
+      return true;
+    }
+  }
   return true;
 }
 

diff --git a/lib/Optimizer/Scalar/FunctionAnalysis.cpp b/lib/Optimizer/Scalar/FunctionAnalysis.cpp
@@ -378,6 +378,15 @@ void analyzeFunctionCallsites(Function *F) {
       continue;
     }
 
+    if (auto *GCSI = llvh::dyn_cast<GetClosureScopeInst>(user)) {
+      // Ignore uses in GetClosureScopeInst.
+      (void)GCSI;
+      assert(
+          GCSI->getFunctionCode() == F &&
+          "invalid use of Function as operand of GetClosureScopeInst");
+      continue;
+    }
+
     // Unknown user of Function.
     LLVM_DEBUG(
         llvh::dbgs() << "Unknown function user: " << user->getKindStr()

diff --git a/lib/Optimizer/Scalar/Inlining.cpp b/lib/Optimizer/Scalar/Inlining.cpp
@@ -352,7 +352,7 @@ static Value *inlineFunction(
           operandMap[&I] = callScope;
         } else {
           operandMap[&I] = builder.createGetClosureScopeInst(
-              GPS->getVariableScope(), CI->getCallee());
+              GPS->getVariableScope(), F, CI->getCallee());
         }
         continue;
       }

diff --git a/test/Optimizer/inline-unavailable-env.js b/test/Optimizer/inline-unavailable-env.js
@@ -56,7 +56,7 @@ function foo(sink){
 // CHECK-NEXT:%BB1:
 // CHECK-NEXT:        ThrowTypeErrorInst "Trying to call a non-function": string
 // CHECK-NEXT:%BB2:
-// CHECK-NEXT:  %11 = GetClosureScopeInst (:environment) %VS2: any, %7: undefined|object
+// CHECK-NEXT:  %11 = GetClosureScopeInst (:environment) %VS2: any, %x(): functionCode, %7: undefined|object
 // CHECK-NEXT:  %12 = LoadFrameInst (:number) %11: environment, [%VS2.y]: number
 // CHECK-NEXT:  %13 = FAddInst (:number) %12: number, 1: number
 // CHECK-NEXT:        StoreFrameInst %11: environment, %13: number, [%VS2.y]: number