[Mesa-dev,02/10] radeonsi: cleanup si_llvm_init_export_args

Submitted by Marek Olšák on Oct. 11, 2015, 1:11 a.m.

Details

Message ID 1444525898-13786-3-git-send-email-maraeo@gmail.com
State New
Headers show

Not browsing as part of any series.

Commit Message

Marek Olšák Oct. 11, 2015, 1:11 a.m.
From: Marek Olšák <marek.olsak@amd.com>

---
 src/gallium/drivers/radeonsi/si_shader.c | 76 ++++++++++++++------------------
 1 file changed, 34 insertions(+), 42 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/gallium/drivers/radeonsi/si_shader.c b/src/gallium/drivers/radeonsi/si_shader.c
index 32a702f..109a805 100644
--- a/src/gallium/drivers/radeonsi/si_shader.c
+++ b/src/gallium/drivers/radeonsi/si_shader.c
@@ -1306,6 +1306,22 @@  static void si_llvm_init_export_args(struct lp_build_tgsi_context *bld_base,
 	unsigned compressed = 0;
 	unsigned chan;
 
+	/* XXX: This controls which components of the output
+	 * registers actually get exported. (e.g bit 0 means export
+	 * X component, bit 1 means export Y component, etc.)  I'm
+	 * hard coding this to 0xf for now.  In the future, we might
+	 * want to do something else. */
+	args[0] = lp_build_const_int32(base->gallivm, 0xf);
+
+	/* Specify whether the EXEC mask represents the valid mask */
+	args[1] = uint->zero;
+
+	/* Specify whether this is the last export */
+	args[2] = uint->zero;
+
+	/* Specify the target we are exporting */
+	args[3] = lp_build_const_int32(base->gallivm, target);
+
 	if (si_shader_ctx->type == TGSI_PROCESSOR_FRAGMENT) {
 		int cbuf = target - V_008DFC_SQ_EXP_MRT;
 
@@ -1323,55 +1339,31 @@  static void si_llvm_init_export_args(struct lp_build_tgsi_context *bld_base,
 		}
 	}
 
+	/* Set COMPR flag */
+	args[4] = compressed ? uint->one : uint->zero;
+
 	if (compressed) {
 		/* Pixel shader needs to pack output values before export */
-		for (chan = 0; chan < 2; chan++ ) {
-			args[0] = values[2 * chan];
-			args[1] = values[2 * chan + 1];
-			args[chan + 5] =
-				lp_build_intrinsic(base->gallivm->builder,
-						"llvm.SI.packf16",
-						LLVMInt32TypeInContext(base->gallivm->context),
-						args, 2,
-						LLVMReadNoneAttribute | LLVMNoUnwindAttribute);
+		for (chan = 0; chan < 2; chan++) {
+			LLVMValueRef pack_args[2] = {
+				values[2 * chan],
+				values[2 * chan + 1]
+			};
+			LLVMValueRef packed;
+
+			packed = lp_build_intrinsic(base->gallivm->builder,
+						    "llvm.SI.packf16",
+						    LLVMInt32TypeInContext(base->gallivm->context),
+						    pack_args, 2,
+						    LLVMReadNoneAttribute | LLVMNoUnwindAttribute);
 			args[chan + 7] = args[chan + 5] =
 				LLVMBuildBitCast(base->gallivm->builder,
-						 args[chan + 5],
+						 packed,
 						 LLVMFloatTypeInContext(base->gallivm->context),
 						 "");
 		}
-
-		/* Set COMPR flag */
-		args[4] = uint->one;
-	} else {
-		for (chan = 0; chan < 4; chan++ )
-			/* +5 because the first output value will be
-			 * the 6th argument to the intrinsic. */
-			args[chan + 5] = values[chan];
-
-		/* Clear COMPR flag */
-		args[4] = uint->zero;
-	}
-
-	/* XXX: This controls which components of the output
-	 * registers actually get exported. (e.g bit 0 means export
-	 * X component, bit 1 means export Y component, etc.)  I'm
-	 * hard coding this to 0xf for now.  In the future, we might
-	 * want to do something else. */
-	args[0] = lp_build_const_int32(base->gallivm, 0xf);
-
-	/* Specify whether the EXEC mask represents the valid mask */
-	args[1] = uint->zero;
-
-	/* Specify whether this is the last export */
-	args[2] = uint->zero;
-
-	/* Specify the target we are exporting */
-	args[3] = lp_build_const_int32(base->gallivm, target);
-
-	/* XXX: We probably need to keep track of the output
-	 * values, so we know what we are passing to the next
-	 * stage. */
+	} else
+		memcpy(&args[5], values, sizeof(values[0]) * 4);
 }
 
 /* Load from output pointers and initialize arguments for the shader export intrinsic */

Comments

On 11.10.2015 10:11, Marek Olšák wrote:
> From: Marek Olšák <marek.olsak@amd.com>

The shortlog should say "clean up" (verb) instead of "cleanup" (noun).
Same for patches 9 & 10.


> diff --git a/src/gallium/drivers/radeonsi/si_shader.c b/src/gallium/drivers/radeonsi/si_shader.c
> index 32a702f..109a805 100644
> --- a/src/gallium/drivers/radeonsi/si_shader.c
> +++ b/src/gallium/drivers/radeonsi/si_shader.c
> @@ -1306,6 +1306,22 @@ static void si_llvm_init_export_args(struct lp_build_tgsi_context *bld_base,
>  	unsigned compressed = 0;
>  	unsigned chan;
>  
> +	/* XXX: This controls which components of the output
> +	 * registers actually get exported. (e.g bit 0 means export
> +	 * X component, bit 1 means export Y component, etc.)  I'm
> +	 * hard coding this to 0xf for now.  In the future, we might
> +	 * want to do something else. */

The "*/" should go on its own line.


With those fixed, this patch and patches 9 & 10 are

Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>