From 46e70d34554a40a23ff2f68b3b560d5ea5fe06d1 Mon Sep 17 00:00:00 2001
From: Michael Leon <fbmal7@meta.com>
Date: Wed, 5 Mar 2025 13:18:47 -0800
Subject: [PATCH] Emit DefineOwnById in bytecode backend

Summary:
In BC lowering, keep the LiteralString intact for
`DefineOwnPropertyInst`. This ensures we can now use the new ById
variant in ISel.

Reviewed By: neildhar

Differential Revision: D67109052

fbshipit-source-id: d01fb6e4c091589bfb2fe64e9c8f18b6752d2680
---
 lib/BCGen/HBC/BytecodeGenerator.cpp |  1 +
 lib/BCGen/HBC/ISel.cpp              | 40 +++++++++++++++++++++--------
 lib/BCGen/HBC/Passes.cpp            |  6 +++++
 3 files changed, 37 insertions(+), 10 deletions(-)

diff --git a/lib/BCGen/HBC/BytecodeGenerator.cpp b/lib/BCGen/HBC/BytecodeGenerator.cpp
index fed9cfc4611..744cac58908 100644
--- a/lib/BCGen/HBC/BytecodeGenerator.cpp
+++ b/lib/BCGen/HBC/BytecodeGenerator.cpp
@@ -287,6 +287,7 @@ static bool isIdOperand(const Instruction *I, unsigned idx) {
     CASE_WITH_PROP_IDX(LoadPropertyInst);
     CASE_WITH_PROP_IDX(LoadPropertyWithReceiverInst);
     CASE_WITH_PROP_IDX(DefineNewOwnPropertyInst);
+    CASE_WITH_PROP_IDX(DefineOwnPropertyInst);
     CASE_WITH_PROP_IDX(StorePropertyLooseInst);
     CASE_WITH_PROP_IDX(StorePropertyStrictInst);
     CASE_WITH_PROP_IDX(TryLoadGlobalPropertyInst);
diff --git a/lib/BCGen/HBC/ISel.cpp b/lib/BCGen/HBC/ISel.cpp
index 05a8963bd11..f440a2e43d2 100644
--- a/lib/BCGen/HBC/ISel.cpp
+++ b/lib/BCGen/HBC/ISel.cpp
@@ -927,23 +927,43 @@ void HBCISel::generateDefineOwnPropertyInst(
   auto valueReg = encodeValue(Inst->getStoredValue());
   auto objReg = encodeValue(Inst->getObject());
   Value *prop = Inst->getProperty();
-  bool isEnumerable = Inst->getIsEnumerable();
 
   // If the property is a LiteralNumber, the property is enumerable, and it is a
   // valid array index, it is coming from an array initialization and we will
   // emit it as DefineOwnByIndex.
   auto *numProp = llvh::dyn_cast<LiteralNumber>(prop);
-  if (numProp && isEnumerable) {
-    if (auto arrayIndex = numProp->convertToArrayIndex()) {
-      uint32_t index = arrayIndex.getValue();
-      if (index <= UINT8_MAX) {
-        BCFGen_->emitDefineOwnByIndex(objReg, valueReg, index);
-      } else {
-        BCFGen_->emitDefineOwnByIndexL(objReg, valueReg, index);
-      }
+  if (numProp) {
+    assert(
+        Inst->getIsEnumerable() &&
+        "Non-enumerable properties with literal keys should be handled by LoadConstants");
+    uint32_t index = numProp->convertToArrayIndex().getValue();
+    if (index <= UINT8_MAX) {
+      BCFGen_->emitDefineOwnByIndex(objReg, valueReg, index);
+    } else {
+      BCFGen_->emitDefineOwnByIndexL(objReg, valueReg, index);
+    }
+    return;
+  }
 
-      return;
+  if (auto *Lit = llvh::dyn_cast<LiteralString>(prop)) {
+    assert(
+        Inst->getIsEnumerable() &&
+        "Non-enumerable properties with literal keys should be handled by LoadConstants");
+    auto id = BCFGen_->getIdentifierID(Lit);
+    if (id <= UINT16_MAX) {
+      BCFGen_->emitDefineOwnById(
+          objReg,
+          valueReg,
+          acquirePropertyWriteCacheIndex(Lit->getValue()),
+          id);
+    } else {
+      BCFGen_->emitDefineOwnByIdLong(
+          objReg,
+          valueReg,
+          acquirePropertyWriteCacheIndex(Lit->getValue()),
+          id);
     }
+    return;
   }
 
   // It is a register operand.
diff --git a/lib/BCGen/HBC/Passes.cpp b/lib/BCGen/HBC/Passes.cpp
index a8afc1c6bd3..77d609a1032 100644
--- a/lib/BCGen/HBC/Passes.cpp
+++ b/lib/BCGen/HBC/Passes.cpp
@@ -99,6 +99,12 @@ bool LoadConstants::operandMustBeLiteral(Instruction *Inst, unsigned opIndex) {
         if (SOP->getIsEnumerable() && LN->convertToArrayIndex().hasValue())
           return true;
       }
+
+      // LiteralStrings are optimized, when they are enumerable.
+      if (llvh::isa<LiteralString>(Inst->getOperand(opIndex)) &&
+          SOP->getIsEnumerable()) {
+        return true;
+      }
     }
 
     // DefineOwnPropertyInst's isEnumerable is a boolean constant.