Re: [Mesa-dev] [PATCH 1/2] radv: Fix up 2_10_10_10 alpha sign.

[Samuel Pitoiset]

On 05/12/2018 11:58 PM, Bas Nieuwenhuizen wrote:

Pre-Vega HW always interprets the alpha for this format as unsigned,
so we have to implement a fixup to do the sign correctly for signed
formats.

CC: 18.0 18.1 
---
  src/amd/vulkan/radv_nir_to_llvm.c | 61 ---
  src/amd/vulkan/radv_pipeline.c| 30 +--
  src/amd/vulkan/radv_private.h |  1 +
  src/amd/vulkan/radv_shader.h  | 12 ++
  4 files changed, 95 insertions(+), 9 deletions(-)

diff --git a/src/amd/vulkan/radv_nir_to_llvm.c 
b/src/amd/vulkan/radv_nir_to_llvm.c
index f98940f0d8f..9d9ecda63cf 100644
--- a/src/amd/vulkan/radv_nir_to_llvm.c
+++ b/src/amd/vulkan/radv_nir_to_llvm.c
@@ -1865,6 +1865,47 @@ static LLVMValueRef radv_get_sampler_desc(struct 
ac_shader_abi *abi,
return ac_build_load_to_sgpr(>ac, list, index);
  }
  
+/* For 2_10_10_10 formats the alpha is handled as unsigned by pre-vega HW.

+ * so we may need to fix it up. */
+static LLVMValueRef
+adjust_vertex_fetch_alpha(struct radv_shader_context *ctx,
+  unsigned adjustment,
+  LLVMValueRef alpha)
+{
+   if (adjustment == RADV_ALPHA_ADJUST_NONE)
+   return alpha;
+
+   LLVMValueRef c30 = LLVMConstInt(ctx->ac.i32, 30, 0);
+
+   if (adjustment == RADV_ALPHA_ADJUST_SSCALED)
+   alpha = LLVMBuildFPToUI(ctx->ac.builder, alpha, ctx->ac.i32, 
"");
+   else
+   alpha = LLVMBuildBitCast(ctx->ac.builder, alpha, ctx->ac.i32, 
"");


ac_to_integer?


+
+   /* For the integer-like cases, do a natural sign extension.
+   *
+   * For the SNORM case, the values are 0.0, 0.333, 0.666, 1.0
+   * and happen to contain 0, 1, 2, 3 as the two LSBs of the
+   * exponent.
+   */


identation.


+   alpha = LLVMBuildShl(ctx->ac.builder, alpha,
+adjustment == RADV_ALPHA_ADJUST_SNORM ?
+LLVMConstInt(ctx->ac.i32, 7, 0) : c30, "");
+   alpha = LLVMBuildAShr(ctx->ac.builder, alpha, c30, "");
+
+   /* Convert back to the right type. */
+   if (adjustment == RADV_ALPHA_ADJUST_SNORM) {
+   LLVMValueRef clamp;
+   LLVMValueRef neg_one = LLVMConstReal(ctx->ac.f32, -1.0);
+   alpha = LLVMBuildSIToFP(ctx->ac.builder, alpha, ctx->ac.f32, 
"");
+   clamp = LLVMBuildFCmp(ctx->ac.builder, LLVMRealULT, alpha, neg_one, 
"");
+   alpha = LLVMBuildSelect(ctx->ac.builder, clamp, neg_one, alpha, 
"");
+   } else if (adjustment == RADV_ALPHA_ADJUST_SSCALED) {
+   alpha = LLVMBuildSIToFP(ctx->ac.builder, alpha, ctx->ac.f32, 
"");
+   }
+
+   return alpha;
+}
  
  static void

  handle_vs_input_decl(struct radv_shader_context *ctx,
@@ -1884,9 +1925,11 @@ handle_vs_input_decl(struct radv_shader_context *ctx,
  
  	variable->data.driver_location = idx * 4;
  
-	for (unsigned i = 0; i < attrib_count; ++i, ++idx) {

-   if (ctx->options->key.vs.instance_rate_inputs & (1u << (index + 
i))) {
-   uint32_t divisor = 
ctx->options->key.vs.instance_rate_divisors[index + i];
+   for (unsigned i = 0; i < attrib_count; ++i, ++idx, ++index) {


Mmh, can you please clean up this? That's going to be quite insane, i, 
idx, index. :)



+   LLVMValueRef output[4];
+
+   if (ctx->options->key.vs.instance_rate_inputs & (1u << index)) {
+   uint32_t divisor = 
ctx->options->key.vs.instance_rate_divisors[index];
  
  			if (divisor) {

buffer_index = LLVMBuildAdd(ctx->ac.builder, 
ctx->abi.instance_id,
@@ -1910,7 +1953,7 @@ handle_vs_input_decl(struct radv_shader_context *ctx,
} else
buffer_index = LLVMBuildAdd(ctx->ac.builder, 
ctx->abi.vertex_id,
ctx->abi.base_vertex, "");
-   t_offset = LLVMConstInt(ctx->ac.i32, index + i, false);
+   t_offset = LLVMConstInt(ctx->ac.i32, index, false);
  
  		t_list = ac_build_load_to_sgpr(>ac, t_list_ptr, t_offset);
  
@@ -1923,9 +1966,15 @@ handle_vs_input_decl(struct radv_shader_context *ctx,
  
  		for (unsigned chan = 0; chan < 4; chan++) {

LLVMValueRef llvm_chan = LLVMConstInt(ctx->ac.i32, 
chan, false);
+   output[chan] = LLVMBuildExtractElement(ctx->ac.builder, input, 
llvm_chan, "");
+   }
+
+   unsigned alpha_adjust = (ctx->options->key.vs.alpha_adjust >> 
(index * 2)) & 3;
+   output[3] = adjust_vertex_fetch_alpha(ctx, alpha_adjust, 
output[3]);
+
+   for (unsigned chan = 0; chan < 4; chan++) {
ctx->inputs[ac_llvm_reg_index_soa(idx, chan)] =
-   ac_to_integer(>ac, 
LLVMBuildExtractElement(ctx->ac.builder,
-   input, 

[Mesa-dev] [PATCH 1/2] radv: Fix up 2_10_10_10 alpha sign.

[Bas Nieuwenhuizen]

Pre-Vega HW always interprets the alpha for this format as unsigned,
so we have to implement a fixup to do the sign correctly for signed
formats.

CC: 18.0 18.1 
---
 src/amd/vulkan/radv_nir_to_llvm.c | 61 ---
 src/amd/vulkan/radv_pipeline.c| 30 +--
 src/amd/vulkan/radv_private.h |  1 +
 src/amd/vulkan/radv_shader.h  | 12 ++
 4 files changed, 95 insertions(+), 9 deletions(-)

diff --git a/src/amd/vulkan/radv_nir_to_llvm.c 
b/src/amd/vulkan/radv_nir_to_llvm.c
index f98940f0d8f..9d9ecda63cf 100644
--- a/src/amd/vulkan/radv_nir_to_llvm.c
+++ b/src/amd/vulkan/radv_nir_to_llvm.c
@@ -1865,6 +1865,47 @@ static LLVMValueRef radv_get_sampler_desc(struct 
ac_shader_abi *abi,
return ac_build_load_to_sgpr(>ac, list, index);
 }
 
+/* For 2_10_10_10 formats the alpha is handled as unsigned by pre-vega HW.
+ * so we may need to fix it up. */
+static LLVMValueRef
+adjust_vertex_fetch_alpha(struct radv_shader_context *ctx,
+  unsigned adjustment,
+  LLVMValueRef alpha)
+{
+   if (adjustment == RADV_ALPHA_ADJUST_NONE)
+   return alpha;
+
+   LLVMValueRef c30 = LLVMConstInt(ctx->ac.i32, 30, 0);
+
+   if (adjustment == RADV_ALPHA_ADJUST_SSCALED)
+   alpha = LLVMBuildFPToUI(ctx->ac.builder, alpha, ctx->ac.i32, 
"");
+   else
+   alpha = LLVMBuildBitCast(ctx->ac.builder, alpha, ctx->ac.i32, 
"");
+
+   /* For the integer-like cases, do a natural sign extension.
+   *
+   * For the SNORM case, the values are 0.0, 0.333, 0.666, 1.0
+   * and happen to contain 0, 1, 2, 3 as the two LSBs of the
+   * exponent.
+   */
+   alpha = LLVMBuildShl(ctx->ac.builder, alpha,
+adjustment == RADV_ALPHA_ADJUST_SNORM ?
+LLVMConstInt(ctx->ac.i32, 7, 0) : c30, "");
+   alpha = LLVMBuildAShr(ctx->ac.builder, alpha, c30, "");
+
+   /* Convert back to the right type. */
+   if (adjustment == RADV_ALPHA_ADJUST_SNORM) {
+   LLVMValueRef clamp;
+   LLVMValueRef neg_one = LLVMConstReal(ctx->ac.f32, -1.0);
+   alpha = LLVMBuildSIToFP(ctx->ac.builder, alpha, ctx->ac.f32, 
"");
+   clamp = LLVMBuildFCmp(ctx->ac.builder, LLVMRealULT, alpha, 
neg_one, "");
+   alpha = LLVMBuildSelect(ctx->ac.builder, clamp, neg_one, alpha, 
"");
+   } else if (adjustment == RADV_ALPHA_ADJUST_SSCALED) {
+   alpha = LLVMBuildSIToFP(ctx->ac.builder, alpha, ctx->ac.f32, 
"");
+   }
+
+   return alpha;
+}
 
 static void
 handle_vs_input_decl(struct radv_shader_context *ctx,
@@ -1884,9 +1925,11 @@ handle_vs_input_decl(struct radv_shader_context *ctx,
 
variable->data.driver_location = idx * 4;
 
-   for (unsigned i = 0; i < attrib_count; ++i, ++idx) {
-   if (ctx->options->key.vs.instance_rate_inputs & (1u << (index + 
i))) {
-   uint32_t divisor = 
ctx->options->key.vs.instance_rate_divisors[index + i];
+   for (unsigned i = 0; i < attrib_count; ++i, ++idx, ++index) {
+   LLVMValueRef output[4];
+
+   if (ctx->options->key.vs.instance_rate_inputs & (1u << index)) {
+   uint32_t divisor = 
ctx->options->key.vs.instance_rate_divisors[index];
 
if (divisor) {
buffer_index = LLVMBuildAdd(ctx->ac.builder, 
ctx->abi.instance_id,
@@ -1910,7 +1953,7 @@ handle_vs_input_decl(struct radv_shader_context *ctx,
} else
buffer_index = LLVMBuildAdd(ctx->ac.builder, 
ctx->abi.vertex_id,
ctx->abi.base_vertex, "");
-   t_offset = LLVMConstInt(ctx->ac.i32, index + i, false);
+   t_offset = LLVMConstInt(ctx->ac.i32, index, false);
 
t_list = ac_build_load_to_sgpr(>ac, t_list_ptr, t_offset);
 
@@ -1923,9 +1966,15 @@ handle_vs_input_decl(struct radv_shader_context *ctx,
 
for (unsigned chan = 0; chan < 4; chan++) {
LLVMValueRef llvm_chan = LLVMConstInt(ctx->ac.i32, 
chan, false);
+   output[chan] = LLVMBuildExtractElement(ctx->ac.builder, 
input, llvm_chan, "");
+   }
+
+   unsigned alpha_adjust = (ctx->options->key.vs.alpha_adjust >> 
(index * 2)) & 3;
+   output[3] = adjust_vertex_fetch_alpha(ctx, alpha_adjust, 
output[3]);
+
+   for (unsigned chan = 0; chan < 4; chan++) {
ctx->inputs[ac_llvm_reg_index_soa(idx, chan)] =
-   ac_to_integer(>ac, 
LLVMBuildExtractElement(ctx->ac.builder,
-   input, llvm_chan, ""));
+   ac_to_integer(>ac, output[chan]);
}
}
 }
diff --git