Use PyTuple_FromArray for creation of tuple objects

Summary:
A new test has arrived in 3.14.1 and 3.15 which tests that tuples which don't have any GC objects are not GC tracked. Currently our allocations are always tracking them.

This replaces the kBatchDecref LIR instruction with a generic VarArgCall instruction which takes a PyObject **args, Py_ssize_t length. We use this for both BatchDecref and make tuple.

Reviewed By: alexmalyshev

Differential Revision: D86136959

fbshipit-source-id: 7337539b91c90c0b407a50377fb5293d2f6a2f9a

Author: DinoV

cinderx/Jit/hir/instr_effects.cpp

@@ -205,15 +205,14 @@ MemoryEffects memoryEffects(const Instr& inst) {
       return commonEffects(inst, AOther);
 
     case Opcode::kMakeCheckedList:
-    case Opcode::kMakeList:
-    case Opcode::kMakeTuple: {
+    case Opcode::kMakeList: {
       // Steal all inputs.
       util::BitVector inputs{inst.NumOperands()};
       inputs.fill(true);
-      auto may_store =
-          inst.opcode() == Opcode::kMakeTuple ? ATupleItem : AListItem;
-      return {false, AEmpty, std::move(inputs), may_store};
+      return {false, AEmpty, std::move(inputs), AListItem};
     }
+    case Opcode::kMakeTuple:
+      return commonEffects(inst, ATupleItem);
 
     case Opcode::kStoreField:
       JIT_DCHECK(inst.NumOperands() == 3, "Unexpected number of operands");

cinderx/Jit/lir/generator.cpp

@@ -1769,7 +1769,8 @@ LIRGenerator::TranslatedBlock LIRGenerator::TranslateOneBasicBlock(
       case Opcode::kBatchDecref: {
         auto instr = static_cast<const BatchDecref*>(&i);
 
-        Instruction* lir = bbb.appendInstr(Instruction::kBatchDecref);
+        Instruction* lir = bbb.appendInstr(Instruction::kVarArgCall);
+        lir->addOperands(Imm{reinterpret_cast<uint64_t>(JITRT_BatchDecref)});
         for (hir::Register* arg : instr->GetOperands()) {
           lir->addOperands(VReg{bbb.getDefInstr(arg)});
         }
@@ -2264,22 +2265,18 @@ LIRGenerator::TranslatedBlock LIRGenerator::TranslateOneBasicBlock(
       }
       case Opcode::kMakeTuple: {
         auto instr = static_cast<const MakeTuple*>(&i);
-        Instruction* tuple = bbb.appendCallInstruction(
-            instr->output(),
-            PyTuple_New,
-            static_cast<Py_ssize_t>(instr->nvalues()));
-        // TODO(T174544781): need to check for 0 before initializing, currently
-        // that check only happens after assigning these values.
-        const size_t ob_item_offset = offsetof(PyTupleObject, ob_item);
-        for (size_t operandIdx = 0; operandIdx < instr->NumOperands();
-             operandIdx++) {
-          bbb.appendInstr(
-              OutInd{
-                  tuple,
-                  static_cast<int32_t>(
-                      ob_item_offset + operandIdx * kPointerSize)},
-              Instruction::kMove,
-              instr->GetOperand(operandIdx));
+        Instruction* tuple =
+            bbb.appendInstr(instr->output(), Instruction::kVarArgCall);
+        tuple->addOperands(
+            Imm{reinterpret_cast<uint64_t>(
+#if PY_VERSION_HEX >= 0x030F0000
+                PyTuple_FromArray
+#else
+                _PyTuple_FromArray
+#endif
+                )});
+        for (size_t ix = 0; ix < instr->NumOperands(); ix++) {
+          tuple->addOperands(VReg{bbb.getDefInstr(instr->GetOperand(ix))});
         }
         break;
       }

cinderx/Jit/lir/instruction.h

@@ -90,6 +90,7 @@ enum OperandSizeType {
   X(Unreachable, false, FlagEffects::kNone, kDefault, 0, {}, 1)               \
   X(Call, false, FlagEffects::kInvalidate, kAlways64, 1, {}, 1)               \
   X(VectorCall, false, FlagEffects::kInvalidate, kAlways64, 1, {1}, 1)        \
+  X(VarArgCall, false, FlagEffects::kInvalidate, kDefault, 1, {1})            \
   X(Guard, false, FlagEffects::kInvalidate, kDefault, 1, {0, 0, 1, 1}, 1)     \
   X(DeoptPatchpoint, false, FlagEffects::kInvalidate, kDefault, 0, {1, 1}, 1) \
   X(Sext)                                                                     \
@@ -134,7 +135,6 @@ enum OperandSizeType {
   X(Cdq, false, FlagEffects::kNone, kDefault, 1, {}, 1)                       \
   X(Cwd, false, FlagEffects::kNone, kDefault, 1, {}, 1)                       \
   X(Cqo, false, FlagEffects::kNone, kDefault, 1, {}, 1)                       \
-  X(BatchDecref, false, FlagEffects::kInvalidate, kDefault, 1, {1})           \
   X(Branch)                                                                   \
   X(BranchNZ)                                                                 \
   X(BranchZ)                                                                  \

cinderx/Jit/lir/operand.h

@@ -375,5 +375,3 @@ DECLARE_TYPE_ARG(OutInd, MemoryIndirect, true)
 DECLARE_TYPE_ARG(OutVReg, void, true)
 
 } // namespace jit::lir
-
-#define FUNC_MARKER_BATCHDECREF (void*)0x0001

cinderx/Jit/lir/postalloc.cpp

@@ -251,19 +251,18 @@ int rewriteVectorCallFunctions(instr_iter_t instr_iter) {
   return rsp_sub;
 }
 
-int rewriteBatchDecrefFunction(instr_iter_t instr_iter) {
+int rewriteVarArgCall(instr_iter_t instr_iter) {
   auto instr = instr_iter->get();
-  static_cast<Operand*>(instr->getInput(0))
-      ->setConstant(
-          reinterpret_cast<uint64_t>(JITRT_BatchDecref), Operand::k64bit);
-
-  return prepareArgsArray(
+  instr->setOpcode(Instruction::kCall);
+  auto res = prepareArgsArray(
       instr_iter,
       instr->getNumInputs() - 1, // func is 1st argument
       0,
       1,
       ARGUMENT_REGS[0],
       ARGUMENT_REGS[1]);
+  instr->setNumInputs(1);
+  return res;
 }
 
 // rewrite call instructions:
@@ -272,7 +271,11 @@ int rewriteBatchDecrefFunction(instr_iter_t instr_iter) {
 //   JITRT_(Call|Get)Method, etc.
 RewriteResult rewriteCallInstrs(instr_iter_t instr_iter, Environ* env) {
   auto instr = instr_iter->get();
-  if (!instr->isCall() && !instr->isVectorCall()) {
+  if (instr->isVarArgCall()) {
+    int rsp_sub = rewriteVarArgCall(instr_iter);
+    env->max_arg_buffer_size = std::max<int>(env->max_arg_buffer_size, rsp_sub);
+    return kChanged;
+  } else if (!instr->isCall() && !instr->isVectorCall()) {
     return kUnchanged;
   }
 
@@ -286,13 +289,6 @@ RewriteResult rewriteCallInstrs(instr_iter_t instr_iter, Environ* env) {
 
   if (instr->isVectorCall()) {
     rsp_sub = rewriteVectorCallFunctions(instr_iter);
-  } else if (instr->getInput(0)->isImm()) {
-    void* func = reinterpret_cast<void*>(instr->getInput(0)->getConstant());
-    if (func == FUNC_MARKER_BATCHDECREF) {
-      rsp_sub = rewriteBatchDecrefFunction(instr_iter);
-    } else {
-      rsp_sub = rewriteRegularFunction(instr_iter);
-    }
   } else {
     rsp_sub = rewriteRegularFunction(instr_iter);
   }

cinderx/Jit/lir/postgen.cpp

@@ -228,24 +228,6 @@ RewriteResult rewriteLoadArg(instr_iter_t instr_iter, Environ* env) {
   return kChanged;
 }
 
-RewriteResult rewriteBatchDecrefInstrs(instr_iter_t instr_iter) {
-  auto instr = instr_iter->get();
-  if (!instr->isBatchDecref()) {
-    return kUnchanged;
-  }
-
-  // we translate BatchDecref by converting it to a Call instruction
-  instr->setOpcode(Instruction::kCall);
-
-  instr->prependInput(
-      std::make_unique<Operand>(
-          nullptr,
-          Operand::k64bit,
-          Operand::kImm,
-          reinterpret_cast<uint64_t>(FUNC_MARKER_BATCHDECREF)));
-  return kChanged;
-}
-
 void populateLoadSecondCallResultPhi(
     OperandBase::DataType data_type,
     Instruction* phi1,
@@ -363,7 +345,6 @@ RewriteResult rewriteLoadSecondCallResult(instr_iter_t instr_iter) {
 void PostGenerationRewrite::registerRewrites() {
   // rewriteInlineHelper should occur before other rewrites.
   registerOneRewriteFunction(rewriteInlineHelper, 0);
-  registerOneRewriteFunction(rewriteBatchDecrefInstrs, 0);
 
   registerOneRewriteFunction(rewriteBinaryOpConstantPosition, 1);
   registerOneRewriteFunction(rewriteBinaryOpLargeConstant, 1);

cinderx/Jit/lir/regalloc.cpp

@@ -495,6 +495,7 @@ void LinearScanAllocator::calculateLiveIntervals() {
       }
 
       if (instr_opcode == Instruction::kCall ||
+          instr_opcode == Instruction::kVarArgCall ||
           instr_opcode == Instruction::kVectorCall) {
         reserveCallerSaveRegisters(instr_id);
       }
@@ -1165,6 +1166,7 @@ void LinearScanAllocator::rewriteInstrOutput(
   // TODO: Fix HIR generator to avoid generating unused output/variables.
   // Need a separate pass in HIR to handle the dead code more gracefully.
   if (instr->opcode() == Instruction::kCall ||
+      instr->opcode() == Instruction::kVarArgCall ||
       instr->opcode() == Instruction::kVectorCall) {
     output->setNone();
   } else {

cinderx/RuntimeTests/hir_tests/refcount_insertion_test.txt

@@ -1937,9 +1937,6 @@ def test(a):
 fun jittestmodule:test {
   bb 0 {
     v2:Object = LoadArg<0; "a">
-    Incref v2
-    Incref v2
-    Incref v2
     v6:MortalTupleExact = MakeTuple<3> v2 v2 v2 {
       LiveValues<1> b:v2
       FrameState {
@@ -1971,9 +1968,6 @@ fun jittestmodule:test {
   }
 
   bb 1 (preds 0, 2) {
-    Incref v5
-    Incref v5
-    Incref v5
     v10:MortalTupleExact = MakeTuple<3> v5 v5 v5 {
       LiveValues<1> b:v5
       FrameState {
@@ -2005,9 +1999,6 @@ fun jittestmodule:test {
   }
 
   bb 1 (preds 0, 2) {
-    Incref v5
-    Incref v5
-    Incref v5
     v10:MortalTupleExact = MakeTuple<3> v5 v5 v5 {
       LiveValues<1> b:v5
       FrameState {
@@ -2253,7 +2244,6 @@ fun jittestmodule:test {
     XDecref v18
     Decref v19
     Decref v26
-    Incref v14
     v20:MortalTupleExact = MakeTuple<1> v14 {
       LiveValues<2> unc:v13 o:v14
       FrameState {
@@ -2335,7 +2325,6 @@ fun jittestmodule:test {
     XDecref v26
     Decref v27
     Decref v33
-    Incref v17
     v28:MortalTupleExact = MakeTuple<1> v17 {
       LiveValues<2> unc:v16 o:v17
       FrameState {
@@ -2415,7 +2404,6 @@ fun jittestmodule:test {
     XDecref v26
     Decref v27
     Decref v33
-    Incref v17
     v28:MortalTupleExact = MakeTuple<1> v17 {
       LiveValues<2> unc:v16 o:v17
       FrameState {