Re: [Mesa-dev] [PATCH] ac/nir: fix image dimension for subpass attachments

2018-04-23 Thread Samuel Pitoiset 



On 04/23/2018 01:24 PM, Nicolai Hähnle wrote:

On 20.04.2018 18:06, Samuel Pitoiset wrote:

For subpass attachments we need one more coordinate with
the sample index, so make them array types.


Sorry about that. Shouldn't it be layer index instead of sample index 
though?


No worries!

Yes, should be layer index. I have pushed the patch on Friday but I'm 
going to send a new one with your suggestion.




If I understand this right, I think it would be cleaner to just change 
get_ac_sampler_dim to say:


 case GLSL_SAMPLER_DIM_SUBPASS:
     return ac_image_2darray;
 case GLSL_SAMPLER_DIM_SUBPASS_MS:
     return ac_image_2darraymsaa;

That way, the code very clearly documents that subpasses are always of 
array type.


Thanks,
Nicolai





This fixes a bunch of CTS fails with RADV.

Fixes: 24fb3e6aa1 ("ac/nir: use ac_build_image_opcode for image 
intrinsics")

Signed-off-by: Samuel Pitoiset 
---
  src/amd/common/ac_nir_to_llvm.c | 18 +++---
  1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/src/amd/common/ac_nir_to_llvm.c 
b/src/amd/common/ac_nir_to_llvm.c

index ba7f353a9a..72c773522f 100644
--- a/src/amd/common/ac_nir_to_llvm.c
+++ b/src/amd/common/ac_nir_to_llvm.c
@@ -2090,6 +2090,18 @@ static LLVMValueRef 
adjust_sample_index_using_fmask(struct ac_llvm_context *ctx,

  return sample_index;
  }
+static bool
+glsl_is_array_image(const struct glsl_type *type)
+{
+    const enum glsl_sampler_dim dim = glsl_get_sampler_dim(type);
+
+    if (glsl_sampler_type_is_array(type))
+    return true;
+
+    return dim == GLSL_SAMPLER_DIM_SUBPASS ||
+   dim == GLSL_SAMPLER_DIM_SUBPASS_MS;
+}
+
  static void get_image_coords(struct ac_nir_context *ctx,
   const nir_intrinsic_instr *instr,
   struct ac_image_args *args)
@@ -2235,7 +2247,7 @@ static LLVMValueRef visit_image_load(struct 
ac_nir_context *ctx,

  args.resource = get_sampler_desc(ctx, instr->variables[0],
   AC_DESC_IMAGE, NULL, true, false);
  args.dim = get_ac_image_dim(>ac, 
glsl_get_sampler_dim(type),

-    glsl_sampler_type_is_array(type));
+    glsl_is_array_image(type));
  args.dmask = 15;
  args.attributes = AC_FUNC_ATTR_READONLY;
  if (var->data.image._volatile || var->data.image.coherent)
@@ -2278,7 +2290,7 @@ static void visit_image_store(struct 
ac_nir_context *ctx,

  args.resource = get_sampler_desc(ctx, instr->variables[0],
   AC_DESC_IMAGE, NULL, true, false);
  args.dim = get_ac_image_dim(>ac, 
glsl_get_sampler_dim(type),

-    glsl_sampler_type_is_array(type));
+    glsl_is_array_image(type));
  args.dmask = 15;
  if (force_glc || var->data.image._volatile || 
var->data.image.coherent)

  args.cache_policy |= ac_glc;
@@ -2369,7 +2381,7 @@ static LLVMValueRef visit_image_atomic(struct 
ac_nir_context *ctx,

  args.resource = get_sampler_desc(ctx, instr->variables[0],
   AC_DESC_IMAGE, NULL, true, false);
  args.dim = get_ac_image_dim(>ac, 
glsl_get_sampler_dim(type),

-    glsl_sampler_type_is_array(type));
+    glsl_is_array_image(type));
  return ac_build_image_opcode(>ac, );
  }





___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] ac/nir: fix image dimension for subpass attachments

2018-04-23 Thread Nicolai Hähnle 

On 20.04.2018 18:06, Samuel Pitoiset wrote:

For subpass attachments we need one more coordinate with
the sample index, so make them array types.


Sorry about that. Shouldn't it be layer index instead of sample index 
though?


If I understand this right, I think it would be cleaner to just change 
get_ac_sampler_dim to say:


case GLSL_SAMPLER_DIM_SUBPASS:
return ac_image_2darray;
case GLSL_SAMPLER_DIM_SUBPASS_MS:
return ac_image_2darraymsaa;

That way, the code very clearly documents that subpasses are always of 
array type.


Thanks,
Nicolai





This fixes a bunch of CTS fails with RADV.

Fixes: 24fb3e6aa1 ("ac/nir: use ac_build_image_opcode for image intrinsics")
Signed-off-by: Samuel Pitoiset 
---
  src/amd/common/ac_nir_to_llvm.c | 18 +++---
  1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/src/amd/common/ac_nir_to_llvm.c b/src/amd/common/ac_nir_to_llvm.c
index ba7f353a9a..72c773522f 100644
--- a/src/amd/common/ac_nir_to_llvm.c
+++ b/src/amd/common/ac_nir_to_llvm.c
@@ -2090,6 +2090,18 @@ static LLVMValueRef 
adjust_sample_index_using_fmask(struct ac_llvm_context *ctx,
return sample_index;
  }
  
+static bool

+glsl_is_array_image(const struct glsl_type *type)
+{
+   const enum glsl_sampler_dim dim = glsl_get_sampler_dim(type);
+
+   if (glsl_sampler_type_is_array(type))
+   return true;
+
+   return dim == GLSL_SAMPLER_DIM_SUBPASS ||
+  dim == GLSL_SAMPLER_DIM_SUBPASS_MS;
+}
+
  static void get_image_coords(struct ac_nir_context *ctx,
 const nir_intrinsic_instr *instr,
 struct ac_image_args *args)
@@ -2235,7 +2247,7 @@ static LLVMValueRef visit_image_load(struct 
ac_nir_context *ctx,
args.resource = get_sampler_desc(ctx, instr->variables[0],
 AC_DESC_IMAGE, NULL, true, 
false);
args.dim = get_ac_image_dim(>ac, 
glsl_get_sampler_dim(type),
-   glsl_sampler_type_is_array(type));
+   glsl_is_array_image(type));
args.dmask = 15;
args.attributes = AC_FUNC_ATTR_READONLY;
if (var->data.image._volatile || var->data.image.coherent)
@@ -2278,7 +2290,7 @@ static void visit_image_store(struct ac_nir_context *ctx,
args.resource = get_sampler_desc(ctx, instr->variables[0],
 AC_DESC_IMAGE, NULL, true, 
false);
args.dim = get_ac_image_dim(>ac, 
glsl_get_sampler_dim(type),
-   glsl_sampler_type_is_array(type));
+   glsl_is_array_image(type));
args.dmask = 15;
if (force_glc || var->data.image._volatile || 
var->data.image.coherent)
args.cache_policy |= ac_glc;
@@ -2369,7 +2381,7 @@ static LLVMValueRef visit_image_atomic(struct 
ac_nir_context *ctx,
args.resource = get_sampler_desc(ctx, instr->variables[0],
 AC_DESC_IMAGE, NULL, true, 
false);
args.dim = get_ac_image_dim(>ac, 
glsl_get_sampler_dim(type),
-   glsl_sampler_type_is_array(type));
+   glsl_is_array_image(type));
  
  		return ac_build_image_opcode(>ac, );

}




--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] ac/nir: fix image dimension for subpass attachments

2018-04-20 Thread Samuel Pitoiset 
For subpass attachments we need one more coordinate with
the sample index, so make them array types.

This fixes a bunch of CTS fails with RADV.

Fixes: 24fb3e6aa1 ("ac/nir: use ac_build_image_opcode for image intrinsics")
Signed-off-by: Samuel Pitoiset 
---
 src/amd/common/ac_nir_to_llvm.c | 18 +++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/src/amd/common/ac_nir_to_llvm.c b/src/amd/common/ac_nir_to_llvm.c
index ba7f353a9a..72c773522f 100644
--- a/src/amd/common/ac_nir_to_llvm.c
+++ b/src/amd/common/ac_nir_to_llvm.c
@@ -2090,6 +2090,18 @@ static LLVMValueRef 
adjust_sample_index_using_fmask(struct ac_llvm_context *ctx,
return sample_index;
 }
 
+static bool
+glsl_is_array_image(const struct glsl_type *type)
+{
+   const enum glsl_sampler_dim dim = glsl_get_sampler_dim(type);
+
+   if (glsl_sampler_type_is_array(type))
+   return true;
+
+   return dim == GLSL_SAMPLER_DIM_SUBPASS ||
+  dim == GLSL_SAMPLER_DIM_SUBPASS_MS;
+}
+
 static void get_image_coords(struct ac_nir_context *ctx,
 const nir_intrinsic_instr *instr,
 struct ac_image_args *args)
@@ -2235,7 +2247,7 @@ static LLVMValueRef visit_image_load(struct 
ac_nir_context *ctx,
args.resource = get_sampler_desc(ctx, instr->variables[0],
 AC_DESC_IMAGE, NULL, true, 
false);
args.dim = get_ac_image_dim(>ac, 
glsl_get_sampler_dim(type),
-   glsl_sampler_type_is_array(type));
+   glsl_is_array_image(type));
args.dmask = 15;
args.attributes = AC_FUNC_ATTR_READONLY;
if (var->data.image._volatile || var->data.image.coherent)
@@ -2278,7 +2290,7 @@ static void visit_image_store(struct ac_nir_context *ctx,
args.resource = get_sampler_desc(ctx, instr->variables[0],
 AC_DESC_IMAGE, NULL, true, 
false);
args.dim = get_ac_image_dim(>ac, 
glsl_get_sampler_dim(type),
-   glsl_sampler_type_is_array(type));
+   glsl_is_array_image(type));
args.dmask = 15;
if (force_glc || var->data.image._volatile || 
var->data.image.coherent)
args.cache_policy |= ac_glc;
@@ -2369,7 +2381,7 @@ static LLVMValueRef visit_image_atomic(struct 
ac_nir_context *ctx,
args.resource = get_sampler_desc(ctx, instr->variables[0],
 AC_DESC_IMAGE, NULL, true, 
false);
args.dim = get_ac_image_dim(>ac, 
glsl_get_sampler_dim(type),
-   glsl_sampler_type_is_array(type));
+   glsl_is_array_image(type));
 
return ac_build_image_opcode(>ac, );
}
-- 
2.17.0

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev