On Fri, Jun 9, 2017 at 12:36 PM, Jan Vesely <jan.ves...@rutgers.edu> wrote:
> On Fri, 2017-06-09 at 10:12 -0500, Aaron Watry wrote:
>> On Fri, Jun 9, 2017 at 8:20 AM, Jan Vesely <jan.ves...@rutgers.edu> wrote:
>> > This is a verbatim copy of the code. The functions can be cleaned up since
>> > r600 does not use all the stuff that gcn does.
>> > The symbol names have been changed since we still use ac_binary.h header
>> > (for struct definition)
>> >
>> > Signed-off-by: Jan Vesely <jan.ves...@rutgers.edu>
>> > ---
>> > Emil, Aaron,
>> >
>> > this is the last patch to get rid of libamd_common dependency (and thus
>> > libdrm_amdgpu). I have only remote access to the machine atm, so it's
>> > compile tested only.
>> >
>> > Jan
>> >
>> > configure.ac | 5 +-
>> > src/gallium/drivers/r600/Automake.inc | 10 +-
>> > src/gallium/drivers/r600/Makefile.am | 2 +
>> > src/gallium/drivers/r600/evergreen_compute.c | 197
>> > ++++++++++++++++++++-
>> > .../drivers/r600/evergreen_compute_internal.h | 2 +-
>> > src/gallium/drivers/radeon/r600_pipe_common.c | 21 ---
>> > src/gallium/drivers/radeon/r600_pipe_common.h | 5 -
>> > src/gallium/targets/pipe-loader/Makefile.am | 10 +-
>> > 8 files changed, 200 insertions(+), 52 deletions(-)
>> >
>> > diff --git a/configure.ac b/configure.ac
>> > index 9433e3c..fc4a58f 100644
>> > --- a/configure.ac
>> > +++ b/configure.ac
>> > @@ -2631,10 +2631,7 @@ AM_CONDITIONAL(HAVE_SWRAST_DRI, test
>> > x$HAVE_SWRAST_DRI = xyes)
>> > AM_CONDITIONAL(HAVE_RADEON_VULKAN, test "x$HAVE_RADEON_VULKAN" = xyes)
>> > AM_CONDITIONAL(HAVE_INTEL_VULKAN, test "x$HAVE_INTEL_VULKAN" = xyes)
>> >
>> > -# FIXME: r600g still depends and amd_common (ac_binary*) when building
>> > OpenCL
>> > -AM_CONDITIONAL(HAVE_AMD_DRIVERS, test \( "x$HAVE_GALLIUM_R600" = xyes -a \
>> > - "x$enable_opencl" = xyes \) -o \
>> > - "x$HAVE_GALLIUM_RADEONSI" = xyes -o
>> > \
>> > +AM_CONDITIONAL(HAVE_AMD_DRIVERS, test "x$HAVE_GALLIUM_RADEONSI" = xyes -o
>> > \
>> > "x$HAVE_RADEON_VULKAN" = xyes)
>> >
>> > AM_CONDITIONAL(HAVE_INTEL_DRIVERS, test "x$HAVE_INTEL_VULKAN" = xyes -o \
>> > diff --git a/src/gallium/drivers/r600/Automake.inc
>> > b/src/gallium/drivers/r600/Automake.inc
>> > index 642d527..bb9f6ec 100644
>> > --- a/src/gallium/drivers/r600/Automake.inc
>> > +++ b/src/gallium/drivers/r600/Automake.inc
>> > @@ -5,18 +5,12 @@ TARGET_CPPFLAGS += -DGALLIUM_R600
>> > TARGET_LIB_DEPS += \
>> > $(top_builddir)/src/gallium/drivers/r600/libr600.la \
>> > $(RADEON_LIBS) \
>> > - $(LIBDRM_LIBS)
>> > + $(LIBDRM_LIBS) \
>> > + $(LIBELF_LIBS)
>> >
>> > TARGET_RADEON_WINSYS = \
>> > $(top_builddir)/src/gallium/winsys/radeon/drm/libradeonwinsys.la
>> >
>> > TARGET_RADEON_COMMON = \
>> > $(top_builddir)/src/gallium/drivers/radeon/libradeon.la
>> > -
>> > -# TODO: drop this dependency. libamd_common requires libdrm_amdgpu.
>> > -if HAVE_AMD_DRIVERS
>> > -TARGET_RADEON_COMMON += \
>> > - $(top_builddir)/src/amd/common/libamd_common.la
>> > -endif
>> > -
>> > endif
>> > diff --git a/src/gallium/drivers/r600/Makefile.am
>> > b/src/gallium/drivers/r600/Makefile.am
>> > index 44fd51d..fbfb6e6 100644
>> > --- a/src/gallium/drivers/r600/Makefile.am
>> > +++ b/src/gallium/drivers/r600/Makefile.am
>> > @@ -9,11 +9,13 @@ BUILT_SOURCES = $(R600_GENERATED_FILES)
>> > AM_CFLAGS = \
>> > $(GALLIUM_DRIVER_CFLAGS) \
>> > $(RADEON_CFLAGS) \
>> > + $(LIBELF_CFLAGS) \
>> > -I$(top_srcdir)/src/amd/common
>> >
>> > AM_CXXFLAGS = \
>> > $(GALLIUM_DRIVER_CXXFLAGS) \
>> > $(RADEON_CFLAGS) \
>> > + $(LIBELF_CFLAGS) \
>> > -I$(top_srcdir)/src/amd/common
>> >
>> > noinst_LTLIBRARIES = libr600.la
>> > diff --git a/src/gallium/drivers/r600/evergreen_compute.c
>> > b/src/gallium/drivers/r600/evergreen_compute.c
>> > index d30024d..69a6d8b 100644
>> > --- a/src/gallium/drivers/r600/evergreen_compute.c
>> > +++ b/src/gallium/drivers/r600/evergreen_compute.c
>> > @@ -24,9 +24,10 @@
>> > * Adam Rak <adam....@streamnovation.com>
>> > */
>> >
>> > +#include <gelf.h>
>> > +#include <libelf.h>
>> > #include <stdio.h>
>> > #include <errno.h>
>> > -#include "ac_binary.h"
>> > #include "pipe/p_defines.h"
>> > #include "pipe/p_state.h"
>> > #include "pipe/p_context.h"
>> > @@ -179,6 +180,192 @@ static void evergreen_cs_set_constant_buffer(struct
>> > r600_context *rctx,
>> > #define R_028850_SQ_PGM_RESOURCES_PS 0x028850
>> >
>> > #ifdef HAVE_OPENCL
>> > +/*
>> > + * shader binary helpers.
>> > + */
>> > +static void r600_shader_binary_init(struct ac_shader_binary *b)
>> > +{
>> > + memset(b, 0, sizeof(*b));
>> > +}
>> > +
>> > +static void r600_shader_binary_clean(struct ac_shader_binary *b)
>> > +{
>> > + if (!b)
>> > + return;
>> > + FREE(b->code);
>> > + FREE(b->config);
>> > + FREE(b->rodata);
>> > + FREE(b->global_symbol_offsets);
>> > + FREE(b->relocs);
>> > + FREE(b->disasm_string);
>> > + FREE(b->llvm_ir_string);
>> > +}
>> > +
>> > +static void parse_symbol_table(Elf_Data *symbol_table_data,
>> > + const GElf_Shdr *symbol_table_header,
>> > + struct ac_shader_binary *binary)
>> > +{
>> > + GElf_Sym symbol;
>> > + unsigned i = 0;
>> > + unsigned symbol_count =
>> > + symbol_table_header->sh_size /
>> > symbol_table_header->sh_entsize;
>> > +
>> > + /* We are over allocating this list, because symbol_count gives the
>> > + * total number of symbols, and we will only be filling the list
>> > + * with offsets of global symbols. The memory savings from
>> > + * allocating the correct size of this list will be small, and
>> > + * I don't think it is worth the cost of pre-computing the number
>> > + * of global symbols.
>> > + */
>> > + binary->global_symbol_offsets = CALLOC(symbol_count,
>> > sizeof(uint64_t));
>> > +
>> > + while (gelf_getsym(symbol_table_data, i++, &symbol)) {
>> > + unsigned i;
>> > + if (GELF_ST_BIND(symbol.st_info) != STB_GLOBAL ||
>> > + symbol.st_shndx == 0 /* Undefined symbol */) {
>> > + continue;
>> > + }
>> > +
>> > + binary->global_symbol_offsets[binary->global_symbol_count]
>> > =
>> > + symbol.st_value;
>> > +
>> > + /* Sort the list using bubble sort. This list will usually
>> > + * be small. */
>> > + for (i = binary->global_symbol_count; i > 0; --i) {
>> > + uint64_t lhs = binary->global_symbol_offsets[i -
>> > 1];
>> > + uint64_t rhs = binary->global_symbol_offsets[i];
>> > + if (lhs < rhs) {
>> > + break;
>> > + }
>> > + binary->global_symbol_offsets[i] = lhs;
>> > + binary->global_symbol_offsets[i - 1] = rhs;
>> > + }
>> > + ++binary->global_symbol_count;
>> > + }
>> > +}
>> > +
>> > +
>> > +static void parse_relocs(Elf *elf, Elf_Data *relocs, Elf_Data *symbols,
>> > + unsigned symbol_sh_link,
>> > + struct ac_shader_binary *binary)
>> > +{
>> > + unsigned i;
>> > +
>> > + if (!relocs || !symbols || !binary->reloc_count) {
>> > + return;
>> > + }
>> > + binary->relocs = CALLOC(binary->reloc_count,
>> > + sizeof(struct ac_shader_reloc));
>> > + for (i = 0; i < binary->reloc_count; i++) {
>> > + GElf_Sym symbol;
>> > + GElf_Rel rel;
>> > + char *symbol_name;
>> > + struct ac_shader_reloc *reloc = &binary->relocs[i];
>> > +
>> > + gelf_getrel(relocs, i, &rel);
>> > + gelf_getsym(symbols, GELF_R_SYM(rel.r_info), &symbol);
>> > + symbol_name = elf_strptr(elf, symbol_sh_link,
>> > symbol.st_name);
>> > +
>> > + reloc->offset = rel.r_offset;
>> > + strncpy(reloc->name, symbol_name, sizeof(reloc->name)-1);
>> > + reloc->name[sizeof(reloc->name)-1] = 0;
>> > + }
>> > +}
>> > +
>> > +//ac_elf_read
>> > +
>> > +static void r600_elf_read(const char *elf_data, unsigned elf_size,
>> > + struct ac_shader_binary *binary)
>> > +{
>> > + char *elf_buffer;
>> > + Elf *elf;
>> > + Elf_Scn *section = NULL;
>> > + Elf_Data *symbols = NULL, *relocs = NULL;
>> > + size_t section_str_index;
>> > + unsigned symbol_sh_link = 0;
>> > +
>> > + /* One of the libelf implementations
>> > + * (http://www.mr511.de/software/english.htm) requires calling
>> > + * elf_version() before elf_memory().
>> > + */
>> > + elf_version(EV_CURRENT);
>> > + elf_buffer = MALLOC(elf_size);
>> > + memcpy(elf_buffer, elf_data, elf_size);
>> > +
>> > + elf = elf_memory(elf_buffer, elf_size);
>> > +
>> > + elf_getshdrstrndx(elf, §ion_str_index);
>> > +
>> > + while ((section = elf_nextscn(elf, section))) {
>> > + const char *name;
>> > + Elf_Data *section_data = NULL;
>> > + GElf_Shdr section_header;
>> > + if (gelf_getshdr(section, §ion_header) !=
>> > §ion_header) {
>> > + fprintf(stderr, "Failed to read ELF section
>> > header\n");
>> > + return;
>> > + }
>> > + name = elf_strptr(elf, section_str_index,
>> > section_header.sh_name);
>> > + if (!strcmp(name, ".text")) {
>> > + section_data = elf_getdata(section, section_data);
>> > + binary->code_size = section_data->d_size;
>> > + binary->code = MALLOC(binary->code_size *
>> > sizeof(unsigned char));
>> > + memcpy(binary->code, section_data->d_buf,
>> > binary->code_size);
>> > + } else if (!strcmp(name, ".AMDGPU.config")) {
>> > + section_data = elf_getdata(section, section_data);
>> > + binary->config_size = section_data->d_size;
>> > + binary->config = MALLOC(binary->config_size *
>> > sizeof(unsigned char));
>> > + memcpy(binary->config, section_data->d_buf,
>> > binary->config_size);
>> > + } else if (!strcmp(name, ".AMDGPU.disasm")) {
>> > + /* Always read disassembly if it's available. */
>> > + section_data = elf_getdata(section, section_data);
>> > + binary->disasm_string =
>> > strndup(section_data->d_buf,
>> > +
>> > section_data->d_size);
>> > + } else if (!strncmp(name, ".rodata", 7)) {
>> > + section_data = elf_getdata(section, section_data);
>> > + binary->rodata_size = section_data->d_size;
>> > + binary->rodata = MALLOC(binary->rodata_size *
>> > sizeof(unsigned char));
>> > + memcpy(binary->rodata, section_data->d_buf,
>> > binary->rodata_size);
>> > + } else if (!strncmp(name, ".symtab", 7)) {
>> > + symbols = elf_getdata(section, section_data);
>> > + symbol_sh_link = section_header.sh_link;
>> > + parse_symbol_table(symbols, §ion_header,
>> > binary);
>> > + } else if (!strcmp(name, ".rel.text")) {
>> > + relocs = elf_getdata(section, section_data);
>> > + binary->reloc_count = section_header.sh_size /
>> > + section_header.sh_entsize;
>> > + }
>> > + }
>> > +
>> > + parse_relocs(elf, relocs, symbols, symbol_sh_link, binary);
>> > +
>> > + if (elf){
>> > + elf_end(elf);
>> > + }
>> > + FREE(elf_buffer);
>> > +
>> > + /* Cache the config size per symbol */
>> > + if (binary->global_symbol_count) {
>> > + binary->config_size_per_symbol =
>> > + binary->config_size / binary->global_symbol_count;
>> > + } else {
>> > + binary->global_symbol_count = 1;
>> > + binary->config_size_per_symbol = binary->config_size;
>> > + }
>> > +}
>> > +
>> > +static const unsigned char *r600_shader_binary_config_start(
>> > + const struct ac_shader_binary *binary,
>> > + uint64_t symbol_offset)
>> > +{
>> > + unsigned i;
>> > + for (i = 0; i < binary->global_symbol_count; ++i) {
>> > + if (binary->global_symbol_offsets[i] == symbol_offset) {
>> > + unsigned offset = i *
>> > binary->config_size_per_symbol;
>> > + return binary->config + offset;
>> > + }
>> > + }
>> > + return binary->config;
>> > +}
>> >
>> > static void r600_shader_binary_read_config(const struct ac_shader_binary
>> > *binary,
>> > struct r600_bytecode *bc,
>> > @@ -187,7 +374,7 @@ static void r600_shader_binary_read_config(const
>> > struct ac_shader_binary *binary
>> > {
>> > unsigned i;
>> > const unsigned char *config =
>> > - ac_shader_binary_config_start(binary, symbol_offset);
>> > + r600_shader_binary_config_start(binary, symbol_offset);
>> >
>> > for (i = 0; i < binary->config_size_per_symbol; i+= 8) {
>> > unsigned reg =
>> > @@ -250,8 +437,8 @@ static void *evergreen_create_compute_state(struct
>> > pipe_context *ctx,
>> > COMPUTE_DBG(rctx->screen, "*** evergreen_create_compute_state\n");
>> > header = cso->prog;
>> > code = cso->prog + sizeof(struct pipe_llvm_program_header);
>> > - radeon_shader_binary_init(&shader->binary);
>> > - ac_elf_read(code, header->num_bytes, &shader->binary);
>> > + r600_shader_binary_init(&shader->binary);
>> > + r600_elf_read(code, header->num_bytes, &shader->binary);
>> > r600_create_shader(&shader->bc, &shader->binary, &use_kill);
>> >
>> > /* Upload code + ROdata */
>> > @@ -281,7 +468,7 @@ static void evergreen_delete_compute_state(struct
>> > pipe_context *ctx, void *state
>> > if (!shader)
>> > return;
>> >
>> > - radeon_shader_binary_clean(&shader->binary);
>> > + r600_shader_binary_clean(&shader->binary);
>>
>> This call to r600_shader_binary_clean needs an #ifdef HAVE_OPENCL
>> guard around it.
>>
>> That being said, we still have a dependency on amdgpu.h in
>> amd/common/ac_gpu_info.h here, which breaks the build if the amdgpu.h
>> header isn't installed. Emil's patch 4/5 fixes that issue, so I think
>> we'll have to include that somewhere in this series when we commit it.
>
> right, thanks for clarification. Yours or Emil's patches address that
> so I did not include them. I don't have a problem if it's two different
> series.
>
>>
>> With the aforementioned HAVE_OPENCL guard and Emil's 4/5, I've got
>> the following builds results on my BARTS/6850 system:
>>
>> r600g, no CL, no amdgpu.h: Succeeds
>> r600g, with CL, no amdgpu.h: Succeeds, clinfo works
>> r600g, no CL, with amdgpu.h: Succeeds
>> r600g, with CL, with amdgpu.h: Succeeds, clinfo works
>>
>> With that #ifdef change from above added, this is:
>> Tested-By: Aaron Watry <awa...@gmail.com>
>
> fixed in v2. It might be worth testing at least one piglit, since
> clinfo does not hit dispatch codepaths.
I retested both CL builds (with/without amdgpu.h present), and both
work fine when I run:
$ bin/cl-program-tester tests/cl/program/execute/builtin/math/tan.cl
So I think we're ok here. I can run a full piglit run if we feel it's
necessary, but it wouldn't be until Sunday/Monday, as I'm headed out
of town for the weekend.
--Aaron
>
> thanks,
> Jan
>
>>
>> --Aaron
>>
>> > r600_destroy_shader(&shader->bc);
>> >
>> > /* TODO destroy shader->code_bo, shader->const_bo
>> > diff --git a/src/gallium/drivers/r600/evergreen_compute_internal.h
>> > b/src/gallium/drivers/r600/evergreen_compute_internal.h
>> > index 6f4be3e..32f53ad 100644
>> > --- a/src/gallium/drivers/r600/evergreen_compute_internal.h
>> > +++ b/src/gallium/drivers/r600/evergreen_compute_internal.h
>> > @@ -21,10 +21,10 @@
>> > * Authors:
>> > * Adam Rak <adam....@streamnovation.com>
>> > */
>> > -
>> > #ifndef EVERGREEN_COMPUTE_INTERNAL_H
>> > #define EVERGREEN_COMPUTE_INTERNAL_H
>> >
>> > +#include "ac_binary.h"
>> > #include "r600_asm.h"
>> > #ifdef HAVE_OPENCL
>> > #include <llvm-c/Core.h>
>> > diff --git a/src/gallium/drivers/radeon/r600_pipe_common.c
>> > b/src/gallium/drivers/radeon/r600_pipe_common.c
>> > index 48d136a..3939623 100644
>> > --- a/src/gallium/drivers/radeon/r600_pipe_common.c
>> > +++ b/src/gallium/drivers/radeon/r600_pipe_common.c
>> > @@ -64,27 +64,6 @@ struct r600_multi_fence {
>> > };
>> >
>> > /*
>> > - * shader binary helpers.
>> > - */
>> > -void radeon_shader_binary_init(struct ac_shader_binary *b)
>> > -{
>> > - memset(b, 0, sizeof(*b));
>> > -}
>> > -
>> > -void radeon_shader_binary_clean(struct ac_shader_binary *b)
>> > -{
>> > - if (!b)
>> > - return;
>> > - FREE(b->code);
>> > - FREE(b->config);
>> > - FREE(b->rodata);
>> > - FREE(b->global_symbol_offsets);
>> > - FREE(b->relocs);
>> > - FREE(b->disasm_string);
>> > - FREE(b->llvm_ir_string);
>> > -}
>> > -
>> > -/*
>> > * pipe_context
>> > */
>> >
>> > diff --git a/src/gallium/drivers/radeon/r600_pipe_common.h
>> > b/src/gallium/drivers/radeon/r600_pipe_common.h
>> > index 84d38fb..2d2a05f 100644
>> > --- a/src/gallium/drivers/radeon/r600_pipe_common.h
>> > +++ b/src/gallium/drivers/radeon/r600_pipe_common.h
>> > @@ -34,8 +34,6 @@
>> >
>> > #include <stdio.h>
>> >
>> > -#include "amd/common/ac_binary.h"
>> > -
>> > #include "radeon/radeon_winsys.h"
>> >
>> > #include "util/disk_cache.h"
>> > @@ -134,9 +132,6 @@ struct r600_perfcounters;
>> > struct tgsi_shader_info;
>> > struct r600_qbo_state;
>> >
>> > -void radeon_shader_binary_init(struct ac_shader_binary *b);
>> > -void radeon_shader_binary_clean(struct ac_shader_binary *b);
>> > -
>> > /* Only 32-bit buffer allocations are supported, gallium doesn't support
>> > more
>> > * at the moment.
>> > */
>> > diff --git a/src/gallium/targets/pipe-loader/Makefile.am
>> > b/src/gallium/targets/pipe-loader/Makefile.am
>> > index b1ef07e..e06d9321 100644
>> > --- a/src/gallium/targets/pipe-loader/Makefile.am
>> > +++ b/src/gallium/targets/pipe-loader/Makefile.am
>> > @@ -129,14 +129,8 @@ pipe_r600_la_LIBADD = \
>> > $(top_builddir)/src/gallium/drivers/radeon/libradeon.la \
>> > $(top_builddir)/src/gallium/drivers/r600/libr600.la \
>> > $(LIBDRM_LIBS) \
>> > - $(RADEON_LIBS)
>> > -
>> > -# TODO: drop this dependency. libamd_common requires libdrm_amdgpu.
>> > -if HAVE_AMD_DRIVERS
>> > -pipe_r600_la_LIBADD += \
>> > - $(top_builddir)/src/amd/common/libamd_common.la
>> > -endif
>> > -
>> > + $(RADEON_LIBS) \
>> > + $(LIBELF_LIBS)
>> > endif
>> >
>> > if HAVE_GALLIUM_RADEONSI
>> > --
>> > 2.9.4
>> >
>
> --
> Jan Vesely <jan.ves...@rutgers.edu>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
