[Mesa-dev] [PATCH 4/4] i965: Silence unused parameter warnings
Ian Romanick
idr at freedesktop.org
Sat Oct 15 02:26:38 UTC 2016
On 10/14/2016 05:44 PM, Eric Engestrom wrote:
>> Subject: [PATCH 4/4] i965: Silence unused parameter warnings
>
> How about "remove unused parameters" instead?
> Silencing the warnings is nothing more than a side effect of this
> change, albeit the reason you realised it was needed.
There are already quite a few commits in Mesa with subjects that match
this pattern. I've generally used this when the purpose of the change
is to make the compiler complain less.
Also... not all of the changes in this commit remove the unused
parameter. :)
> (Sorry, I've seen so many "silence warning" commits at $DAYJOB that
> this has become a pet peeve of mine)
>
> One suggestion below, but the series looks good:
> Reviewed-by: Eric Engestrom <eric at engestrom.ch>
>
>
> On Fri, Oct 14, 2016 at 11:59:47AM -0700, Ian Romanick wrote:
>> From: Ian Romanick <ian.d.romanick at intel.com>
>>
>> brw_link.cpp:76:44: warning: unused parameter ‘shader_type’ [-Wunused-parameter]
>> gl_shader_stage shader_type,
>> ^
>> brw_nir.c: In function ‘brw_nir_lower_vs_inputs’:
>> brw_nir.c:194:55: warning: unused parameter ‘devinfo’ [-Wunused-parameter]
>> const struct gen_device_info *devinfo,
>> ^
>> brw_vec4_visitor.cpp:914:37: warning: unused parameter ‘sampler’ [-Wunused-parameter]
>> uint32_t sampler,
>> ^
>> brw_vec4_visitor.cpp:1146:34: warning: unused parameter ‘stream_id’ [-Wunused-parameter]
>> vec4_visitor::gs_emit_vertex(int stream_id)
>> ^
>>
>> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
>> ---
>> src/mesa/drivers/dri/i965/brw_link.cpp | 3 +--
>> src/mesa/drivers/dri/i965/brw_nir.c | 1 -
>> src/mesa/drivers/dri/i965/brw_nir.h | 1 -
>> src/mesa/drivers/dri/i965/brw_vec4.cpp | 2 +-
>> src/mesa/drivers/dri/i965/brw_vec4.h | 2 +-
>> src/mesa/drivers/dri/i965/brw_vec4_nir.cpp | 2 +-
>> src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp | 3 +--
>> 7 files changed, 5 insertions(+), 9 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_link.cpp b/src/mesa/drivers/dri/i965/brw_link.cpp
>> index 02151d6..5ea9773 100644
>> --- a/src/mesa/drivers/dri/i965/brw_link.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_link.cpp
>> @@ -73,7 +73,6 @@ brw_shader_precompile(struct gl_context *ctx,
>>
>> static void
>> brw_lower_packing_builtins(struct brw_context *brw,
>> - gl_shader_stage shader_type,
>> exec_list *ir)
>> {
>> /* Gens < 7 don't have instructions to convert to or from half-precision,
>> @@ -105,7 +104,7 @@ process_glsl_ir(struct brw_context *brw,
>> /* lower_packing_builtins() inserts arithmetic instructions, so it
>> * must precede lower_instructions().
>> */
>> - brw_lower_packing_builtins(brw, shader->Stage, shader->ir);
>> + brw_lower_packing_builtins(brw, shader->ir);
>> do_mat_op_to_vec(shader->ir);
>>
>> unsigned instructions_to_lower = (DIV_TO_MUL_RCP |
>> diff --git a/src/mesa/drivers/dri/i965/brw_nir.c b/src/mesa/drivers/dri/i965/brw_nir.c
>> index 744865b..a935f42 100644
>> --- a/src/mesa/drivers/dri/i965/brw_nir.c
>> +++ b/src/mesa/drivers/dri/i965/brw_nir.c
>> @@ -191,7 +191,6 @@ remap_patch_urb_offsets(nir_block *block, nir_builder *b,
>>
>> void
>> brw_nir_lower_vs_inputs(nir_shader *nir,
>> - const struct gen_device_info *devinfo,
>> bool is_scalar,
>> bool use_legacy_snorm_formula,
>> const uint8_t *vs_attrib_wa_flags)
>> diff --git a/src/mesa/drivers/dri/i965/brw_nir.h b/src/mesa/drivers/dri/i965/brw_nir.h
>> index 425d6ce..aef5c53 100644
>> --- a/src/mesa/drivers/dri/i965/brw_nir.h
>> +++ b/src/mesa/drivers/dri/i965/brw_nir.h
>> @@ -99,7 +99,6 @@ nir_shader *brw_preprocess_nir(const struct brw_compiler *compiler,
>> bool brw_nir_lower_intrinsics(nir_shader *nir,
>> struct brw_stage_prog_data *prog_data);
>> void brw_nir_lower_vs_inputs(nir_shader *nir,
>> - const struct gen_device_info *devinfo,
>> bool is_scalar,
>> bool use_legacy_snorm_formula,
>> const uint8_t *vs_attrib_wa_flags);
>> diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp b/src/mesa/drivers/dri/i965/brw_vec4.cpp
>> index 6aa9102..362f32b 100644
>> --- a/src/mesa/drivers/dri/i965/brw_vec4.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp
>> @@ -2114,7 +2114,7 @@ brw_compile_vs(const struct brw_compiler *compiler, void *log_data,
>> nir_shader *shader = nir_shader_clone(mem_ctx, src_shader);
>> shader = brw_nir_apply_sampler_key(shader, compiler->devinfo, &key->tex,
>> is_scalar);
>> - brw_nir_lower_vs_inputs(shader, compiler->devinfo, is_scalar,
>> + brw_nir_lower_vs_inputs(shader, is_scalar,
>> use_legacy_snorm_formula, key->gl_attrib_wa_flags);
>> brw_nir_lower_vue_outputs(shader, is_scalar);
>> shader = brw_postprocess_nir(shader, compiler->devinfo, is_scalar);
>> diff --git a/src/mesa/drivers/dri/i965/brw_vec4.h b/src/mesa/drivers/dri/i965/brw_vec4.h
>> index 1505ba6..62c6007 100644
>> --- a/src/mesa/drivers/dri/i965/brw_vec4.h
>> +++ b/src/mesa/drivers/dri/i965/brw_vec4.h
>> @@ -262,7 +262,7 @@ public:
>> src_reg offset_value,
>> src_reg mcs,
>> uint32_t surface, src_reg surface_reg,
>> - uint32_t sampler, src_reg sampler_reg);
>> + src_reg sampler_reg);
>>
>> src_reg emit_mcs_fetch(const glsl_type *coordinate_type, src_reg coordinate,
>> src_reg surface);
>> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp b/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
>> index 1d834a4..7b36fca 100644
>> --- a/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
>> @@ -1967,7 +1967,7 @@ vec4_visitor::nir_emit_texture(nir_tex_instr *instr)
>> shadow_comparitor,
>> lod, lod2, sample_index,
>> constant_offset, offset_value, mcs,
>> - texture, texture_reg, sampler, sampler_reg);
>> + texture, texture_reg, sampler_reg);
>> }
>>
>> void
>> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
>> index 3e785bc..eca753c 100644
>> --- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
>> @@ -909,7 +909,6 @@ vec4_visitor::emit_texture(ir_texture_opcode op,
>> src_reg mcs,
>> uint32_t surface,
>> src_reg surface_reg,
>> - uint32_t sampler,
>> src_reg sampler_reg)
>> {
>> /* The sampler can only meaningfully compute LOD for fragment shader
>> @@ -1141,7 +1140,7 @@ vec4_visitor::emit_gen6_gather_wa(uint8_t wa, dst_reg dst)
>> }
>>
>> void
>> -vec4_visitor::gs_emit_vertex(int stream_id)
>> +vec4_visitor::gs_emit_vertex(int /* stream_id */)
>
> `UNUSED`?
I generally prefer this method in C++ code. With UNUSED you don't get
any notification when you start to use a previously unused parameter.
We previously used (void) casting, and that had the same problem. If
the parameter has no name, you know, without a doubt, when you start
using a parameter that was previously unused.
It also makes it easier to scan all the method implementations and
determine that the parameter is never used in any derived class. Right
now I can do this:
$ grep -r gs_emit_vertex src/mesa/drivers/dri/
src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp:vec4_visitor::gs_emit_vertex(int /* stream_id */)
src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.cpp:vec4_gs_visitor::gs_emit_vertex(int stream_id)
src/mesa/drivers/dri/i965/brw_vec4_gs_nir.cpp: gs_emit_vertex(stream_id);
src/mesa/drivers/dri/i965/gen6_gs_visitor.h: virtual void gs_emit_vertex(int stream_id);
src/mesa/drivers/dri/i965/gen6_gs_visitor.cpp:gen6_gs_visitor::gs_emit_vertex(int stream_id)
src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.h: virtual void gs_emit_vertex(int stream_id);
src/mesa/drivers/dri/i965/brw_vec4.h: virtual void gs_emit_vertex(int stream_id);
Assuming that everyone uses the same technique, I see that stream_id is
almost always used. I don't even have to look at the implementations.
If they were marked UNUSED, it might be incorrect. Someone may have
started using the parameter without removing UNUSED. Ditto for (void)
casting.
>> {
>> unreachable("not reached");
>> }
>> --
>> 2.5.5
>
More information about the mesa-dev
mailing list