[Mesa-dev] [PATCH 1/4] amd/common: add nir->llvm translation.

[Emil Velikov]

Hi Dave,

On 4 October 2016 at 02:48, Dave Airlie <airlied at gmail.com> wrote:
> From: Bas Nieuwenhuizen <bas at basnieuwenhuizen.nl>
>
> This adds the basic files for the NIR->LLVM translation layer,
> along with some hopefully generic code to load the binary
> result, and other helpers required.
>
> The hope is in the future we could share this with an
> GL_ARB_spirv implementation or a push to replace TGSI
> with NIR in radeonsi.
>
Some of this code is copied from/based on existing one in mesa. Please
mention so in the commit summary and the respective files ?


> --- /dev/null
> +++ b/src/amd/common/Makefile.am

> +AM_CPPFLAGS = \
> +       $(VALGRIND_CFLAGS) \
> +       $(DEFINES) \
> +       -I$(top_srcdir)/include \
> +       -I$(top_builddir)/src \
> +       -I$(top_srcdir)/src \
> +       -I$(top_builddir)/src/compiler \
> +       -I$(top_builddir)/src/compiler/nir \
> +       -I$(top_srcdir)/src/compiler \
> +       -I$(top_srcdir)/src/mapi \
> +       -I$(top_srcdir)/src/mesa \
> +       -I$(top_srcdir)/src/mesa/drivers/dri/common \
> +       -I$(top_srcdir)/src/gallium/auxiliary \
> +       -I$(top_srcdir)/src/gallium/include
> +
I'm leaning that at least some of the above can be nuked, but if it's
too much of a hassle just add a comment on top - XXX/TODO or other.


> +AM_CFLAGS = -Wno-override-init -msse2 \
>From a curtecy skim though, neither of these two are required. Please drop them.

> +       $(VISIBILITY_CFLAGS) \
> +       $(PTHREAD_CFLAGS) \
> +       $(LLVM_CFLAGS) \
> +       $(LIBELF_CFLAGS)
> +
> +AM_CXXFLAGS = \
> +       $(VISIBILITY_CXXFLAGS) \
> +       $(MSVC2013_COMPAT_CXXFLAGS) \
I don't think we're about to build things with MSVC anytime soon, so
we can drop this line.


> --- /dev/null
> +++ b/src/amd/common/Makefile.sources


> +AMD_COMPILER_SOURCES := \
> +       ac_binary.c \
> +       ac_llvm_util.c \
> +       ac_nir_to_llvm.c \
> +       ac_llvm_helper.cpp
Please list all the files.

ac_binary.c
ac_binary.h
ac_llvm_helper.cpp
ac_llvm_util.c
ac_llvm_util.h
ac_nir_to_llvm.c
ac_nir_to_llvm.h
ac_radeon_winsys.h



> +#pragma once
> +
Please don't use pragma once, but ifdef FOO/define FOO guards.


Thanks
Emil