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

2014-12-02 Thread Kenneth Graunke
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.

2014-12-02 Thread Ian Romanick
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]