[Mesa-dev] [PATCH 2/4] i965: Store floating point mode choice in brw_stage_prog_data.

Ian Romanick idr at freedesktop.org
Tue Dec 2 13:19:46 PST 2014


This patch is

Reviewed-by: Ian Romanick <ian.d.romanick at 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 <kenneth at 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 a/src/mesa/drivers/dri/i965/gen6_vs_state.c b/src/mesa/drivers/dri/i965/gen6_vs_state.c
> index fc0fd1d..e365cc6 100644
> --- a/src/mesa/drivers/dri/i965/gen6_vs_state.c
> +++ b/src/mesa/drivers/dri/i965/gen6_vs_state.c
> @@ -160,7 +160,6 @@ const struct brw_tracked_state gen6_vs_push_constants = {
>  static void
>  upload_vs_state(struct brw_context *brw)
>  {
> -   struct gl_context *ctx = &brw->ctx;
>     const struct brw_stage_state *stage_state = &brw->vs.base;
>     uint32_t floating_point_mode = 0;
>  
> @@ -202,10 +201,7 @@ upload_vs_state(struct brw_context *brw)
>        ADVANCE_BATCH();
>     }
>  
> -   /* Use ALT floating point mode for ARB vertex programs, because they
> -    * require 0^0 == 1.
> -    */
> -   if (ctx->_Shader->CurrentProgram[MESA_SHADER_VERTEX] == NULL)
> +   if (brw->vs.prog_data->base.base.use_alt_mode)
>        floating_point_mode = GEN6_VS_FLOATING_POINT_MODE_ALT;
>  
>     BEGIN_BATCH(6);
> diff --git a/src/mesa/drivers/dri/i965/gen6_wm_state.c b/src/mesa/drivers/dri/i965/gen6_wm_state.c
> index 4ac91af..e57b7f6 100644
> --- a/src/mesa/drivers/dri/i965/gen6_wm_state.c
> +++ b/src/mesa/drivers/dri/i965/gen6_wm_state.c
> @@ -115,12 +115,7 @@ upload_wm_state(struct brw_context *brw)
>     dw5 |= GEN6_WM_LINE_AA_WIDTH_1_0;
>     dw5 |= GEN6_WM_LINE_END_CAP_AA_WIDTH_0_5;
>  
> -   /* 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)
>        dw2 |= GEN6_WM_FLOATING_POINT_MODE_ALT;
>  
>     dw2 |= (ALIGN(brw->wm.base.sampler_count, 4) / 4) <<
> diff --git a/src/mesa/drivers/dri/i965/gen7_vs_state.c b/src/mesa/drivers/dri/i965/gen7_vs_state.c
> index 3b0126d..5a11588 100644
> --- a/src/mesa/drivers/dri/i965/gen7_vs_state.c
> +++ b/src/mesa/drivers/dri/i965/gen7_vs_state.c
> @@ -66,7 +66,6 @@ gen7_upload_constant_state(struct brw_context *brw,
>  static void
>  upload_vs_state(struct brw_context *brw)
>  {
> -   struct gl_context *ctx = &brw->ctx;
>     const struct brw_stage_state *stage_state = &brw->vs.base;
>     uint32_t floating_point_mode = 0;
>     const int max_threads_shift = brw->is_haswell ?
> @@ -75,10 +74,7 @@ upload_vs_state(struct brw_context *brw)
>     if (!brw->is_haswell && !brw->is_baytrail)
>        gen7_emit_vs_workaround_flush(brw);
>  
> -   /* Use ALT floating point mode for ARB vertex programs, because they
> -    * require 0^0 == 1.
> -    */
> -   if (ctx->_Shader->CurrentProgram[MESA_SHADER_VERTEX] == NULL)
> +   if (brw->vs.prog_data->base.base.use_alt_mode)
>        floating_point_mode = GEN6_VS_FLOATING_POINT_MODE_ALT;
>  
>     BEGIN_BATCH(6);
> diff --git a/src/mesa/drivers/dri/i965/gen7_wm_state.c b/src/mesa/drivers/dri/i965/gen7_wm_state.c
> index 5a5c726..923414e 100644
> --- a/src/mesa/drivers/dri/i965/gen7_wm_state.c
> +++ b/src/mesa/drivers/dri/i965/gen7_wm_state.c
> @@ -141,13 +141,7 @@ upload_ps_state(struct brw_context *brw)
>     dw2 |= ((prog_data->base.binding_table.size_bytes / 4) <<
>             GEN7_PS_BINDING_TABLE_ENTRY_COUNT_SHIFT);
>  
> -   /* 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.
> -    */
> -   /* BRW_NEW_FRAGMENT_PROGRAM */
> -   if (ctx->_Shader->CurrentProgram[MESA_SHADER_FRAGMENT] == NULL)
> +   if (prog_data->base.use_alt_mode)
>        dw2 |= GEN7_PS_FLOATING_POINT_MODE_ALT;
>  
>     /* Haswell requires the sample mask to be set in this packet as well as
> diff --git a/src/mesa/drivers/dri/i965/gen8_ps_state.c b/src/mesa/drivers/dri/i965/gen8_ps_state.c
> index a3ce1d4..d4a58e4 100644
> --- a/src/mesa/drivers/dri/i965/gen8_ps_state.c
> +++ b/src/mesa/drivers/dri/i965/gen8_ps_state.c
> @@ -141,12 +141,7 @@ upload_ps_state(struct brw_context *brw)
>        ((prog_data->base.binding_table.size_bytes / 4) <<
>         GEN7_PS_BINDING_TABLE_ENTRY_COUNT_SHIFT);
>  
> -   /* Use ALT floating point mode for ARB fragment programs, because they
> -    * require 0^0 == 1.  Even though _CurrentFragmentProgram is used for
> -    * rendering, CurrentFragmentProgram 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)
>        dw3 |= GEN7_PS_FLOATING_POINT_MODE_ALT;
>  
>     /* 3DSTATE_PS expects the number of threads per PSD, which is always 64;
> diff --git a/src/mesa/drivers/dri/i965/gen8_vs_state.c b/src/mesa/drivers/dri/i965/gen8_vs_state.c
> index 5a2021f..e5b7e03 100644
> --- a/src/mesa/drivers/dri/i965/gen8_vs_state.c
> +++ b/src/mesa/drivers/dri/i965/gen8_vs_state.c
> @@ -39,10 +39,7 @@ upload_vs_state(struct brw_context *brw)
>     /* BRW_NEW_VS_PROG_DATA */
>     const struct brw_vec4_prog_data *prog_data = &brw->vs.prog_data->base;
>  
> -   /* Use ALT floating point mode for ARB vertex programs, because they
> -    * require 0^0 == 1.
> -    */
> -   if (ctx->_Shader->CurrentProgram[MESA_SHADER_VERTEX] == NULL)
> +   if (prog_data->base.use_alt_mode)
>        floating_point_mode = GEN6_VS_FLOATING_POINT_MODE_ALT;
>  
>     BEGIN_BATCH(9);
> 



More information about the mesa-dev mailing list