<div dir="ltr"><div>Reviewed-by: Marek Olšák <<a href="mailto:marek.olsak@amd.com">marek.olsak@amd.com</a>></div><div> </div><div>Marek </div></div> <div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Jun 17, 2019 at 2:47 PM Haehnle, Nicolai <<a href="mailto:Nicolai.Haehnle@amd.com">Nicolai.Haehnle@amd.com</a>> wrote: </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">The following hunk needs to be added: > @@ -503,7 +521,9 @@ static bool resolve_symbol(const struct ac_rtld_upload_info *u, > unsigned part_idx, const Elf64_Sym *sym, > const char *name, uint64_t *value) > { > - if (sym->st_shndx == SHN_UNDEF) { > + /* TODO: properly disentangle the undef and the LDS cases once > + * STT_AMDGPU_LDS is retired. */ > + if (sym->st_shndx == SHN_UNDEF || sym->st_shndx == SHN_AMDGPU_LDS) { > const struct ac_rtld_symbol *lds_sym = > find_symbol(&u->binary->lds_symbols, name, part_idx); > Cheers, Nicolai On 17.06.19 01:34, Nicolai Hähnle wrote: > From: Nicolai Hähnle <<a href="mailto:nicolai.haehnle@amd.com" target="_blank">nicolai.haehnle@amd.com</a>> > > The initial prototype used a processor-specific symbol type, but > feedback suggests that an approach using processor-specific section > name that encodes the alignment analogous to SHN_COMMON symbols is > preferred. > > This patch keeps both variants around for now to reduce problems > with LLVM compatibility as we switch branches around. > > This also cleans up the error reporting in this function. > --- > src/amd/common/ac_rtld.c | 30 ++++++++++++++++++++++++------ > 1 file changed, 24 insertions(+), 6 deletions(-) > > diff --git a/src/amd/common/ac_rtld.c b/src/amd/common/ac_rtld.c > index dc9cc04705b..6379e55120d 100644 > --- a/src/amd/common/ac_rtld.c > +++ b/src/amd/common/ac_rtld.c > @@ -32,21 +32,25 @@ > > #include "ac_binary.h" > #include "ac_gpu_info.h" > #include "util/u_dynarray.h" > #include "util/u_math.h" > > // Old distributions may not have this enum constant > #define MY_EM_AMDGPU 224 > > #ifndef STT_AMDGPU_LDS > -#define STT_AMDGPU_LDS 13 > +#define STT_AMDGPU_LDS 13 // this is deprecated -- remove > +#endif > + > +#ifndef SHN_AMDGPU_LDS > +#define SHN_AMDGPU_LDS 0xff00 > #endif > > #ifndef R_AMDGPU_NONE > #define R_AMDGPU_NONE 0 > #define R_AMDGPU_ABS32_LO 1 > #define R_AMDGPU_ABS32_HI 2 > #define R_AMDGPU_ABS64 3 > #define R_AMDGPU_REL32 4 > #define R_AMDGPU_REL64 5 > #define R_AMDGPU_ABS32 6 > @@ -169,47 +173,60 @@ static bool layout_symbols(struct ac_rtld_symbol *symbols, unsigned num_symbols, > * Read LDS symbols from the given \p section of the ELF of \p part and append > * them to the LDS symbols list. > * > * Shared LDS symbols are filtered out. > */ > static bool read_private_lds_symbols(struct ac_rtld_binary *binary, > unsigned part_idx, > Elf_Scn *section, > uint32_t *lds_end_align) > { > -#define report_elf_if(cond) \ > +#define report_if(cond) \ > do { \ > if ((cond)) { \ > report_errorf(#cond); \ > return false; \ > } \ > } while (false) > +#define report_elf_if(cond) \ > + do { \ > + if ((cond)) { \ > + report_elf_errorf(#cond); \ > + return false; \ > + } \ > + } while (false) > > struct ac_rtld_part *part = &binary->parts[part_idx]; > Elf64_Shdr *shdr = elf64_getshdr(section); > uint32_t strtabidx = shdr->sh_link; > Elf_Data *symbols_data = elf_getdata(section, NULL); > report_elf_if(!symbols_data); > > const Elf64_Sym *symbol = symbols_data->d_buf; > size_t num_symbols = symbols_data->d_size / sizeof(Elf64_Sym); > > for (size_t j = 0; j < num_symbols; ++j, ++symbol) { > - if (ELF64_ST_TYPE(symbol->st_info) != STT_AMDGPU_LDS) > + struct ac_rtld_symbol s = {}; > + > + if (ELF64_ST_TYPE(symbol->st_info) == STT_AMDGPU_LDS) { > + /* old-style LDS symbols from initial prototype -- remove eventually */ > + s.align = MIN2(1u << (symbol->st_other >> 3), 1u << 16); > + } else if (symbol->st_shndx == SHN_AMDGPU_LDS) { > + s.align = MIN2(symbol->st_value, 1u << 16); > + report_if(!util_is_power_of_two_nonzero(s.align)); > + } else > continue; > > - report_elf_if(symbol->st_size > 1u << 29); > + report_if(symbol->st_size > 1u << 29); > > - struct ac_rtld_symbol s = {}; > <a href="http://s.name" rel="noreferrer" target="_blank">s.name</a> = elf_strptr(part->elf, strtabidx, symbol->st_name); > s.size = symbol->st_size; > - s.align = MIN2(1u << (symbol->st_other >> 3), 1u << 16); > s.part_idx = part_idx; > > if (!strcmp(<a href="http://s.name" rel="noreferrer" target="_blank">s.name</a>, "__lds_end")) { > report_elf_if(s.size != 0); > *lds_end_align = MAX2(*lds_end_align, s.align); > continue; > } > > const struct ac_rtld_symbol *shared = > find_symbol(&binary->lds_symbols, <a href="http://s.name" rel="noreferrer" target="_blank">s.name</a>, part_idx); > @@ -217,20 +234,21 @@ static bool read_private_lds_symbols(struct ac_rtld_binary *binary, > report_elf_if(s.align > shared->align); > report_elf_if(s.size > shared->size); > continue; > } > > util_dynarray_append(&binary->lds_symbols, struct ac_rtld_symbol, s); > } > > return true; > > +#undef report_if > #undef report_elf_if > } > > /** > * Open a binary consisting of one or more shader parts. > * > * \param binary the uninitialized struct > * \param i binary opening parameters > */ > bool ac_rtld_open(struct ac_rtld_binary *binary, > _______________________________________________ mesa-dev mailing list <a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a> <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a></blockquote></div>