[Libclc-dev] [PATCH 4/4] spirv: add ability to build spirv from libclc
David Airlie via Libclc-dev
libclc-dev at lists.llvm.org
Sun Oct 13 21:10:01 PDT 2019
On Sun, Oct 13, 2019 at 2:58 PM Jan Vesely <jan.vesely at rutgers.edu> wrote:
>
> On Fri, 2019-09-27 at 10:59 +1000, Dave Airlie via Libclc-dev wrote:
> > This adds the ability to create two new targets (spirv--.spv and spirv64--.spv)
> > that contain the SPIR-V builds of the major libclc components.
> >
> > It uses a separate sources file as it doesn't require any of the sources
> > that directly wrap llvm intrinsics or use __builtin intrinsics.
> >
> > It builds unoptimised bytecode as the llvm -> spirv translator requires it.
> > It inlines only some functions, but in general we don't want inlining as
> > it makes the binary quite large.
> >
> > v2: drop shuffle. add vload/store_half variants
> > ---
> > libclc/CMakeLists.txt | 67 +++++++++++++++++++++---
> > libclc/generic/lib/SOURCES-SPIRV | 82 ++++++++++++++++++++++++++++++
> > libclc/generic/lib/shared/vload.cl | 3 +-
> > 3 files changed, 143 insertions(+), 9 deletions(-)
> > create mode 100644 libclc/generic/lib/SOURCES-SPIRV
> >
> > diff --git a/libclc/CMakeLists.txt b/libclc/CMakeLists.txt
> > index 440eab07650..7af3eaa51ee 100644
> > --- a/libclc/CMakeLists.txt
> > +++ b/libclc/CMakeLists.txt
> > @@ -39,6 +39,13 @@ if( ${LLVM_VERSION} VERSION_GREATER "3.9.0" )
> > set( LIBCLC_TARGETS_ALL ${LIBCLC_TARGETS_ALL} amdgcn-mesa-mesa3d )
> > endif()
> >
> > +find_program( LLVM_SPIRV llvm-spirv PATHS ${LLVM_BINDIR} )
> > +if( NOT LLVM_SPIRV )
> > + message( "libclc won't build spir-v support." )
> > +else()
> > + set ( LIBCLC_TARGETS_ALL ${LIBCLC_TARGETS_ALL} spirv-- spirv64-- )
> > +endif()
>
> No automagic dependencies please. The build should fail if spirv was
> requested and llvm-spirv not found.
The question then is if we should have spirv generation be a separate
item rather than part of the "ALL" as otherwise people who don't
install the spirv tools won't be able to generate their current
configuations.
> >
> > # Print toolchain
> > message( "clang: ${LLVM_CLANG}" )
> > message( "llvm-as: ${LLVM_AS}" )
> > message( "llvm-link: ${LLVM_LINK}" )
> > message( "opt: ${LLVM_OPT}" )
> > +message( "llvm-spirv: ${LLVM_SPIRV}" )
> > message( "" )
> > if( NOT LLVM_CLANG OR NOT LLVM_OPT OR NOT LLVM_AS OR NOT LLVM_LINK )
> > message( FATAL_ERROR "toolchain incomplete!" )
> > @@ -125,6 +134,10 @@ set( nvptx--_devices none )
> > set( nvptx64--_devices none )
> > set( nvptx--nvidiacl_devices none )
> > set( nvptx64--nvidiacl_devices none )
> > +if ( LLVM_SPIRV )
> > + set( spirv--_devices none )
> > + set( spirv64--_devices none )
> > +endif()
>
> I don't think there needs to be a conditional here.
>
> >
> > # Setup aliases
> > set( cedar_aliases palm sumo sumo2 redwood juniper )
> > @@ -187,10 +200,14 @@ foreach( t ${LIBCLC_TARGETS_TO_BUILD} )
> > set( DARCH ${ARCH} )
> > endif()
> >
> > + set( sourcefilename "SOURCES" )
> > + if ( ${ARCH} STREQUAL spirv OR ${ARCH} STREQUAL spirv64 )
> > + set( sourcefilename "SOURCES-SPIRV" )
> > + endif()
>
> I haven't gone through the details. How many files from SOURCES are
> not present in SOURCES_SPIRV?
Lots, there are 82 in the spirv sources vs 219 in sources here. The
SPIR-V runtime library doesn't quite need a lot of the stuff that can
just be done in spir-v->nir.
There may be cases in the future to add some more bits, but it seems
fine with the current baseline.
> > foreach( d ${${t}_devices} )
> > +
> > + set (dflags "")
> > + set (oflags "")
> > + if ( ${t} STREQUAL "spirv--" )
> > + set (target "spir--")
> > + set (dflags "-DCLC_SPIRV")
> > + set (oflags -O0 -finline-hint-functions)
> > + elseif ( ${t} STREQUAL "spirv64--" )
> > + set (target "spir64--")
> > + set (dflags "-DCLC_SPIRV")
> > + set (oflags -O0 -finline-hint-functions)
> > + else()
> > + set (target ${t})
> > + endif()
> > # Some targets don't have a specific GPU to target
> > if( ${d} STREQUAL "none" )
> > set( mcpu )
> > @@ -250,11 +286,12 @@ foreach( t ${LIBCLC_TARGETS_TO_BUILD} )
> > target_compile_definitions( builtins.link.${arch_suffix} PRIVATE
> > "__CLC_INTERNAL" )
> > target_compile_options( builtins.link.${arch_suffix} PRIVATE -target
> > - ${t} ${mcpu} -fno-builtin )
> > + ${target} ${mcpu} -fno-builtin ${oflags} ${dflags} )
> definitions should be handled in the target_compile_defitions above.
> I think we can just add generic LIBCLC_TARGET_${t}, alternatively
> clang defines __SPIR32__ and __SPIR64__ when targetting spir so we
> don't really need a custom define.
>
> Why -O0 necessary? is that not clang default?
> -finline-hint-functions can be probably enabled for all targets, but I
> need to check if it includes the always-inliner pass.
The LLVM->SPIRV converter only works on unoptimised LLVM IR, so we
have to pass -O0, but we want to inline any hinted functions.
My build here is defaulting clang to -O2 by the looks of it.
Do you suggest I change all the internal CLC_SPIRV to if
defined(__SPIR32__) || defined(__SPIR64__) ? might be a bit uglier but
it's not too many of them I suppose.
>
> > set_target_properties( builtins.link.${arch_suffix} PROPERTIES
> > LINKER_LANGUAGE CLC )
> >
> > set( obj_suffix ${arch_suffix}.bc )
> > + set( spv_suffix ${arch_suffix}.spv )
> >
> > # Add opt target
> > add_custom_command( OUTPUT "builtins.opt.${obj_suffix}"
> > @@ -265,6 +302,12 @@ foreach( t ${LIBCLC_TARGETS_TO_BUILD} )
> > add_custom_target( "opt.${obj_suffix}" ALL
> > DEPENDS "builtins.opt.${obj_suffix}" )
> >
> > + add_custom_command( OUTPUT "${spv_suffix}"
> > + COMMAND ${LLVM_SPIRV} -o
> > + "${spv_suffix}"
> > + "builtins.link.${obj_suffix}"
> > + DEPENDS "builtins.link.${arch_suffix}")
> > +
> > # Add prepare target
> > add_custom_command( OUTPUT "${obj_suffix}"
> > COMMAND prepare_builtins -o
> > @@ -274,8 +317,16 @@ foreach( t ${LIBCLC_TARGETS_TO_BUILD} )
> > "builtins.opt.${obj_suffix}"
> > prepare_builtins )
> > add_custom_target( "prepare-${obj_suffix}" ALL
> > - DEPENDS "${obj_suffix}" )
> > - install( FILES ${CMAKE_CURRENT_BINARY_DIR}/${obj_suffix} DESTINATION ${CMAKE_INSTALL_DATADIR}/clc )
> > + DEPENDS "${obj_suffix}" )
> > +
> > + if ( ${t} STREQUAL "spirv--" OR ${t} STREQUAL "spirv64--")
> > + # Add spv target
> > + add_custom_target( "spv.${spv_suffix}" ALL
> > + DEPENDS "${spv_suffix}" )
> > + install( FILES ${CMAKE_CURRENT_BINARY_DIR}/${spv_suffix} DESTINATION ${CMAKE_INSTALL_DATADIR}/clc )
> > + else()
> > + install( FILES ${CMAKE_CURRENT_BINARY_DIR}/${obj_suffix} DESTINATION ${CMAKE_INSTALL_DATADIR}/clc )
> > + endif()
> > # nvptx-- targets don't include workitem builtins
> > if( NOT ${t} MATCHES ".*ptx.*--$" )
> > add_test( NAME external-calls-${obj_suffix}
> > diff --git a/libclc/generic/lib/shared/vload.cl b/libclc/generic/lib/shared/vload.cl
> > index 9c37fcf9981..9d28aac10c0 100644
> > --- a/libclc/generic/lib/shared/vload.cl
> > +++ b/libclc/generic/lib/shared/vload.cl
> > @@ -33,6 +33,7 @@
> > VLOAD_VECTORIZE(__CLC_SCALAR_GENTYPE, __constant) \
> > VLOAD_VECTORIZE(__CLC_SCALAR_GENTYPE, __global) \
> >
> > +#ifndef __CLC_SPIRV__
> > #define VLOAD_TYPES() \
> > VLOAD_ADDR_SPACES(char) \
> > VLOAD_ADDR_SPACES(uchar) \
> > @@ -54,7 +55,7 @@ VLOAD_TYPES()
> > #pragma OPENCL EXTENSION cl_khr_fp16 : enable
> > VLOAD_ADDR_SPACES(half)
> > #endif
> > -
> > +#endif
>
> can you either split the .cl files changes, or include all of them?
Not sure what you mean, merge the previous ones into this patch or
split them all out? I think merging them makes more sense.
>
> I plan to take a more detailed look and actually build the library
> sometime this week. I think there's a way to simplify couple of things
> to make this integration easier, but it can always be cleaned up
> later.
I'm trying to rebase my mesa patches to use this onto master instead
of one of Karol's trees.
Thanks,
Dave.
More information about the Libclc-dev
mailing list