From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 1461) id 6E6693858D35; Wed, 22 Dec 2021 12:51:39 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 6E6693858D35 Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: Andrew Stubbs To: gcc-cvs@gcc.gnu.org Subject: [gcc/devel/omp/gcc-11] amdgcn: Change offload variable table discovery X-Act-Checkin: gcc X-Git-Author: Andrew Stubbs X-Git-Refname: refs/heads/devel/omp/gcc-11 X-Git-Oldrev: 4da9ae405b0b6c6b08bb0d0605da0a96c02babb7 X-Git-Newrev: 028b55a93e16015543f3a6cabda2e9b043bd3028 Message-Id: <20211222125139.6E6693858D35@sourceware.org> Date: Wed, 22 Dec 2021 12:51:39 +0000 (GMT) X-BeenThere: gcc-cvs@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-cvs mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 22 Dec 2021 12:51:39 -0000 https://gcc.gnu.org/g:028b55a93e16015543f3a6cabda2e9b043bd3028 commit 028b55a93e16015543f3a6cabda2e9b043bd3028 Author: Andrew Stubbs Date: Tue Nov 16 10:32:35 2021 +0000 amdgcn: Change offload variable table discovery Up to now the libgomp GCN plugin has been finding the offload variables by using a symbol lookup, but the AMD runtime requires that the symbols are global for that to work. This was ensured by mkoffload as a post-procssing step, but the LLVM 13 assembler no longer accepts this in the case where the variable was previously declared differently. This patch switches to locating the symbols directly from the offload_var_table, which means that only one symbol needs to be forced global. This changes breaks the libgomp image compatibility so GOMP_VERSION_GCN has also been bumped. gcc/ChangeLog: * config/gcn/mkoffload.c (process_asm): Process the variable table completely differently. (process_obj): Encode the varaible data differently. include/ChangeLog: * gomp-constants.h (GOMP_VERSION_GCN): Bump. libgomp/ChangeLog: * plugin/plugin-gcn.c (struct gcn_image_desc): Remove global_variables. (GOMP_OFFLOAD_load_image): Locate the offload variables via the table, not individual symbols. (cherry picked from commit 4a87a8e4b13e979e7c8a626a8f4082715a48e21e) Diff: --- gcc/ChangeLog.omp | 7 +++++++ gcc/config/gcn/mkoffload.c | 51 +++++++++++++++++---------------------------- include/ChangeLog.omp | 5 +++++ include/gomp-constants.h | 2 +- libgomp/ChangeLog.omp | 7 +++++++ libgomp/plugin/plugin-gcn.c | 39 ++++++++++++++++++---------------- 6 files changed, 60 insertions(+), 51 deletions(-) diff --git a/gcc/ChangeLog.omp b/gcc/ChangeLog.omp index 012cc52a9e0..88826ce2944 100644 --- a/gcc/ChangeLog.omp +++ b/gcc/ChangeLog.omp @@ -1,3 +1,10 @@ +2021-12-22 Andrew Stubbs + + Backport from master: + * config/gcn/mkoffload.c (process_asm): Process the variable table + completely differently. + (process_obj): Encode the varaible data differently. + 2021-12-22 Andrew Stubbs * config/nvptx/nvptx-opts.h (ptx_version): Change PTX_VERSION_3_1 to diff --git a/gcc/config/gcn/mkoffload.c b/gcc/config/gcn/mkoffload.c index 2b8350e1653..78c36ed5f7a 100644 --- a/gcc/config/gcn/mkoffload.c +++ b/gcc/config/gcn/mkoffload.c @@ -495,10 +495,8 @@ static void process_asm (FILE *in, FILE *out, FILE *cfile) { int fn_count = 0, var_count = 0, dims_count = 0, regcount_count = 0; - struct obstack fns_os, vars_os, varsizes_os, dims_os, regcounts_os; + struct obstack fns_os, dims_os, regcounts_os; obstack_init (&fns_os); - obstack_init (&vars_os); - obstack_init (&varsizes_os); obstack_init (&dims_os); obstack_init (®counts_os); @@ -567,16 +565,11 @@ process_asm (FILE *in, FILE *out, FILE *cfile) unsigned varsize; if (sscanf (buf, " .8byte %ms\n", &varname)) { - obstack_ptr_grow (&vars_os, varname); + fputs (buf, out); fgets (buf, sizeof (buf), in); if (!sscanf (buf, " .8byte %u\n", &varsize)) abort (); - obstack_int_grow (&varsizes_os, varsize); var_count++; - - /* The HSA Runtime cannot locate the symbol if it is not - exported from the kernel. */ - fprintf (out, "\t.global %s\n", varname); } break; } @@ -595,7 +588,19 @@ process_asm (FILE *in, FILE *out, FILE *cfile) char dummy; if (sscanf (buf, " .section .gnu.offload_vars%c", &dummy) > 0) - state = IN_VARS; + { + state = IN_VARS; + + /* Add a global symbol to allow plugin-gcn.c to locate the table + at runtime. It can't use the "offload_var_table.N" emitted by + the compiler because a) they're not global, and b) there's one + for each input file combined into the binary. */ + fputs (buf, out); + fputs ("\t.global .offload_var_table\n" + "\t.type .offload_var_table, @object\n" + ".offload_var_table:\n", + out); + } else if (sscanf (buf, " .section .gnu.offload_funcs%c", &dummy) > 0) state = IN_FUNCS; else if (sscanf (buf, " .amdgpu_metadata%c", &dummy) > 0) @@ -622,7 +627,7 @@ process_asm (FILE *in, FILE *out, FILE *cfile) regcount.sgpr_count = regcount.vgpr_count = -1; } - if (state == IN_CODE || state == IN_METADATA) + if (state == IN_CODE || state == IN_METADATA || state == IN_VARS) fputs (buf, out); } @@ -633,24 +638,7 @@ process_asm (FILE *in, FILE *out, FILE *cfile) fprintf (cfile, "#include \n"); fprintf (cfile, "#include \n\n"); - char **vars = XOBFINISH (&vars_os, char **); - unsigned *varsizes = XOBFINISH (&varsizes_os, unsigned *); - fprintf (cfile, - "static const struct global_var_info {\n" - " const char *name;\n" - " void *address;\n" - "} vars[] = {\n"); - int i; - for (i = 0; i < var_count; ++i) - { - const char *sep = i < var_count - 1 ? "," : " "; - fprintf (cfile, " { \"%s\", NULL }%s /* size: %u */\n", vars[i], sep, - varsizes[i]); - } - fprintf (cfile, "};\n\n"); - - obstack_free (&vars_os, NULL); - obstack_free (&varsizes_os, NULL); + fprintf (cfile, "static const int gcn_num_vars = %d;\n\n", var_count); /* Dump out function idents. */ fprintf (cfile, "static const struct hsa_kernel_description {\n" @@ -661,6 +649,7 @@ process_asm (FILE *in, FILE *out, FILE *cfile) "} gcn_kernels[] = {\n "); dim.d[0] = dim.d[1] = dim.d[2] = 0; const char *comma; + int i; for (comma = "", i = 0; i < fn_count; comma = ",\n ", i++) { /* Find if we recorded dimensions for this function. */ @@ -732,13 +721,11 @@ process_obj (FILE *in, FILE *cfile) " unsigned kernel_count;\n" " const struct hsa_kernel_description *kernel_infos;\n" " unsigned global_variable_count;\n" - " const struct global_var_info *global_variables;\n" "} target_data = {\n" " &gcn_image,\n" " sizeof (gcn_kernels) / sizeof (gcn_kernels[0]),\n" " gcn_kernels,\n" - " sizeof (vars) / sizeof (vars[0]),\n" - " vars\n" + " gcn_num_vars\n" "};\n\n"); fprintf (cfile, diff --git a/include/ChangeLog.omp b/include/ChangeLog.omp index 7c79ccd203d..641e1d9a158 100644 --- a/include/ChangeLog.omp +++ b/include/ChangeLog.omp @@ -1,3 +1,8 @@ +2021-12-22 Andrew Stubbs + + Backport from master: + * gomp-constants.h (GOMP_VERSION_GCN): Bump. + 2021-08-23 Tobias Burnus Backported from master: diff --git a/include/gomp-constants.h b/include/gomp-constants.h index 77cac00a3aa..c2cbe798492 100644 --- a/include/gomp-constants.h +++ b/include/gomp-constants.h @@ -303,7 +303,7 @@ enum gomp_map_kind #define GOMP_VERSION 1 #define GOMP_VERSION_NVIDIA_PTX 1 #define GOMP_VERSION_INTEL_MIC 0 -#define GOMP_VERSION_GCN 1 +#define GOMP_VERSION_GCN 2 #define GOMP_VERSION_PACK(LIB, DEV) (((LIB) << 16) | (DEV)) #define GOMP_VERSION_LIB(PACK) (((PACK) >> 16) & 0xffff) diff --git a/libgomp/ChangeLog.omp b/libgomp/ChangeLog.omp index fb4a98e3298..ec846079e87 100644 --- a/libgomp/ChangeLog.omp +++ b/libgomp/ChangeLog.omp @@ -1,3 +1,10 @@ +2021-12-22 Andrew Stubbs + + Backport from master: + * plugin/plugin-gcn.c (struct gcn_image_desc): Remove global_variables. + (GOMP_OFFLOAD_load_image): Locate the offload variables via the + table, not individual symbols. + 2021-12-22 Andrew Stubbs * allocator.c (MEMSPACE_ALLOC): New macro. diff --git a/libgomp/plugin/plugin-gcn.c b/libgomp/plugin/plugin-gcn.c index 1455bdceb70..c760c172229 100644 --- a/libgomp/plugin/plugin-gcn.c +++ b/libgomp/plugin/plugin-gcn.c @@ -393,7 +393,6 @@ struct gcn_image_desc const unsigned kernel_count; struct hsa_kernel_description *kernel_infos; const unsigned global_variable_count; - struct global_var_info *global_variables; }; /* This enum mirrors the corresponding LLVM enum's values for all ISAs that we @@ -3422,37 +3421,41 @@ GOMP_OFFLOAD_load_image (int ord, unsigned version, const void *target_data, if (!create_and_finalize_hsa_program (agent)) return -1; - for (unsigned i = 0; i < var_count; i++) + if (var_count > 0) { - struct global_var_info *v = &image_desc->global_variables[i]; - GCN_DEBUG ("Looking for variable %s\n", v->name); - hsa_status_t status; hsa_executable_symbol_t var_symbol; status = hsa_fns.hsa_executable_get_symbol_fn (agent->executable, NULL, - v->name, agent->id, + ".offload_var_table", + agent->id, 0, &var_symbol); if (status != HSA_STATUS_SUCCESS) hsa_fatal ("Could not find symbol for variable in the code object", status); - uint64_t var_addr; - uint32_t var_size; + uint64_t var_table_addr; status = hsa_fns.hsa_executable_symbol_get_info_fn - (var_symbol, HSA_EXECUTABLE_SYMBOL_INFO_VARIABLE_ADDRESS, &var_addr); + (var_symbol, HSA_EXECUTABLE_SYMBOL_INFO_VARIABLE_ADDRESS, + &var_table_addr); if (status != HSA_STATUS_SUCCESS) hsa_fatal ("Could not extract a variable from its symbol", status); - status = hsa_fns.hsa_executable_symbol_get_info_fn - (var_symbol, HSA_EXECUTABLE_SYMBOL_INFO_VARIABLE_SIZE, &var_size); - if (status != HSA_STATUS_SUCCESS) - hsa_fatal ("Could not extract a variable size from its symbol", status); - pair->start = var_addr; - pair->end = var_addr + var_size; - GCN_DEBUG ("Found variable %s at %p with size %u\n", v->name, - (void *)var_addr, var_size); - pair++; + struct { + uint64_t addr; + uint64_t size; + } var_table[var_count]; + GOMP_OFFLOAD_dev2host (agent->device_id, var_table, + (void*)var_table_addr, sizeof (var_table)); + + for (unsigned i = 0; i < var_count; i++) + { + pair->start = var_table[i].addr; + pair->end = var_table[i].addr + var_table[i].size; + GCN_DEBUG ("Found variable at %p with size %lu\n", + (void *)var_table[i].addr, var_table[i].size); + pair++; + } } GCN_DEBUG ("Looking for variable %s\n", STRINGX (GOMP_DEVICE_NUM_VAR));