Re: [Mesa-dev] [PATCH 2/2] ac: use the common helper ac_apply_fmask_to_sample

2019-04-12 Thread Samuel Pitoiset


On 4/12/19 5:04 PM, Marek Olšák wrote:


On Thu, Apr 11, 2019 at 3:15 AM Samuel Pitoiset 
mailto:samuel.pitoi...@gmail.com>> wrote:



On 4/11/19 3:30 AM, Marek Olšák wrote:
> From: Marek Olšák mailto:marek.ol...@amd.com>>
>
> ---
>   src/amd/common/ac_nir_to_llvm.c | 70
+++--
>   1 file changed, 5 insertions(+), 65 deletions(-)
>
> diff --git a/src/amd/common/ac_nir_to_llvm.c
b/src/amd/common/ac_nir_to_llvm.c
> index 3d2f738edec..3abde6e0969 100644
> --- a/src/amd/common/ac_nir_to_llvm.c
> +++ b/src/amd/common/ac_nir_to_llvm.c
> @@ -2323,92 +2323,32 @@ static int
image_type_to_components_count(enum glsl_sampler_dim dim, bool array)
>       case GLSL_SAMPLER_DIM_SUBPASS:
>               return 2;
>       case GLSL_SAMPLER_DIM_SUBPASS_MS:
>               return 3;
>       default:
>               break;
>       }
>       return 0;
>   }
>
> -
> -/* Adjust the sample index according to FMASK.
> - *
> - * For uncompressed MSAA surfaces, FMASK should return 0x76543210,
> - * which is the identity mapping. Each nibble says which
physical sample
> - * should be fetched to get that sample.
> - *
> - * For example, 0x1100 means there are only 2 samples
stored and
> - * the second sample covers 3/4 of the pixel. When reading
samples 0
> - * and 1, return physical sample 0 (determined by the first two 0s
> - * in FMASK), otherwise return physical sample 1.
> - *
> - * The sample index should be adjusted as follows:
> - *   sample_index = (fmask >> (sample_index * 4)) & 0xF;
> - */
>   static LLVMValueRef adjust_sample_index_using_fmask(struct
ac_llvm_context *ctx,
>  LLVMValueRef coord_x, LLVMValueRef coord_y,
>  LLVMValueRef coord_z,
>  LLVMValueRef sample_index,
>  LLVMValueRef fmask_desc_ptr)
>   {
> -     struct ac_image_args args = {0};
> -     LLVMValueRef res;
> +     unsigned sample_chan = coord_z ? 3 : 2;
> +     LLVMValueRef addr[4] = {coord_x, coord_y, coord_z};
> +     addr[sample_chan] = sample_index;
>
> -     args.coords[0] = coord_x;
> -     args.coords[1] = coord_y;
> -     if (coord_z)
> -             args.coords[2] = coord_z;
> -
> -     args.opcode = ac_image_load;
> -     args.dim = coord_z ? ac_image_2darray : ac_image_2d;
> -     args.resource = fmask_desc_ptr;
> -     args.dmask = 0xf;
> -     args.attributes = AC_FUNC_ATTR_READNONE;
> -
> -     res = ac_build_image_opcode(ctx, );
> -
> -     res = ac_to_integer(ctx, res);
> -     LLVMValueRef four = LLVMConstInt(ctx->i32, 4, false);
> -     LLVMValueRef F = LLVMConstInt(ctx->i32, 0xf, false);
> -
> -     LLVMValueRef fmask = LLVMBuildExtractElement(ctx->builder,
> -                                                  res,
> - ctx->i32_0, "");
> -
> -     LLVMValueRef sample_index4 =
> -             LLVMBuildMul(ctx->builder, sample_index, four, "");
> -     LLVMValueRef shifted_fmask =
> -             LLVMBuildLShr(ctx->builder, fmask, sample_index4, "");
> -     LLVMValueRef final_sample =
> -             LLVMBuildAnd(ctx->builder, shifted_fmask, F, "");

The only difference is the mask (ie. ac_apply_fmask_to_sample uses
0x7)
while this code uses 0xF.


Yes.


According to the comment in that function, I assume 0x7 is the
correct
value?


Yes, it's for EQAA. Only samples 0-7 can occur with MSAA. If EQAA is 
used, 0x8 means the color of the sample is unknown, which is mapped to 
sample 0 by the 0x7 mask.

Reviewed-by: Samuel Pitoiset 


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

Re: [Mesa-dev] [PATCH 2/2] ac: use the common helper ac_apply_fmask_to_sample

2019-04-12 Thread Marek Olšák
On Thu, Apr 11, 2019 at 3:15 AM Samuel Pitoiset 
wrote:

>
> On 4/11/19 3:30 AM, Marek Olšák wrote:
> > From: Marek Olšák 
> >
> > ---
> >   src/amd/common/ac_nir_to_llvm.c | 70 +++--
> >   1 file changed, 5 insertions(+), 65 deletions(-)
> >
> > diff --git a/src/amd/common/ac_nir_to_llvm.c
> b/src/amd/common/ac_nir_to_llvm.c
> > index 3d2f738edec..3abde6e0969 100644
> > --- a/src/amd/common/ac_nir_to_llvm.c
> > +++ b/src/amd/common/ac_nir_to_llvm.c
> > @@ -2323,92 +2323,32 @@ static int image_type_to_components_count(enum
> glsl_sampler_dim dim, bool array)
> >   case GLSL_SAMPLER_DIM_SUBPASS:
> >   return 2;
> >   case GLSL_SAMPLER_DIM_SUBPASS_MS:
> >   return 3;
> >   default:
> >   break;
> >   }
> >   return 0;
> >   }
> >
> > -
> > -/* Adjust the sample index according to FMASK.
> > - *
> > - * For uncompressed MSAA surfaces, FMASK should return 0x76543210,
> > - * which is the identity mapping. Each nibble says which physical sample
> > - * should be fetched to get that sample.
> > - *
> > - * For example, 0x1100 means there are only 2 samples stored and
> > - * the second sample covers 3/4 of the pixel. When reading samples 0
> > - * and 1, return physical sample 0 (determined by the first two 0s
> > - * in FMASK), otherwise return physical sample 1.
> > - *
> > - * The sample index should be adjusted as follows:
> > - *   sample_index = (fmask >> (sample_index * 4)) & 0xF;
> > - */
> >   static LLVMValueRef adjust_sample_index_using_fmask(struct
> ac_llvm_context *ctx,
> >   LLVMValueRef coord_x,
> LLVMValueRef coord_y,
> >   LLVMValueRef coord_z,
> >   LLVMValueRef
> sample_index,
> >   LLVMValueRef
> fmask_desc_ptr)
> >   {
> > - struct ac_image_args args = {0};
> > - LLVMValueRef res;
> > + unsigned sample_chan = coord_z ? 3 : 2;
> > + LLVMValueRef addr[4] = {coord_x, coord_y, coord_z};
> > + addr[sample_chan] = sample_index;
> >
> > - args.coords[0] = coord_x;
> > - args.coords[1] = coord_y;
> > - if (coord_z)
> > - args.coords[2] = coord_z;
> > -
> > - args.opcode = ac_image_load;
> > - args.dim = coord_z ? ac_image_2darray : ac_image_2d;
> > - args.resource = fmask_desc_ptr;
> > - args.dmask = 0xf;
> > - args.attributes = AC_FUNC_ATTR_READNONE;
> > -
> > - res = ac_build_image_opcode(ctx, );
> > -
> > - res = ac_to_integer(ctx, res);
> > - LLVMValueRef four = LLVMConstInt(ctx->i32, 4, false);
> > - LLVMValueRef F = LLVMConstInt(ctx->i32, 0xf, false);
> > -
> > - LLVMValueRef fmask = LLVMBuildExtractElement(ctx->builder,
> > -  res,
> > -  ctx->i32_0, "");
> > -
> > - LLVMValueRef sample_index4 =
> > - LLVMBuildMul(ctx->builder, sample_index, four, "");
> > - LLVMValueRef shifted_fmask =
> > - LLVMBuildLShr(ctx->builder, fmask, sample_index4, "");
> > - LLVMValueRef final_sample =
> > - LLVMBuildAnd(ctx->builder, shifted_fmask, F, "");
>
> The only difference is the mask (ie. ac_apply_fmask_to_sample uses 0x7)
> while this code uses 0xF.
>

Yes.


>
> According to the comment in that function, I assume 0x7 is the correct
> value?
>

Yes, it's for EQAA. Only samples 0-7 can occur with MSAA. If EQAA is used,
0x8 means the color of the sample is unknown, which is mapped to sample 0
by the 0x7 mask.

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

Re: [Mesa-dev] [PATCH 2/2] ac: use the common helper ac_apply_fmask_to_sample

2019-04-11 Thread Samuel Pitoiset


On 4/11/19 3:30 AM, Marek Olšák wrote:

From: Marek Olšák 

---
  src/amd/common/ac_nir_to_llvm.c | 70 +++--
  1 file changed, 5 insertions(+), 65 deletions(-)

diff --git a/src/amd/common/ac_nir_to_llvm.c b/src/amd/common/ac_nir_to_llvm.c
index 3d2f738edec..3abde6e0969 100644
--- a/src/amd/common/ac_nir_to_llvm.c
+++ b/src/amd/common/ac_nir_to_llvm.c
@@ -2323,92 +2323,32 @@ static int image_type_to_components_count(enum 
glsl_sampler_dim dim, bool array)
case GLSL_SAMPLER_DIM_SUBPASS:
return 2;
case GLSL_SAMPLER_DIM_SUBPASS_MS:
return 3;
default:
break;
}
return 0;
  }
  
-

-/* Adjust the sample index according to FMASK.
- *
- * For uncompressed MSAA surfaces, FMASK should return 0x76543210,
- * which is the identity mapping. Each nibble says which physical sample
- * should be fetched to get that sample.
- *
- * For example, 0x1100 means there are only 2 samples stored and
- * the second sample covers 3/4 of the pixel. When reading samples 0
- * and 1, return physical sample 0 (determined by the first two 0s
- * in FMASK), otherwise return physical sample 1.
- *
- * The sample index should be adjusted as follows:
- *   sample_index = (fmask >> (sample_index * 4)) & 0xF;
- */
  static LLVMValueRef adjust_sample_index_using_fmask(struct ac_llvm_context 
*ctx,
LLVMValueRef coord_x, 
LLVMValueRef coord_y,
LLVMValueRef coord_z,
LLVMValueRef sample_index,
LLVMValueRef fmask_desc_ptr)
  {
-   struct ac_image_args args = {0};
-   LLVMValueRef res;
+   unsigned sample_chan = coord_z ? 3 : 2;
+   LLVMValueRef addr[4] = {coord_x, coord_y, coord_z};
+   addr[sample_chan] = sample_index;
  
-	args.coords[0] = coord_x;

-   args.coords[1] = coord_y;
-   if (coord_z)
-   args.coords[2] = coord_z;
-
-   args.opcode = ac_image_load;
-   args.dim = coord_z ? ac_image_2darray : ac_image_2d;
-   args.resource = fmask_desc_ptr;
-   args.dmask = 0xf;
-   args.attributes = AC_FUNC_ATTR_READNONE;
-
-   res = ac_build_image_opcode(ctx, );
-
-   res = ac_to_integer(ctx, res);
-   LLVMValueRef four = LLVMConstInt(ctx->i32, 4, false);
-   LLVMValueRef F = LLVMConstInt(ctx->i32, 0xf, false);
-
-   LLVMValueRef fmask = LLVMBuildExtractElement(ctx->builder,
-res,
-ctx->i32_0, "");
-
-   LLVMValueRef sample_index4 =
-   LLVMBuildMul(ctx->builder, sample_index, four, "");
-   LLVMValueRef shifted_fmask =
-   LLVMBuildLShr(ctx->builder, fmask, sample_index4, "");
-   LLVMValueRef final_sample =
-   LLVMBuildAnd(ctx->builder, shifted_fmask, F, "");


The only difference is the mask (ie. ac_apply_fmask_to_sample uses 0x7) 
while this code uses 0xF.


According to the comment in that function, I assume 0x7 is the correct 
value?



-
-   /* Don't rewrite the sample index if WORD1.DATA_FORMAT of the FMASK
-* resource descriptor is 0 (invalid),
-*/
-   LLVMValueRef fmask_desc =
-   LLVMBuildBitCast(ctx->builder, fmask_desc_ptr,
-ctx->v8i32, "");
-
-   LLVMValueRef fmask_word1 =
-   LLVMBuildExtractElement(ctx->builder, fmask_desc,
-   ctx->i32_1, "");
-
-   LLVMValueRef word1_is_nonzero =
-   LLVMBuildICmp(ctx->builder, LLVMIntNE,
- fmask_word1, ctx->i32_0, "");
-
-   /* Replace the MSAA sample index. */
-   sample_index =
-   LLVMBuildSelect(ctx->builder, word1_is_nonzero,
-   final_sample, sample_index, "");
-   return sample_index;
+   ac_apply_fmask_to_sample(ctx, fmask_desc_ptr, addr, coord_z != NULL);
+   return addr[sample_chan];
  }
  
  static nir_deref_instr *get_image_deref(const nir_intrinsic_instr *instr)

  {
assert(instr->src[0].is_ssa);
return nir_instr_as_deref(instr->src[0].ssa->parent_instr);
  }
  
  static LLVMValueRef get_image_descriptor(struct ac_nir_context *ctx,

   const nir_intrinsic_instr *instr,

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

[Mesa-dev] [PATCH 2/2] ac: use the common helper ac_apply_fmask_to_sample

2019-04-10 Thread Marek Olšák
From: Marek Olšák 

---
 src/amd/common/ac_nir_to_llvm.c | 70 +++--
 1 file changed, 5 insertions(+), 65 deletions(-)

diff --git a/src/amd/common/ac_nir_to_llvm.c b/src/amd/common/ac_nir_to_llvm.c
index 3d2f738edec..3abde6e0969 100644
--- a/src/amd/common/ac_nir_to_llvm.c
+++ b/src/amd/common/ac_nir_to_llvm.c
@@ -2323,92 +2323,32 @@ static int image_type_to_components_count(enum 
glsl_sampler_dim dim, bool array)
case GLSL_SAMPLER_DIM_SUBPASS:
return 2;
case GLSL_SAMPLER_DIM_SUBPASS_MS:
return 3;
default:
break;
}
return 0;
 }
 
-
-/* Adjust the sample index according to FMASK.
- *
- * For uncompressed MSAA surfaces, FMASK should return 0x76543210,
- * which is the identity mapping. Each nibble says which physical sample
- * should be fetched to get that sample.
- *
- * For example, 0x1100 means there are only 2 samples stored and
- * the second sample covers 3/4 of the pixel. When reading samples 0
- * and 1, return physical sample 0 (determined by the first two 0s
- * in FMASK), otherwise return physical sample 1.
- *
- * The sample index should be adjusted as follows:
- *   sample_index = (fmask >> (sample_index * 4)) & 0xF;
- */
 static LLVMValueRef adjust_sample_index_using_fmask(struct ac_llvm_context 
*ctx,
LLVMValueRef coord_x, 
LLVMValueRef coord_y,
LLVMValueRef coord_z,
LLVMValueRef sample_index,
LLVMValueRef fmask_desc_ptr)
 {
-   struct ac_image_args args = {0};
-   LLVMValueRef res;
+   unsigned sample_chan = coord_z ? 3 : 2;
+   LLVMValueRef addr[4] = {coord_x, coord_y, coord_z};
+   addr[sample_chan] = sample_index;
 
-   args.coords[0] = coord_x;
-   args.coords[1] = coord_y;
-   if (coord_z)
-   args.coords[2] = coord_z;
-
-   args.opcode = ac_image_load;
-   args.dim = coord_z ? ac_image_2darray : ac_image_2d;
-   args.resource = fmask_desc_ptr;
-   args.dmask = 0xf;
-   args.attributes = AC_FUNC_ATTR_READNONE;
-
-   res = ac_build_image_opcode(ctx, );
-
-   res = ac_to_integer(ctx, res);
-   LLVMValueRef four = LLVMConstInt(ctx->i32, 4, false);
-   LLVMValueRef F = LLVMConstInt(ctx->i32, 0xf, false);
-
-   LLVMValueRef fmask = LLVMBuildExtractElement(ctx->builder,
-res,
-ctx->i32_0, "");
-
-   LLVMValueRef sample_index4 =
-   LLVMBuildMul(ctx->builder, sample_index, four, "");
-   LLVMValueRef shifted_fmask =
-   LLVMBuildLShr(ctx->builder, fmask, sample_index4, "");
-   LLVMValueRef final_sample =
-   LLVMBuildAnd(ctx->builder, shifted_fmask, F, "");
-
-   /* Don't rewrite the sample index if WORD1.DATA_FORMAT of the FMASK
-* resource descriptor is 0 (invalid),
-*/
-   LLVMValueRef fmask_desc =
-   LLVMBuildBitCast(ctx->builder, fmask_desc_ptr,
-ctx->v8i32, "");
-
-   LLVMValueRef fmask_word1 =
-   LLVMBuildExtractElement(ctx->builder, fmask_desc,
-   ctx->i32_1, "");
-
-   LLVMValueRef word1_is_nonzero =
-   LLVMBuildICmp(ctx->builder, LLVMIntNE,
- fmask_word1, ctx->i32_0, "");
-
-   /* Replace the MSAA sample index. */
-   sample_index =
-   LLVMBuildSelect(ctx->builder, word1_is_nonzero,
-   final_sample, sample_index, "");
-   return sample_index;
+   ac_apply_fmask_to_sample(ctx, fmask_desc_ptr, addr, coord_z != NULL);
+   return addr[sample_chan];
 }
 
 static nir_deref_instr *get_image_deref(const nir_intrinsic_instr *instr)
 {
assert(instr->src[0].is_ssa);
return nir_instr_as_deref(instr->src[0].ssa->parent_instr);
 }
 
 static LLVMValueRef get_image_descriptor(struct ac_nir_context *ctx,
  const nir_intrinsic_instr *instr,
-- 
2.17.1

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