On Thu, May 15, 2014 at 06:22:11PM +0200, Francisco Jerez wrote: > Bruno Jimenez <brunoji...@gmail.com> writes: > > > Hi, > > > > I'm trying to make clover fail in the case that a kernel fails to build. > > My first attempt has been to track a llvm compile failure up from > > 'radeon_llvm_compile' (at radeon/radeon_llvm_emit.c), through > > 'r600_llvm_compile' (at r600/r600_llvm.c) and through > > 'evergreen_launch_grid' (at r600/evergreen_compute.c), then detect the > > failure when launching a kernel and throw an error. > > > > The attached patch is based on this, and it kind of works, as for the > > first kernel that fails to compile it will return the corresponding > > error (CL_INVALID_PROGRAM_EXECUTABLE), but for the following kernels it > > crashes completely. > > > > I think that this is not how we should detect compile errors, nor where > > we should. Because, after reading the OpenCL 1.1 spec, I think that they > > should be handled at 'clBuildProgram'. > > > > The spec for OpenCL 1.1 says about 'clBuildProgram': > > 'builds (compiles & links) a program executable [...]' > > And there's an error for build failures: > > 'CL_BUILD_PROGRAM_FAILURE if there is a failure to build the program > > executable' > > > > Any thoughts about this? > > Yeah... I think you're right and this is a dead end, it's probably not > the way we should handle this situation. Compiler errors should be > returned from the compiler, not from launch_grid(). This is likely to > be an LLVM back-end bug. Clang shouldn't return successfully if the > program is malformed and the pipe driver won't be able to deal with the > generated code. If the fact that clover::compile_program_llvm() doesn't > detect the failure is caused by radeon's two-step compilation process, I > guess that this is another reason to refactor the radeon compiler code > so the whole compilation happens in one step from the LLVM back-end and > most of what radeon_llvm_compile() does now is handled from within clang > as it's called from clover::compile_program_llvm(), which would generate > machine code directly instead of LLVM IR. >
I've mostly implemented moving the compilation phase into clover, but it is dependent on this patch: http://lists.freedesktop.org/archives/mesa-dev/2014-March/055424.html until function calls are implemented, which may be a while. Is there some other way to handle this? Maybe a flag on struct pipe_compute_state? -Tom > Thanks. > > > Bruno > > From e92bc2804b60ebb69268b02df7f57301aea8b20b Mon Sep 17 00:00:00 2001 > > From: =?UTF-8?q?Bruno=20Jim=C3=A9nez?= <brunoji...@gmail.com> > > Date: Wed, 14 May 2014 17:04:50 +0200 > > Subject: [PATCH] exit if llvm_compile fails > > > > --- > > src/gallium/drivers/ilo/ilo_gpgpu.c | 2 +- > > src/gallium/drivers/nouveau/nvc0/nvc0_compute.c | 2 +- > > src/gallium/drivers/nouveau/nvc0/nvc0_context.h | 8 ++++---- > > src/gallium/drivers/nouveau/nvc0/nve4_compute.c | 2 +- > > src/gallium/drivers/r600/evergreen_compute.c | 12 ++++++++---- > > src/gallium/drivers/radeonsi/si_compute.c | 2 +- > > src/gallium/include/pipe/p_context.h | 6 +++--- > > src/gallium/state_trackers/clover/core/kernel.cpp | 14 +++++++++----- > > 8 files changed, 28 insertions(+), 20 deletions(-) > > > > diff --git a/src/gallium/drivers/ilo/ilo_gpgpu.c > > b/src/gallium/drivers/ilo/ilo_gpgpu.c > > index b17a518..19990fc 100644 > > --- a/src/gallium/drivers/ilo/ilo_gpgpu.c > > +++ b/src/gallium/drivers/ilo/ilo_gpgpu.c > > @@ -32,7 +32,7 @@ > > * This is a placeholder. We will need something similar to > > ilo_3d_pipeline. > > */ > > > > -static void > > +static unsigned > > ilo_launch_grid(struct pipe_context *pipe, > > const uint *block_layout, const uint *grid_layout, > > uint32_t pc, const void *input) > > diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c > > b/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c > > index ad287a2..26ace0a 100644 > > --- a/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c > > +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c > > @@ -193,7 +193,7 @@ nvc0_compute_upload_input(struct nvc0_context *nvc0, > > const void *input) > > } > > } > > > > -void > > +unsigned > > nvc0_launch_grid(struct pipe_context *pipe, > > const uint *block_layout, const uint *grid_layout, > > uint32_t label, > > diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_context.h > > b/src/gallium/drivers/nouveau/nvc0/nvc0_context.h > > index 76416a0..b1bbc54 100644 > > --- a/src/gallium/drivers/nouveau/nvc0/nvc0_context.h > > +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_context.h > > @@ -349,11 +349,11 @@ nvc0_video_buffer_create(struct pipe_context *pipe, > > void nvc0_push_vbo(struct nvc0_context *, const struct pipe_draw_info *); > > > > /* nve4_compute.c */ > > -void nve4_launch_grid(struct pipe_context *, > > - const uint *, const uint *, uint32_t, const void *); > > +unsigned nve4_launch_grid(struct pipe_context *, > > + const uint *, const uint *, uint32_t, const void > > *); > > > > /* nvc0_compute.c */ > > -void nvc0_launch_grid(struct pipe_context *, > > - const uint *, const uint *, uint32_t, const void *); > > +unsigned nvc0_launch_grid(struct pipe_context *, > > + const uint *, const uint *, uint32_t, const void > > *); > > > > #endif > > diff --git a/src/gallium/drivers/nouveau/nvc0/nve4_compute.c > > b/src/gallium/drivers/nouveau/nvc0/nve4_compute.c > > index f243316..593d455 100644 > > --- a/src/gallium/drivers/nouveau/nvc0/nve4_compute.c > > +++ b/src/gallium/drivers/nouveau/nvc0/nve4_compute.c > > @@ -428,7 +428,7 @@ nve4_compute_alloc_launch_desc(struct nouveau_context > > *nv, > > return (struct nve4_cp_launch_desc *)ptr; > > } > > > > -void > > +unsigned > > nve4_launch_grid(struct pipe_context *pipe, > > const uint *block_layout, const uint *grid_layout, > > uint32_t label, > > diff --git a/src/gallium/drivers/r600/evergreen_compute.c > > b/src/gallium/drivers/r600/evergreen_compute.c > > index 701bb5c..305bd84 100644 > > --- a/src/gallium/drivers/r600/evergreen_compute.c > > +++ b/src/gallium/drivers/r600/evergreen_compute.c > > @@ -538,7 +538,7 @@ void evergreen_emit_cs_shader( > > RADEON_PRIO_SHADER_DATA)); > > } > > > > -static void evergreen_launch_grid( > > +static unsigned evergreen_launch_grid( > > struct pipe_context *ctx_, > > const uint *block_layout, const uint *grid_layout, > > uint32_t pc, const void *input) > > @@ -550,6 +550,8 @@ static void evergreen_launch_grid( > > > > COMPUTE_DBG(ctx->screen, "*** evergreen_launch_grid: pc = %u\n", pc); > > > > + shader->active_kernel = kernel; > > + ctx->cs_shader_state.kernel_index = pc; > > #ifdef HAVE_OPENCL > > > > if (!kernel->code_bo) { > > @@ -566,7 +568,9 @@ static void evergreen_launch_grid( > > ctx->screen->has_compressed_msaa_texturing); > > bc->type = TGSI_PROCESSOR_COMPUTE; > > bc->isa = ctx->isa; > > - r600_llvm_compile(mod, ctx->b.family, bc, &use_kill, dump); > > + > > + if (r600_llvm_compile(mod, ctx->b.family, bc, &use_kill, dump)) > > + return 1; > > > > if (dump && !sb_disasm) { > > r600_bytecode_disasm(bc); > > @@ -582,10 +586,10 @@ static void evergreen_launch_grid( > > ctx->b.ws->buffer_unmap(kernel->code_bo->cs_buf); > > } > > #endif > > - shader->active_kernel = kernel; > > - ctx->cs_shader_state.kernel_index = pc; > > evergreen_compute_upload_input(ctx_, block_layout, grid_layout, input); > > compute_emit_cs(ctx, block_layout, grid_layout); > > + > > + return 0; > > } > > > > static void evergreen_set_compute_resources(struct pipe_context * ctx_, > > diff --git a/src/gallium/drivers/radeonsi/si_compute.c > > b/src/gallium/drivers/radeonsi/si_compute.c > > index c0637f6..4ff7f7a 100644 > > --- a/src/gallium/drivers/radeonsi/si_compute.c > > +++ b/src/gallium/drivers/radeonsi/si_compute.c > > @@ -117,7 +117,7 @@ static void si_set_global_binding( > > } > > } > > > > -static void si_launch_grid( > > +static unsigned si_launch_grid( > > struct pipe_context *ctx, > > const uint *block_layout, const uint *grid_layout, > > uint32_t pc, const void *input) > > diff --git a/src/gallium/include/pipe/p_context.h > > b/src/gallium/include/pipe/p_context.h > > index bc43530..0fa9278 100644 > > --- a/src/gallium/include/pipe/p_context.h > > +++ b/src/gallium/include/pipe/p_context.h > > @@ -520,9 +520,9 @@ struct pipe_context { > > * should point to a buffer of at least > > * pipe_compute_state::req_input_mem bytes. > > */ > > - void (*launch_grid)(struct pipe_context *context, > > - const uint *block_layout, const uint *grid_layout, > > - uint32_t pc, const void *input); > > + unsigned (*launch_grid)(struct pipe_context *context, > > + const uint *block_layout, const uint > > *grid_layout, > > + uint32_t pc, const void *input); > > /*@}*/ > > > > /** > > diff --git a/src/gallium/state_trackers/clover/core/kernel.cpp > > b/src/gallium/state_trackers/clover/core/kernel.cpp > > index 5e5fe51..5517b17 100644 > > --- a/src/gallium/state_trackers/clover/core/kernel.cpp > > +++ b/src/gallium/state_trackers/clover/core/kernel.cpp > > @@ -70,6 +70,7 @@ kernel::launch(command_queue &q, > > const auto reduced_grid_size = > > map(divides(), grid_size, block_size); > > void *st = exec.bind(&q); > > + unsigned err; > > > > // The handles are created during exec_context::bind(), so we need make > > // sure to call exec_context::bind() before retrieving them. > > @@ -89,11 +90,11 @@ kernel::launch(command_queue &q, > > q.pipe->set_global_binding(q.pipe, 0, exec.g_buffers.size(), > > exec.g_buffers.data(), g_handles.data()); > > > > - q.pipe->launch_grid(q.pipe, > > - pad_vector(q, block_size, 1).data(), > > - pad_vector(q, reduced_grid_size, 1).data(), > > - find(name_equals(_name), m.syms).offset, > > - exec.input.data()); > > + err = q.pipe->launch_grid(q.pipe, > > + pad_vector(q, block_size, 1).data(), > > + pad_vector(q, reduced_grid_size, 1).data(), > > + find(name_equals(_name), m.syms).offset, > > + exec.input.data()); > > > > q.pipe->set_global_binding(q.pipe, 0, exec.g_buffers.size(), NULL, > > NULL); > > q.pipe->set_compute_resources(q.pipe, 0, exec.resources.size(), NULL); > > @@ -102,6 +103,9 @@ kernel::launch(command_queue &q, > > q.pipe->bind_sampler_states(q.pipe, PIPE_SHADER_COMPUTE, 0, > > exec.samplers.size(), NULL); > > exec.unbind(); > > + > > + if (err) > > + throw error(CL_INVALID_PROGRAM_EXECUTABLE); > > } > > > > size_t > > -- > > 1.9.2 > _______________________________________________ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
