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.
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
pgpst8mff3E_Q.pgp
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
