Re: [Mesa-dev] [PATCH 2/4] radeonsi: Set range metadata on calls to llvm.SI.tid

[Michel Dänzer]

On 20.04.2016 02:52, Tom Stellard wrote:
> The range metadata tells LLVM the range of expected values for this intrinsic,
> so it can do some additional optimizations on the result.
> ---
>  src/gallium/drivers/radeonsi/si_shader.c | 29 ++---
>  1 file changed, 26 insertions(+), 3 deletions(-)
> 
> diff --git a/src/gallium/drivers/radeonsi/si_shader.c 
> b/src/gallium/drivers/radeonsi/si_shader.c
> index 3b6d6e9..b4f2a42 100644
> --- a/src/gallium/drivers/radeonsi/si_shader.c
> +++ b/src/gallium/drivers/radeonsi/si_shader.c
> @@ -1114,12 +1114,35 @@ static LLVMValueRef get_sample_id(struct 
> radeon_llvm_context *radeon_bld)
>   SI_PARAM_ANCILLARY, 8, 4);
>  }
>  
> +/**
> + * Set range metadata on an instruction.  This can only be used on load and
> + * call instructions.  To specify an instruciton can only produce the values

Typo: instruciton -> instruction

It would also be nice to always write LDS instead of lds in the commit
log of patch 4, but either way, with the above typo fixed, the series is

Reviewed-by: Michel Dänzer 

Nice work!


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/4] radeonsi: Set range metadata on calls to llvm.SI.tid

[Tom Stellard]

On Tue, Apr 19, 2016 at 08:12:08PM +0200, Michael Schellenberger Costa wrote:
> Hi Tom,
> 
> Am 19.04.2016 um 19:52 schrieb Tom Stellard:
> > The range metadata tells LLVM the range of expected values for this 
> > intrinsic,
> > so it can do some additional optimizations on the result.
> > ---
> >  src/gallium/drivers/radeonsi/si_shader.c | 29 ++---
> >  1 file changed, 26 insertions(+), 3 deletions(-)
> >
> > diff --git a/src/gallium/drivers/radeonsi/si_shader.c 
> > b/src/gallium/drivers/radeonsi/si_shader.c
> > index 3b6d6e9..b4f2a42 100644
> > --- a/src/gallium/drivers/radeonsi/si_shader.c
> > +++ b/src/gallium/drivers/radeonsi/si_shader.c
> > @@ -1114,12 +1114,35 @@ static LLVMValueRef get_sample_id(struct 
> > radeon_llvm_context *radeon_bld)
> > SI_PARAM_ANCILLARY, 8, 4);
> >  }
> >  
> > +/**
> > + * Set range metadata on an instruction.  This can only be used on load and
> > + * call instructions.  To specify an instruciton can only produce the 
> > values
> > + * 0, 1, 2, you would do set_range_metadata(value, 0, 3);
> > + * \p lo is the minimum value inclusive.
> > + * \p hi is the maximum value exclusive.
> > + */
> > +static void set_range_metadata(LLVMValueRef value, unsigned lo, unsigned 
> > hi)
> > +{
> > +   const char *range_md_string = "range";
> > +   LLVMValueRef range_md, md_args[2];
> > +   LLVMTypeRef type = LLVMTypeOf(value);
> > +   LLVMContextRef context = LLVMGetTypeContext(type);
> > +   unsigned md_range_id = LLVMGetMDKindIDInContext(context,
> > +   range_md_string, strlen(range_md_string));
> > +
> > +   md_args[0] = LLVMConstInt(type, lo, false);
> > +   md_args[1] = LLVMConstInt(type, hi, false);
> > +   range_md = LLVMMDNodeInContext(context, md_args, 2);
> > +   LLVMSetMetadata(value, md_range_id, range_md);
> > +}
> > +
> >  static LLVMValueRef get_thread_id(struct si_shader_context *ctx)
> >  {
> > struct gallivm_state *gallivm = >radeon_bld.gallivm;
> > -
> > -   return lp_build_intrinsic(gallivm->builder, "llvm.SI.tid", ctx->i32,
> > -  NULL, 0, LLVMReadNoneAttribute);
> > +   LLVMValueRef tid = lp_build_intrinsic(gallivm->builder, "llvm.SI.tid",
> > +   ctx->i32,   NULL, 0, LLVMReadNoneAttribute);
> 
> same here, why not use the helper from patch 1?

This is the helper from patch 1. ;)

-Tom

> Michael
> > +   set_range_metadata(tid, 0, 64);
> > +   return tid;
> >  }
> >  
> >  /**
> 
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/4] radeonsi: Set range metadata on calls to llvm.SI.tid

[Michael Schellenberger Costa]

Hi Tom,

Am 19.04.2016 um 19:52 schrieb Tom Stellard:
> The range metadata tells LLVM the range of expected values for this intrinsic,
> so it can do some additional optimizations on the result.
> ---
>  src/gallium/drivers/radeonsi/si_shader.c | 29 ++---
>  1 file changed, 26 insertions(+), 3 deletions(-)
>
> diff --git a/src/gallium/drivers/radeonsi/si_shader.c 
> b/src/gallium/drivers/radeonsi/si_shader.c
> index 3b6d6e9..b4f2a42 100644
> --- a/src/gallium/drivers/radeonsi/si_shader.c
> +++ b/src/gallium/drivers/radeonsi/si_shader.c
> @@ -1114,12 +1114,35 @@ static LLVMValueRef get_sample_id(struct 
> radeon_llvm_context *radeon_bld)
>   SI_PARAM_ANCILLARY, 8, 4);
>  }
>  
> +/**
> + * Set range metadata on an instruction.  This can only be used on load and
> + * call instructions.  To specify an instruciton can only produce the values
> + * 0, 1, 2, you would do set_range_metadata(value, 0, 3);
> + * \p lo is the minimum value inclusive.
> + * \p hi is the maximum value exclusive.
> + */
> +static void set_range_metadata(LLVMValueRef value, unsigned lo, unsigned hi)
> +{
> + const char *range_md_string = "range";
> + LLVMValueRef range_md, md_args[2];
> + LLVMTypeRef type = LLVMTypeOf(value);
> + LLVMContextRef context = LLVMGetTypeContext(type);
> + unsigned md_range_id = LLVMGetMDKindIDInContext(context,
> + range_md_string, strlen(range_md_string));
> +
> + md_args[0] = LLVMConstInt(type, lo, false);
> + md_args[1] = LLVMConstInt(type, hi, false);
> + range_md = LLVMMDNodeInContext(context, md_args, 2);
> + LLVMSetMetadata(value, md_range_id, range_md);
> +}
> +
>  static LLVMValueRef get_thread_id(struct si_shader_context *ctx)
>  {
>   struct gallivm_state *gallivm = >radeon_bld.gallivm;
> -
> - return lp_build_intrinsic(gallivm->builder, "llvm.SI.tid", ctx->i32,
> -NULL, 0, LLVMReadNoneAttribute);
> + LLVMValueRef tid = lp_build_intrinsic(gallivm->builder, "llvm.SI.tid",
> + ctx->i32,   NULL, 0, LLVMReadNoneAttribute);

same here, why not use the helper from patch 1?
Michael
> + set_range_metadata(tid, 0, 64);
> + return tid;
>  }
>  
>  /**

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

[Mesa-dev] [PATCH 2/4] radeonsi: Set range metadata on calls to llvm.SI.tid

[Tom Stellard]

The range metadata tells LLVM the range of expected values for this intrinsic,
so it can do some additional optimizations on the result.
---
 src/gallium/drivers/radeonsi/si_shader.c | 29 ++---
 1 file changed, 26 insertions(+), 3 deletions(-)

diff --git a/src/gallium/drivers/radeonsi/si_shader.c 
b/src/gallium/drivers/radeonsi/si_shader.c
index 3b6d6e9..b4f2a42 100644
--- a/src/gallium/drivers/radeonsi/si_shader.c
+++ b/src/gallium/drivers/radeonsi/si_shader.c
@@ -1114,12 +1114,35 @@ static LLVMValueRef get_sample_id(struct 
radeon_llvm_context *radeon_bld)
SI_PARAM_ANCILLARY, 8, 4);
 }
 
+/**
+ * Set range metadata on an instruction.  This can only be used on load and
+ * call instructions.  To specify an instruciton can only produce the values
+ * 0, 1, 2, you would do set_range_metadata(value, 0, 3);
+ * \p lo is the minimum value inclusive.
+ * \p hi is the maximum value exclusive.
+ */
+static void set_range_metadata(LLVMValueRef value, unsigned lo, unsigned hi)
+{
+   const char *range_md_string = "range";
+   LLVMValueRef range_md, md_args[2];
+   LLVMTypeRef type = LLVMTypeOf(value);
+   LLVMContextRef context = LLVMGetTypeContext(type);
+   unsigned md_range_id = LLVMGetMDKindIDInContext(context,
+   range_md_string, strlen(range_md_string));
+
+   md_args[0] = LLVMConstInt(type, lo, false);
+   md_args[1] = LLVMConstInt(type, hi, false);
+   range_md = LLVMMDNodeInContext(context, md_args, 2);
+   LLVMSetMetadata(value, md_range_id, range_md);
+}
+
 static LLVMValueRef get_thread_id(struct si_shader_context *ctx)
 {
struct gallivm_state *gallivm = >radeon_bld.gallivm;
-
-   return lp_build_intrinsic(gallivm->builder, "llvm.SI.tid", ctx->i32,
-  NULL, 0, LLVMReadNoneAttribute);
+   LLVMValueRef tid = lp_build_intrinsic(gallivm->builder, "llvm.SI.tid",
+   ctx->i32,   NULL, 0, LLVMReadNoneAttribute);
+   set_range_metadata(tid, 0, 64);
+   return tid;
 }
 
 /**
-- 
2.1.0

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