On 11/29/2014 12:10 PM, Kenneth Graunke wrote: > On Thursday, November 27, 2014 10:45:00 AM Abdiel Janulgue wrote: >> Add the the same restriction as in the previous try_emit_sat when trying >> to optimize clamp. Fixes an infinite loop in swrast where the lowering >> pass unpacks saturate into clamp but the opt_algebraic pass tries to do >> the opposite. >> >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=83463 >> >> Tested-by: Vinson Lee <v...@freedesktop.org> >> Signed-off-by: Abdiel Janulgue <abdiel.janul...@linux.intel.com> >> --- >> src/glsl/opt_algebraic.cpp | 3 ++- >> src/mesa/main/mtypes.h | 1 + >> src/mesa/program/ir_to_mesa.cpp | 3 ++- >> 3 files changed, 5 insertions(+), 2 deletions(-) >> >> diff --git a/src/glsl/opt_algebraic.cpp b/src/glsl/opt_algebraic.cpp >> index 430f5cb..62b14dd 100644 >> --- a/src/glsl/opt_algebraic.cpp >> +++ b/src/glsl/opt_algebraic.cpp >> @@ -679,7 +679,8 @@ ir_algebraic_visitor::handle_expression(ir_expression >> *ir) >> >> case ir_binop_min: >> case ir_binop_max: >> - if (ir->type->base_type != GLSL_TYPE_FLOAT) >> + if (ir->type->base_type != GLSL_TYPE_FLOAT || >> + options->EmitNoSat) >> break; >> >> /* Replace min(max) operations and its commutative combinations with >> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h >> index 7389baa..199443a 100644 >> --- a/src/mesa/main/mtypes.h >> +++ b/src/mesa/main/mtypes.h >> @@ -2990,6 +2990,7 @@ struct gl_shader_compiler_options >> GLboolean EmitNoMainReturn; /**< Emit CONT/RET opcodes? */ >> GLboolean EmitNoNoise; /**< Emit NOISE opcodes? */ >> GLboolean EmitNoPow; /**< Emit POW opcodes? */ >> + GLboolean EmitNoSat; >> GLboolean LowerClipDistance; /**< Lower gl_ClipDistance from float[8] to >> vec4[2]? */ >> >> /** >> diff --git a/src/mesa/program/ir_to_mesa.cpp >> b/src/mesa/program/ir_to_mesa.cpp >> index 5cd9058..44fd315 100644 >> --- a/src/mesa/program/ir_to_mesa.cpp >> +++ b/src/mesa/program/ir_to_mesa.cpp >> @@ -2935,7 +2935,7 @@ _mesa_ir_link_shader(struct gl_context *ctx, struct >> gl_shader_program *prog) >> >> bool progress; >> exec_list *ir = prog->_LinkedShaders[i]->ir; >> - const struct gl_shader_compiler_options *options = >> + struct gl_shader_compiler_options *options = >> >> &ctx->Const.ShaderCompilerOptions[prog->_LinkedShaders[i]->Stage]; >> >> do { >> @@ -2949,6 +2949,7 @@ _mesa_ir_link_shader(struct gl_context *ctx, struct >> gl_shader_program *prog) >> | ((options->EmitNoPow) ? POW_TO_EXP2 : 0) >> | ((target == GL_VERTEX_PROGRAM_ARB) ? >> SAT_TO_CLAMP >> : 0))); >> + options->EmitNoSat = (target == GL_VERTEX_PROGRAM_ARB); >> >> progress = do_lower_jumps(ir, true, true, options->EmitNoMainReturn, >> options->EmitNoCont, options->EmitNoLoops) || progress; > > NAK. You don't get to edit ctx->Const. It should be set up at context > initialization time (either in core mesa or in the driver), and never > touched after that. > > You should: > 1. Create an gl_shader_compiler_options::EmitNoSat field (hunk 2 of this > patch). > 2. Set it appropriately in all relevant drivers: > - swrast > - r200 > - i915 > - i965 > - Gallium (st_extensions.c) > You can skip radeon and nouveau_vieux as they don't support shaders. > 3. Make opt_algebraic pay attention to it (hunk 1 of this patch). > 4. Change all callers of lower_instructions with SAT_TO_CLAMP to > use options->EmitNoSat, not target == GL_VERTEX_PROGRAM stuff. > > It's unclear to me that we're making the correct decisions today - in other > words, setting EmitNoSat /properly/ for drivers may be a change in behavior. > > In particular, in commit cfa8c1cb, your st_glsl_to_tgsi.cpp hunk appears to be > doing SAT_TO_CLAMP lowering only for vertex shaders on pipe drivers that > *support SM3*. It seems like you meant to do lowering for ones that > *don't* support SM3. > > Please try to get this fixed ASAP - we have a release coming up in under a > week; we can't afford to wait two months to attend to simple regressions. >
Will do. Sending v2 now... > > > _______________________________________________ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
