[Mesa-dev] [PATCH 2/4] i965: Store floating point mode choice in brw_stage_prog_data.
We use IEEE mode for GLSL programs, but need to use ALT mode for ARB programs so that 0^0 == 1. The choice is based entirely on the shader source language. Previously, our code to determine which mode we wanted was duplicated in 8 different places (VS and FS for Gen4-5, Gen6, Gen7, and Gen8). The ctx-_Shader-CurrentProgram[stage] == NULL check was confusing as well - we use CurrentProgram (non-derived state), but _Shader (derived state). It also relies on knowing that ARB programs don't use gl_shader_program structures today. The compiler already makes this assumption in a few places, but I'd rather keep that assumption out of the state upload code. With this patch, we select the mode at compile time, and store that choice in prog_data. The state upload code simply uses that decision. This eliminates a BRW_NEW_*_PROGRAM dependency in the state upload code. Signed-off-by: Kenneth Graunke kenn...@whitecape.org --- src/mesa/drivers/dri/i965/brw_context.h | 2 ++ src/mesa/drivers/dri/i965/brw_vs.c| 4 src/mesa/drivers/dri/i965/brw_vs_state.c | 5 + src/mesa/drivers/dri/i965/brw_wm.c| 4 src/mesa/drivers/dri/i965/brw_wm_state.c | 7 +-- src/mesa/drivers/dri/i965/gen6_vs_state.c | 6 +- src/mesa/drivers/dri/i965/gen6_wm_state.c | 7 +-- src/mesa/drivers/dri/i965/gen7_vs_state.c | 6 +- src/mesa/drivers/dri/i965/gen7_wm_state.c | 8 +--- src/mesa/drivers/dri/i965/gen8_ps_state.c | 7 +-- src/mesa/drivers/dri/i965/gen8_vs_state.c | 5 + 11 files changed, 18 insertions(+), 43 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_context.h b/src/mesa/drivers/dri/i965/brw_context.h index b4ddc17..4bb3b17 100644 --- a/src/mesa/drivers/dri/i965/brw_context.h +++ b/src/mesa/drivers/dri/i965/brw_context.h @@ -355,6 +355,8 @@ struct brw_stage_prog_data { */ unsigned dispatch_grf_start_reg; + bool use_alt_mode; /** Use ALT floating point mode? Otherwise, IEEE. */ + /* Pointers to tracked values (only valid once * _mesa_load_state_parameters has been called at runtime). * diff --git a/src/mesa/drivers/dri/i965/brw_vs.c b/src/mesa/drivers/dri/i965/brw_vs.c index 2f628e5..970d86c 100644 --- a/src/mesa/drivers/dri/i965/brw_vs.c +++ b/src/mesa/drivers/dri/i965/brw_vs.c @@ -209,6 +209,10 @@ do_vs_prog(struct brw_context *brw, memcpy(c.key, key, sizeof(*key)); memset(prog_data, 0, sizeof(prog_data)); + /* Use ALT floating point mode for ARB programs so that 0^0 == 1. */ + if (!prog) + stage_prog_data-use_alt_mode = true; + mem_ctx = ralloc_context(NULL); c.vp = vp; diff --git a/src/mesa/drivers/dri/i965/brw_vs_state.c b/src/mesa/drivers/dri/i965/brw_vs_state.c index abd6771..5371f71 100644 --- a/src/mesa/drivers/dri/i965/brw_vs_state.c +++ b/src/mesa/drivers/dri/i965/brw_vs_state.c @@ -57,10 +57,7 @@ brw_upload_vs_unit(struct brw_context *brw) stage_state-prog_offset + (vs-thread0.grf_reg_count 1)) 6; - /* Use ALT floating point mode for ARB vertex programs, because they -* require 0^0 == 1. -*/ - if (brw-ctx._Shader-CurrentProgram[MESA_SHADER_VERTEX] == NULL) + if (brw-vs.prog_data-base.base.use_alt_mode) vs-thread1.floating_point_mode = BRW_FLOATING_POINT_NON_IEEE_754; else vs-thread1.floating_point_mode = BRW_FLOATING_POINT_IEEE_754; diff --git a/src/mesa/drivers/dri/i965/brw_wm.c b/src/mesa/drivers/dri/i965/brw_wm.c index 7badb23..e7939f0 100644 --- a/src/mesa/drivers/dri/i965/brw_wm.c +++ b/src/mesa/drivers/dri/i965/brw_wm.c @@ -182,6 +182,10 @@ bool do_wm_prog(struct brw_context *brw, prog_data.computed_depth_mode = computed_depth_mode(fp-program); + /* Use ALT floating point mode for ARB programs so that 0^0 == 1. */ + if (!prog) + prog_data.base.use_alt_mode = true; + /* Allocate the references to the uniforms that will end up in the * prog_data associated with the compiled program, and which will be freed * by the state cache. diff --git a/src/mesa/drivers/dri/i965/brw_wm_state.c b/src/mesa/drivers/dri/i965/brw_wm_state.c index d2903c7..0dee1f8 100644 --- a/src/mesa/drivers/dri/i965/brw_wm_state.c +++ b/src/mesa/drivers/dri/i965/brw_wm_state.c @@ -114,12 +114,7 @@ brw_upload_wm_unit(struct brw_context *brw) (wm-wm9.grf_reg_count_2 1)) 6; wm-thread1.depth_coef_urb_read_offset = 1; - /* Use ALT floating point mode for ARB fragment programs, because they -* require 0^0 == 1. Even though _CurrentFragmentProgram is used for -* rendering, CurrentProgram[MESA_SHADER_FRAGMENT] is used for this check -* to differentiate between the GLSL and non-GLSL cases. -*/ - if (ctx-_Shader-CurrentProgram[MESA_SHADER_FRAGMENT] == NULL) + if (prog_data-base.use_alt_mode) wm-thread1.floating_point_mode = BRW_FLOATING_POINT_NON_IEEE_754; else wm-thread1.floating_point_mode = BRW_FLOATING_POINT_IEEE_754; diff --git
Re: [Mesa-dev] [PATCH 2/4] i965: Store floating point mode choice in brw_stage_prog_data.
This patch is Reviewed-by: Ian Romanick ian.d.roman...@intel.com On 12/02/2014 03:51 AM, Kenneth Graunke wrote: We use IEEE mode for GLSL programs, but need to use ALT mode for ARB programs so that 0^0 == 1. The choice is based entirely on the shader source language. Previously, our code to determine which mode we wanted was duplicated in 8 different places (VS and FS for Gen4-5, Gen6, Gen7, and Gen8). The ctx-_Shader-CurrentProgram[stage] == NULL check was confusing as well - we use CurrentProgram (non-derived state), but _Shader (derived state). It also relies on knowing that ARB programs don't use gl_shader_program structures today. The compiler already makes this assumption in a few places, but I'd rather keep that assumption out of the state upload code. With this patch, we select the mode at compile time, and store that choice in prog_data. The state upload code simply uses that decision. This eliminates a BRW_NEW_*_PROGRAM dependency in the state upload code. Signed-off-by: Kenneth Graunke kenn...@whitecape.org --- src/mesa/drivers/dri/i965/brw_context.h | 2 ++ src/mesa/drivers/dri/i965/brw_vs.c| 4 src/mesa/drivers/dri/i965/brw_vs_state.c | 5 + src/mesa/drivers/dri/i965/brw_wm.c| 4 src/mesa/drivers/dri/i965/brw_wm_state.c | 7 +-- src/mesa/drivers/dri/i965/gen6_vs_state.c | 6 +- src/mesa/drivers/dri/i965/gen6_wm_state.c | 7 +-- src/mesa/drivers/dri/i965/gen7_vs_state.c | 6 +- src/mesa/drivers/dri/i965/gen7_wm_state.c | 8 +--- src/mesa/drivers/dri/i965/gen8_ps_state.c | 7 +-- src/mesa/drivers/dri/i965/gen8_vs_state.c | 5 + 11 files changed, 18 insertions(+), 43 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_context.h b/src/mesa/drivers/dri/i965/brw_context.h index b4ddc17..4bb3b17 100644 --- a/src/mesa/drivers/dri/i965/brw_context.h +++ b/src/mesa/drivers/dri/i965/brw_context.h @@ -355,6 +355,8 @@ struct brw_stage_prog_data { */ unsigned dispatch_grf_start_reg; + bool use_alt_mode; /** Use ALT floating point mode? Otherwise, IEEE. */ + /* Pointers to tracked values (only valid once * _mesa_load_state_parameters has been called at runtime). * diff --git a/src/mesa/drivers/dri/i965/brw_vs.c b/src/mesa/drivers/dri/i965/brw_vs.c index 2f628e5..970d86c 100644 --- a/src/mesa/drivers/dri/i965/brw_vs.c +++ b/src/mesa/drivers/dri/i965/brw_vs.c @@ -209,6 +209,10 @@ do_vs_prog(struct brw_context *brw, memcpy(c.key, key, sizeof(*key)); memset(prog_data, 0, sizeof(prog_data)); + /* Use ALT floating point mode for ARB programs so that 0^0 == 1. */ + if (!prog) + stage_prog_data-use_alt_mode = true; + mem_ctx = ralloc_context(NULL); c.vp = vp; diff --git a/src/mesa/drivers/dri/i965/brw_vs_state.c b/src/mesa/drivers/dri/i965/brw_vs_state.c index abd6771..5371f71 100644 --- a/src/mesa/drivers/dri/i965/brw_vs_state.c +++ b/src/mesa/drivers/dri/i965/brw_vs_state.c @@ -57,10 +57,7 @@ brw_upload_vs_unit(struct brw_context *brw) stage_state-prog_offset + (vs-thread0.grf_reg_count 1)) 6; - /* Use ALT floating point mode for ARB vertex programs, because they -* require 0^0 == 1. -*/ - if (brw-ctx._Shader-CurrentProgram[MESA_SHADER_VERTEX] == NULL) + if (brw-vs.prog_data-base.base.use_alt_mode) vs-thread1.floating_point_mode = BRW_FLOATING_POINT_NON_IEEE_754; else vs-thread1.floating_point_mode = BRW_FLOATING_POINT_IEEE_754; diff --git a/src/mesa/drivers/dri/i965/brw_wm.c b/src/mesa/drivers/dri/i965/brw_wm.c index 7badb23..e7939f0 100644 --- a/src/mesa/drivers/dri/i965/brw_wm.c +++ b/src/mesa/drivers/dri/i965/brw_wm.c @@ -182,6 +182,10 @@ bool do_wm_prog(struct brw_context *brw, prog_data.computed_depth_mode = computed_depth_mode(fp-program); + /* Use ALT floating point mode for ARB programs so that 0^0 == 1. */ + if (!prog) + prog_data.base.use_alt_mode = true; + /* Allocate the references to the uniforms that will end up in the * prog_data associated with the compiled program, and which will be freed * by the state cache. diff --git a/src/mesa/drivers/dri/i965/brw_wm_state.c b/src/mesa/drivers/dri/i965/brw_wm_state.c index d2903c7..0dee1f8 100644 --- a/src/mesa/drivers/dri/i965/brw_wm_state.c +++ b/src/mesa/drivers/dri/i965/brw_wm_state.c @@ -114,12 +114,7 @@ brw_upload_wm_unit(struct brw_context *brw) (wm-wm9.grf_reg_count_2 1)) 6; wm-thread1.depth_coef_urb_read_offset = 1; - /* Use ALT floating point mode for ARB fragment programs, because they -* require 0^0 == 1. Even though _CurrentFragmentProgram is used for -* rendering, CurrentProgram[MESA_SHADER_FRAGMENT] is used for this check -* to differentiate between the GLSL and non-GLSL cases. -*/ - if (ctx-_Shader-CurrentProgram[MESA_SHADER_FRAGMENT]