On 2018-08-17 01:07:06 -0700, Andres Freund wrote:
> Hi,
> 
> On 2018-08-17 12:10:20 +0530, Ashutosh Bapat wrote:
> > We need to add LLVM code to fetch tts_flags and
> > perform bit operation on it to get or set slow property. I haven't
> > found any precedence for LLVM bit operations in postgresql's JIT code.
> 
> There are several, look for the infomask accesses in
> slot_compiler_deform.
> 
> I'll try to do the adaption later today.

Attached is a series of patches doing so.  The previous implementation
of sysvar accesses wasn't actually working - the slot argument was
uninitialized.

I also noticed an independent issue in your changes to
ExecInitScanTupleSlot(): You can't assume that the plan belonging to the
ScanState have a Scan node in their plan. Look e.g. at Material, Sort
etc. So currently your scanrelid access is often just uninitialized
data.

Greetings,

Andres Freund
>From 68ab969d94964a8e6cdfac974d73ef559546ffa0 Mon Sep 17 00:00:00 2001
From: Andres Freund <and...@anarazel.de>
Date: Sat, 18 Aug 2018 08:12:52 -0700
Subject: [PATCH 1/4] Fix slot type used in subqueryscan.

This later becomes relevant because it prevents upper layers from
making assumptions about the format of the slots.
---
 src/backend/executor/nodeSubqueryscan.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/backend/executor/nodeSubqueryscan.c b/src/backend/executor/nodeSubqueryscan.c
index 1e83e673939..6da918894fd 100644
--- a/src/backend/executor/nodeSubqueryscan.c
+++ b/src/backend/executor/nodeSubqueryscan.c
@@ -130,7 +130,7 @@ ExecInitSubqueryScan(SubqueryScan *node, EState *estate, int eflags)
 	 */
 	ExecInitScanTupleSlot(estate, &subquerystate->ss,
 						  ExecGetResultType(subquerystate->subplan),
-						  &TTSOpsHeapTuple);
+						  &TTSOpsVirtual);
 
 	/*
 	 * Initialize result slot, type and projection.
-- 
2.18.0.rc2.dirty

>From 50ca33b96bf6017f04863486763e2e3250c5cc0a Mon Sep 17 00:00:00 2001
From: Andres Freund <and...@anarazel.de>
Date: Sat, 18 Aug 2018 08:13:49 -0700
Subject: [PATCH 2/4] XXX: Copy slot in nodeMaterial.c

This is needed because otherwise upper layers can't make correct
assumptions about the type of slot handed up.

Author:
Reviewed-By:
Discussion: https://postgr.es/m/
Backpatch:
---
 src/backend/executor/nodeMaterial.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/src/backend/executor/nodeMaterial.c b/src/backend/executor/nodeMaterial.c
index 66c4ef6ca90..fc36aedfa26 100644
--- a/src/backend/executor/nodeMaterial.c
+++ b/src/backend/executor/nodeMaterial.c
@@ -146,10 +146,8 @@ ExecMaterial(PlanState *pstate)
 		if (tuplestorestate)
 			tuplestore_puttupleslot(tuplestorestate, outerslot);
 
-		/*
-		 * We can just return the subplan's returned tuple, without copying.
-		 */
-		return outerslot;
+		ExecCopySlot(slot, outerslot);
+		return slot;
 	}
 
 	/*
-- 
2.18.0.rc2.dirty

>From 5d6168601419ff044f197dfe5c1a2479f97496d1 Mon Sep 17 00:00:00 2001
From: Andres Freund <and...@anarazel.de>
Date: Sat, 18 Aug 2018 08:15:52 -0700
Subject: [PATCH 3/4] Fix JIT calls to ExecEvalSysVar().

The previous attempt at this called the function without initializing
the slot argument. Which unsurprisingly leads to crashes.

Author:
Reviewed-By:
Discussion: https://postgr.es/m/
Backpatch:
---
 src/backend/jit/llvm/llvmjit_expr.c | 58 ++++++++++++++++++++++++++---
 1 file changed, 52 insertions(+), 6 deletions(-)

diff --git a/src/backend/jit/llvm/llvmjit_expr.c b/src/backend/jit/llvm/llvmjit_expr.c
index 432f81e0dfb..5c878e45285 100644
--- a/src/backend/jit/llvm/llvmjit_expr.c
+++ b/src/backend/jit/llvm/llvmjit_expr.c
@@ -64,6 +64,9 @@ static void build_EvalXFunc(LLVMBuilderRef b, LLVMModuleRef mod,
 				const char *funcname,
 				LLVMValueRef v_state, LLVMValueRef v_econtext,
 				ExprEvalStep *op);
+static void build_ExecEvalSysVar(LLVMBuilderRef b, LLVMModuleRef mod,
+				LLVMValueRef v_state, LLVMValueRef v_econtext,
+				ExprEvalStep *op, LLVMValueRef v_slot);
 static LLVMValueRef create_LifetimeEnd(LLVMModuleRef mod);
 
 
@@ -403,14 +406,22 @@ llvm_compile_expr(ExprState *state)
 				}
 
 			case EEOP_INNER_SYSVAR:
+				build_ExecEvalSysVar(b, mod, v_state,
+									 v_econtext, op, v_innerslot);
+				LLVMBuildBr(b, opblocks[i + 1]);
+				break;
+
 			case EEOP_OUTER_SYSVAR:
+				build_ExecEvalSysVar(b, mod, v_state,
+									 v_econtext, op, v_outerslot);
+				LLVMBuildBr(b, opblocks[i + 1]);
+				break;
+
 			case EEOP_SCAN_SYSVAR:
-				{
-					build_EvalXFunc(b, mod, "ExecEvalSysVar",
-									v_state, v_econtext, op);
-					LLVMBuildBr(b, opblocks[i + 1]);
-					break;
-				}
+				build_ExecEvalSysVar(b, mod, v_state,
+										 v_econtext, op, v_scanslot);
+				LLVMBuildBr(b, opblocks[i + 1]);
+				break;
 
 			case EEOP_WHOLEROW:
 				build_EvalXFunc(b, mod, "ExecEvalWholeRowVar",
@@ -2634,6 +2645,41 @@ build_EvalXFunc(LLVMBuilderRef b, LLVMModuleRef mod, const char *funcname,
 				  params, lengthof(params), "");
 }
 
+static void
+build_ExecEvalSysVar(LLVMBuilderRef b, LLVMModuleRef mod,
+					 LLVMValueRef v_state, LLVMValueRef v_econtext,
+					 ExprEvalStep *op, LLVMValueRef v_slot)
+{
+	LLVMTypeRef sig;
+	LLVMValueRef v_fn;
+	LLVMTypeRef param_types[4];
+	LLVMValueRef params[4];
+	const char *funcname = "ExecEvalSysVar";
+
+	v_fn = LLVMGetNamedFunction(mod, funcname);
+	if (!v_fn)
+	{
+		param_types[0] = l_ptr(StructExprState);
+		param_types[1] = l_ptr(StructExprEvalStep);
+		param_types[2] = l_ptr(StructExprContext);
+		param_types[3] = l_ptr(StructTupleTableSlot);
+
+		sig = LLVMFunctionType(LLVMVoidType(),
+							   param_types, lengthof(param_types),
+							   false);
+		v_fn = LLVMAddFunction(mod, funcname, sig);
+	}
+
+	params[0] = v_state;
+	params[1] = l_ptr_const(op, l_ptr(StructExprEvalStep));
+	params[2] = v_econtext;
+	params[3] = v_slot;
+
+	LLVMBuildCall(b,
+				  v_fn,
+				  params, lengthof(params), "");
+}
+
 static LLVMValueRef
 create_LifetimeEnd(LLVMModuleRef mod)
 {
-- 
2.18.0.rc2.dirty

>From 6ce4e9632ebb5d55693588e25b0f45c2005c7b43 Mon Sep 17 00:00:00 2001
From: Andres Freund <and...@anarazel.de>
Date: Sat, 18 Aug 2018 08:17:13 -0700
Subject: [PATCH 4/4] First attempt at support JITing of tuple deforming with
 abstract slots.

This now assumes that slots for which we JIT compile deforming allways
have the same slot type, not just tupledesc. Previous commits attempt
to make that the case.

With that knowledge this commit JIT compiles differently for different
slot types. Besides just making things work, this has the advantage
that we no longer JIT compile in cases virtual slots are
used. Previously we did so entirely unnecessarily.

Author:
Reviewed-By:
Discussion: https://postgr.es/m/
Backpatch:
---
 src/backend/jit/llvm/llvmjit.c        |  4 ++
 src/backend/jit/llvm/llvmjit_deform.c | 61 +++++++++++++++++++++++----
 src/backend/jit/llvm/llvmjit_expr.c   | 25 +++++++++--
 src/backend/jit/llvm/llvmjit_types.c  |  2 +
 src/include/executor/tuptable.h       |  5 +++
 src/include/jit/llvmjit.h             |  5 ++-
 6 files changed, 88 insertions(+), 14 deletions(-)

diff --git a/src/backend/jit/llvm/llvmjit.c b/src/backend/jit/llvm/llvmjit.c
index 640c27fc408..55ee46d292b 100644
--- a/src/backend/jit/llvm/llvmjit.c
+++ b/src/backend/jit/llvm/llvmjit.c
@@ -65,6 +65,8 @@ LLVMTypeRef StructFormPgAttribute;
 LLVMTypeRef StructTupleConstr;
 LLVMTypeRef StructtupleDesc;
 LLVMTypeRef StructTupleTableSlot;
+LLVMTypeRef StructHeapTupleTableSlot;
+LLVMTypeRef StructMinimalTupleTableSlot;
 LLVMTypeRef StructMemoryContextData;
 LLVMTypeRef StructPGFinfoRecord;
 LLVMTypeRef StructFmgrInfo;
@@ -811,6 +813,8 @@ llvm_create_types(void)
 	StructFunctionCallInfoData = load_type(mod, "StructFunctionCallInfoData");
 	StructMemoryContextData = load_type(mod, "StructMemoryContextData");
 	StructTupleTableSlot = load_type(mod, "StructTupleTableSlot");
+	StructHeapTupleTableSlot = load_type(mod, "StructHeapTupleTableSlot");
+	StructMinimalTupleTableSlot = load_type(mod, "StructMinimalTupleTableSlot");
 	StructHeapTupleData = load_type(mod, "StructHeapTupleData");
 	StructtupleDesc = load_type(mod, "StructtupleDesc");
 	StructAggState = load_type(mod, "StructAggState");
diff --git a/src/backend/jit/llvm/llvmjit_deform.c b/src/backend/jit/llvm/llvmjit_deform.c
index 6d7ce21865c..f213480f32d 100644
--- a/src/backend/jit/llvm/llvmjit_deform.c
+++ b/src/backend/jit/llvm/llvmjit_deform.c
@@ -31,7 +31,7 @@
  * Create a function that deforms a tuple of type desc up to natts columns.
  */
 LLVMValueRef
-slot_compile_deform(LLVMJitContext *context, TupleDesc desc, int natts)
+slot_compile_deform(LLVMJitContext *context, TupleDesc desc, const TupleTableSlotOps *ops, int natts)
 {
 	char	   *funcname;
 
@@ -60,7 +60,7 @@ slot_compile_deform(LLVMJitContext *context, TupleDesc desc, int natts)
 	LLVMValueRef v_tts_values;
 	LLVMValueRef v_tts_nulls;
 	LLVMValueRef v_slotoffp;
-	LLVMValueRef v_slowp;
+	LLVMValueRef v_flagsp;
 	LLVMValueRef v_nvalidp;
 	LLVMValueRef v_nvalid;
 	LLVMValueRef v_maxatt;
@@ -88,6 +88,16 @@ slot_compile_deform(LLVMJitContext *context, TupleDesc desc, int natts)
 
 	int			attnum;
 
+	if (ops != &TTSOpsHeapTuple && ops != &TTSOpsBufferTuple &&
+		ops != &TTSOpsMinimalTuple)
+	{
+		/*
+		 * Decline to JIT for slot types we don't know to handle, or don't
+		 * want to handle (say virtual slots).
+		 */
+		return NULL;
+	}
+
 	mod = llvm_mutable_module(context);
 
 	funcname = llvm_expand_funcname(context, "deform");
@@ -166,14 +176,44 @@ slot_compile_deform(LLVMJitContext *context, TupleDesc desc, int natts)
 	v_tts_nulls =
 		l_load_struct_gep(b, v_slot, FIELDNO_TUPLETABLESLOT_ISNULL,
 						  "tts_ISNULL");
-
-	v_slotoffp = LLVMBuildStructGEP(b, v_slot, FIELDNO_TUPLETABLESLOT_OFF, "");
-	v_slowp = LLVMBuildStructGEP(b, v_slot, FIELDNO_TUPLETABLESLOT_SLOW, "");
+	v_flagsp = LLVMBuildStructGEP(b, v_slot, FIELDNO_TUPLETABLESLOT_FLAGS, "");
 	v_nvalidp = LLVMBuildStructGEP(b, v_slot, FIELDNO_TUPLETABLESLOT_NVALID, "");
 
-	v_tupleheaderp =
-		l_load_struct_gep(b, v_slot, FIELDNO_TUPLETABLESLOT_TUPLE,
-						  "tupleheader");
+	if (ops == &TTSOpsHeapTuple || ops == &TTSOpsBufferTuple)
+	{
+		LLVMValueRef v_heapslot;
+
+		v_heapslot =
+			LLVMBuildBitCast(b,
+							 v_slot,
+							 l_ptr(StructHeapTupleTableSlot),
+							 "heapslot");
+		v_slotoffp = LLVMBuildStructGEP(b, v_heapslot, FIELDNO_HEAPTUPLETABLESLOT_OFF, "");
+		v_tupleheaderp =
+			l_load_struct_gep(b, v_heapslot, FIELDNO_HEAPTUPLETABLESLOT_TUPLE,
+							  "tupleheader");
+
+	}
+	else if (ops == &TTSOpsMinimalTuple)
+	{
+		LLVMValueRef v_minimalslot;
+
+		v_minimalslot =
+			LLVMBuildBitCast(b,
+							 v_slot,
+							 l_ptr(StructMinimalTupleTableSlot),
+							 "minimalslotslot");
+		v_slotoffp = LLVMBuildStructGEP(b, v_minimalslot, FIELDNO_MINIMALTUPLETABLESLOT_OFF, "");
+		v_tupleheaderp =
+			l_load_struct_gep(b, v_minimalslot, FIELDNO_MINIMALTUPLETABLESLOT_TUPLE,
+							  "tupleheader");
+	}
+	else
+	{
+		/* should've returned at the start of the function */
+		pg_unreachable();
+	}
+
 	v_tuplep =
 		l_load_struct_gep(b, v_tupleheaderp, FIELDNO_HEAPTUPLEDATA_DATA,
 						  "tuple");
@@ -690,11 +730,14 @@ slot_compile_deform(LLVMJitContext *context, TupleDesc desc, int natts)
 
 	{
 		LLVMValueRef v_off = LLVMBuildLoad(b, v_offp, "");
+		LLVMValueRef v_flags;
 
 		LLVMBuildStore(b, l_int16_const(natts), v_nvalidp);
 		v_off = LLVMBuildTrunc(b, v_off, LLVMInt32Type(), "");
 		LLVMBuildStore(b, v_off, v_slotoffp);
-		LLVMBuildStore(b, l_int8_const(1), v_slowp);
+		v_flags = LLVMBuildLoad(b, v_flagsp, "tts_flags");
+		v_flags = LLVMBuildOr(b, v_flags, l_int16_const(TTS_SLOW), "");
+		LLVMBuildStore(b, v_flags, v_flagsp);
 		LLVMBuildRetVoid(b);
 	}
 
diff --git a/src/backend/jit/llvm/llvmjit_expr.c b/src/backend/jit/llvm/llvmjit_expr.c
index 5c878e45285..546c0bc0785 100644
--- a/src/backend/jit/llvm/llvmjit_expr.c
+++ b/src/backend/jit/llvm/llvmjit_expr.c
@@ -276,9 +276,11 @@ llvm_compile_expr(ExprState *state)
 			case EEOP_SCAN_FETCHSOME:
 				{
 					TupleDesc	desc = NULL;
+					TupleTableSlot *slot;
 					LLVMValueRef v_slot;
 					LLVMBasicBlockRef b_fetch;
 					LLVMValueRef v_nvalid;
+					LLVMValueRef l_jit_deform = NULL;
 
 					b_fetch = l_bb_before_v(opblocks[i + 1],
 											"op.%d.fetch", i);
@@ -296,7 +298,10 @@ llvm_compile_expr(ExprState *state)
 							is &&
 							is->ps_ResultTupleSlot &&
 							IS_TTS_FIXED(is->ps_ResultTupleSlot))
+						{
+							slot = is->ps_ResultTupleSlot;
 							desc = is->ps_ResultTupleSlot->tts_tupleDescriptor;
+						}
 					}
 					else if (opcode == EEOP_OUTER_FETCHSOME)
 					{
@@ -308,13 +313,20 @@ llvm_compile_expr(ExprState *state)
 							os &&
 							os->ps_ResultTupleSlot &&
 							IS_TTS_FIXED(os->ps_ResultTupleSlot))
+						{
+							slot = os->ps_ResultTupleSlot;
 							desc = os->ps_ResultTupleSlot->tts_tupleDescriptor;
+						}
 					}
 					else
 					{
 						v_slot = v_scanslot;
+
 						if (!desc && parent)
+						{
+							slot = ((ScanState*) parent)->ss_ScanTupleSlot;
 							desc = parent->scandesc;
+						}
 					}
 
 					/*
@@ -339,14 +351,19 @@ llvm_compile_expr(ExprState *state)
 					 * function specific to tupledesc and the exact number of
 					 * to-be-extracted attributes.
 					 */
-					if (desc && (context->base.flags & PGJIT_DEFORM))
+					if (slot && desc && (context->base.flags & PGJIT_DEFORM))
 					{
-						LLVMValueRef params[1];
-						LLVMValueRef l_jit_deform;
-
 						l_jit_deform =
 							slot_compile_deform(context, desc,
+												slot->tts_cb,
 												op->d.fetch.last_var);
+					}
+
+
+					if (l_jit_deform)
+					{
+						LLVMValueRef params[1];
+
 						params[0] = v_slot;
 
 						LLVMBuildCall(b, l_jit_deform,
diff --git a/src/backend/jit/llvm/llvmjit_types.c b/src/backend/jit/llvm/llvmjit_types.c
index 58316a760d4..ea24866938f 100644
--- a/src/backend/jit/llvm/llvmjit_types.c
+++ b/src/backend/jit/llvm/llvmjit_types.c
@@ -59,6 +59,8 @@ FunctionCallInfoData StructFunctionCallInfoData;
 HeapTupleData StructHeapTupleData;
 MemoryContextData StructMemoryContextData;
 TupleTableSlot StructTupleTableSlot;
+HeapTupleTableSlot StructHeapTupleTableSlot;
+MinimalTupleTableSlot StructMinimalTupleTableSlot;
 struct tupleDesc StructtupleDesc;
 
 
diff --git a/src/include/executor/tuptable.h b/src/include/executor/tuptable.h
index e16bb26f6fc..fa5997cc3dc 100644
--- a/src/include/executor/tuptable.h
+++ b/src/include/executor/tuptable.h
@@ -273,6 +273,7 @@ extern Datum ExecFetchSlotTupleDatum(TupleTableSlot *slot);
 struct TupleTableSlot
 {
 	NodeTag		type;
+#define FIELDNO_TUPLETABLESLOT_FLAGS 1
 	uint16		tts_flags;
 #define FIELDNO_TUPLETABLESLOT_NVALID 2
 	AttrNumber	tts_nvalid;		/* # of valid values in tts_values */
@@ -300,7 +301,9 @@ struct TupleTableSlot
 typedef struct HeapTupleTableSlot
 {
 	TupleTableSlot base;
+#define FIELDNO_HEAPTUPLETABLESLOT_TUPLE 1
 	HeapTuple	tuple;		/* physical tuple */
+#define FIELDNO_HEAPTUPLETABLESLOT_OFF 2
 	uint32		off;		/* saved state for slot_deform_tuple */
 } HeapTupleTableSlot;
 
@@ -314,9 +317,11 @@ typedef struct BufferHeapTupleTableSlot
 typedef struct MinimalTupleTableSlot
 {
 	TupleTableSlot base;
+#define FIELDNO_MINIMALTUPLETABLESLOT_TUPLE 1
 	HeapTuple	tuple;		/* tuple wrapper */
 	MinimalTuple mintuple;	/* minimal tuple, or NULL if none */
 	HeapTupleData minhdr;	/* workspace for minimal-tuple-only case */
+#define FIELDNO_MINIMALTUPLETABLESLOT_OFF 4
 	uint32		off;		/* saved state for slot_deform_tuple */
 } MinimalTupleTableSlot;
 
diff --git a/src/include/jit/llvmjit.h b/src/include/jit/llvmjit.h
index b0093db49d7..da2f088cd31 100644
--- a/src/include/jit/llvmjit.h
+++ b/src/include/jit/llvmjit.h
@@ -65,6 +65,8 @@ extern LLVMTypeRef TypeStorageBool;
 extern LLVMTypeRef StructtupleDesc;
 extern LLVMTypeRef StructHeapTupleData;
 extern LLVMTypeRef StructTupleTableSlot;
+extern LLVMTypeRef StructHeapTupleTableSlot;
+extern LLVMTypeRef StructMinimalTupleTableSlot;
 extern LLVMTypeRef StructMemoryContextData;
 extern LLVMTypeRef StructFunctionCallInfoData;
 extern LLVMTypeRef StructExprContext;
@@ -111,7 +113,8 @@ extern void llvm_inline(LLVMModuleRef mod);
  ****************************************************************************
  */
 extern bool llvm_compile_expr(struct ExprState *state);
-extern LLVMValueRef slot_compile_deform(struct LLVMJitContext *context, TupleDesc desc, int natts);
+struct TupleTableSlotOps;
+extern LLVMValueRef slot_compile_deform(struct LLVMJitContext *context, TupleDesc desc, const struct TupleTableSlotOps *ops, int natts);
 
 /*
  ****************************************************************************
-- 
2.18.0.rc2.dirty

Reply via email to