v1-0001-jit-Eliminate-const-pointer-to-fmgrinfo.patch

From dcee28d76d28a396cc6e925af61690afecd9c516 Mon Sep 17 00:00:00 2001
From: soumyadeep2007 <sochakraborty@pivotal.io>
Date: Tue, 3 Mar 2020 09:25:52 -0800
Subject: [PATCH v1 1/1] jit: Eliminate const pointer to fmgrinfo

---
 src/backend/executor/execExpr.c     | 96 ++++++++++++++++++++++-------
 src/backend/jit/llvm/llvmjit_expr.c | 84 +++++++++++++++++++++----
 src/include/executor/execExpr.h     |  7 +++
 src/include/jit/llvmjit.h           |  6 +-
 4 files changed, 156 insertions(+), 37 deletions(-)

diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c
index 4e10f9aea8..a1ac44133c 100644
--- a/src/backend/executor/execExpr.c
+++ b/src/backend/executor/execExpr.c
@@ -149,8 +149,23 @@ ExprBuilderAllocNullableDatumArrayInit(ExprStateBuilder *esb, int nelems, Nullab
 	return (RelNullableDatumArray){alloc->ptr};
 }
 
+static RelFunctionMgrInfo
+ExprBuilderAllocFunctionMgrInfo(ExprStateBuilder *esb, FmgrInfo **ptr)
+{
+	Size sz = sizeof(FmgrInfo);
+	ExprStateAllocation *alloc;
+
+	alloc = ExprBuilderAllocInternal(esb, ERP_FUNCTIONMGRINFO, sz);
+
+	alloc->zeroed = true;
+	alloc->initial_content = palloc0(sz);
+	*ptr = alloc->initial_content;
+
+	return (RelFunctionMgrInfo){alloc->ptr};
+}
+
 static RelFunctionCallInfo
-ExprBuilderAllocFunctionCallInfo(ExprStateBuilder *esb, int nargs, FunctionCallInfo *ptr)
+ExprBuilderAllocFunctionCallInfo(ExprStateBuilder *esb, int nargs, RelFunctionMgrInfo finfoRel, FunctionCallInfo *ptr)
 {
 	ExprStateAllocation *alloc;
 	Size sz = SizeForFunctionCallInfo(nargs);
@@ -161,7 +176,7 @@ ExprBuilderAllocFunctionCallInfo(ExprStateBuilder *esb, int nargs, FunctionCallI
 	alloc->initial_content = palloc0(sz);
 	*ptr = alloc->initial_content;
 
-	return (RelFunctionCallInfo){alloc->ptr};
+	return (RelFunctionCallInfo){alloc->ptr, finfoRel.ptr};
 }
 
 static RelNullableDatum
@@ -1148,6 +1163,7 @@ ExecInitExprRec(ExprStateBuilder *esb, Expr *node, RelNullableDatum result)
 				ScalarArrayOpExpr *opexpr = (ScalarArrayOpExpr *) node;
 				Expr	   *scalararg;
 				Expr	   *arrayarg;
+				RelFunctionMgrInfo finfo_rel;
 				FmgrInfo   *finfo;
 				RelFunctionCallInfo fcinfo_rel;
 				FunctionCallInfo fcinfo;
@@ -1167,8 +1183,11 @@ ExecInitExprRec(ExprStateBuilder *esb, Expr *node, RelNullableDatum result)
 				InvokeFunctionExecuteHook(opexpr->opfuncid);
 
 				/* Set up the primary fmgr lookup information */
-				finfo = palloc0(sizeof(FmgrInfo));
-				fcinfo_rel = ExprBuilderAllocFunctionCallInfo(esb, 2, &fcinfo);
+				finfo_rel = ExprBuilderAllocFunctionMgrInfo(esb, &finfo);
+				fcinfo_rel = ExprBuilderAllocFunctionCallInfo(esb,
+															  2,
+															  finfo_rel,
+															  &fcinfo);
 
 				fmgr_info(opexpr->opfuncid, finfo);
 				fmgr_info_set_expr((Node *) node, finfo);
@@ -1441,8 +1460,10 @@ ExecInitExprRec(ExprStateBuilder *esb, Expr *node, RelNullableDatum result)
 				Oid			typioparam;
 				FunctionCallInfo fcinfo_in;
 				FunctionCallInfo fcinfo_out;
-				FmgrInfo   *finfo_in = palloc0(sizeof(FmgrInfo));
-				FmgrInfo   *finfo_out = palloc0(sizeof(FmgrInfo));
+				RelFunctionMgrInfo finfo_in_rel;
+				FmgrInfo   *finfo_in;
+				RelFunctionMgrInfo finfo_out_rel;
+				FmgrInfo   *finfo_out;
 
 				/* evaluate argument into step's result area */
 				ExecInitExprRec(esb, iocoerce->arg, result);
@@ -1456,10 +1477,13 @@ ExecInitExprRec(ExprStateBuilder *esb, Expr *node, RelNullableDatum result)
 				 * function are assumed to be executable by everyone.
 				 */
 				scratch.opcode = EEOP_IOCOERCE;
-
 				/* lookup the source type's output function */
+				finfo_out_rel = ExprBuilderAllocFunctionMgrInfo(esb, &finfo_out);
 				scratch.d.iocoerce.fcinfo_data_out =
-					ExprBuilderAllocFunctionCallInfo(esb, 1, &fcinfo_out);
+					ExprBuilderAllocFunctionCallInfo(esb,
+													 1,
+													 finfo_out_rel,
+													 &fcinfo_out);
 
 				getTypeOutputInfo(exprType((Node *) iocoerce->arg),
 								  &iofunc, &typisvarlena);
@@ -1470,8 +1494,12 @@ ExecInitExprRec(ExprStateBuilder *esb, Expr *node, RelNullableDatum result)
 				scratch.d.iocoerce.fn_addr_out = finfo_out->fn_addr;
 
 				/* lookup the result type's input function */
+				finfo_in_rel = ExprBuilderAllocFunctionMgrInfo(esb, &finfo_in);
 				scratch.d.iocoerce.fcinfo_data_in =
-					ExprBuilderAllocFunctionCallInfo(esb, 3, &fcinfo_in);
+					ExprBuilderAllocFunctionCallInfo(esb,
+													 3,
+													 finfo_in_rel,
+													 &fcinfo_in);
 
 				getTypeInputInfo(iocoerce->resulttype,
 								 &iofunc, &typioparam);
@@ -1888,6 +1916,7 @@ ExecInitExprRec(ExprStateBuilder *esb, Expr *node, RelNullableDatum result)
 					Oid			lefttype;
 					Oid			righttype;
 					Oid			proc;
+					RelFunctionMgrInfo finfo_rel;
 					FmgrInfo   *finfo;
 					RelFunctionCallInfo fcinfo_rel;
 					FunctionCallInfo fcinfo;
@@ -1905,9 +1934,12 @@ ExecInitExprRec(ExprStateBuilder *esb, Expr *node, RelNullableDatum result)
 							 BTORDER_PROC, lefttype, righttype, opfamily);
 
 					/* Set up the primary fmgr lookup information */
-					finfo = palloc0(sizeof(FmgrInfo));
+					finfo_rel = ExprBuilderAllocFunctionMgrInfo(esb, &finfo);
 					fcinfo_rel =
-						ExprBuilderAllocFunctionCallInfo(esb, 3, &fcinfo);
+						ExprBuilderAllocFunctionCallInfo(esb,
+														 3,
+														 finfo_rel,
+														 &fcinfo);
 
 					fmgr_info(proc, finfo);
 					fmgr_info_set_expr((Node *) node, finfo);
@@ -2026,6 +2058,7 @@ ExecInitExprRec(ExprStateBuilder *esb, Expr *node, RelNullableDatum result)
 				MinMaxExpr *minmaxexpr = (MinMaxExpr *) node;
 				int			nelems = list_length(minmaxexpr->args);
 				TypeCacheEntry *typentry;
+				RelFunctionMgrInfo finfo_rel;
 				FmgrInfo   *finfo;
 				RelFunctionCallInfo fcinfo_rel;
 				FunctionCallInfo fcinfo;
@@ -2049,9 +2082,12 @@ ExecInitExprRec(ExprStateBuilder *esb, Expr *node, RelNullableDatum result)
 				 */
 
 				/* Perform function lookup */
-				finfo = palloc0(sizeof(FmgrInfo));
+				finfo_rel = ExprBuilderAllocFunctionMgrInfo(esb, &finfo);
 				fcinfo_rel =
-					ExprBuilderAllocFunctionCallInfo(esb, 2, &fcinfo);
+					ExprBuilderAllocFunctionCallInfo(esb,
+													 2,
+													 finfo_rel,
+													 &fcinfo);
 				fmgr_info(typentry->cmp_proc, finfo);
 				fmgr_info_set_expr((Node *) node, finfo);
 				InitFunctionCallInfoData(*fcinfo, finfo, 2,
@@ -2327,6 +2363,7 @@ ExecInitFunc(ExprEvalStep *scratch, Expr *node, List *args, Oid funcid,
 {
 	int			nargs = list_length(args);
 	AclResult	aclresult;
+	RelFunctionMgrInfo flinfo_rel;
 	FmgrInfo   *flinfo;
 	RelFunctionCallInfo fcinfo_rel;
 	FunctionCallInfo fcinfo;
@@ -2354,8 +2391,11 @@ ExecInitFunc(ExprEvalStep *scratch, Expr *node, List *args, Oid funcid,
 							   FUNC_MAX_ARGS)));
 
 	/* Allocate function lookup data and parameter workspace for this call */
-	flinfo = palloc0(sizeof(FmgrInfo));
-	fcinfo_rel = ExprBuilderAllocFunctionCallInfo(esb, nargs, &fcinfo);
+	flinfo_rel = ExprBuilderAllocFunctionMgrInfo(esb, &flinfo);
+	fcinfo_rel = ExprBuilderAllocFunctionCallInfo(esb,
+												  nargs,
+												  flinfo_rel,
+												  &fcinfo);
 	scratch->d.func.fcinfo_data = fcinfo_rel;
 
 	/* Set up the primary fmgr lookup information */
@@ -3145,6 +3185,8 @@ ExecBuildAggTrans(AggState *aggstate, AggStatePerPhase phase,
 	for (int transno = 0; transno < aggstate->numtrans; transno++)
 	{
 		AggStatePerTrans pertrans = &aggstate->pertrans[transno];
+		RelFunctionMgrInfo trans_finfo_rel;
+		FmgrInfo *trans_finfo;
 		RelFunctionCallInfo trans_fcinfo_rel;
 		FunctionCallInfo trans_fcinfo;
 		List	   *adjust_bailout = NIL;
@@ -3154,13 +3196,17 @@ ExecBuildAggTrans(AggState *aggstate, AggStatePerPhase phase,
 		ListCell   *bail;
 		size_t		numTransArgs;
 
+		trans_finfo_rel = ExprBuilderAllocFunctionMgrInfo(&esb, &trans_finfo);
+		*trans_finfo = pertrans->transfn;
 		/* account for the current transition state */
 		numTransArgs = pertrans->numTransInputs + 1;
 		trans_fcinfo_rel =
-			ExprBuilderAllocFunctionCallInfo(&esb, numTransArgs,
+			ExprBuilderAllocFunctionCallInfo(&esb,
+											 numTransArgs,
+											 trans_finfo_rel,
 											 &trans_fcinfo);
 		InitFunctionCallInfoData(*trans_fcinfo,
-								 &pertrans->transfn,
+								 trans_finfo,
 								 numTransArgs,
 								 pertrans->aggCollation,
 								 NULL, /* will get set when calling */
@@ -3218,6 +3264,8 @@ ExecBuildAggTrans(AggState *aggstate, AggStatePerPhase phase,
 			}
 			else
 			{
+				RelFunctionMgrInfo ds_finfo_rel;
+				FmgrInfo *ds_finfo;
 				FunctionCallInfo ds_fcinfo;
 				RelFunctionCallInfo ds_fcinfo_rel;
 				AggStatePerCallContext *percall;
@@ -3226,12 +3274,17 @@ ExecBuildAggTrans(AggState *aggstate, AggStatePerPhase phase,
 				percall->aggstate = aggstate;
 				percall->pertrans = pertrans;
 
+				ds_finfo_rel = ExprBuilderAllocFunctionMgrInfo(&esb, &ds_finfo);
+				*ds_finfo = pertrans->deserialfn;
+
 				ds_fcinfo_rel =
-					ExprBuilderAllocFunctionCallInfo(&esb, numTransArgs,
+					ExprBuilderAllocFunctionCallInfo(&esb,
+													 numTransArgs,
+													 ds_finfo_rel,
 													 &ds_fcinfo);
 
 				InitFunctionCallInfoData(*ds_fcinfo,
-										 &pertrans->deserialfn,
+										 ds_finfo,
 										 2,
 										 InvalidOid,
 										 (void *) percall, NULL);
@@ -3603,6 +3656,7 @@ ExecBuildGroupingEqual(TupleDesc ldesc, TupleDesc rdesc,
 		Form_pg_attribute ratt = TupleDescAttr(rdesc, attno - 1);
 		Oid			foid = eqfunctions[natt];
 		Oid			collid = collations[natt];
+		RelFunctionMgrInfo finfo_rel;
 		FmgrInfo   *finfo;
 		RelFunctionCallInfo fcinfo_rel;
 		FunctionCallInfo fcinfo;
@@ -3616,8 +3670,8 @@ ExecBuildGroupingEqual(TupleDesc ldesc, TupleDesc rdesc,
 		InvokeFunctionExecuteHook(foid);
 
 		/* Set up the primary fmgr lookup information */
-		finfo = palloc0(sizeof(FmgrInfo));
-		fcinfo_rel = ExprBuilderAllocFunctionCallInfo(&esb, 2, &fcinfo);
+		finfo_rel = ExprBuilderAllocFunctionMgrInfo(&esb, &finfo);
+		fcinfo_rel = ExprBuilderAllocFunctionCallInfo(&esb,2, finfo_rel, &fcinfo);
 		fmgr_info(foid, finfo);
 		fmgr_info_set_expr(NULL, finfo);
 		InitFunctionCallInfoData(*fcinfo, finfo, 2,
diff --git a/src/backend/jit/llvm/llvmjit_expr.c b/src/backend/jit/llvm/llvmjit_expr.c
index 43501a5bb6..f9af5482f8 100644
--- a/src/backend/jit/llvm/llvmjit_expr.c
+++ b/src/backend/jit/llvm/llvmjit_expr.c
@@ -110,6 +110,7 @@ typedef struct ExprCompileState
 static Datum ExecRunCompiledExpr(ExprState *state, ExprContext *econtext, bool *isNull);
 
 static void expr_emit_allocations(ExprCompileState *ecs);
+static void expr_init_fmgri(ExprCompileState *ecs, FmgrInfo *fmgrinfo, LLVMValueRef v_fmgrinfo);
 static void expr_init_fci(ExprCompileState *ecs, FunctionCallInfo fcinfo, LLVMValueRef v_fcinfo);
 static void expr_init_nullable_datums(ExprCompileState *ecs, int nelems, NullableDatum *datums, LLVMValueRef v_datums);
 
@@ -2672,6 +2673,13 @@ expr_emit_allocations(ExprCompileState *ecs)
 											  v_allocation);
 				break;
 
+			case ERP_FUNCTIONMGRINFO:
+				Assert(allocation->initial_content != NULL);
+				allocname = psprintf("alloc.%u.fmgr_info", i + 1);
+				v_allocation = LLVMBuildAlloca(ecs->b, StructFmgrInfo, allocname);
+				expr_init_fmgri(ecs, (FmgrInfo *) allocation->initial_content, v_allocation);
+				break;
+
 			case ERP_FUNCTIONCALLINFO:
 				Assert(allocation->initial_content != NULL);
 
@@ -2704,22 +2712,49 @@ expr_emit_allocations(ExprCompileState *ecs)
 	}
 }
 
+static void
+expr_init_fmgri(ExprCompileState *ecs, FmgrInfo *fmgrinfo, LLVMValueRef v_fmgrinfo)
+{
+	/*
+	 * There is no need to set fn_addr as the function will be resolved via
+	 * llvm_function_reference() followed by llvm_resolve_symbl().
+	 */
+
+	LLVMBuildStore(ecs->b,
+				   l_int32_const(fmgrinfo->fn_oid),
+				   LLVMBuildStructGEP(ecs->b, v_fmgrinfo, 1, ""));
+	LLVMBuildStore(ecs->b,
+				   l_int16_const(fmgrinfo->fn_nargs),
+				   LLVMBuildStructGEP(ecs->b, v_fmgrinfo, 2, ""));
+	LLVMBuildStore(ecs->b,
+				   l_sbool_const(fmgrinfo->fn_strict),
+				   LLVMBuildStructGEP(ecs->b, v_fmgrinfo, 3, ""));
+	LLVMBuildStore(ecs->b,
+				   l_sbool_const(fmgrinfo->fn_retset),
+				   LLVMBuildStructGEP(ecs->b, v_fmgrinfo, 4, ""));
+	LLVMBuildStore(ecs->b,
+				   l_int8_const(fmgrinfo->fn_stats),
+				   LLVMBuildStructGEP(ecs->b, v_fmgrinfo, 5, ""));
+	LLVMBuildStore(ecs->b,
+				   l_ptr_const(fmgrinfo->fn_extra, LLVMStructGetTypeAtIndex(StructFmgrInfo, 6)),
+				   LLVMBuildStructGEP(ecs->b, v_fmgrinfo, 6, "")); /* TODO */
+	LLVMBuildStore(ecs->b,
+				   l_ptr_const(fmgrinfo->fn_mcxt, l_ptr(StructMemoryContextData)),
+				   LLVMBuildStructGEP(ecs->b, v_fmgrinfo, 7, ""));
+	LLVMBuildStore(ecs->b,
+				   l_ptr_const(fmgrinfo->fn_expr, l_ptr(StructNode)),
+				   LLVMBuildStructGEP(ecs->b, v_fmgrinfo, 8, "")); /* TODO */
+}
+
 static void
 expr_init_fci(ExprCompileState *ecs, FunctionCallInfo fcinfo, LLVMValueRef v_fcinfo)
 {
 	LLVMValueRef v_args;
 
 	/*
-	 * FIXME: should be allocated as part of the ExprState, allowing this to
-	 * be referenced efficiently / without pointer constants.
+	 * We can't set fcinfo->flinfo here. It will be set inside expr_fcinfo_ref()
+	 * where we have access to the flinfo relptr.
 	 */
-	if (fcinfo->flinfo)
-		LLVMBuildStore(ecs->b,
-					   l_ptr_const(fcinfo->flinfo, l_ptr(StructFmgrInfo)),
-					   LLVMBuildStructGEP(ecs->b,
-										  v_fcinfo,
-										  FIELDNO_FUNCTIONCALLINFODATA_FLINFO,
-										  ""));
 
 	/*
 	 * FIXME: should be allocated as part of the ExprState, allowing this to
@@ -2910,13 +2945,36 @@ expr_allocation_ref(ExprCompileState *ecs, ExprRelPtr rel)
 static LLVMValueRef
 expr_fcinfo_ref(ExprCompileState *ecs, RelFunctionCallInfo relfc, FunctionCallInfo *fcinfo)
 {
-	ExprStateAllocation *alloc = list_nth(ecs->esb->allocations, relfc.ptr.allocno - 1);
+	FmgrInfo            *fmgrInfo;
+	LLVMValueRef        v_fmgrInfo;
+	LLVMValueRef        v_fcinfo;
+	ExprStateAllocation *fcinfo_alloc;
+	ExprStateAllocation *fmgr_alloc;
+
+	fcinfo_alloc = list_nth(ecs->esb->allocations, relfc.ptr.allocno - 1);
+	Assert(fcinfo_alloc->initial_content != NULL);
+
+	fmgr_alloc = list_nth(ecs->esb->allocations, relfc.fmgrInfoPtr.allocno - 1);
+	Assert(fmgr_alloc->initial_content != NULL);
 
-	Assert(alloc->initial_content != NULL);
 	if (fcinfo)
-		*fcinfo = (FunctionCallInfo) alloc->initial_content;
+	{
+		*fcinfo = (FunctionCallInfo) fcinfo_alloc->initial_content;
+		fmgrInfo = (FmgrInfo *) fmgr_alloc->initial_content;
+		(*fcinfo)->flinfo = fmgrInfo;
+	}
+	v_fmgrInfo = expr_allocation_ref(ecs, relfc.fmgrInfoPtr);
+
+	v_fcinfo = expr_allocation_ref(ecs, relfc.ptr);
+
+	LLVMBuildStore(ecs->b,
+				   v_fmgrInfo,
+				   LLVMBuildStructGEP(ecs->b,
+										  v_fcinfo,
+										  FIELDNO_FUNCTIONCALLINFODATA_FLINFO,
+										  ""));
 
-   return expr_allocation_ref(ecs, relfc.ptr);
+	return v_fcinfo;
 }
 
 static LLVMValueRef
diff --git a/src/include/executor/execExpr.h b/src/include/executor/execExpr.h
index b523e839bb..a3f9e8676f 100644
--- a/src/include/executor/execExpr.h
+++ b/src/include/executor/execExpr.h
@@ -265,9 +265,15 @@ typedef struct RelNullableDatumArray
 	ExprRelPtr ptr;
 } RelNullableDatumArray;
 
+typedef struct RelFunctionMgrInfo
+{
+	ExprRelPtr ptr;
+} RelFunctionMgrInfo;
+
 typedef struct RelFunctionCallInfo
 {
 	ExprRelPtr ptr;
+	ExprRelPtr fmgrInfoPtr;
 } RelFunctionCallInfo;
 
 typedef struct ExprEvalStep
@@ -682,6 +688,7 @@ typedef enum ExprRelPtrKind
 	ERP_BOOL,
 	ERP_NULLABLE_DATUM,
 	ERP_NULLABLE_DATUM_ARRAY,
+	ERP_FUNCTIONMGRINFO,
 	ERP_FUNCTIONCALLINFO
 } ExprRelPtrKind;
 
diff --git a/src/include/jit/llvmjit.h b/src/include/jit/llvmjit.h
index 6250148159..ae874fca03 100644
--- a/src/include/jit/llvmjit.h
+++ b/src/include/jit/llvmjit.h
@@ -112,9 +112,9 @@ extern void llvm_split_symbol_name(const char *name, char **modname, char **func
 extern LLVMValueRef llvm_get_decl(LLVMModuleRef mod, LLVMValueRef f);
 extern void llvm_copy_attributes(LLVMValueRef from, LLVMValueRef to);
 extern LLVMValueRef llvm_function_reference(LLVMJitContext *context,
-						LLVMBuilderRef builder,
-						LLVMModuleRef mod,
-						FunctionCallInfo fcinfo);
+											LLVMBuilderRef builder,
+											LLVMModuleRef mod,
+											FunctionCallInfo fcinfo);
 
 extern void llvm_inline(LLVMModuleRef mod);
 
-- 
2.24.1