[Mesa-dev] [PATCH 01/12] i965: Move brw_dump_ir() out of brw_*_emit() functions
Iago Toral
itoral at igalia.com
Thu Oct 8 04:23:27 PDT 2015
Well, this is weird.... this patch makes this crash:
INTEL_DEBUG=vs glxgears
because that uses a vertex program, so &vs->base is a NULL pointer
dereference in this call:
brw_dump_ir("vertex", prog, &vs->base, &vp->program.Base);
The surprising part is that this is the same without this patch (!) just
that the NULL dereference is not actually producing a segfault in that
case. Apparently, dereferencing a NULL pointer is undefined behavior
[1], so I guess that explains why the behavior is inconsistent and that
we have just being lucky all this time.
I suppose we want to do:
brw_dump_ir("vertex", prog,
vs ? &vs->base : NULL,
vp ? &vp->program.Base: NULL);
Ans the same for the other calls to brw_dump_ir...
[1]
http://stackoverflow.com/questions/6793262/why-dereferencing-a-null-pointer-is-undefined-behaviour
On Wed, 2015-10-07 at 07:11 -0700, Kristian Høgsberg Kristensen wrote:
> We move these calls one level up into the codegen functions.
>
> Signed-off-by: Kristian Høgsberg Kristensen <krh at bitplanet.net>
> ---
> src/mesa/drivers/dri/i965/brw_cs.c | 3 +++
> src/mesa/drivers/dri/i965/brw_fs.cpp | 13 -------------
> src/mesa/drivers/dri/i965/brw_gs.c | 3 +++
> src/mesa/drivers/dri/i965/brw_vec4.cpp | 7 -------
> src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.cpp | 3 ---
> src/mesa/drivers/dri/i965/brw_vs.c | 3 +++
> src/mesa/drivers/dri/i965/brw_wm.c | 3 +++
> 7 files changed, 12 insertions(+), 23 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_cs.c b/src/mesa/drivers/dri/i965/brw_cs.c
> index 6b64030..34680ee 100644
> --- a/src/mesa/drivers/dri/i965/brw_cs.c
> +++ b/src/mesa/drivers/dri/i965/brw_cs.c
> @@ -98,6 +98,9 @@ brw_codegen_cs_prog(struct brw_context *brw,
> start_time = get_time();
> }
>
> + if (unlikely(INTEL_DEBUG & DEBUG_CS))
> + brw_dump_ir("compute", prog, &cs->base, &cp->program.Base);
> +
> program = brw_cs_emit(brw, mem_ctx, key, &prog_data,
> &cp->program, prog, &program_size);
> if (program == NULL) {
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index 1187c67..1dee888 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -5100,13 +5100,6 @@ brw_wm_fs_emit(struct brw_context *brw,
> struct gl_shader_program *prog,
> unsigned *final_assembly_size)
> {
> - struct brw_shader *shader = NULL;
> - if (prog)
> - shader = (brw_shader *) prog->_LinkedShaders[MESA_SHADER_FRAGMENT];
> -
> - if (unlikely(INTEL_DEBUG & DEBUG_WM))
> - brw_dump_ir("fragment", prog, &shader->base, &fp->Base);
> -
> int st_index8 = -1, st_index16 = -1;
> if (INTEL_DEBUG & DEBUG_SHADER_TIME) {
> st_index8 = brw_get_shader_time_index(brw, prog, &fp->Base, ST_FS8);
> @@ -5224,12 +5217,6 @@ brw_cs_emit(struct brw_context *brw,
> struct gl_shader_program *prog,
> unsigned *final_assembly_size)
> {
> - struct brw_shader *shader =
> - (struct brw_shader *) prog->_LinkedShaders[MESA_SHADER_COMPUTE];
> -
> - if (unlikely(INTEL_DEBUG & DEBUG_CS))
> - brw_dump_ir("compute", prog, &shader->base, &cp->Base);
> -
> prog_data->local_size[0] = cp->LocalSize[0];
> prog_data->local_size[1] = cp->LocalSize[1];
> prog_data->local_size[2] = cp->LocalSize[2];
> diff --git a/src/mesa/drivers/dri/i965/brw_gs.c b/src/mesa/drivers/dri/i965/brw_gs.c
> index f6b9874..26c91e4 100644
> --- a/src/mesa/drivers/dri/i965/brw_gs.c
> +++ b/src/mesa/drivers/dri/i965/brw_gs.c
> @@ -291,6 +291,9 @@ brw_codegen_gs_prog(struct brw_context *brw,
> */
> c.prog_data.base.urb_read_length = (c.input_vue_map.num_slots + 1) / 2;
>
> + if (unlikely(INTEL_DEBUG & DEBUG_GS))
> + brw_dump_ir("geometry", prog, gs, NULL);
> +
> void *mem_ctx = ralloc_context(NULL);
> unsigned program_size;
> const unsigned *program =
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> index 76ce0c4..4b4a216 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> @@ -1947,17 +1947,10 @@ brw_vs_emit(struct brw_context *brw,
> {
> const unsigned *assembly = NULL;
>
> - struct brw_shader *shader = NULL;
> - if (prog)
> - shader = (brw_shader *) prog->_LinkedShaders[MESA_SHADER_VERTEX];
> -
> int st_index = -1;
> if (INTEL_DEBUG & DEBUG_SHADER_TIME)
> st_index = brw_get_shader_time_index(brw, prog, &vp->Base, ST_VS);
>
> - if (unlikely(INTEL_DEBUG & DEBUG_VS))
> - brw_dump_ir("vertex", prog, &shader->base, &vp->Base);
> -
> if (brw->intelScreen->compiler->scalar_vs) {
> prog_data->base.dispatch_mode = DISPATCH_MODE_SIMD8;
>
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.cpp b/src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.cpp
> index f6967a7..36ab25d 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.cpp
> @@ -622,9 +622,6 @@ brw_gs_emit(struct brw_context *brw,
> {
> struct gl_shader *shader = prog->_LinkedShaders[MESA_SHADER_GEOMETRY];
>
> - if (unlikely(INTEL_DEBUG & DEBUG_GS))
> - brw_dump_ir("geometry", prog, shader, NULL);
> -
> int st_index = -1;
> if (INTEL_DEBUG & DEBUG_SHADER_TIME)
> st_index = brw_get_shader_time_index(brw, prog, NULL, ST_GS);
> diff --git a/src/mesa/drivers/dri/i965/brw_vs.c b/src/mesa/drivers/dri/i965/brw_vs.c
> index 3c6ee0a..34c58b8 100644
> --- a/src/mesa/drivers/dri/i965/brw_vs.c
> +++ b/src/mesa/drivers/dri/i965/brw_vs.c
> @@ -195,6 +195,9 @@ brw_codegen_vs_prog(struct brw_context *brw,
> start_time = get_time();
> }
>
> + if (unlikely(INTEL_DEBUG & DEBUG_VS) && vs->base.ir)
> + brw_dump_ir("vertex", prog, &vs->base, &vp->program.Base);
> +
> /* Emit GEN4 code.
> */
> program = brw_vs_emit(brw, mem_ctx, key, &prog_data,
> diff --git a/src/mesa/drivers/dri/i965/brw_wm.c b/src/mesa/drivers/dri/i965/brw_wm.c
> index 9892046..242114f 100644
> --- a/src/mesa/drivers/dri/i965/brw_wm.c
> +++ b/src/mesa/drivers/dri/i965/brw_wm.c
> @@ -221,6 +221,9 @@ brw_codegen_wm_prog(struct brw_context *brw,
> start_time = get_time();
> }
>
> + if (unlikely(INTEL_DEBUG & DEBUG_WM) && fs->base.ir)
> + brw_dump_ir("fragment", prog, &fs->base, &fp->program.Base);
> +
> program = brw_wm_fs_emit(brw, mem_ctx, key, &prog_data,
> &fp->program, prog, &program_size);
> if (program == NULL) {
More information about the mesa-dev
mailing list