[Mesa-dev] [PATCH] amd/rtld: update the ELF representation of LDS symbols

[Haehnle, Nicolai]

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 <nicolai.haehnle at amd.com>
> 
> 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 = {};
>   		s.name = 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(s.name, "__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, s.name, 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,
>