Re: [Mesa-dev] [PATCH 4/4] i965: Silence unused parameter warnings
On Friday, 2016-10-14 19:26:38 -0700, Ian Romanick wrote:
> 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. :)
Fair enough :)
>
> > (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
> >
> >
> > On Fri, Oct 14, 2016 at 11:59:47AM -0700, Ian Romanick wrote:
> >> From: Ian Romanick
> >>
> >> 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
> >> ---
> >> 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
Re: [Mesa-dev] [PATCH 4/4] i965: Silence unused parameter warnings
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
>
>
> On Fri, Oct 14, 2016 at 11:59:47AM -0700, Ian Romanick wrote:
>> From: Ian Romanick
>>
>> 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
>> ---
>> 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);
>
Re: [Mesa-dev] [PATCH 4/4] i965: Silence unused parameter warnings
> 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.
(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
On Fri, Oct 14, 2016 at 11:59:47AM -0700, Ian Romanick wrote:
> From: Ian Romanick
>
> 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
> ---
> 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
[Mesa-dev] [PATCH 4/4] i965: Silence unused parameter warnings
From: Ian Romanick
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
---
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
