[Tarantool-patches] [PATCH v3 02/15] sql: use register P1 for number of arguments

[Mergen Imeev via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

This patch makes OP_FunctionByName, OP_AggStep and OP_BuiltinFunction to
use register P1 for the number of arguments instead of register P5. This
makes it easier to use these opcodes.

Needed for #4145
---
 src/box/sql/expr.c    |  5 ++--
 src/box/sql/select.c  |  3 +-
 src/box/sql/vdbe.c    | 64 ++++++++++++++++++++-----------------------
 src/box/sql/vdbeInt.h |  1 -
 4 files changed, 32 insertions(+), 41 deletions(-)

diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index ee21c1ede..3b5df5292 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -4107,18 +4107,17 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target)
 						  (char *)coll, P4_COLLSEQ);
 			}
 			if (func->def->language == FUNC_LANGUAGE_SQL_BUILTIN) {
-				sqlVdbeAddOp4(v, OP_BuiltinFunction0, constMask,
+				sqlVdbeAddOp4(v, OP_BuiltinFunction0, nFarg,
 					      r1, target, (char *)func,
 					      P4_FUNC);
 			} else {
-				sqlVdbeAddOp4(v, OP_FunctionByName, constMask,
+				sqlVdbeAddOp4(v, OP_FunctionByName, nFarg,
 					      r1, target,
 					      sqlDbStrNDup(pParse->db,
 							   func->def->name,
 							   func->def->name_len),
 					      P4_DYNAMIC);
 			}
-			sqlVdbeChangeP5(v, (u8) nFarg);
 			if (nFarg && constMask == 0) {
 				sqlReleaseTempRange(pParse, r1, nFarg);
 			}
diff --git a/src/box/sql/select.c b/src/box/sql/select.c
index 2fe38e319..92e40aef6 100644
--- a/src/box/sql/select.c
+++ b/src/box/sql/select.c
@@ -5639,9 +5639,8 @@ updateAccumulator(Parse * pParse, AggInfo * pAggInfo)
 			sqlVdbeAddOp4(v, OP_CollSeq, regHit, 0, 0,
 					  (char *)coll, P4_COLLSEQ);
 		}
-		sqlVdbeAddOp3(v, OP_AggStep0, 0, regAgg, pF->iMem);
+		sqlVdbeAddOp3(v, OP_AggStep0, nArg, regAgg, pF->iMem);
 		sqlVdbeAppendP4(v, pF->func, P4_FUNC);
-		sqlVdbeChangeP5(v, (u8) nArg);
 		sql_expr_type_cache_change(pParse, regAgg, nArg);
 		sqlReleaseTempRange(pParse, regAgg, nArg);
 		if (addrNext) {
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 44533fb3e..a0f1be454 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -1182,32 +1182,24 @@ case OP_CollSeq: {
 	break;
 }
 
-/* Opcode: BuiltinFunction0 P1 P2 P3 P4 P5
- * Synopsis: r[P3]=func(r[P2@P5])
+/* Opcode: BuiltinFunction0 P1 P2 P3 P4 *
+ * Synopsis: r[P3]=func(r[P2@P1])
  *
  * Invoke a user function (P4 is a pointer to a FuncDef object that
- * defines the function) with P5 arguments taken from register P2 and
+ * defines the function) with P1 arguments taken from register P2 and
  * successors.  The result of the function is stored in register P3.
  * Register P3 must not be one of the function inputs.
  *
- * P1 is a 32-bit bitmask indicating whether or not each argument to the
- * function was determined to be constant at compile time. If the first
- * argument was constant then bit 0 of P1 is set.
- *
  * See also: BuiltinFunction, AggStep, AggFinal
  */
-/* Opcode: BuiltinFunction P1 P2 P3 P4 P5
- * Synopsis: r[P3]=func(r[P2@P5])
+/* Opcode: BuiltinFunction P1 P2 P3 P4 *
+ * Synopsis: r[P3]=func(r[P2@P1])
  *
  * Invoke a user function (P4 is a pointer to an sql_context object that
- * contains a pointer to the function to be run) with P5 arguments taken
+ * contains a pointer to the function to be run) with P1 arguments taken
  * from register P2 and successors.  The result of the function is stored
  * in register P3.  Register P3 must not be one of the function inputs.
  *
- * P1 is a 32-bit bitmask indicating whether or not each argument to the
- * function was determined to be constant at compile time. If the first
- * argument was constant then bit 0 of P1 is set.
- *
  * SQL functions are initially coded as OP_BuiltinFunction0 with
  * P4 pointing to a FuncDef object.  But on first evaluation,
  * the P4 operand is automatically converted into an sql_context
@@ -1223,7 +1215,7 @@ case OP_BuiltinFunction0: {
 	sql_context *pCtx;
 
 	assert(pOp->p4type == P4_FUNC);
-	n = pOp->p5;
+	n = pOp->p1;
 	assert(pOp->p3>0 && pOp->p3<=(p->nMem+1 - p->nCursor));
 	assert(n==0 || (pOp->p2>0 && pOp->p2+n<=(p->nMem+1 - p->nCursor)+1));
 	assert(pOp->p3<pOp->p2 || pOp->p3>=pOp->p2+n);
@@ -1233,7 +1225,6 @@ case OP_BuiltinFunction0: {
 	pCtx->func = pOp->p4.func;
 	pCtx->iOp = (int)(pOp - aOp);
 	pCtx->pVdbe = p;
-	pCtx->argc = n;
 	pOp->p4type = P4_FUNCCTX;
 	pOp->p4.pCtx = pCtx;
 	pOp->opcode = OP_BuiltinFunction;
@@ -1242,6 +1233,7 @@ case OP_BuiltinFunction0: {
 }
 case OP_BuiltinFunction: {
 	int i;
+	int argc = pOp->p1;
 	sql_context *pCtx;
 
 	assert(pOp->p4type==P4_FUNCCTX);
@@ -1255,11 +1247,12 @@ case OP_BuiltinFunction: {
 	pOut = vdbe_prepare_null_out(p, pOp->p3);
 	if (pCtx->pOut != pOut) {
 		pCtx->pOut = pOut;
-		for(i=pCtx->argc-1; i>=0; i--) pCtx->argv[i] = &aMem[pOp->p2+i];
+		for(i = 0; i < argc; ++i)
+			pCtx->argv[i] = &aMem[pOp->p2 + i];
 	}
 
 #ifdef SQL_DEBUG
-	for(i=0; i<pCtx->argc; i++) {
+	for(i = 0; i < argc; i++) {
 		assert(memIsValid(pCtx->argv[i]));
 		REGISTER_TRACE(p, pOp->p2+i, pCtx->argv[i]);
 	}
@@ -1267,7 +1260,7 @@ case OP_BuiltinFunction: {
 	pCtx->is_aborted = false;
 	assert(pCtx->func->def->language == FUNC_LANGUAGE_SQL_BUILTIN);
 	struct func_sql_builtin *func = (struct func_sql_builtin *)pCtx->func;
-	func->call(pCtx, pCtx->argc, pCtx->argv);
+	func->call(pCtx, argc, pCtx->argv);
 
 	/* If the function returned an error, throw an exception */
 	if (pCtx->is_aborted)
@@ -1283,11 +1276,11 @@ case OP_BuiltinFunction: {
 	break;
 }
 
-/* Opcode: FunctionByName * P2 P3 P4 P5
- * Synopsis: r[P3]=func(r[P2@P5])
+/* Opcode: FunctionByName P1 P2 P3 P4 *
+ * Synopsis: r[P3]=func(r[P2@P1])
  *
  * Invoke a user function (P4 is a pointer to a function object
- * that defines the function) with P5 arguments taken from
+ * that defines the function) with P1 arguments taken from
  * register P2 and successors. The result of the function is
  * stored in register P3.
  */
@@ -1303,7 +1296,7 @@ case OP_FunctionByName: {
 	 * turn out to be invalid after call.
 	 */
 	enum field_type returns = func->def->returns;
-	int argc = pOp->p5;
+	int argc = pOp->p1;
 	struct Mem *argv = &aMem[pOp->p2];
 	struct port args, ret;
 
@@ -4185,26 +4178,26 @@ case OP_DecrJumpZero: {      /* jump, in1 */
 }
 
 
-/* Opcode: AggStep0 * P2 P3 P4 P5
- * Synopsis: accum=r[P3] step(r[P2@P5])
+/* Opcode: AggStep0 P1 P2 P3 P4 *
+ * Synopsis: accum=r[P3] step(r[P2@P1])
  *
  * Execute the step function for an aggregate.  The
- * function has P5 arguments.   P4 is a pointer to the FuncDef
+ * function has P1 arguments.   P4 is a pointer to the FuncDef
  * structure that specifies the function.  Register P3 is the
  * accumulator.
  *
- * The P5 arguments are taken from register P2 and its
+ * The P1 arguments are taken from register P2 and its
  * successors.
  */
-/* Opcode: AggStep * P2 P3 P4 P5
- * Synopsis: accum=r[P3] step(r[P2@P5])
+/* Opcode: AggStep P1 P2 P3 P4 *
+ * Synopsis: accum=r[P3] step(r[P2@P1])
  *
  * Execute the step function for an aggregate.  The
- * function has P5 arguments.   P4 is a pointer to an sql_context
+ * function has P1 arguments.   P4 is a pointer to an sql_context
  * object that is used to run the function.  Register P3 is
  * as the accumulator.
  *
- * The P5 arguments are taken from register P2 and its
+ * The P1 arguments are taken from register P2 and its
  * successors.
  *
  * This opcode is initially coded as OP_AggStep0.  On first evaluation,
@@ -4228,7 +4221,6 @@ case OP_AggStep0: {
 	pCtx->func = pOp->p4.func;
 	pCtx->iOp = (int)(pOp - aOp);
 	pCtx->pVdbe = p;
-	pCtx->argc = n;
 	pOp->p4type = P4_FUNCCTX;
 	pOp->p4.pCtx = pCtx;
 	pOp->opcode = OP_AggStep;
@@ -4237,6 +4229,7 @@ case OP_AggStep0: {
 }
 case OP_AggStep: {
 	int i;
+	int argc = pOp->p1;
 	sql_context *pCtx;
 	Mem *pMem;
 	Mem t;
@@ -4252,11 +4245,12 @@ case OP_AggStep: {
 	 */
 	if (pCtx->pMem != pMem) {
 		pCtx->pMem = pMem;
-		for(i=pCtx->argc-1; i>=0; i--) pCtx->argv[i] = &aMem[pOp->p2+i];
+		for(i = 0; i < argc; ++i)
+			pCtx->argv[i] = &aMem[pOp->p2 + i];
 	}
 
 #ifdef SQL_DEBUG
-	for(i=0; i<pCtx->argc; i++) {
+	for(i = 0; i < argc; i++) {
 		assert(memIsValid(pCtx->argv[i]));
 		REGISTER_TRACE(p, pOp->p2+i, pCtx->argv[i]);
 	}
@@ -4269,7 +4263,7 @@ case OP_AggStep: {
 	pCtx->skipFlag = 0;
 	assert(pCtx->func->def->language == FUNC_LANGUAGE_SQL_BUILTIN);
 	struct func_sql_builtin *func = (struct func_sql_builtin *)pCtx->func;
-	func->call(pCtx, pCtx->argc, pCtx->argv);
+	func->call(pCtx, argc, pCtx->argv);
 	if (pCtx->is_aborted) {
 		mem_destroy(&t);
 		goto abort_due_to_error;
diff --git a/src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h
index cfe743b94..575ab3f3d 100644
--- a/src/box/sql/vdbeInt.h
+++ b/src/box/sql/vdbeInt.h
@@ -182,7 +182,6 @@ struct sql_context {
 	 */
 	bool is_aborted;
 	u8 skipFlag;		/* Skip accumulator loading if true */
-	u8 argc;		/* Number of arguments */
 	sql_value *argv[1];	/* Argument set */
 };
 
-- 
2.25.1