Re: [Mesa-dev] [PATCH 1/2] amd/common: import get_{load, store}_intr_attribs() from RadeonSI

2018-01-10 Thread Samuel Pitoiset 



On 01/10/2018 02:24 PM, Grazvydas Ignotas wrote:

On Wed, Jan 10, 2018 at 1:57 PM, Samuel Pitoiset
 wrote:

Signed-off-by: Samuel Pitoiset 
---
  src/amd/common/ac_llvm_build.c| 12 ++-
  src/amd/common/ac_llvm_util.c | 18 
  src/amd/common/ac_llvm_util.h |  6 ++
  src/gallium/drivers/radeonsi/si_shader_tgsi_mem.c | 26 +--
  4 files changed, 31 insertions(+), 31 deletions(-)

diff --git a/src/amd/common/ac_llvm_build.c b/src/amd/common/ac_llvm_build.c
index efc6fa12e5..07044142b0 100644
--- a/src/amd/common/ac_llvm_build.c
+++ b/src/amd/common/ac_llvm_build.c
@@ -983,11 +983,7 @@ ac_build_buffer_load(struct ac_llvm_context *ctx,

 return ac_build_intrinsic(ctx, name, types[func], args,
   ARRAY_SIZE(args),
- /* READNONE means writes can't affect it, 
while
-  * READONLY means that writes can affect it. 
*/
- can_speculate && HAVE_LLVM >= 0x0400 ?
- AC_FUNC_ATTR_READNONE :
- AC_FUNC_ATTR_READONLY);
+ ac_get_load_intr_attribs(can_speculate));
  }

  LLVMValueRef ac_build_buffer_load_format(struct ac_llvm_context *ctx,
@@ -1007,11 +1003,7 @@ LLVMValueRef ac_build_buffer_load_format(struct 
ac_llvm_context *ctx,
 return ac_build_intrinsic(ctx,
   "llvm.amdgcn.buffer.load.format.v4f32",
   ctx->v4f32, args, ARRAY_SIZE(args),
- /* READNONE means writes can't affect it, 
while
-  * READONLY means that writes can affect it. 
*/
- can_speculate && HAVE_LLVM >= 0x0400 ?
- AC_FUNC_ATTR_READNONE :
- AC_FUNC_ATTR_READONLY);
+ ac_get_load_intr_attribs(can_speculate));
  }

  /**
diff --git a/src/amd/common/ac_llvm_util.c b/src/amd/common/ac_llvm_util.c
index 429904c040..976d6540a1 100644
--- a/src/amd/common/ac_llvm_util.c
+++ b/src/amd/common/ac_llvm_util.c
@@ -240,3 +240,21 @@ ac_llvm_add_target_dep_function_attr(LLVMValueRef F,
 snprintf(str, sizeof(str), "%i", value);
 LLVMAddTargetDependentFunctionAttr(F, name, str);
  }
+
+unsigned
+ac_get_load_intr_attribs(bool can_speculate)
+{
+   /* READNONE means writes can't affect it, while READONLY means that
+* writes can affect it. */
+   return can_speculate && HAVE_LLVM >= 0x0400 ?
+AC_FUNC_ATTR_READNONE :
+AC_FUNC_ATTR_READONLY;
+}
+
+unsigned
+ac_get_store_intr_attribs(bool writeonly_memory)
+{
+   return writeonly_memory && HAVE_LLVM >= 0x0400 ?
+ AC_FUNC_ATTR_INACCESSIBLE_MEM_ONLY :
+ AC_FUNC_ATTR_WRITEONLY;
+}


What about putting such tiny functions in the header as static inline
to avoid function call overheads?


I can do that, yes.



Gražvydas


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

Re: [Mesa-dev] [PATCH 1/2] amd/common: import get_{load, store}_intr_attribs() from RadeonSI

2018-01-10 Thread Grazvydas Ignotas 
On Wed, Jan 10, 2018 at 1:57 PM, Samuel Pitoiset
 wrote:
> Signed-off-by: Samuel Pitoiset 
> ---
>  src/amd/common/ac_llvm_build.c| 12 ++-
>  src/amd/common/ac_llvm_util.c | 18 
>  src/amd/common/ac_llvm_util.h |  6 ++
>  src/gallium/drivers/radeonsi/si_shader_tgsi_mem.c | 26 
> +--
>  4 files changed, 31 insertions(+), 31 deletions(-)
>
> diff --git a/src/amd/common/ac_llvm_build.c b/src/amd/common/ac_llvm_build.c
> index efc6fa12e5..07044142b0 100644
> --- a/src/amd/common/ac_llvm_build.c
> +++ b/src/amd/common/ac_llvm_build.c
> @@ -983,11 +983,7 @@ ac_build_buffer_load(struct ac_llvm_context *ctx,
>
> return ac_build_intrinsic(ctx, name, types[func], args,
>   ARRAY_SIZE(args),
> - /* READNONE means writes can't affect it, 
> while
> -  * READONLY means that writes can affect 
> it. */
> - can_speculate && HAVE_LLVM >= 0x0400 ?
> - AC_FUNC_ATTR_READNONE :
> - AC_FUNC_ATTR_READONLY);
> + ac_get_load_intr_attribs(can_speculate));
>  }
>
>  LLVMValueRef ac_build_buffer_load_format(struct ac_llvm_context *ctx,
> @@ -1007,11 +1003,7 @@ LLVMValueRef ac_build_buffer_load_format(struct 
> ac_llvm_context *ctx,
> return ac_build_intrinsic(ctx,
>   "llvm.amdgcn.buffer.load.format.v4f32",
>   ctx->v4f32, args, ARRAY_SIZE(args),
> - /* READNONE means writes can't affect it, 
> while
> -  * READONLY means that writes can affect 
> it. */
> - can_speculate && HAVE_LLVM >= 0x0400 ?
> - AC_FUNC_ATTR_READNONE :
> - AC_FUNC_ATTR_READONLY);
> + ac_get_load_intr_attribs(can_speculate));
>  }
>
>  /**
> diff --git a/src/amd/common/ac_llvm_util.c b/src/amd/common/ac_llvm_util.c
> index 429904c040..976d6540a1 100644
> --- a/src/amd/common/ac_llvm_util.c
> +++ b/src/amd/common/ac_llvm_util.c
> @@ -240,3 +240,21 @@ ac_llvm_add_target_dep_function_attr(LLVMValueRef F,
> snprintf(str, sizeof(str), "%i", value);
> LLVMAddTargetDependentFunctionAttr(F, name, str);
>  }
> +
> +unsigned
> +ac_get_load_intr_attribs(bool can_speculate)
> +{
> +   /* READNONE means writes can't affect it, while READONLY means that
> +* writes can affect it. */
> +   return can_speculate && HAVE_LLVM >= 0x0400 ?
> +AC_FUNC_ATTR_READNONE :
> +AC_FUNC_ATTR_READONLY;
> +}
> +
> +unsigned
> +ac_get_store_intr_attribs(bool writeonly_memory)
> +{
> +   return writeonly_memory && HAVE_LLVM >= 0x0400 ?
> + AC_FUNC_ATTR_INACCESSIBLE_MEM_ONLY :
> + AC_FUNC_ATTR_WRITEONLY;
> +}

What about putting such tiny functions in the header as static inline
to avoid function call overheads?

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

[Mesa-dev] [PATCH 1/2] amd/common: import get_{load, store}_intr_attribs() from RadeonSI

2018-01-10 Thread Samuel Pitoiset 
Signed-off-by: Samuel Pitoiset 
---
 src/amd/common/ac_llvm_build.c| 12 ++-
 src/amd/common/ac_llvm_util.c | 18 
 src/amd/common/ac_llvm_util.h |  6 ++
 src/gallium/drivers/radeonsi/si_shader_tgsi_mem.c | 26 +--
 4 files changed, 31 insertions(+), 31 deletions(-)

diff --git a/src/amd/common/ac_llvm_build.c b/src/amd/common/ac_llvm_build.c
index efc6fa12e5..07044142b0 100644
--- a/src/amd/common/ac_llvm_build.c
+++ b/src/amd/common/ac_llvm_build.c
@@ -983,11 +983,7 @@ ac_build_buffer_load(struct ac_llvm_context *ctx,
 
return ac_build_intrinsic(ctx, name, types[func], args,
  ARRAY_SIZE(args),
- /* READNONE means writes can't affect it, 
while
-  * READONLY means that writes can affect it. 
*/
- can_speculate && HAVE_LLVM >= 0x0400 ?
- AC_FUNC_ATTR_READNONE :
- AC_FUNC_ATTR_READONLY);
+ ac_get_load_intr_attribs(can_speculate));
 }
 
 LLVMValueRef ac_build_buffer_load_format(struct ac_llvm_context *ctx,
@@ -1007,11 +1003,7 @@ LLVMValueRef ac_build_buffer_load_format(struct 
ac_llvm_context *ctx,
return ac_build_intrinsic(ctx,
  "llvm.amdgcn.buffer.load.format.v4f32",
  ctx->v4f32, args, ARRAY_SIZE(args),
- /* READNONE means writes can't affect it, 
while
-  * READONLY means that writes can affect it. 
*/
- can_speculate && HAVE_LLVM >= 0x0400 ?
- AC_FUNC_ATTR_READNONE :
- AC_FUNC_ATTR_READONLY);
+ ac_get_load_intr_attribs(can_speculate));
 }
 
 /**
diff --git a/src/amd/common/ac_llvm_util.c b/src/amd/common/ac_llvm_util.c
index 429904c040..976d6540a1 100644
--- a/src/amd/common/ac_llvm_util.c
+++ b/src/amd/common/ac_llvm_util.c
@@ -240,3 +240,21 @@ ac_llvm_add_target_dep_function_attr(LLVMValueRef F,
snprintf(str, sizeof(str), "%i", value);
LLVMAddTargetDependentFunctionAttr(F, name, str);
 }
+
+unsigned
+ac_get_load_intr_attribs(bool can_speculate)
+{
+   /* READNONE means writes can't affect it, while READONLY means that
+* writes can affect it. */
+   return can_speculate && HAVE_LLVM >= 0x0400 ?
+AC_FUNC_ATTR_READNONE :
+AC_FUNC_ATTR_READONLY;
+}
+
+unsigned
+ac_get_store_intr_attribs(bool writeonly_memory)
+{
+   return writeonly_memory && HAVE_LLVM >= 0x0400 ?
+ AC_FUNC_ATTR_INACCESSIBLE_MEM_ONLY :
+ AC_FUNC_ATTR_WRITEONLY;
+}
diff --git a/src/amd/common/ac_llvm_util.h b/src/amd/common/ac_llvm_util.h
index 7c8b6b0a13..79c6029436 100644
--- a/src/amd/common/ac_llvm_util.h
+++ b/src/amd/common/ac_llvm_util.h
@@ -81,6 +81,12 @@ void
 ac_llvm_add_target_dep_function_attr(LLVMValueRef F,
 const char *name, int value);
 
+unsigned
+ac_get_load_intr_attribs(bool can_speculate);
+
+unsigned
+ac_get_store_intr_attribs(bool writeonly_memory);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/src/gallium/drivers/radeonsi/si_shader_tgsi_mem.c 
b/src/gallium/drivers/radeonsi/si_shader_tgsi_mem.c
index fe0cfcef99..d5c9470974 100644
--- a/src/gallium/drivers/radeonsi/si_shader_tgsi_mem.c
+++ b/src/gallium/drivers/radeonsi/si_shader_tgsi_mem.c
@@ -390,22 +390,6 @@ static void load_fetch_args(
}
 }
 
-static unsigned get_load_intr_attribs(bool can_speculate)
-{
-   /* READNONE means writes can't affect it, while READONLY means that
-* writes can affect it. */
-   return can_speculate && HAVE_LLVM >= 0x0400 ?
-LP_FUNC_ATTR_READNONE :
-LP_FUNC_ATTR_READONLY;
-}
-
-static unsigned get_store_intr_attribs(bool writeonly_memory)
-{
-   return writeonly_memory && HAVE_LLVM >= 0x0400 ?
- LP_FUNC_ATTR_INACCESSIBLE_MEM_ONLY :
- LP_FUNC_ATTR_WRITEONLY;
-}
-
 static void load_emit_buffer(struct si_shader_context *ctx,
 struct lp_build_emit_data *emit_data,
 bool can_speculate, bool allow_smem)
@@ -586,7 +570,7 @@ static void load_emit(
lp_build_intrinsic(
builder, 
"llvm.amdgcn.buffer.load.format.v4f32", emit_data->dst_type,
emit_data->args, emit_data->arg_count,
-   get_load_intr_attribs(can_speculate));
+