[Mesa-dev] [PATCH mesa 2/3] tgsi: Add support for global / local / input MEMORY
Hans de Goede
hdegoede at redhat.com
Thu Mar 10 18:43:40 UTC 2016
Hi,
On 10-03-16 16:35, Aaron Watry wrote:
> On Thu, Mar 10, 2016 at 9:14 AM, Hans de Goede <hdegoede at redhat.com> wrote:
>
>> Extend the MEMORY file support to differentiate between global, local
>> and shared memory, as well as "input" memory.
>>
>> "MEMORY[x], INPUT" is intended to access OpenCL kernel parameters, a
>> special memory type is added for this, since the actual storage of these
>> (e.g. UBO-s) may differ per implementation. The uploading of kernel
>> parameters is handled by launch_grid, "MEMORY[x], INPUT" allows drivers
>> to use an access mechanism for parameter reads which matches with the
>> upload method.
>>
>> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
>> ---
>> src/gallium/auxiliary/tgsi/tgsi_build.c | 8 +++----
>> src/gallium/auxiliary/tgsi/tgsi_dump.c | 9 ++++++--
>> src/gallium/auxiliary/tgsi/tgsi_text.c | 14 ++++++++++--
>> src/gallium/auxiliary/tgsi/tgsi_ureg.c | 25
>> ++++++++++++----------
>> src/gallium/auxiliary/tgsi/tgsi_ureg.h | 2 +-
>> .../drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp | 7 +++---
>> src/gallium/include/pipe/p_shader_tokens.h | 10 +++++++--
>> src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 2 +-
>> 8 files changed, 51 insertions(+), 26 deletions(-)
>>
>> diff --git a/src/gallium/auxiliary/tgsi/tgsi_build.c
>> b/src/gallium/auxiliary/tgsi/tgsi_build.c
>> index c420ae1..b108ade 100644
>> --- a/src/gallium/auxiliary/tgsi/tgsi_build.c
>> +++ b/src/gallium/auxiliary/tgsi/tgsi_build.c
>> @@ -111,7 +111,7 @@ tgsi_default_declaration( void )
>> declaration.Local = 0;
>> declaration.Array = 0;
>> declaration.Atomic = 0;
>> - declaration.Shared = 0;
>> + declaration.MemType = TGSI_MEMORY_TYPE_GLOBAL;
>> declaration.Padding = 0;
>>
>> return declaration;
>> @@ -128,7 +128,7 @@ tgsi_build_declaration(
>> unsigned local,
>> unsigned array,
>> unsigned atomic,
>> - unsigned shared,
>> + unsigned mem_type,
>> struct tgsi_header *header )
>> {
>> struct tgsi_declaration declaration;
>> @@ -146,7 +146,7 @@ tgsi_build_declaration(
>> declaration.Local = local;
>> declaration.Array = array;
>> declaration.Atomic = atomic;
>> - declaration.Shared = shared;
>> + declaration.MemType = mem_type;
>> header_bodysize_grow( header );
>>
>> return declaration;
>> @@ -406,7 +406,7 @@ tgsi_build_full_declaration(
>> full_decl->Declaration.Local,
>> full_decl->Declaration.Array,
>> full_decl->Declaration.Atomic,
>> - full_decl->Declaration.Shared,
>> + full_decl->Declaration.MemType,
>> header );
>>
>> if (maxsize <= size)
>> diff --git a/src/gallium/auxiliary/tgsi/tgsi_dump.c
>> b/src/gallium/auxiliary/tgsi/tgsi_dump.c
>> index f232f38..273f0ae 100644
>> --- a/src/gallium/auxiliary/tgsi/tgsi_dump.c
>> +++ b/src/gallium/auxiliary/tgsi/tgsi_dump.c
>> @@ -365,8 +365,13 @@ iter_declaration(
>> }
>>
>> if (decl->Declaration.File == TGSI_FILE_MEMORY) {
>> - if (decl->Declaration.Shared)
>> - TXT(", SHARED");
>> + switch (decl->Declaration.MemType) {
>> + /* Note: ,GLOBAL is optional / the default */
>> + case TGSI_MEMORY_TYPE_GLOBAL: TXT(", GLOBAL"); break;
>> + case TGSI_MEMORY_TYPE_LOCAL: TXT(", LOCAL"); break;
>> + case TGSI_MEMORY_TYPE_SHARED: TXT(", SHARED"); break;
>> + case TGSI_MEMORY_TYPE_INPUT: TXT(", INPUT"); break;
>> + }
>> }
>>
>> if (decl->Declaration.File == TGSI_FILE_SAMPLER_VIEW) {
>> diff --git a/src/gallium/auxiliary/tgsi/tgsi_text.c
>> b/src/gallium/auxiliary/tgsi/tgsi_text.c
>> index 77598d2..9438e3b 100644
>> --- a/src/gallium/auxiliary/tgsi/tgsi_text.c
>> +++ b/src/gallium/auxiliary/tgsi/tgsi_text.c
>> @@ -1390,8 +1390,18 @@ static boolean parse_declaration( struct
>> translate_ctx *ctx )
>> ctx->cur = cur;
>> }
>> } else if (file == TGSI_FILE_MEMORY) {
>> - if (str_match_nocase_whole(&cur, "SHARED")) {
>> - decl.Declaration.Shared = 1;
>> + if (str_match_nocase_whole(&cur, "GLOBAL")) {
>> + /* Note this is a no-op global is the default */
>> + decl.Declaration.MemType = TGSI_MEMORY_TYPE_GLOBAL;
>> + ctx->cur = cur;
>> + } else if (str_match_nocase_whole(&cur, "LOCAL")) {
>> + decl.Declaration.MemType = TGSI_MEMORY_TYPE_LOCAL;
>> + ctx->cur = cur;
>> + } else if (str_match_nocase_whole(&cur, "SHARED")) {
>> + decl.Declaration.MemType = TGSI_MEMORY_TYPE_SHARED;
>> + ctx->cur = cur;
>> + } else if (str_match_nocase_whole(&cur, "INPUT")) {
>> + decl.Declaration.MemType = TGSI_MEMORY_TYPE_INPUT;
>> ctx->cur = cur;
>> }
>> } else {
>> diff --git a/src/gallium/auxiliary/tgsi/tgsi_ureg.c
>> b/src/gallium/auxiliary/tgsi/tgsi_ureg.c
>> index e1a7278..9e10044 100644
>> --- a/src/gallium/auxiliary/tgsi/tgsi_ureg.c
>> +++ b/src/gallium/auxiliary/tgsi/tgsi_ureg.c
>> @@ -190,7 +190,7 @@ struct ureg_program
>>
>> struct ureg_tokens domain[2];
>>
>> - bool use_shared_memory;
>> + bool use_memory[TGSI_MEMORY_TYPE_COUNT];
>> };
>>
>> static union tgsi_any_token error_tokens[32];
>> @@ -729,13 +729,14 @@ struct ureg_src ureg_DECL_buffer(struct ureg_program
>> *ureg, unsigned nr,
>> return reg;
>> }
>>
>> -/* Allocate a shared memory area.
>> +/* Allocate a memory area.
>> */
>> -struct ureg_src ureg_DECL_shared_memory(struct ureg_program *ureg)
>> +struct ureg_src ureg_DECL_memory(struct ureg_program *ureg,
>> + unsigned memory_type)
>> {
>> - struct ureg_src reg = ureg_src_register(TGSI_FILE_MEMORY, 0);
>> + struct ureg_src reg = ureg_src_register(TGSI_FILE_MEMORY, memory_type);
>>
>> - ureg->use_shared_memory = true;
>> + ureg->use_memory[memory_type] = true;
>> return reg;
>> }
>>
>> @@ -1666,7 +1667,7 @@ emit_decl_buffer(struct ureg_program *ureg,
>> }
>>
>> static void
>> -emit_decl_shared_memory(struct ureg_program *ureg)
>> +emit_decl_memory(struct ureg_program *ureg, unsigned memory_type)
>> {
>> union tgsi_any_token *out = get_tokens(ureg, DOMAIN_DECL, 2);
>>
>> @@ -1675,11 +1676,11 @@ emit_decl_shared_memory(struct ureg_program *ureg)
>> out[0].decl.NrTokens = 2;
>> out[0].decl.File = TGSI_FILE_MEMORY;
>> out[0].decl.UsageMask = TGSI_WRITEMASK_XYZW;
>> - out[0].decl.Shared = true;
>> + out[0].decl.MemType = memory_type;
>>
>> out[1].value = 0;
>> - out[1].decl_range.First = 0;
>> - out[1].decl_range.Last = 0;
>> + out[1].decl_range.First = memory_type;
>> + out[1].decl_range.Last = memory_type;
>> }
>>
>> static void
>> @@ -1854,8 +1855,10 @@ static void emit_decls( struct ureg_program *ureg )
>> emit_decl_buffer(ureg, ureg->buffer[i].index,
>> ureg->buffer[i].atomic);
>> }
>>
>> - if (ureg->use_shared_memory)
>> - emit_decl_shared_memory(ureg);
>> + for (i = 0; i < TGSI_MEMORY_TYPE_COUNT; i++) {
>> + if (ureg->use_memory[i])
>> + emit_decl_memory(ureg, i);
>> + }
>>
>> if (ureg->const_decls.nr_constant_ranges) {
>> for (i = 0; i < ureg->const_decls.nr_constant_ranges; i++) {
>> diff --git a/src/gallium/auxiliary/tgsi/tgsi_ureg.h
>> b/src/gallium/auxiliary/tgsi/tgsi_ureg.h
>> index 6a3b5dd..7c7a89e 100644
>> --- a/src/gallium/auxiliary/tgsi/tgsi_ureg.h
>> +++ b/src/gallium/auxiliary/tgsi/tgsi_ureg.h
>> @@ -338,7 +338,7 @@ struct ureg_src
>> ureg_DECL_buffer(struct ureg_program *ureg, unsigned nr, bool atomic);
>>
>> struct ureg_src
>> -ureg_DECL_shared_memory(struct ureg_program *ureg);
>> +ureg_DECL_memory(struct ureg_program *ureg, unsigned memory_type);
>>
>> static inline struct ureg_src
>> ureg_imm4f( struct ureg_program *ureg,
>> diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
>> b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
>> index 8683722..a8258af 100644
>> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
>> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
>> @@ -860,7 +860,7 @@ public:
>> std::vector<Resource> resources;
>>
>> struct MemoryFile {
>> - bool shared;
>> + uint8_t mem_type; // TGSI_MEMORY_TYPE_*
>> };
>> std::vector<MemoryFile> memoryFiles;
>>
>> @@ -1218,7 +1218,7 @@ bool Source::scanDeclaration(const struct
>> tgsi_full_declaration *decl)
>> break;
>> case TGSI_FILE_MEMORY:
>> for (i = first; i <= last; ++i)
>> - memoryFiles[i].shared = decl->Declaration.Shared;
>> + memoryFiles[i].mem_type = decl->Declaration.MemType;
>> break;
>> case TGSI_FILE_NULL:
>> case TGSI_FILE_TEMPORARY:
>> @@ -1523,7 +1523,8 @@ Converter::makeSym(uint tgsiFile, int fileIdx, int
>> idx, int c, uint32_t address)
>>
>> sym->reg.fileIndex = fileIdx;
>>
>> - if (tgsiFile == TGSI_FILE_MEMORY && code->memoryFiles[fileIdx].shared)
>> + if (tgsiFile == TGSI_FILE_MEMORY &&
>> + code->memoryFiles[fileIdx].mem_type == TGSI_MEMORY_TYPE_SHARED)
>> sym->setFile(FILE_MEMORY_SHARED);
>>
>> if (idx >= 0) {
>> diff --git a/src/gallium/include/pipe/p_shader_tokens.h
>> b/src/gallium/include/pipe/p_shader_tokens.h
>> index 9d4a96a..0b8d7fb 100644
>> --- a/src/gallium/include/pipe/p_shader_tokens.h
>> +++ b/src/gallium/include/pipe/p_shader_tokens.h
>> @@ -117,6 +117,12 @@ enum tgsi_file_type {
>> #define TGSI_CYLINDRICAL_WRAP_Z (1 << 2)
>> #define TGSI_CYLINDRICAL_WRAP_W (1 << 3)
>>
>> +#define TGSI_MEMORY_TYPE_GLOBAL 0 /* GLSL global / OpenCL global
>> */
>> +#define TGSI_MEMORY_TYPE_LOCAL 1 /* GLSL local / OpenCL private
>> */
>> +#define TGSI_MEMORY_TYPE_SHARED 2 /* GLSL shared / OpenCL local
>> */
>> +#define TGSI_MEMORY_TYPE_INPUT 3 /* For OpenCL kernel input
>> params */
>>
>
> Note: I know little about how you are approaching the CL implementation for
> Nouveau...
The plan is to use a tgsi backend for llvm:
http://cgit.freedesktop.org/~jwrdegoede/llvm
http://cgit.freedesktop.org/~jwrdegoede/clang
And then have clover use that to feed tgsi to the mesa nouveau code.
> Should I expect to see an entry in here anywhere for the CONSTANT memory
> type?
>
> My GL knowledge is pretty basic. Would that map reasonably to something
> like a GL UBO? Do we need an explicit type for CL constant buffers?
That is a good question, CONST has not really been on my radar
yet because it is kinda tricky. Since OpenCL is really C-like, the
best match for const is global mem, and then (optionally) map the
const buffers read-only. This is what amd is doing:
src/gallium/state_trackers/clover/llvm/invocation.cpp:
// XXX: Correctly handle constant address space. There is no
// way for r600g to pass a handle for constant buffers back
// to clover like it can for global buffers, so
// creating constant arguments will break r600g. For now,
// continue treating constant buffers as global buffers
// until we can come up with a way to create handles for
// constant buffers.
args.push_back(module::argument(module::argument::global,
arg_api_size, target_size,
target_align,
module::argument::zero_ext));
The same more or less applies to nouveau. One day we would ideally map
const buffers to UBO-s but that is non trivial. I guess it would be
better for the llvm tgsi backend to do:
DCL MEMORY[42], CONST
and things like:
LOAD TEMP[0].x, MEMORY[42], IMM[0]
For const accesses so that if we later want to change things the TGSI
code already clearly differentiates between global and const accesses,
or maybe:
DCL MEMORY[42], GLOBAL, RO
Nah, I like:
DCL MEMORY[42], CONST
Better it is more consistent. Ilia, what is your take on this ?
The obvious implementation of this will eat another bit in
tgsi_declaration though.
Regards,
Hans
>
> --Aaron
>
>
>
>> +#define TGSI_MEMORY_TYPE_COUNT 4
>> +
>> struct tgsi_declaration
>> {
>> unsigned Type : 4; /**< TGSI_TOKEN_TYPE_DECLARATION */
>> @@ -130,8 +136,8 @@ struct tgsi_declaration
>> unsigned Local : 1; /**< optimize as subroutine local variable?
>> */
>> unsigned Array : 1; /**< extra array info? */
>> unsigned Atomic : 1; /**< atomic only? for TGSI_FILE_BUFFER */
>> - unsigned Shared : 1; /**< shared storage for TGSI_FILE_MEMORY */
>> - unsigned Padding : 4;
>> + unsigned MemType : 2; /**< TGSI_MEMORY_TYPE_x for
>> TGSI_FILE_MEMORY */
>> + unsigned Padding : 3;
>> };
>>
>> struct tgsi_declaration_range
>> diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
>> b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
>> index 26e463e..9b8bf8e 100644
>> --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
>> +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
>> @@ -6281,7 +6281,7 @@ st_translate_program(
>> }
>>
>> if (program->use_shared_memory)
>> - t->shared_memory = ureg_DECL_shared_memory(ureg);
>> + t->shared_memory = ureg_DECL_memory(ureg, TGSI_MEMORY_TYPE_SHARED);
>>
>> for (i = 0; i < program->shader->NumImages; i++) {
>> if (program->images_used & (1 << i)) {
>> --
>> 2.7.2
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
>
More information about the mesa-dev
mailing list