Fix simplifyGetConstructedObjectInst

Summary:
We need to make sure that when replacing `GetConstructedObjectInst`, the
value we are replacing it with is also an object type. It may not have
been inferred yet.

Reviewed By: avp

Differential Revision: D66516220

fbshipit-source-id: 4046d0d0147fdbdb15a169b70ef4f4c5abb0078b

lib/Optimizer/Scalar/InstSimplify.cpp

@@ -655,7 +655,13 @@ class InstSimplifyImpl {
     auto opTy = GCOI->getConstructorReturnValue()->getType();
     if (opTy.isObjectType())
       return GCOI->getConstructorReturnValue();
-    if (!opTy.canBeObject())
+    // Ideally, all we would need to check is that opTy cannot be an object.
+    // However, there might be cases where we have inferred the return type of
+    // ConstructorReturnValue, and have not yet inferred that ThisValue is an
+    // object. In that case, it would be invalid to replace
+    // GetConstructedObject, which is of type object, with ThisValue, which is
+    // not of type object.
+    if (!opTy.canBeObject() && GCOI->getThisValue()->getType().isObjectType())
       return GCOI->getThisValue();
     return nullptr;
   }

test/Optimizer/callee.js

@@ -82,7 +82,8 @@ function load_store_multiple_test() {
 // CHECK-NEXT:  %1 = CreateFunctionInst (:object) %0: environment, %"foo 1#"(): functionCode
 // CHECK-NEXT:  %2 = CreateThisInst (:undefined|object) %1: object, empty: any
 // CHECK-NEXT:  %3 = CallInst (:number) %1: object, %"foo 1#"(): functionCode, true: boolean, empty: any, undefined: undefined, 0: number, 12: number
-// CHECK-NEXT:       ReturnInst %2: undefined|object
+// CHECK-NEXT:  %4 = GetConstructedObjectInst (:object) %2: undefined|object, 12: number
+// CHECK-NEXT:       ReturnInst %4: object
 // CHECK-NEXT:function_end
 
 // CHECK:scope %VS1 [k: object]

test/Optimizer/constructor_callee.js

@@ -89,5 +89,6 @@ function ctor_load_store_test() {
 // CHECK-NEXT:  %1 = LoadFrameInst (:object) %0: environment, [%VS1.use_this]: object
 // CHECK-NEXT:  %2 = CreateThisInst (:undefined|object) %1: object, empty: any
 // CHECK-NEXT:  %3 = CallInst (:undefined) %1: object, %"use_this 1#"(): functionCode, true: boolean, empty: any, undefined: undefined, %2: undefined|object, 12: number
-// CHECK-NEXT:       ReturnInst %2: undefined|object
+// CHECK-NEXT:  %4 = GetConstructedObjectInst (:object) %2: undefined|object, undefined: undefined
+// CHECK-NEXT:       ReturnInst %4: object
 // CHECK-NEXT:function_end

test/Optimizer/function-analysis-construction-store.js

@@ -35,7 +35,8 @@ function main() {
 // CHECK-NEXT:  %1 = CreateFunctionInst (:object) %0: environment, %f(): functionCode
 // CHECK-NEXT:  %2 = CreateThisInst (:undefined|object) %1: object, empty: any
 // CHECK-NEXT:  %3 = CallInst (:undefined) %1: object, %f(): functionCode, true: boolean, empty: any, undefined: undefined, 0: number
-// CHECK-NEXT:       ReturnInst %2: undefined|object
+// CHECK-NEXT:  %4 = GetConstructedObjectInst (:object) %2: undefined|object, undefined: undefined
+// CHECK-NEXT:       ReturnInst %4: object
 // CHECK-NEXT:function_end
 
 // CHECK:function f(): undefined [allCallsitesKnownInStrictMode]

test/Optimizer/function-analysis-construction.js

@@ -34,7 +34,8 @@ function main() {
 // CHECK-NEXT:  %1 = CreateFunctionInst (:object) %0: environment, %f(): functionCode
 // CHECK-NEXT:  %2 = CreateThisInst (:undefined|object) %1: object, empty: any
 // CHECK-NEXT:  %3 = CallInst (:undefined) %1: object, %f(): functionCode, true: boolean, empty: any, undefined: undefined, 0: number
-// CHECK-NEXT:       ReturnInst %2: undefined|object
+// CHECK-NEXT:  %4 = GetConstructedObjectInst (:object) %2: undefined|object, undefined: undefined
+// CHECK-NEXT:       ReturnInst %4: object
 // CHECK-NEXT:function_end
 
 // CHECK:function f(): undefined [allCallsitesKnownInStrictMode]

test/Optimizer/function-analysis-new-target.js

@@ -37,7 +37,8 @@ function main() {
 // CHECK-NEXT:  %1 = CreateFunctionInst (:object) %0: environment, %f(): functionCode
 // CHECK-NEXT:  %2 = CreateThisInst (:undefined|object) %1: object, empty: any
 // CHECK-NEXT:  %3 = CallInst (:undefined) %1: object, %f(): functionCode, true: boolean, empty: any, %1: object, %2: undefined|object
-// CHECK-NEXT:       ReturnInst %2: undefined|object
+// CHECK-NEXT:  %4 = GetConstructedObjectInst (:object) %2: undefined|object, undefined: undefined
+// CHECK-NEXT:       ReturnInst %4: object
 // CHECK-NEXT:function_end
 
 // CHECK:function f(): undefined

test/Optimizer/inline-sibling-constructors.js

@@ -61,5 +61,6 @@ function outer() {
 // CHECK-NEXT:       StorePropertyStrictInst %1: any, %5: undefined|object, "x": string
 // CHECK-NEXT:       StorePropertyStrictInst %2: any, %5: undefined|object, "y": string
 // CHECK-NEXT:       StorePropertyStrictInst %3: any, %5: undefined|object, "z": string
-// CHECK-NEXT:       ReturnInst %5: undefined|object
+// CHECK-NEXT:  %9 = GetConstructedObjectInst (:object) %5: undefined|object, undefined: undefined
+// CHECK-NEXT:        ReturnInst %9: object
 // CHECK-NEXT:function_end

test/Optimizer/regress-construct.js

@@ -0,0 +1,68 @@
+/**
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+// RUN: %hermesc %s -dump-ir | %FileCheckOrRegen --match-full-lines %s
+
+// Check that we don't replace GetConstructedObjectInst with a
+// value which does not also have an object type.
+//
+// IRVerifier error: StoreFrameInst: Value type mismatch in function
+
+function main() {
+  var x = null;
+  function y() {}
+  return function value() {
+    return x || (x = new y());
+  };
+}
+
+// Auto-generated content below. Please do not modify manually.
+
+// CHECK:scope %VS0 []
+
+// CHECK:function global(): undefined
+// CHECK-NEXT:%BB0:
+// CHECK-NEXT:  %0 = CreateScopeInst (:environment) %VS0: any, empty: any
+// CHECK-NEXT:       DeclareGlobalVarInst "main": string
+// CHECK-NEXT:  %2 = CreateFunctionInst (:object) %0: environment, %main(): functionCode
+// CHECK-NEXT:       StorePropertyLooseInst %2: object, globalObject: object, "main": string
+// CHECK-NEXT:       ReturnInst undefined: undefined
+// CHECK-NEXT:function_end
+
+// CHECK:scope %VS1 [x: null|object, y: object]
+
+// CHECK:function main(): object
+// CHECK-NEXT:%BB0:
+// CHECK-NEXT:  %0 = GetParentScopeInst (:environment) %VS0: any, %parentScope: environment
+// CHECK-NEXT:  %1 = CreateScopeInst (:environment) %VS1: any, %0: environment
+// CHECK-NEXT:  %2 = CreateFunctionInst (:object) %1: environment, %y(): functionCode
+// CHECK-NEXT:       StoreFrameInst %1: environment, %2: object, [%VS1.y]: object
+// CHECK-NEXT:       StoreFrameInst %1: environment, null: null, [%VS1.x]: null|object
+// CHECK-NEXT:  %5 = CreateFunctionInst (:object) %1: environment, %value(): functionCode
+// CHECK-NEXT:       ReturnInst %5: object
+// CHECK-NEXT:function_end
+
+// CHECK:function y(): undefined [allCallsitesKnownInStrictMode,unreachable]
+// CHECK-NEXT:%BB0:
+// CHECK-NEXT:       UnreachableInst
+// CHECK-NEXT:function_end
+
+// CHECK:function value(): null|object
+// CHECK-NEXT:%BB0:
+// CHECK-NEXT:  %0 = GetParentScopeInst (:environment) %VS1: any, %parentScope: environment
+// CHECK-NEXT:  %1 = LoadFrameInst (:null|object) %0: environment, [%VS1.x]: null|object
+// CHECK-NEXT:       CondBranchInst %1: null|object, %BB2, %BB1
+// CHECK-NEXT:%BB1:
+// CHECK-NEXT:  %3 = LoadFrameInst (:object) %0: environment, [%VS1.y]: object
+// CHECK-NEXT:  %4 = CreateThisInst (:undefined|object) %3: object, empty: any
+// CHECK-NEXT:  %5 = GetConstructedObjectInst (:object) %4: undefined|object, undefined: undefined
+// CHECK-NEXT:       StoreFrameInst %0: environment, %5: object, [%VS1.x]: null|object
+// CHECK-NEXT:       BranchInst %BB2
+// CHECK-NEXT:%BB2:
+// CHECK-NEXT:  %8 = PhiInst (:null|object) %1: null|object, %BB0, %5: object, %BB1
+// CHECK-NEXT:       ReturnInst %8: null|object
+// CHECK-NEXT:function_end