Rename StoreNewOwnPropertyInst -> DefineNewOwnPropertyInst

Summary: Name the store instructions more accurately.

Reviewed By: neildhar

Differential Revision: D67061812

fbshipit-source-id: 60907d8ffaf880f26b3069d646d8407f7977430e

doc/IR.md

@@ -527,12 +527,12 @@ Arguments | %value is the value to be stored. %object *must* be of an object typ
 Semantics | Implements ES15 7.3.8 DefinePropertyOrThrow. The instruction follows the rules of JavaScript *own* property access. The property is created or updated in the instance of the object, regardless of whether the same property already exists earlier in the prototype chain.
 Effects | May read and write memory.
 
-### StoreNewOwnPropertyInst
+### DefineNewOwnPropertyInst
 
-StoreNewOwnPropertyInst | _
+DefineNewOwnPropertyInst | _
 --- | --- |
 Description | Create a new *own property* in what is known to be a JavaScript object.
-Example |   `%4 = StoreNewOwnPropertyInst %value, %object, %property, %enumerable : boolean`
+Example |   `%4 = DefineNewOwnPropertyInst %value, %object, %property, %enumerable : boolean`
 Arguments | *%value* is the value to be stored. *%object*, which must be an object, is where the field with name *%property* will be created. *%property* must be a string or index-like number literal, otherwise it is impossible to guarantee that it is new. *%enumerable* determines whether the new property will be created as enumerable or not.
 Semantics | The instruction follows the rules of JavaScript *own* property access. The property is created in the instance of the object, regardless of whether the same property already exists earlier in the prototype chain.
 Effects | May read and write memory.

include/hermes/IR/IRBuilder.h

@@ -446,7 +446,7 @@ class IRBuilder {
       Value *object,
       Value *property,
       PropEnumerable isEnumerable);
-  StoreNewOwnPropertyInst *createStoreNewOwnPropertyInst(
+  DefineNewOwnPropertyInst *createDefineNewOwnPropertyInst(
       Value *storedValue,
       Value *object,
       Literal *property,

include/hermes/IR/Instrs.def

@@ -109,7 +109,7 @@ MARK_LAST(BaseStorePropertyInst)
 
 MARK_FIRST(BaseDefineOwnPropertyInst, Instruction)
 DEF_VALUE(DefineOwnPropertyInst, BaseDefineOwnPropertyInst)
-DEF_VALUE(StoreNewOwnPropertyInst, BaseDefineOwnPropertyInst)
+DEF_VALUE(DefineNewOwnPropertyInst, BaseDefineOwnPropertyInst)
 MARK_LAST(BaseDefineOwnPropertyInst)
 
 DEF_VALUE(DefineOwnGetterSetterInst, Instruction)

include/hermes/IR/Instrs.h

@@ -1741,18 +1741,18 @@ class DefineOwnPropertyInst : public BaseDefineOwnPropertyInst {
   }
 };
 
-class StoreNewOwnPropertyInst : public BaseDefineOwnPropertyInst {
-  StoreNewOwnPropertyInst(const StoreNewOwnPropertyInst &) = delete;
-  void operator=(const StoreNewOwnPropertyInst &) = delete;
+class DefineNewOwnPropertyInst : public BaseDefineOwnPropertyInst {
+  DefineNewOwnPropertyInst(const DefineNewOwnPropertyInst &) = delete;
+  void operator=(const DefineNewOwnPropertyInst &) = delete;
 
  public:
-  explicit StoreNewOwnPropertyInst(
+  explicit DefineNewOwnPropertyInst(
       Value *storedValue,
       Value *object,
       Literal *property,
       LiteralBool *isEnumerable)
       : BaseDefineOwnPropertyInst(
-            ValueKind::StoreNewOwnPropertyInstKind,
+            ValueKind::DefineNewOwnPropertyInstKind,
             storedValue,
             object,
             property,
@@ -1766,14 +1766,14 @@ class StoreNewOwnPropertyInst : public BaseDefineOwnPropertyInst {
         "object operand must be known to be an object");
   }
 
-  explicit StoreNewOwnPropertyInst(
-      const StoreNewOwnPropertyInst *src,
+  explicit DefineNewOwnPropertyInst(
+      const DefineNewOwnPropertyInst *src,
       llvh::ArrayRef<Value *> operands)
       : BaseDefineOwnPropertyInst(src, operands) {}
 
   static bool classof(const Value *V) {
     ValueKind kind = V->getKind();
-    return kind == ValueKind::StoreNewOwnPropertyInstKind;
+    return kind == ValueKind::DefineNewOwnPropertyInstKind;
   }
 };
 

include/hermes/Optimizer/PassManager/Passes.def

@@ -46,7 +46,7 @@ PASS(
 PASS(
     ObjectMergeNewStores,
     "objectmergenewstores",
-    "Merge StoreNewOwnPropertyInsts into AllocObjectLiteral")
+    "Merge DefineNewOwnPropertyInsts into AllocObjectLiteral")
 PASS(SimplifyCFG, "simplifycfg", "Simplify CFG")
 PASS(TypeInference, "typeinference", "Type inference")
 PASS(FunctionAnalysis, "functionanalysis", "Function analysis")

lib/BCGen/HBC/BytecodeGenerator.cpp

@@ -281,7 +281,7 @@ static bool isIdOperand(const Instruction *I, unsigned idx) {
     CASE_WITH_PROP_IDX(DeletePropertyLooseInst);
     CASE_WITH_PROP_IDX(DeletePropertyStrictInst);
     CASE_WITH_PROP_IDX(LoadPropertyInst);
-    CASE_WITH_PROP_IDX(StoreNewOwnPropertyInst);
+    CASE_WITH_PROP_IDX(DefineNewOwnPropertyInst);
     CASE_WITH_PROP_IDX(StorePropertyLooseInst);
     CASE_WITH_PROP_IDX(StorePropertyStrictInst);
     CASE_WITH_PROP_IDX(TryLoadGlobalPropertyInst);

lib/BCGen/HBC/ISel.cpp

@@ -931,8 +931,8 @@ void HBCISel::generateDefineOwnPropertyInst(
   BCFGen_->emitPutOwnByVal(objReg, valueReg, propReg, Inst->getIsEnumerable());
 }
 
-void HBCISel::generateStoreNewOwnPropertyInst(
-    StoreNewOwnPropertyInst *Inst,
+void HBCISel::generateDefineNewOwnPropertyInst(
+    DefineNewOwnPropertyInst *Inst,
     BasicBlock *next) {
   auto valueReg = encodeValue(Inst->getStoredValue());
   auto objReg = encodeValue(Inst->getObject());
@@ -942,7 +942,7 @@ void HBCISel::generateStoreNewOwnPropertyInst(
   if (auto *numProp = llvh::dyn_cast<LiteralNumber>(prop)) {
     assert(
         isEnumerable &&
-        "No way to generate non-enumerable indexed StoreNewOwnPropertyInst.");
+        "No way to generate non-enumerable indexed DefineNewOwnPropertyInst.");
     uint32_t index = *numProp->convertToArrayIndex();
     if (index <= UINT8_MAX) {
       BCFGen_->emitPutOwnByIndex(objReg, valueReg, index);

lib/BCGen/HBC/LoweringPipelines.cpp

@@ -44,7 +44,7 @@ void lowerModuleIR(Module *M, const BytecodeGenerationOptions &options) {
   // as LowerNumericProperties could generate new constants.
   PM.addPass(new LowerNumericProperties());
   // Lower AllocObjectLiteral into a mixture of HBCAllocObjectFromBufferInst,
-  // AllocObjectInst, StoreNewOwnPropertyInst and StorePropertyInst.
+  // AllocObjectInst, DefineNewOwnPropertyInst and StorePropertyInst.
   PM.addPass(new LowerAllocObjectLiteral());
   PM.addPass(new LowerArgumentsArray());
   PM.addPass(new LimitAllocArray(UINT16_MAX));

lib/BCGen/HBC/Passes.cpp

@@ -84,11 +84,11 @@ bool LoadConstants::operandMustBeLiteral(Instruction *Inst, unsigned opIndex) {
   if (llvh::isa<SwitchInst>(Inst) && opIndex > 0)
     return true;
 
-  // DefineOwnPropertyInst and StoreNewOwnPropertyInst.
+  // DefineOwnPropertyInst and DefineNewOwnPropertyInst.
   if (auto *SOP = llvh::dyn_cast<BaseDefineOwnPropertyInst>(Inst)) {
     if (opIndex == DefineOwnPropertyInst::PropertyIdx) {
-      if (llvh::isa<StoreNewOwnPropertyInst>(Inst)) {
-        // In StoreNewOwnPropertyInst the property name must be a literal.
+      if (llvh::isa<DefineNewOwnPropertyInst>(Inst)) {
+        // In DefineNewOwnPropertyInst the property name must be a literal.
         return true;
       }
 

lib/BCGen/SH/LoadConstants.cpp

@@ -41,11 +41,11 @@ bool operandMustBeLiteral(Instruction *Inst, unsigned opIndex) {
   if (llvh::isa<SwitchInst>(Inst) && opIndex > 0)
     return true;
 
-  // DefineOwnPropertyInst and StoreNewOwnPropertyInst.
+  // DefineOwnPropertyInst and DefineNewOwnPropertyInst.
   if (auto *SOP = llvh::dyn_cast<BaseDefineOwnPropertyInst>(Inst)) {
     if (opIndex == DefineOwnPropertyInst::PropertyIdx) {
-      if (llvh::isa<StoreNewOwnPropertyInst>(Inst)) {
-        // In StoreNewOwnPropertyInst the property name must be a literal.
+      if (llvh::isa<DefineNewOwnPropertyInst>(Inst)) {
+        // In DefineNewOwnPropertyInst the property name must be a literal.
         return true;
       }
 

lib/BCGen/SH/SH.cpp

@@ -1238,15 +1238,15 @@ class InstrGen {
     generateRegisterPtr(*inst.getStoredValue());
     os_ << ");\n";
   }
-  void generateStoreNewOwnPropertyInst(StoreNewOwnPropertyInst &inst) {
+  void generateDefineNewOwnPropertyInst(DefineNewOwnPropertyInst &inst) {
     os_.indent(2);
     auto prop = inst.getProperty();
     bool isEnumerable = inst.getIsEnumerable();
 
     if (auto *numProp = llvh::dyn_cast<LiteralNumber>(prop)) {
       assert(
           isEnumerable &&
-          "No way to generate non-enumerable indexed StoreNewOwnPropertyInst.");
+          "No way to generate non-enumerable indexed DefineNewOwnPropertyInst.");
       uint32_t index = *numProp->convertToArrayIndex();
       os_ << "_sh_ljs_put_own_by_index(";
       os_ << "shr, ";

lib/IR/IRBuilder.cpp

@@ -622,12 +622,12 @@ DefineOwnPropertyInst *IRBuilder::createDefineOwnPropertyInst(
   insert(SPI);
   return SPI;
 }
-StoreNewOwnPropertyInst *IRBuilder::createStoreNewOwnPropertyInst(
+DefineNewOwnPropertyInst *IRBuilder::createDefineNewOwnPropertyInst(
     Value *storedValue,
     Value *object,
     Literal *property,
     PropEnumerable isEnumerable) {
-  auto *inst = new StoreNewOwnPropertyInst(
+  auto *inst = new DefineNewOwnPropertyInst(
       storedValue,
       object,
       property,

lib/IR/IRVerifier.cpp

@@ -974,23 +974,23 @@ bool Verifier::visitBaseDefineOwnPropertyInst(
 bool Verifier::visitDefineOwnPropertyInst(const DefineOwnPropertyInst &Inst) {
   return visitBaseDefineOwnPropertyInst(Inst);
 }
-bool Verifier::visitStoreNewOwnPropertyInst(
-    const StoreNewOwnPropertyInst &Inst) {
+bool Verifier::visitDefineNewOwnPropertyInst(
+    const DefineNewOwnPropertyInst &Inst) {
   ReturnIfNot(visitBaseDefineOwnPropertyInst(Inst));
   AssertIWithMsg(
       Inst,
       Inst.getObject()->getType().isObjectType(),
-      "StoreNewOwnPropertyInst::Object must be known to be an object");
+      "DefineNewOwnPropertyInst::Object must be known to be an object");
   if (auto *LN = llvh::dyn_cast<LiteralNumber>(Inst.getProperty())) {
     AssertIWithMsg(
         Inst,
         LN->convertToArrayIndex().hasValue(),
-        "StoreNewOwnPropertyInst::Property can only be an index-like number");
+        "DefineNewOwnPropertyInst::Property can only be an index-like number");
   } else {
     AssertIWithMsg(
         Inst,
         llvh::isa<LiteralString>(Inst.getProperty()),
-        "StoreNewOwnPropertyInst::Property must be a string or number literal");
+        "DefineNewOwnPropertyInst::Property must be a string or number literal");
   }
   return true;
 }

lib/IRGen/ESTreeIRGen-expr.cpp

@@ -1464,7 +1464,7 @@ Value *ESTreeIRGen::genObjectExpr(ESTree::ObjectExpressionNode *Expr) {
 
   // haveSeenComputedProp tracks whether we have processed a computed property.
   // Once we do, for all future properties, we can no longer generate
-  // StoreNewOwnPropertyInst because the computed property could have already
+  // DefineNewOwnPropertyInst because the computed property could have already
   // defined any property.
   bool haveSeenComputedProp = false;
 
@@ -1564,7 +1564,7 @@ Value *ESTreeIRGen::genObjectExpr(ESTree::ObjectExpressionNode *Expr) {
               Key,
               IRBuilder::PropEnumerable::Yes);
         } else {
-          Builder.createStoreNewOwnPropertyInst(
+          Builder.createDefineNewOwnPropertyInst(
               Builder.getLiteralNull(),
               Obj,
               Key,
@@ -1629,7 +1629,7 @@ Value *ESTreeIRGen::genObjectExpr(ESTree::ObjectExpressionNode *Expr) {
         Builder.createDefineOwnPropertyInst(
             value, Obj, Key, IRBuilder::PropEnumerable::Yes);
       } else {
-        Builder.createStoreNewOwnPropertyInst(
+        Builder.createDefineNewOwnPropertyInst(
             value, Obj, Key, IRBuilder::PropEnumerable::Yes);
       }
       propValue->state = PropertyValue::IRGenerated;
@@ -1690,7 +1690,7 @@ Value *ESTreeIRGen::genTypedObjectExpr(
         "Missing stored value in typechecked object literal");
     // Use a literal if possible, otherwise use the default init value.
     // Avoid putting non-literals here because we want to emit PrStore
-    // instead of StoreNewOwnPropertyInst.
+    // instead of DefineNewOwnPropertyInst.
     Value *initValue = nullptr;
     if (llvh::isa<Literal>(it->second.first)) {
       initValue = it->second.first;

lib/IRGen/ESTreeIRGen.cpp

@@ -1066,7 +1066,7 @@ void ESTreeIRGen::emitRestProperty(
         Builder.createAllocObjectLiteralInst({}, Builder.getLiteralNull());
 
     for (Literal *key : literalExcludedItems)
-      Builder.createStoreNewOwnPropertyInst(
+      Builder.createDefineNewOwnPropertyInst(
           zeroValue, excludedObj, key, IRBuilder::PropEnumerable::Yes);
 
     for (Value *key : computedExcludedItems) {

lib/Optimizer/Scalar/ObjectMergeNewStores.cpp

@@ -8,7 +8,7 @@
 //===----------------------------------------------------------------------===//
 /// \file
 ///
-/// This optimization collects StoreNewOwnPropertyInsts and merges them into a
+/// This optimization collects DefineNewOwnPropertyInsts and merges them into a
 /// single AllocObjectLiteral.
 //===----------------------------------------------------------------------===//
 
@@ -25,8 +25,8 @@
 namespace hermes {
 namespace {
 
-/// Define a type for managing lists of StoreNewOwnPropertyInsts.
-using StoreList = llvh::SmallVector<StoreNewOwnPropertyInst *, 4>;
+/// Define a type for managing lists of DefineNewOwnPropertyInsts.
+using StoreList = llvh::SmallVector<DefineNewOwnPropertyInst *, 4>;
 /// Define a type for mapping a given basic block to the stores to a given
 /// AllocObjectLiteralInst in that basic block.
 using BlockUserMap = llvh::DenseMap<BasicBlock *, StoreList>;
@@ -42,11 +42,11 @@ StoreList collectStores(
       DI, allocInst->getParent(), [&userBasicBlockMap](BasicBlock *BB) {
         return userBasicBlockMap.find(BB) != userBasicBlockMap.end();
       });
-  // Iterate over the sorted blocks to collect StoreNewOwnPropertyInst users
+  // Iterate over the sorted blocks to collect DefineNewOwnPropertyInst users
   // until we encounter a nullptr indicating we should stop.
   StoreList instrs;
   for (BasicBlock *BB : sortedBlocks) {
-    for (StoreNewOwnPropertyInst *I : userBasicBlockMap.find(BB)->second) {
+    for (DefineNewOwnPropertyInst *I : userBasicBlockMap.find(BB)->second) {
       // If I is null, we cannot consider additional stores.
       if (!I)
         return instrs;
@@ -56,7 +56,7 @@ StoreList collectStores(
   return instrs;
 }
 
-/// Merge StoreNewOwnPropertyInsts into a single AllocObjectLiteralInst.
+/// Merge DefineNewOwnPropertyInsts into a single AllocObjectLiteralInst.
 /// Non-literal values are set with placeholders and later patched with the
 /// correct value.
 /// \p allocInst the instruction to transform
@@ -78,7 +78,7 @@ bool mergeStoresToObjectLiteral(
   // Keep track of if we have encountered a numeric key yet.
   bool hasSeenNumericKey = false;
   for (uint32_t i = 0; i < size; ++i) {
-    StoreNewOwnPropertyInst *I = users[i];
+    DefineNewOwnPropertyInst *I = users[i];
     Literal *propKey = llvh::cast<Literal>(I->getProperty());
     auto *propVal = I->getStoredValue();
     hasSeenNumericKey |= llvh::isa<LiteralNumber>(propKey);
@@ -124,24 +124,26 @@ bool mergeNewStores(Function *F) {
   auto tryAdd =
       [](AllocObjectLiteralInst *A, Instruction *U, StoreList &stores) {
         // If the store list has been terminated by a nullptr, we have already
-        // encountered a non-SNOP user of A in this block. Ignore this user.
+        // encountered a non-define new user of A in this block. Ignore this
+        // user.
         if (!stores.empty() && !stores.back())
           return;
-        auto *SI = llvh::dyn_cast<StoreNewOwnPropertyInst>(U);
-        if (!SI || SI->getStoredValue() == A || !SI->getIsEnumerable()) {
-          // A user that's not a StoreNewOwnPropertyInst storing into the object
-          // created by allocInst. We have to stop processing here. Note that we
-          // check the stored value instead of the target object so that we omit
-          // the case where an object is stored into itself. While it should
-          // technically be safe, this maintains the invariant that stop as soon
-          // the allocated object is used as something other than the target of
-          // a StoreNewOwnPropertyInst.
+        auto *newDef = llvh::dyn_cast<DefineNewOwnPropertyInst>(U);
+        if (!newDef || newDef->getStoredValue() == A ||
+            !newDef->getIsEnumerable()) {
+          // A user that's not a DefineNewOwnPropertyInst storing into the
+          // object created by allocInst. We have to stop processing here. Note
+          // that we check the stored value instead of the target object so that
+          // we omit the case where an object is stored into itself. While it
+          // should technically be safe, this maintains the invariant that stop
+          // as soon the allocated object is used as something other than the
+          // target of a DefineNewOwnPropertyInst.
           stores.push_back(nullptr);
         } else {
           assert(
-              SI->getObject() == A &&
-              "SNOP using allocInst must use it as object or value");
-          stores.push_back(SI);
+              newDef->getObject() == A &&
+              "DefineNew using allocInst must use it as object or value");
+          stores.push_back(newDef);
         }
       };
 
@@ -152,8 +154,8 @@ bool mergeNewStores(Function *F) {
     for (Instruction &I : BB) {
       for (size_t i = 0; i < I.getNumOperands(); ++i) {
         if (auto *A = llvh::dyn_cast<AllocObjectLiteralInst>(I.getOperand(i))) {
-          // For now, we only consider merging StoreNewOwnPropertyInsts that are
-          // writing into an empty object to start.
+          // For now, we only consider merging DefineNewOwnPropertyInsts that
+          // are writing into an empty object to start.
           if (A->getKeyValuePairCount() == 0)
             tryAdd(A, &I, allocUsers[A][&BB]);
         }

lib/Optimizer/Scalar/TypeInference.cpp

@@ -565,7 +565,7 @@ class TypeInferenceImpl {
   Type inferDefineOwnPropertyInst(DefineOwnPropertyInst *inst) {
     return Type::createNoType();
   }
-  Type inferStoreNewOwnPropertyInst(StoreNewOwnPropertyInst *inst) {
+  Type inferDefineNewOwnPropertyInst(DefineNewOwnPropertyInst *inst) {
     return Type::createNoType();
   }
 

test/BCGen/HBC/hbc_object_literals-lowering.js

@@ -117,7 +117,7 @@ function accessorObjectLiteral(func) {
 // IRGEN-NEXT:  %4 = HBCLoadConstInst (:string) "c": string
 // IRGEN-NEXT:       DefineOwnGetterSetterInst %2: object, %3: undefined, %0: object, %4: string, true: boolean
 // IRGEN-NEXT:  %6 = HBCLoadConstInst (:null) null: null
-// IRGEN-NEXT:       StoreNewOwnPropertyInst %6: null, %0: object, "d": string, true: boolean
+// IRGEN-NEXT:       DefineNewOwnPropertyInst %6: null, %0: object, "d": string, true: boolean
 // IRGEN-NEXT:       ReturnInst %0: object
 // IRGEN-NEXT:function_end