On 04/11/2014 10:29 PM, Matt Turner wrote:
> ---
> src/mesa/drivers/dri/i965/brw_fs.cpp | 129
> +++++++++++++---------
> src/mesa/drivers/dri/i965/brw_fs.h | 2 +
> src/mesa/drivers/dri/i965/brw_fs_reg_allocate.cpp | 2 +-
> src/mesa/drivers/dri/i965/brw_shader.cpp | 20 +++-
> src/mesa/drivers/dri/i965/brw_shader.h | 2 +
> src/mesa/drivers/dri/i965/brw_vec4.cpp | 104 +++++++++--------
> src/mesa/drivers/dri/i965/brw_vec4.h | 1 +
> 7 files changed, 154 insertions(+), 106 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp
> b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index 85a5463..c1d79e1 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -2810,197 +2810,218 @@ fs_visitor::lower_uniform_pull_constant_loads()
> void
> fs_visitor::dump_instructions()
> {
> + dump_instructions(NULL);
> +}
> +
> +void
> +fs_visitor::dump_instructions(const char *name)
> +{
> + invalidate_live_intervals();
> calculate_register_pressure();
> + FILE *file = stderr;
> + if (name) {
> + file = fopen(name, "w");
> + }You're playing with fire here. Opening files is dangerous, considering that the X server ofter runs as root and executes this code. One mistake in debugging code, and...root exploit. We may want to restrict this to non-root users by default, and maybe only in debug builds... At any rate, I'd like to see patches 3-5 respun as follows: 1. Make dump_instruction() and dump_instructions() take FILE * parameters, and do the s/stderr/file/g sed job you've done here. This is the bulk of the change, and easily justifiable - we can always use stdout vs. stderr or the like. 2. Introduce any code that opens files for writing. With this being separate, we can think through the security implications. 3. Your actual optimization debugging. I would probably squash patch 3 into this patch, since it doesn't really stand alone. *shrug* [snip] > diff --git a/src/mesa/drivers/dri/i965/brw_vec4.h > b/src/mesa/drivers/dri/i965/brw_vec4.h > index 159a5bd..43c3f64 100644 > --- a/src/mesa/drivers/dri/i965/brw_vec4.h > +++ b/src/mesa/drivers/dri/i965/brw_vec4.h > @@ -597,6 +597,7 @@ public: > bool process_move_condition(ir_rvalue *ir); > > void dump_instruction(backend_instruction *inst); > + void dump_instruction(backend_instruction *inst, FILE *file); Usually, this is done via a default argument: void dump_instruction(backend_instruction *inst, FILE *file = stderr); That would probably be cleaner. > > void visit_atomic_counter_intrinsic(ir_call *ir);
signature.asc
Description: OpenPGP digital signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
