Re: [Mesa-dev] [PATCH v2 3/4] i965/state: Create separate dirty state bits for each pipeline

2015-03-17 Thread Kristian Høgsberg
On Tue, Mar 17, 2015 at 2:22 AM, Kenneth Graunke kenn...@whitecape.org wrote:
 On Monday, March 16, 2015 10:21:31 PM Kristian Høgsberg wrote:
 On Wed, Mar 11, 2015 at 11:53 AM, Jordan Justen
 jordan.l.jus...@intel.com wrote:
  When uploading state for a pipeline, we will save changed state for
  the other pipelines.
 
  Signed-off-by: Jordan Justen jordan.l.jus...@intel.com

 In reviewing this again, I realize I'm not completely comfortable with
 how this is done. The overall approach is great, I'm happy to see the
 dual pipeline flags contained in brw_state_upload.c, but I think we
 need to keep brw_upload_pipeline_state() a no-op in case it fails.

 Given that brw_upload_state() has always done:

struct brw_state_flags *state = brw-state.dirty;

state-mesa |= brw-NewGLState;
brw-NewGLState = 0;

state-brw |= ctx-NewDriverState;
ctx-NewDriverState = 0;

 I think there's a pretty good precedence for having it update the
 driver's notion of what's dirty.  But you can still run it several
 times back to back with no issues.

 I can't think of a case where it might bite us, but currently, the
 expectation is that if it fails, nothing changes and you can call it
 again after flushing the batch.  This patch changes that in two ways:
 1) we merge the pipeline brw-state.dirty into state and clear the
 pipeline state and 2) we copy the dirty state into the other pipeline.
 Suppose we call with the render pipeline and fail, flush the batch,
 then call with the compute pipeline and clear the dirty flags [1]. If
 we then go back and try to emit render state again, there are no
 longer any dirty bits and we fail to emit render state.

 I'm pretty comfortable with this.  Your concerns seem to revolve around
 implicitly switching pipelines in the middle of state upload.  I don't
 believe that can happen (and we'd better keep it that way!).

 You're always uploading state for some particular operation - drawing
 (on the render pipe), or glDispatchCompute (on the compute pipe).  When
 processing an operation, we first save the batch state (bytes used for
 commands, number of relocations).  Then we upload state.

 If we run out of batch space(*), or refer to more objects than we can
 fit in the aperture, we roll back to that saved state and flush the
 batch, executing everything queued except the most recent operation.

 We then re-upload state for that single failed operation in a fresh
 batch.  There's no pipeline switch here - when we retry that same
 operation, it will dispatch to the same pipe as before.  Notably,
 there are no _mesa_update_state calls, so we can never hit resolves.
 (We did those at the very beginning, before even beginning to process
 the operation at hand!)  It's a pretty tight loop - 1. upload state,
 2. check aperture space, 3. flush, go back to step 1.

 This may be far-fetched and, as I said, I don't think we can hit this.

 So we agree :)

 But I'd rather not break the contract that brw_upload_pipeline_state()
 doesn't change state in case of failure - I know I'd hate to debug
 that.
 We can just introduce a brw_clear_pipeline_dirty_bits() function
 that will be called on successful state upload and merge the state
 bits to the other pipeline and then clear the selected pipeline dirty
 flags as well as brw-state.dirty.  And we need to use a local copy of
 brw-state.dirty in brw_upload_pipeline_state() for merging in the
 pipeline dirty flags so we don't modify brw-state.dirty on error.

 [1] I don't see how we'd get into that, but with fast clear resolve
 using meta we end up running the render pipeline at unexpected places.

 But not here - and frankly, doing resolves in the middle of state upload
 would be crazy.  Resolves inherently need to set render state that the
 actual draw will want to set, too.  You have to serialize them.

 Which is what we do today - we handle resolves when Mesa first asks us
 to draw or dispatch a compute workload, before we ever start processing
 the operation itself.  The resolve will space-check-and-retry...but it
 will get done.  Only then do we try the next thing, which will
 space-check-and-retry independently.

 Thanks for thinking about this carefully - it's easy to mess up, and we
 really need to get it right.  I think we have, though.

You've argued and agreed that the case I said couldn't happen can't
happen. Fine. My point however was that with a small change we can
eliminate any concerns like that and not have to think through all the
different paths we can call brw_update_state(). I'm not asking for a
big rewrite of this, just a small tweak in how and when we clear and
propagate the state bits.  And you ignored the part below about how we
keep carrying dirty state around as we switch pipelines.

Kristian


  ---
   src/mesa/drivers/dri/i965/brw_context.h  |  1 +
   src/mesa/drivers/dri/i965/brw_state_upload.c | 42 
  ++--
   2 files changed, 35 insertions(+), 8 deletions(-)
 
  diff --git 

[Mesa-dev] [Bug 89622] Drivers/Gallium/swrast: Pixel image limited to GL_MAX_TEXTURE_SIZE

2015-03-17 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=89622

--- Comment #6 from Dan Sebald daniel.seb...@ieee.org ---
Correct.  -1  xfactor  1, excluding 0.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [Bug 89622] Drivers/Gallium/swrast: Pixel image limited to GL_MAX_TEXTURE_SIZE

2015-03-17 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=89622

--- Comment #7 from Dan Sebald daniel.seb...@ieee.org ---
Oh, and attempting to load suitable driver first is fair.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [Bug 89622] Drivers/Gallium/swrast: Pixel image limited to GL_MAX_TEXTURE_SIZE

2015-03-17 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=89622

--- Comment #8 from Ilia Mirkin imir...@alum.mit.edu ---
(In reply to Dan Sebald from comment #0)
 So, what is the problem here?  Is it that not all systems are ensured to
 have non-paged memory, and my system just happens to have it?  Can this be
 tested and adjusted accordingly?  More generally, it seems that the Gallium
 version of pixel zoom/draw can be done in a more straightforward way than
 legacy swrast is doing.  Notice that Gallium is treating the image as blocks
 (i.e., the clip affects both width and height).  It shouldn't be difficult
 to write a double loop to write to the frame buffer in blocks (unless there
 is a problem of not being able to select the memory page in the
 st_DrawPixels() routine because that is done prior to the function call). 
 What role does the st-pipe play in affecting size of writes?

Worrying about mmu-less (or non-page-on-demand) issues is not something we do.

Basically st_DrawPixels takes the pixel data, sticks it into a texture, and
then uses shaders to texture that onto the framebuffer. Obviously the texture
can't be larger than MAX_TEXTURE_SIZE. However no harm done if you cut up the
source data into MAX_TEXTURE_SIZE-sized blocks and discarded fragments that
were not going to be covered by that texture slice (or even better, set the
viewport or scissors appropriately). If you were going to be sneaky, you could
even concoct some scheme to use a 2d array texture, although ultimately even
that has limits.

I don't immediately see a way to transfer a non-0,0-origin block of pixels from
memory into a texture, but I'm sure it's reasonably easy to do, and a helper
probably exists for some other functions. (Maybe it can be specified as a
gl_pixelstore_attrib? It has a SkipRows/SkipPixels attribute.)

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 9/9] i965: Use NIR by default for Fragment shaders on SNB+

2015-03-17 Thread Jason Ekstrand
I did this for SNB+ only because we did have some shader-db
regressions.  However, with the patch 8.5 I just sent out, we're down
to about +0.2% on ILK-.  Arguably, that's probably close enough for
the old platforms especially if it lets us move forward.

The majority of hurt shaders there are planeshift shaders which are
hurt because they use indirect uniforms which causes NIR to do pull
constants for everything and that's bad.  We need to get our uniform
story figured out better for NIR soon.

Also, I didn't recommend it for SIMD8 FS on BDW because the shader-db
result there are not great.  I don't know that I'd say dire, but it
needs some work/looking into.
--Jason

On Tue, Mar 17, 2015 at 7:17 PM, Jason Ekstrand ja...@jlekstrand.net wrote:
 GLSL IR vs. NIR FS instructions on snb:
 total instructions in shared programs: 4976784 - 4973072 (-0.07%)
 instructions in affected programs: 3309521 - 3305809 (-0.11%)
 helped:7161
 HURT:  9839
 GAINED:55
 LOST:  23

 GLSL IR vs. NIR FS instructions on ivb:
 total instructions in shared programs: 4593938 - 4588496 (-0.12%)
 instructions in affected programs: 2948822 - 2943380 (-0.18%)
 helped:6878
 HURT:  8801
 GAINED:69
 LOST:  44

 GLSL IR vs. NIR FS instructions on hsw:
 total instructions in shared programs: 4089459 - 4081368 (-0.20%)
 instructions in affected programs: 2592474 - 2584383 (-0.31%)
 helped:6492
 HURT:  9403
 GAINED:69
 LOST:  32

 GLSL IR vs. NIR FS instructions on bdw:
 total instructions in shared programs: 4065050 - 4058653 (-0.16%)
 instructions in affected programs: 255 - 2549158 (-0.25%)
 helped:5910
 HURT:  9765
 GAINED:45
 LOST:  53
 ---
  src/mesa/drivers/dri/i965/brw_fs.cpp | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp 
 b/src/mesa/drivers/dri/i965/brw_fs.cpp
 index 3d4d31a..42f6604 100644
 --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
 +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
 @@ -3943,7 +3943,7 @@ fs_visitor::run_fs()
 * functions called main).
 */
if (shader) {
 - if (env_var_as_boolean(INTEL_USE_NIR, false)) {
 + if (env_var_as_boolean(INTEL_USE_NIR, brw-gen = 6)) {
  emit_nir_code();
   } else {
  foreach_in_list(ir_instruction, ir, shader-base.ir) {
 --
 2.3.2

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 8.5/9] i965/nir: Use signed integer type for booleans

2015-03-17 Thread Jason Ekstrand
FS instructions with NIR on i965:
total instructions in shared programs: 2663561 - 2619051 (-1.67%)
instructions in affected programs: 1612965 - 1568455 (-2.76%)
helped:5455
HURT:  12

FS instructions with NIR on g4x:
total instructions in shared programs: 2352633 - 2307908 (-1.90%)
instructions in affected programs: 1441842 - 1397117 (-3.10%)
helped:5463
HURT:  11

FS instructions with NIR on ilk:
total instructions in shared programs: 3997305 - 3934278 (-1.58%)
instructions in affected programs: 2189409 - 2126382 (-2.88%)
helped:8969
HURT:  22

FS instructions with NIR on hsw (snb and ivb were similar):
total instructions in shared programs: 4109389 - 4109242 (-0.00%)
instructions in affected programs: 109869 - 109722 (-0.13%)
helped:339
HURT:  190

No SIMD16 programs were gained or lost on any platform

More signed integers
---
 src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp 
b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
index 1ef4602..69c5680 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
@@ -518,7 +518,7 @@ fs_visitor::nir_emit_if(nir_if *if_stmt)
/* first, put the condition into f0 */
fs_inst *inst = emit(MOV(reg_null_d,
 retype(get_nir_src(if_stmt-condition),
-   BRW_REGISTER_TYPE_UD)));
+   BRW_REGISTER_TYPE_D)));
inst-conditional_mod = BRW_CONDITIONAL_NZ;
 
emit(IF(BRW_PREDICATE_NORMAL));
@@ -594,9 +594,9 @@ static brw_reg_type
 brw_type_for_nir_type(nir_alu_type type)
 {
switch (type) {
-   case nir_type_bool:
case nir_type_unsigned:
   return BRW_REGISTER_TYPE_UD;
+   case nir_type_bool:
case nir_type_int:
   return BRW_REGISTER_TYPE_D;
case nir_type_float:
@@ -1282,7 +1282,7 @@ fs_visitor::nir_emit_alu(nir_alu_instr *instr)
   fs_reg masked = vgrf(glsl_type::int_type);
   emit(AND(masked, result, fs_reg(1)));
   masked.negate = true;
-  emit(MOV(result, masked));
+  emit(MOV(retype(result, BRW_REGISTER_TYPE_D), masked));
}
 }
 
-- 
2.3.2

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 16/18] i965/cs: Support CS program precompile

2015-03-17 Thread Kristian Høgsberg
On Sat, Mar 14, 2015 at 9:54 PM, Jordan Justen
jordan.l.jus...@intel.com wrote:
 Signed-off-by: Jordan Justen jordan.l.jus...@intel.com
 ---
  src/mesa/drivers/dri/i965/brw_context.h  |  6 ++
  src/mesa/drivers/dri/i965/brw_cs.cpp | 26 ++
  src/mesa/drivers/dri/i965/brw_shader.cpp |  4 
  src/mesa/drivers/dri/i965/brw_shader.h   |  3 +++
  4 files changed, 39 insertions(+)

 diff --git a/src/mesa/drivers/dri/i965/brw_context.h 
 b/src/mesa/drivers/dri/i965/brw_context.h
 index 170c0c6..27a4ff4 100644
 --- a/src/mesa/drivers/dri/i965/brw_context.h
 +++ b/src/mesa/drivers/dri/i965/brw_context.h
 @@ -1844,6 +1844,12 @@ brw_fragment_program_const(const struct 
 gl_fragment_program *p)
 return (const struct brw_fragment_program *) p;
  }

 +static inline struct brw_compute_program *
 +brw_compute_program(struct gl_compute_program *p)
 +{
 +   return (struct brw_compute_program *) p;
 +}
 +
  /**
   * Pre-gen6, the register file of the EUs was shared between threads,
   * and each thread used some subset allocated on a 16-register block
 diff --git a/src/mesa/drivers/dri/i965/brw_cs.cpp 
 b/src/mesa/drivers/dri/i965/brw_cs.cpp
 index 5be740c..61c35ae 100644
 --- a/src/mesa/drivers/dri/i965/brw_cs.cpp
 +++ b/src/mesa/drivers/dri/i965/brw_cs.cpp
 @@ -262,3 +262,29 @@ brw_upload_cs_prog(struct brw_context *brw)
 }
 brw-cs.base.prog_data = brw-cs.prog_data-base;
  }
 +
 +
 +extern C bool
 +brw_cs_precompile(struct gl_context *ctx,
 +  struct gl_shader_program *shader_prog,
 +  struct gl_program *prog)
 +{
 +   struct brw_context *brw = brw_context(ctx);
 +   struct brw_cs_prog_key key;
 +
 +   struct gl_compute_program *cp = (struct gl_compute_program *) prog;
 +   struct brw_compute_program *bcp = brw_compute_program(cp);
 +
 +   memset(key, 0, sizeof(key));
 +   key.program_string_id = bcp-id;

This should initialize key.tex.swizzles similar to how brw_fs_comp

 +   uint32_t old_prog_offset = brw-cs.base.prog_offset;
 +   struct brw_cs_prog_data *old_prog_data = brw-cs.prog_data;
 +
 +   bool success = do_cs_prog(brw, shader_prog, bcp, key);
 +
 +   brw-cs.base.prog_offset = old_prog_offset;
 +   brw-cs.prog_data = old_prog_data;
 +
 +   return success;
 +}
 diff --git a/src/mesa/drivers/dri/i965/brw_shader.cpp 
 b/src/mesa/drivers/dri/i965/brw_shader.cpp
 index 499bd94..4392fbc 100644
 --- a/src/mesa/drivers/dri/i965/brw_shader.cpp
 +++ b/src/mesa/drivers/dri/i965/brw_shader.cpp
 @@ -59,6 +59,7 @@ brw_shader_precompile(struct gl_context *ctx,
 struct gl_shader *vs = sh_prog-_LinkedShaders[MESA_SHADER_VERTEX];
 struct gl_shader *gs = sh_prog-_LinkedShaders[MESA_SHADER_GEOMETRY];
 struct gl_shader *fs = sh_prog-_LinkedShaders[MESA_SHADER_FRAGMENT];
 +   struct gl_shader *cs = sh_prog-_LinkedShaders[MESA_SHADER_COMPUTE];

 if (fs  !brw_fs_precompile(ctx, sh_prog, fs-Program))
return false;
 @@ -69,6 +70,9 @@ brw_shader_precompile(struct gl_context *ctx,
 if (vs  !brw_vs_precompile(ctx, sh_prog, vs-Program))
return false;

 +   if (cs  !brw_cs_precompile(ctx, sh_prog, cs-Program))
 +  return false;
 +
 return true;
  }

 diff --git a/src/mesa/drivers/dri/i965/brw_shader.h 
 b/src/mesa/drivers/dri/i965/brw_shader.h
 index 5c95355..53aa727 100644
 --- a/src/mesa/drivers/dri/i965/brw_shader.h
 +++ b/src/mesa/drivers/dri/i965/brw_shader.h
 @@ -230,6 +230,9 @@ bool brw_gs_precompile(struct gl_context *ctx,
  bool brw_fs_precompile(struct gl_context *ctx,
 struct gl_shader_program *shader_prog,
 struct gl_program *prog);
 +bool brw_cs_precompile(struct gl_context *ctx,
 +   struct gl_shader_program *shader_prog,
 +   struct gl_program *prog);

  #ifdef __cplusplus
  }
 --
 2.1.4

 ___
 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


Re: [Mesa-dev] [PATCH 2/2] main: Cosmetic updates to GetBufferPointerv.

2015-03-17 Thread Fredrik Höglund
For both patches:

Reviewed-by: Fredrik Höglund fred...@kde.org

On Tuesday 17 March 2015, Laura Ekstrand wrote:
 v3: Review from Fredrik Hoglund
-Split cosmetic refactor of GetBufferPointerv out into a separate commit
 ---
  src/mesa/main/bufferobj.c | 9 +
  1 file changed, 5 insertions(+), 4 deletions(-)
 
 diff --git a/src/mesa/main/bufferobj.c b/src/mesa/main/bufferobj.c
 index 2811604..49d6d32 100644
 --- a/src/mesa/main/bufferobj.c
 +++ b/src/mesa/main/bufferobj.c
 @@ -2051,14 +2051,15 @@ void GLAPIENTRY
  _mesa_GetBufferPointerv(GLenum target, GLenum pname, GLvoid **params)
  {
 GET_CURRENT_CONTEXT(ctx);
 -   struct gl_buffer_object * bufObj;
 +   struct gl_buffer_object *bufObj;
  
 -   if (pname != GL_BUFFER_MAP_POINTER_ARB) {
 -  _mesa_error(ctx, GL_INVALID_ENUM, glGetBufferPointervARB(pname));
 +   if (pname != GL_BUFFER_MAP_POINTER) {
 +  _mesa_error(ctx, GL_INVALID_ENUM, glGetBufferPointerv(pname != 
 +  GL_BUFFER_MAP_POINTER));
return;
 }
  
 -   bufObj = get_buffer(ctx, glGetBufferPointervARB, target,
 +   bufObj = get_buffer(ctx, glGetBufferPointerv, target,
 GL_INVALID_OPERATION);
 if (!bufObj)
return;
 

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [Bug 88806] nir/nir_constant_expressions.c:2754:15: error: controlling expression type 'unsigned int' not compatible with any generic association type

2015-03-17 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=88806

Vinson Lee v...@freedesktop.org changed:

   What|Removed |Added

 Status|NEEDINFO|NEW

--- Comment #7 from Vinson Lee v...@freedesktop.org ---
mesa: 627c68308683abbd6e563a09af6013a33938a790 (master 10.6.0-devel)

The build error doesn't occur anymore.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 11/23] mesa: refactor GetAttribLocation

2015-03-17 Thread Tapani Pälli


On 03/17/2015 12:30 AM, Ilia Mirkin wrote:

On Fri, Mar 13, 2015 at 4:37 AM, Tapani Pälli tapani.pa...@intel.com wrote:

Use program_resource_location to fetch location.

Signed-off-by: Tapani Pälli tapani.pa...@intel.com
---
  src/mesa/main/shader_query.cpp | 33 ++---
  1 file changed, 10 insertions(+), 23 deletions(-)

diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp
index 8134d4b..7e8cf9c 100644
--- a/src/mesa/main/shader_query.cpp
+++ b/src/mesa/main/shader_query.cpp
@@ -41,6 +41,10 @@ extern C {
  #include shaderapi.h
  }

+static GLint
+program_resource_location(struct gl_shader_program *shProg,
+  struct gl_program_resource *res, const char *name);
+
  /**
   * Declare convenience functions to return resource data in a given type.
   * Warning! this is not type safe so be *very* careful when using these.
@@ -266,31 +270,14 @@ _mesa_GetAttribLocation(GLhandleARB program, const 
GLcharARB * name)
 if (shProg-_LinkedShaders[MESA_SHADER_VERTEX] == NULL)
return -1;

-   exec_list *ir = shProg-_LinkedShaders[MESA_SHADER_VERTEX]-ir;
-   foreach_in_list(ir_instruction, node, ir) {
-  const ir_variable *const var = node-as_variable();
-
-  /* The extra check against VERT_ATTRIB_GENERIC0 is because
-   * glGetAttribLocation cannot be used on conventional attributes.
-   *
-   * From page 95 of the OpenGL 3.0 spec:
-   *
-   * If name is not an active attribute, if name is a conventional
-   * attribute, or if an error occurs, -1 will be returned.
-   */
-  if (var == NULL
- || var-data.mode != ir_var_shader_in
- || var-data.location == -1
- || var-data.location  VERT_ATTRIB_GENERIC0)
-continue;
-
-  int index = get_matching_index(var, (const char *) name);
+   struct gl_program_resource *res =
+  _mesa_program_resource_find_name(shProg, GL_PROGRAM_INPUT, name);

-  if (index = 0)
- return var-data.location + index - VERT_ATTRIB_GENERIC0;
-   }
+   if (!res)
+  return -1;

-   return -1;
+   GLint loc = program_resource_location(shProg, res, name);
+   return (loc = 0) ? loc : -1;


What does program_resource_location return? Can you just do

return program_resource_location(...)?


Argh sorry, I forgot to add some comments to address and discuss this. 
When we assign locations to attributes, VERT_ATTRIB_GENERIC0 offset is 
added so that we don't overlap 'conventional' attributes .. 
'program_resource_location()' subtracts this from the location stored. 
This is how it is specified to work before the extension and OpenGL 4.3.


So right here the location returned by program_resource_location() would 
be  0 if it is one of the built-in attributes. This means that 
implementation here might be actually wrong. If I read spec correctly, 
everything should be returned, no matter if it is builtin or not;


See issue 24:
https://www.opengl.org/registry/specs/ARB/program_interface_query.txt

So, in my current understanding, I should either just return loc + 
VERT_ATTRIB_GENERIC0 if loc  0 or get rid of the offset subtraction 
from program_resource_location(), this will make GetActiveAttrib to work 
in different way though than before so some Piglit tests may need to be 
updated as well.


Ian, any comments on this?

// Tapani
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [Bug 89586] Drivers/DRI/swrast

2015-03-17 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=89586

--- Comment #10 from Dan Sebald daniel.seb...@ieee.org ---
OK, src/mesa/state_tracker/st_cb_drawpixels.c is it, thanks.  Saved me a lot of
time.

And here's a comment about image size in the code:

   /* Limit the size of the glDrawPixels to the max texture size.
* Strictly speaking, that's not correct but since we don't handle
* larger images yet, this is better than crashing.
*/

I simply removed the limit and the image displays fine with no segfaults.

There is a tiny discrepancy compared with the swrast/ result, however.  There
may still be a bug floating about.  I will explain in a different bug report.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH] SConstruct: Fix PEP 8 issues.

2015-03-17 Thread Vinson Lee
Signed-off-by: Vinson Lee v...@freedesktop.org
---
 SConstruct | 66 --
 1 file changed, 34 insertions(+), 32 deletions(-)

diff --git a/SConstruct b/SConstruct
index ef71ab6..f9d3d4c 100644
--- a/SConstruct
+++ b/SConstruct
@@ -1,7 +1,7 @@
 ###
 # Top-level SConstruct
 #
-# For example, invoke scons as 
+# For example, invoke scons as
 #
 #   scons build=debug llvm=yes machine=x86
 #
@@ -12,13 +12,13 @@
 #   build='debug'
 #   llvm=True
 #   machine='x86'
-# 
+#
 # Invoke
 #
 #   scons -h
 #
 # to get the full list of options. See scons manpage for more info.
-#  
+#
 
 import os
 import os.path
@@ -34,14 +34,14 @@ opts = Variables('config.py')
 common.AddOptions(opts)
 
 env = Environment(
-   options = opts,
-   tools = ['gallium'],
-   toolpath = ['#scons'],  
-   ENV = os.environ,
+options=opts,
+tools=['gallium'],
+toolpath=['#scons'],
+ENV=os.environ,
 )
 
 # XXX: This creates a many problems as it saves...
-#opts.Save('config.py', env)
+# opts.Save('config.py', env)
 
 # Backwards compatability with old target configuration variable
 try:
@@ -50,10 +50,11 @@ except KeyError:
 pass
 else:
 targets = targets.split(',')
-print 'scons: warning: targets option is deprecated; pass the targets on 
their own such as'
+print 'scons: warning: targets option is deprecated; ' \
+'pass the targets on their own such as'
 print
 print '  scons %s' % ' '.join(targets)
-print 
+print
 COMMAND_LINE_TARGETS.append(targets)
 
 
@@ -63,30 +64,31 @@ Help(opts.GenerateHelpText(env))
 # Environment setup
 
 with open(VERSION) as f:
-  mesa_version = f.read().strip()
-env.Append(CPPDEFINES = [
+mesa_version = f.read().strip()
+env.Append(CPPDEFINES=[
 ('PACKAGE_VERSION', '\\%s\\' % mesa_version),
-('PACKAGE_BUGREPORT', 
'\\https://bugs.freedesktop.org/enter_bug.cgi?product=Mesa\\;'),
+('PACKAGE_BUGREPORT',
+ '\\https://bugs.freedesktop.org/enter_bug.cgi?product=Mesa\\;'),
 ])
 
 # Includes
-env.Prepend(CPPPATH = [
-   '#/include',
+env.Prepend(CPPPATH=[
+'#/include',
 ])
-env.Append(CPPPATH = [
-   '#/src/gallium/include',
-   '#/src/gallium/auxiliary',
-   '#/src/gallium/drivers',
-   '#/src/gallium/winsys',
+env.Append(CPPPATH=[
+'#/src/gallium/include',
+'#/src/gallium/auxiliary',
+'#/src/gallium/drivers',
+'#/src/gallium/winsys',
 ])
 
 # for debugging
-#print env.Dump()
+# print env.Dump()
 
 
 ###
-# Invoke host SConscripts 
-# 
+# Invoke host SConscripts
+#
 # For things that are meant to be run on the native host build machine, instead
 # of the target machine.
 #
@@ -94,11 +96,11 @@ env.Append(CPPPATH = [
 # Create host environent
 if env['crosscompile'] and not env['embedded']:
 host_env = Environment(
-options = opts,
+options=opts,
 # no tool used
-tools = [],
-toolpath = ['#scons'],
-ENV = os.environ,
+tools=[],
+toolpath=['#scons'],
+ENV=os.environ,
 )
 
 # Override options
@@ -118,8 +120,8 @@ if env['crosscompile'] and not env['embedded']:
 
 SConscript(
 'src/SConscript',
-variant_dir = host_env['build_dir'],
-duplicate = 0, # 
http://www.scons.org/doc/0.97/HTML/scons-user/x2261.html
+variant_dir=host_env['build_dir'],
+duplicate=0  # http://www.scons.org/doc/0.97/HTML/scons-user/x2261.html
 )
 
 env = target_env
@@ -133,9 +135,9 @@ Export('env')
 # http://www.scons.org/wiki/SimultaneousVariantBuilds
 
 SConscript(
-   'src/SConscript',
-   variant_dir = env['build_dir'],
-   duplicate = 0 # http://www.scons.org/doc/0.97/HTML/scons-user/x2261.html
+'src/SConscript',
+variant_dir=env['build_dir'],
+duplicate=0  # http://www.scons.org/doc/0.97/HTML/scons-user/x2261.html
 )
 
 
-- 
2.3.2

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 05/23] mesa: glGetProgramResourceName

2015-03-17 Thread Tapani Pälli



On 03/16/2015 07:16 PM, Ilia Mirkin wrote:

On Fri, Mar 13, 2015 at 4:37 AM, Tapani Pälli tapani.pa...@intel.com wrote:

Patch adds required helper functions to shaderapi.h and
the actual implementation.

Name generation copied from '_mesa_get_uniform_name' which can
be removed later by refactoring functions to use resource list.

The added functionality can be tested by tests for following
functions that are refactored by later patches:

GetActiveUniformName
GetActiveUniformBlockName

Signed-off-by: Tapani Pälli tapani.pa...@intel.com
---
  src/mesa/main/program_resource.c | 22 +
  src/mesa/main/shader_query.cpp   | 96 
  src/mesa/main/shaderapi.h| 10 +
  3 files changed, 128 insertions(+)

diff --git a/src/mesa/main/program_resource.c b/src/mesa/main/program_resource.c
index 4190f98..4fa6ac6 100644
--- a/src/mesa/main/program_resource.c
+++ b/src/mesa/main/program_resource.c
@@ -211,6 +211,28 @@ _mesa_GetProgramResourceName(GLuint program, GLenum 
programInterface,
   GLuint index, GLsizei bufSize, GLsizei *length,
   GLchar *name)
  {
+   GET_CURRENT_CONTEXT(ctx);
+   struct gl_shader_program *shProg =
+  _mesa_lookup_shader_program_err(ctx, program,
+  glGetProgramResourceIndex);
+   if (!shProg)
+  return;
+
+   /* Set user friendly return values in case of errors. */


Should this be done above the if (!shProg) return thing then?


Sure, for the case of consistency.


+   *name = '\0';
+   if (length)
+  *length = 0;
+
+   if (programInterface == GL_ATOMIC_COUNTER_BUFFER) {
+  _mesa_error(ctx, GL_INVALID_ENUM,
+  glGetProgramResourceName(%s),
+  _mesa_lookup_enum_by_nr(programInterface));
+  return;
+   }
+
+   _mesa_get_program_resource_name(shProg, programInterface, index,
+   bufSize, length, name,
+   glGetProgramResourceName);
  }

  void GLAPIENTRY
diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp
index d1974a4..77a4af0 100644
--- a/src/mesa/main/shader_query.cpp
+++ b/src/mesa/main/shader_query.cpp
@@ -647,3 +647,99 @@ _mesa_program_resource_index(struct gl_shader_program 
*shProg,
return calc_resource_index(shProg, res);
 }
  }
+
+/* Find a program resource with specific index in given interface.
+ */
+struct gl_program_resource *
+_mesa_program_resource_find_index(struct gl_shader_program *shProg,
+  GLenum interface, GLuint index)


I feel like I've seen a very similar function before. TBH I can't
remember which patch it was in, but if there's any unification
possibility, please do so.


OK, will see if possible.


+{
+   struct gl_program_resource *res = shProg-ProgramResourceList;
+   int idx = -1;
+
+   for (unsigned i = 0; i  shProg-NumProgramResourceList; i++, res++) {
+  if (res-Type != interface)
+ continue;
+
+  switch (res-Type) {
+  case GL_UNIFORM_BLOCK:
+  case GL_ATOMIC_COUNTER_BUFFER:
+ if (_mesa_program_resource_index(shProg, res) == index)
+return res;
+
+  case GL_TRANSFORM_FEEDBACK_VARYING:
+  case GL_PROGRAM_INPUT:
+  case GL_PROGRAM_OUTPUT:
+  case GL_UNIFORM:
+ if (++idx == (int) index)
+return res;
+ break;
+  default:
+ assert(!not implemented for given interface);
+  }
+   }
+   return NULL;
+}
+
+/* Get full name of a program resource.
+ */
+bool
+_mesa_get_program_resource_name(struct gl_shader_program *shProg,
+GLenum interface, GLuint index,
+GLsizei bufSize, GLsizei *length,
+GLchar *name, const char *caller)
+{
+   GET_CURRENT_CONTEXT(ctx);
+
+   /* Find resource with given interface and index. */
+   struct gl_program_resource *res =
+  _mesa_program_resource_find_index(shProg, interface, index);
+
+   /* The error INVALID_VALUE is generated if index is greater than
+   * or equal to the number of entries in the active resource list for
+   * programInterface.
+   */
+   if (!res) {
+  _mesa_error(ctx, GL_INVALID_VALUE, %s(index %u), caller, index);
+  return false;
+   }
+
+   GLsizei localLength;
+
+   if (length == NULL)
+  length = localLength;
+
+   _mesa_copy_string(name, bufSize, length, _mesa_program_resource_name(res));
+
+   /* Page 61 (page 73 of the PDF) in section 2.11 of the OpenGL ES 3.0
+* spec says:
+*
+* If the active uniform is an array, the uniform name returned in
+* name will always be the name of the uniform array appended with
+* [0].
+*
+* The same text also appears in the OpenGL 4.2 spec.  It does not,
+* however, appear in any previous spec.  Previous specifications are
+* ambiguous in this regard.  However, either name can later 

[Mesa-dev] [Bug 89586] Drivers/DRI/swrast

2015-03-17 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=89586

--- Comment #9 from Ilia Mirkin imir...@alum.mit.edu ---
(In reply to Dan Sebald from comment #8)
 I cloned the piglit repository and will see if I can write a program.  It
 shouldn't be difficult once I figure out the source tree organization, but
 it might take me a couple days.

Basically the way it works is... find a simple test, and copy it. I recently
wrote tests/spec/ext_polygon_offset_clamp/*.c -- iirc 2 tests, both very
simple. Not sure when glPixelZoom came about, but it would probably go into
tests/spec/gl-1.0 or gl-2.0 or whatever. Add it to the CMakeFile.gl.txt and
you're good to go -- rerun 'cmake .' and then you should be able to do 'make
your-great-test-name'. Feel free to ask for help if you get stuck... it's all a
bit annoying as it uses cmake. At least waffle has made it to most distro's
packaging systems by now.

 
 I see the transition happening away from swrast to Gallium, but I'm guessing
 linux bundles might still be relying on swrast as I know the other folks who
 mentioned the lines keep their systems more up-to-date.  There is also at
 least one driver in Gallium that will fall back on swrast under certain
 circumstances.

No. Gallium drivers can't fall back on swrast. There is a 'draw' module which
is used by a few drivers though to handle some (or all, in case of i915g)
vertex shader matters. Note that there's a lot of confusion with the 'swrast'
name, since the gallium software renderers can also be called swrast, and there
are 2 of them.

 
 I was just in the process of creating a bug report for a related type of
 problem in the Gallium driver for glPixelZoom.  However, the Gallium
 organization isn't so straightforward and I've not yet figured out exactly
 which code gets run via Gallium.  Whatever is compiled is also named
 swrast_dri.so in the lib/gallium subdirectory, but it is clearly a different
 implementation of pixel zoom.  (Only the first 8192 pixels zoomed appear in
 the image while the rest is background-color, in both x and y dimensions. 
 So I can't tell if there are dropped pixels in that case just yet.)  I will
 look through softpipe and llvmpipe for the source code.

Take a look at src/mesa/state_tracker/st_cb_drawpixels.c. This is the state
tracker which is effectively an adapter between the OpenGL and Gallium APIs.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 15/18] i965/cs: Emit compute shader code and upload programs

2015-03-17 Thread Kristian Høgsberg
On Sat, Mar 14, 2015 at 9:54 PM, Jordan Justen
jordan.l.jus...@intel.com wrote:
 Signed-off-by: Jordan Justen jordan.l.jus...@intel.com
 ---
  src/mesa/drivers/dri/i965/brw_context.h  |   1 +
  src/mesa/drivers/dri/i965/brw_cs.cpp | 216 
 +++
  src/mesa/drivers/dri/i965/brw_state_upload.c |   3 +
  3 files changed, 220 insertions(+)

 diff --git a/src/mesa/drivers/dri/i965/brw_context.h 
 b/src/mesa/drivers/dri/i965/brw_context.h
 index fb24f0e..170c0c6 100644
 --- a/src/mesa/drivers/dri/i965/brw_context.h
 +++ b/src/mesa/drivers/dri/i965/brw_context.h
 @@ -148,6 +148,7 @@ struct brw_vs_prog_key;
  struct brw_vue_prog_key;
  struct brw_wm_prog_key;
  struct brw_wm_prog_data;
 +struct brw_cs_prog_key;
  struct brw_cs_prog_data;

  enum brw_pipeline {
 diff --git a/src/mesa/drivers/dri/i965/brw_cs.cpp 
 b/src/mesa/drivers/dri/i965/brw_cs.cpp
 index 8021147..5be740c 100644
 --- a/src/mesa/drivers/dri/i965/brw_cs.cpp
 +++ b/src/mesa/drivers/dri/i965/brw_cs.cpp
 @@ -22,8 +22,15 @@
   */


 +#include util/ralloc.h
  #include brw_context.h
  #include brw_cs.h
 +#include brw_fs.h
 +#include brw_eu.h
 +#include brw_wm.h
 +#include intel_mipmap_tree.h
 +#include brw_state.h
 +#include intel_batchbuffer.h

  extern C
  bool
 @@ -46,3 +53,212 @@ brw_cs_prog_data_compare(const void *in_a, const void 
 *in_b)

 return true;
  }
 +
 +
 +static const unsigned *
 +brw_cs_emit(struct brw_context *brw,
 +void *mem_ctx,
 +const struct brw_cs_prog_key *key,
 +struct brw_cs_prog_data *prog_data,
 +struct gl_compute_program *cp,
 +struct gl_shader_program *prog,
 +unsigned *final_assembly_size)
 +{
 +   bool start_busy = false;
 +   double start_time = 0;
 +
 +   if (unlikely(brw-perf_debug)) {
 +  start_busy = (brw-batch.last_bo 
 +drm_intel_bo_busy(brw-batch.last_bo));
 +  start_time = get_time();
 +   }
 +
 +   struct brw_shader *shader = NULL;
 +   if (prog)
 +  shader = (struct brw_shader *) 
 prog-_LinkedShaders[MESA_SHADER_COMPUTE];
 +
 +   if (unlikely(INTEL_DEBUG  DEBUG_CS))
 +  brw_dump_ir(compute, prog, shader-base, cp-Base);
 +
 +   /* Now the main event: Visit the shader IR and generate our CS IR for it.
 +*/
 +   fs_visitor v(brw, mem_ctx, key, prog_data, prog, cp, 8);
 +   if (!v.run_cs()) {
 +  if (prog) {
 + prog-LinkStatus = false;
 + ralloc_strcat(prog-InfoLog, v.fail_msg);
 +  }
 +
 +  _mesa_problem(NULL, Failed to compile fragment shader: %s\n,
 +v.fail_msg);
 +
 +  return NULL;
 +   }
 +
 +   cfg_t *simd16_cfg = NULL;
 +   fs_visitor v2(brw, mem_ctx, key, prog_data, prog, cp, 16);
 +   if (brw-gen = 5  likely(!(INTEL_DEBUG  DEBUG_NO16))) {

If CS is Gen7+ we don't need the = 5 check here.

 +  if (!v.simd16_unsupported) {
 + /* Try a SIMD16 compile */
 + v2.import_uniforms(v);
 + if (!v2.run_cs()) {
 +perf_debug(SIMD16 shader failed to compile, falling back to 
 +   SIMD8 at a 10-20%% performance cost: %s, 
 v2.fail_msg);
 + } else {
 +simd16_cfg = v2.cfg;
 + }
 +  } else {
 + perf_debug(SIMD16 shader unsupported, falling back to 
 +SIMD8 at a 10-20%% performance cost: %s, v.no16_msg);
 +  }
 +   }
 +
 +   prog_data-local_size[0] = cp-LocalSize[0];
 +   prog_data-local_size[1] = cp-LocalSize[1];
 +   prog_data-local_size[2] = cp-LocalSize[2];
 +
 +   cfg_t *simd8_cfg;
 +   int no_simd8 = (INTEL_DEBUG  DEBUG_NO8) || brw-no_simd8;
 +   if (no_simd8  simd16_cfg) {
 +  simd8_cfg = NULL;
 +  prog_data-no_8 = true;
 +   } else {
 +  simd8_cfg = v.cfg;
 +  prog_data-no_8 = false;
 +   }
 +
 +   fs_generator g(brw, mem_ctx, (void*) key, prog_data-base, cp-Base,
 +  v.runtime_check_aads_emit, CS);
 +   if (INTEL_DEBUG  DEBUG_CS) {
 +  char *name = ralloc_asprintf(mem_ctx, %s compute shader %d,
 +   prog-Label ? prog-Label : unnamed,
 +   prog-Name);
 +  g.enable_debug(name);
 +   }
 +   if (simd16_cfg) {
 +  prog_data-simd_size = 16;
 +  g.generate_code(simd16_cfg, 16);
 +   } else if (simd8_cfg) {
 +  prog_data-simd_size = 8;
 +  g.generate_code(simd8_cfg, 8);
 +   }
 +
 +   if (unlikely(brw-perf_debug)  shader) {
 +  if (shader-compiled_once) {
 + _mesa_problem(brw-ctx, CS programs shouldn't need recompiles);
 +  }
 +  shader-compiled_once = true;
 +
 +  if (start_busy  !drm_intel_bo_busy(brw-batch.last_bo)) {
 + perf_debug(CS compile took %.03f ms and stalled the GPU\n,
 +(get_time() - start_time) * 1000);
 +  }
 +   }
 +
 +   return g.get_assembly(final_assembly_size);
 +}
 +
 +static bool
 +do_cs_prog(struct brw_context *brw,
 +   struct gl_shader_program *prog,
 +   struct brw_compute_program *cp,
 

[Mesa-dev] [Bug 89586] Drivers/DRI/swrast

2015-03-17 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=89586

--- Comment #8 from Dan Sebald daniel.seb...@ieee.org ---
I cloned the piglit repository and will see if I can write a program.  It
shouldn't be difficult once I figure out the source tree organization, but it
might take me a couple days.

I see the transition happening away from swrast to Gallium, but I'm guessing
linux bundles might still be relying on swrast as I know the other folks who
mentioned the lines keep their systems more up-to-date.  There is also at least
one driver in Gallium that will fall back on swrast under certain
circumstances.

I was just in the process of creating a bug report for a related type of
problem in the Gallium driver for glPixelZoom.  However, the Gallium
organization isn't so straightforward and I've not yet figured out exactly
which code gets run via Gallium.  Whatever is compiled is also named
swrast_dri.so in the lib/gallium subdirectory, but it is clearly a different
implementation of pixel zoom.  (Only the first 8192 pixels zoomed appear in the
image while the rest is background-color, in both x and y dimensions.  So I
can't tell if there are dropped pixels in that case just yet.)  I will look
through softpipe and llvmpipe for the source code.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 16/18] i965/cs: Support CS program precompile

2015-03-17 Thread Kristian Høgsberg
On Mon, Mar 16, 2015 at 11:20 PM, Kristian Høgsberg k...@bitplanet.net wrote:
 On Sat, Mar 14, 2015 at 9:54 PM, Jordan Justen
 jordan.l.jus...@intel.com wrote:
 Signed-off-by: Jordan Justen jordan.l.jus...@intel.com
 ---
  src/mesa/drivers/dri/i965/brw_context.h  |  6 ++
  src/mesa/drivers/dri/i965/brw_cs.cpp | 26 ++
  src/mesa/drivers/dri/i965/brw_shader.cpp |  4 
  src/mesa/drivers/dri/i965/brw_shader.h   |  3 +++
  4 files changed, 39 insertions(+)

 diff --git a/src/mesa/drivers/dri/i965/brw_context.h 
 b/src/mesa/drivers/dri/i965/brw_context.h
 index 170c0c6..27a4ff4 100644
 --- a/src/mesa/drivers/dri/i965/brw_context.h
 +++ b/src/mesa/drivers/dri/i965/brw_context.h
 @@ -1844,6 +1844,12 @@ brw_fragment_program_const(const struct 
 gl_fragment_program *p)
 return (const struct brw_fragment_program *) p;
  }

 +static inline struct brw_compute_program *
 +brw_compute_program(struct gl_compute_program *p)
 +{
 +   return (struct brw_compute_program *) p;
 +}
 +
  /**
   * Pre-gen6, the register file of the EUs was shared between threads,
   * and each thread used some subset allocated on a 16-register block
 diff --git a/src/mesa/drivers/dri/i965/brw_cs.cpp 
 b/src/mesa/drivers/dri/i965/brw_cs.cpp
 index 5be740c..61c35ae 100644
 --- a/src/mesa/drivers/dri/i965/brw_cs.cpp
 +++ b/src/mesa/drivers/dri/i965/brw_cs.cpp
 @@ -262,3 +262,29 @@ brw_upload_cs_prog(struct brw_context *brw)
 }
 brw-cs.base.prog_data = brw-cs.prog_data-base;
  }
 +
 +
 +extern C bool
 +brw_cs_precompile(struct gl_context *ctx,
 +  struct gl_shader_program *shader_prog,
 +  struct gl_program *prog)
 +{
 +   struct brw_context *brw = brw_context(ctx);
 +   struct brw_cs_prog_key key;
 +
 +   struct gl_compute_program *cp = (struct gl_compute_program *) prog;
 +   struct brw_compute_program *bcp = brw_compute_program(cp);
 +
 +   memset(key, 0, sizeof(key));
 +   key.program_string_id = bcp-id;

 This should initialize key.tex.swizzles similar to how brw_fs_comp

Oops, pressed send too fast: similiar to how brw_fs_precompile does
it.  Or perhaps rename brw_vue_setup_prog_key_for_precompile(), make
it take a struct brw_sampler_prog_key_data and share it across VS, GS,
FS and CS.


 +   uint32_t old_prog_offset = brw-cs.base.prog_offset;
 +   struct brw_cs_prog_data *old_prog_data = brw-cs.prog_data;
 +
 +   bool success = do_cs_prog(brw, shader_prog, bcp, key);
 +
 +   brw-cs.base.prog_offset = old_prog_offset;
 +   brw-cs.prog_data = old_prog_data;
 +
 +   return success;
 +}
 diff --git a/src/mesa/drivers/dri/i965/brw_shader.cpp 
 b/src/mesa/drivers/dri/i965/brw_shader.cpp
 index 499bd94..4392fbc 100644
 --- a/src/mesa/drivers/dri/i965/brw_shader.cpp
 +++ b/src/mesa/drivers/dri/i965/brw_shader.cpp
 @@ -59,6 +59,7 @@ brw_shader_precompile(struct gl_context *ctx,
 struct gl_shader *vs = sh_prog-_LinkedShaders[MESA_SHADER_VERTEX];
 struct gl_shader *gs = sh_prog-_LinkedShaders[MESA_SHADER_GEOMETRY];
 struct gl_shader *fs = sh_prog-_LinkedShaders[MESA_SHADER_FRAGMENT];
 +   struct gl_shader *cs = sh_prog-_LinkedShaders[MESA_SHADER_COMPUTE];

 if (fs  !brw_fs_precompile(ctx, sh_prog, fs-Program))
return false;
 @@ -69,6 +70,9 @@ brw_shader_precompile(struct gl_context *ctx,
 if (vs  !brw_vs_precompile(ctx, sh_prog, vs-Program))
return false;

 +   if (cs  !brw_cs_precompile(ctx, sh_prog, cs-Program))
 +  return false;
 +
 return true;
  }

 diff --git a/src/mesa/drivers/dri/i965/brw_shader.h 
 b/src/mesa/drivers/dri/i965/brw_shader.h
 index 5c95355..53aa727 100644
 --- a/src/mesa/drivers/dri/i965/brw_shader.h
 +++ b/src/mesa/drivers/dri/i965/brw_shader.h
 @@ -230,6 +230,9 @@ bool brw_gs_precompile(struct gl_context *ctx,
  bool brw_fs_precompile(struct gl_context *ctx,
 struct gl_shader_program *shader_prog,
 struct gl_program *prog);
 +bool brw_cs_precompile(struct gl_context *ctx,
 +   struct gl_shader_program *shader_prog,
 +   struct gl_program *prog);

  #ifdef __cplusplus
  }
 --
 2.1.4

 ___
 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


Re: [Mesa-dev] [PATCH 2/2] i965/nir: Re-emit instructions instead of doing mov-to-flag when possible

2015-03-17 Thread Jason Ekstrand
On Tue, Mar 17, 2015 at 11:38 AM, Matt Turner matts...@gmail.com wrote:
 On Mon, Mar 16, 2015 at 9:21 PM, Jason Ekstrand ja...@jlekstrand.net wrote:
 Because of the way that NIR does conditionals, we get them in any old SSA
 value.  The actual boolean value used in the select or if is x != 0.
 Previously, we handled this by emitting a mov.nz to move the value to the
 flag register.  However, this almost always adds at least one if not two
 instructions because we have to go through the VGRF when we could be
 comparing directly to the flag and then using the flag.

 With this commit, we simply re-emit the instruction that produces the value
 when we can.  By doing so, we can use the flag directly and we trust in CSE
 to clean up for us if it can.

 Shader-db results:

 total instructions in shared programs: 4164120 - 4110511 (-1.29%)
 instructions in affected programs: 2397042 - 2343433 (-2.24%)
 helped:13167
 HURT:  31
 GAINED:4
 LOST:  4
 ---

 A small change to the cmod_propagation branch accomplishes most of this --

 total instructions in shared programs: 7850607 - 7793875 (-0.72%)
 instructions in affected programs: 2425140 - 2368408 (-2.34%)
 helped:13502
 HURT:  0
 GAINED:1
 LOST:  5

 We always emit the MOV.NZ with a D-typed destination, which
 cmod_propagate won't combine with a CMP with an F-typed destination.

 I don't think we ever emit MOV.NZ without NIR, so no changes without it.

 Let me play with this some more.

Another aspect here is how this is going to interact with boolean
resolves.  I've got patches in my wip/nir-booleans branch that I'm
testing right now.  I think the general approach is solid, I just need
to test and work out the bugs.  It could take some time as I am
testing on ILK after all.
--Jason
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH v2 3/4] i965/state: Create separate dirty state bits for each pipeline

2015-03-17 Thread Kristian Høgsberg
On Tue, Mar 17, 2015 at 10:46 AM, Kenneth Graunke kenn...@whitecape.org wrote:
 On Tuesday, March 17, 2015 08:20:23 AM Kristian Høgsberg wrote:
 On Tue, Mar 17, 2015 at 2:22 AM, Kenneth Graunke kenn...@whitecape.org 
 wrote:
  On Monday, March 16, 2015 10:21:31 PM Kristian Høgsberg wrote:
  On Wed, Mar 11, 2015 at 11:53 AM, Jordan Justen
  jordan.l.jus...@intel.com wrote:
 [snip]
   @@ -691,8 +702,23 @@ void brw_upload_render_state(struct brw_context 
   *brw)
fprintf(stderr, \n);
  }
   }
   +
   +   /* Save all dirty state into the other pipelines */
   +   for (int i = 0; i  BRW_NUM_PIPELINES; i++) {
   +  if (i != pipeline) {
   + brw-state.pipelines[i].mesa |= state-mesa;
   + brw-state.pipelines[i].brw |= state-brw;
   +  }
   +   }
 
  When we merge this in at this point, state-mesa/brw includes dirty
  bits from the active pipelines pipeline flags. For example, suppose we
  do a bunch of render pipeline stuff, which queues up a lot of dirty
  flags for the compute pipeline. Then when we go to emit state for the
  compute pipeline we first merge all these dirty flags into
  brw-state.dirty, then merge that into the render pipeline flags. We
  only need to merge the new dirty bits, that is, the initial value of
  brw-state.dirty into the render pipeline flags. If we don't modify
  brw-state.dirty in this function and merge the flags in clear as
  described above, this problem is easy to fix.
 
  Kristian

 OK, I see what you mean now - very good catch.

 1. We do a render operation.  Any new dirty bits get saved in
brw-state.pipelines[COMPUTE], so they'll happen when we
do the next compute workload.

This completes successfully (ignore retries).

 2. We do a compute operation.

brw-state.pipelines[COMPUTE] gets merged into brw-state.dirty.
This includes bits that happened during #1 (and were missed on
the compute pipe).

This all works fine.  At the end, we save brw-state.dirty into
brw-state.pipelines[RENDER].

 3. We do another render operation.

brw-state.pipelines[RENDER] gets merged into brw-state.dirty.
We now have bits from #1 again, which are not necessary.

 So yeah.  That's bad.  Sorry, I thought this was to handle the
 hypothetical case of pipe-switch-during-retries.  But it's just
 switching pipelines at all!

I still think both points are valid. Just because we don't hit the
retry problem doesn't mean we shouldn't fix it.  Yes, there's
precedence for moving the flag bits around in brw_upload_state() even
in case it fails, but that just propagates the bits from where higher
level mesa sets them to where we emit state from them. Conceptually
the state remains the same after brw_upload_state() fails. There is
also precedence for *not* changing state destructively in
brw_upload_state(), that's the reason brw_clear_dirty_bits() is there
in the first place. When we clear brw-state.pipelines[pipline] and
merge the flags into brw-state.dirty, we change the state in a way
that makes it a requirement that we call again with the same pipeline
to get correct behavior. Given that we have the brw_clear_dirty_bits()
in place already and that this is all new code (as opposed to changing
behavior of a piece of battle-tested, stable logic), I really don't
see why we would make it work this way.

 If I understand your suggestion correctly, you mean that
 a) We should eliminate brw-state.dirty.
 b) brw_upload_pipeline_state() should do:

struct brw_state_flags *pipeline_state =
   brw-state.pipelines[pipeline];

/* Create a local copy of the dirty state */
struct brw_state_flags dirty;
dirty.mesa = brw-NewGLState | pipeline_state-mesa;
dirty.brw = ctx-NewDriverState | pipeline_state-brw;

Use this for uploads.

 c) The existing brw_clear_dirty_bits() should do:

/* Save the dirty flags into the other pipelines */
for (int i = 0; i  BRW_NUM_PIPELINES; i++) {
   if (i != pipeline) {
  brw-state.pipelines[i].mesa |= brw-NewGLState;
  brw-state.pipelines[i].brw |= ctx-NewDriverState;
   }
}

/* We've handled any bits passed to us by other pipeline uploads */
brw-state.pipelines[pipe] = 0;

/* We've handled the flags passed to us by Mesa */
brw-NewGLState = 0;
ctx-NewDriverState = 0;

 Does that sound like a correct interpretation?  This does seem to solve
 the problem, and I think it's a bit cleaner too.

Yes, pretty much. I wasn't considering eliminating brw-state.dirty,
but that would be nice. We still have atoms that set dirty bits there
as they're emitted, but we can use ctx-NewDriverState for that and or
it into dirty.brw after each atom emit call.  That's a bigger change,
I see around 90 cases of setting bits in brw-state.dirty.brw, but
getting rid of brw-state.dirty may be worth the churn.

 Re-evaluating my example to verify that this approach works...

 1. We do a render operation.

The local dirty flags are bits from Mesa | 

Re: [Mesa-dev] Summer of Code ideas (maybe just an idea wishlist?)

2015-03-17 Thread Bruno Jimenez
Hi,

Last year, I participated in GSoC, (yeah, I still read the mailing list
every day :) ) so I will give my 2 cents to the topic.

My background isn't at all related to graphics or computer science, I am
a physicist, although my main interest is in simulation. So that took me
to OpenCL, which in turn took me to clover and finally landed in Mesa. I
started looking at the code to see how things worked in the backstage
(you know, we physicists usually like to know how things work :) ), and
I spotted a couple of things that I thought could be done better. At
first, I was mostly afraid of doing anything, mostly because I am not a
programmer (I still don't think of myself as one) and I didn't know if
what I was doing and how I was doing it was ok. But I ended sending some
patches to the mailing list, and one thing followed another and I ended
doing a GSoC.

And here I am still, mostly without free time to try to continue where I
left everything, but mostly willing to help again with whatever I can.

My points would be more or less two: First, not to just think about
graphics and cs people as possible help, it is true that they may be the
best for the job, but other backgrounds may also help. And second, be
welcoming to new people, for some of us, the step of sending something
to the list, specially the first time, may be very stressfull, and
receiving an answer with a plain 'do this' may be a bit discouraging.

As said, just my 2 cents.
Bruno

On Mon, 2015-03-16 at 09:32 -0700, Laura Ekstrand wrote:
 That was basically my background (mechanical engineering + lots of
 OpenGL) when I started six months ago, but I have found the lack of
 mentoring to be a large roadblock.  At that time, I wrote tests, but
 there were few people willing to review them and give timely feedback.
 I was advised to go ahead and push the tests after a month, but then
 others came back weeks later with lots of late reviews after the fact.
 They were highly critical and made me feel unwelcome in the community.
 I've had more success working directly on the Mesa driver.
 
 
 So I'm not sure we can attract and retain these types of students.
 
 
 On Mon, Mar 16, 2015 at 6:23 AM, Marek Olšák mar...@gmail.com wrote:
 On Fri, Mar 13, 2015 at 6:24 PM, Laura Ekstrand
 la...@jlekstrand.net wrote:
  We should try to steer people away from just writing Piglit
 tests for GSoC,
  unless they have a specific mentor in mind and have already
 talked to him or
  her.  In my experience, Piglit tests are difficult to do
 well because each
  one is drastically different from the others and involves
 cultivating a
  fairly deep understanding of the the OpenGL function in
 question.
 
  A project pairing a specific extension with relevant tests
 (like Martin and
  I have done with ARB_dsa) would be better as long as it's a
 fairly specific
  domain of the OpenGL spec.  That way, the student can study
 the spec for one
  specific set of objects or entry points and cultivate the
 necessary
  understanding they need to write the related tests.
 
  A lot of the emails we've gotten from students saying I
 want to write 4.x
  Piglit tests have been too broad/generic and would be
 difficult for a
  student to master in a summer without lots of
 mentoring/direction from the
  community.
 
 We should also take into account that there are people having
 a degree
 in or studying computer science with specialization in
 computer
 graphics or having strong knowledge of OpenGL already. Such
 people are
 difficult to find, but they would be very effective with very
 little
 (if any) mentoring. Gamedev-related forums (gamedev.net,
 opengl.org,
 etc.) should have a lot of talented people suited for this
 job, but
 none of them are probably aware of the Mesa/Piglit GSoC.
 
 Marek
 
 
 ___
 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


Re: [Mesa-dev] [PATCH] i965/fs: Print spills:fills and number of promoted constants.

2015-03-17 Thread Matt Turner
On Tue, Mar 17, 2015 at 2:15 PM, Jason Ekstrand ja...@jlekstrand.net wrote:
 On Tue, Mar 17, 2015 at 2:09 PM, Matt Turner matts...@gmail.com wrote:
 diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp 
 b/src/mesa/drivers/dri/i965/brw_vec4.cpp
 index 8edb4d0..63dedae 100644
 --- a/src/mesa/drivers/dri/i965/brw_vec4.cpp
 +++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp
 @@ -1969,7 +1969,7 @@ brw_vs_emit(struct brw_context *brw,
}

fs_generator g(brw, mem_ctx, (void *) c-key, prog_data-base.base,
 - c-vp-program.Base, v.runtime_check_aads_emit, VS);
 + c-vp-program.Base, v.runtime_check_aads_emit, 
 v.promoted_constants, VS);

 Promoted constants and aads_emit need to be flipped around.  You got
 it right for FS.

Thanks!

 Also, does this require any adaptations to shader-db or does it work as-is?

Works as is, but shader-db report.py doesn't know about the new
things. I'd like to add some switches to it like --spills or
--compaction and have it print stats based on those.

 Other than that,
 Reviewed-by: Jason Ekstrand jason.ekstr...@intel.com

Thanks!
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] i965/fs: Print spills:fills and number of promoted constants.

2015-03-17 Thread Chris Forbes
With the fix Jason mentioned:

Reviewed-by: Chris Forbes chr...@ijw.co.nz

On Wed, Mar 18, 2015 at 10:19 AM, Matt Turner matts...@gmail.com wrote:
 On Tue, Mar 17, 2015 at 2:15 PM, Jason Ekstrand ja...@jlekstrand.net wrote:
 On Tue, Mar 17, 2015 at 2:09 PM, Matt Turner matts...@gmail.com wrote:
 diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp 
 b/src/mesa/drivers/dri/i965/brw_vec4.cpp
 index 8edb4d0..63dedae 100644
 --- a/src/mesa/drivers/dri/i965/brw_vec4.cpp
 +++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp
 @@ -1969,7 +1969,7 @@ brw_vs_emit(struct brw_context *brw,
}

fs_generator g(brw, mem_ctx, (void *) c-key, prog_data-base.base,
 - c-vp-program.Base, v.runtime_check_aads_emit, 
 VS);
 + c-vp-program.Base, v.runtime_check_aads_emit, 
 v.promoted_constants, VS);

 Promoted constants and aads_emit need to be flipped around.  You got
 it right for FS.

 Thanks!

 Also, does this require any adaptations to shader-db or does it work as-is?

 Works as is, but shader-db report.py doesn't know about the new
 things. I'd like to add some switches to it like --spills or
 --compaction and have it print stats based on those.

 Other than that,
 Reviewed-by: Jason Ekstrand jason.ekstr...@intel.com

 Thanks!
 ___
 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


[Mesa-dev] [PATCH] i965/aa: fixing anti-aliasing bug for thinnest width lines - GEN7

2015-03-17 Thread Marius Predut
On SNB and IVB hw, for 1 pixel line thickness or less,
the general anti-aliasing algorithm give up - garbage line is generated.
Setting a Line Width of 0.0 specifies the rasterization
of the “thinnest” (one-pixel-wide), non-antialiased lines.
Lines rendered with zero Line Width are rasterized using
Grid Intersection Quantization rules as specified by
bspec section 6.3.12.1 Zero-Width (Cosmetic) Line Rasterization.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=28832
Signed-off-by: Marius Predut marius.pre...@intel.com
Signed-off-by: Marius Predut Marius Predut marius.pre...@intel.com
---
 src/mesa/drivers/dri/i965/gen7_sf_state.c |   12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/gen7_sf_state.c 
b/src/mesa/drivers/dri/i965/gen7_sf_state.c
index c9815b0..38b4f2f 100644
--- a/src/mesa/drivers/dri/i965/gen7_sf_state.c
+++ b/src/mesa/drivers/dri/i965/gen7_sf_state.c
@@ -198,9 +198,15 @@ upload_sf_state(struct brw_context *brw)
   float line_width =
  roundf(CLAMP(ctx-Line.Width, 0.0, ctx-Const.MaxLineWidth));
   uint32_t line_width_u3_7 = U_FIXED(line_width, 7);
-  /* TODO: line width of 0 is not allowed when MSAA enabled */
-  if (line_width_u3_7 == 0)
- line_width_u3_7 = 1;
+
+  if (!(multisampled_fbo  ctx-Multisample.Enabled)) {
+ if (ctx-Line.SmoothFlag  ctx-Line.Width = 1)
+line_width_u3_7 = 0;
+  } else {
+  if (line_width_u3_7 == 0)
+ line_width_u3_7 = 1;
+  }
+
   dw2 |= line_width_u3_7  GEN6_SF_LINE_WIDTH_SHIFT;
}
if (ctx-Line.SmoothFlag) {
-- 
1.7.9.5

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH] i965/fs: Print spills:fills and number of promoted constants.

2015-03-17 Thread Matt Turner
---
 src/mesa/drivers/dri/i965/brw_blorp_blit_eu.cpp  |  2 +-
 src/mesa/drivers/dri/i965/brw_fs.cpp |  2 +-
 src/mesa/drivers/dri/i965/brw_fs.h   |  4 
 .../drivers/dri/i965/brw_fs_combine_constants.cpp|  1 +
 src/mesa/drivers/dri/i965/brw_fs_generator.cpp   | 20 +---
 src/mesa/drivers/dri/i965/brw_fs_visitor.cpp |  4 ++--
 src/mesa/drivers/dri/i965/brw_vec4.cpp   |  2 +-
 7 files changed, 23 insertions(+), 12 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_blorp_blit_eu.cpp 
b/src/mesa/drivers/dri/i965/brw_blorp_blit_eu.cpp
index f9b1737..32919b1 100644
--- a/src/mesa/drivers/dri/i965/brw_blorp_blit_eu.cpp
+++ b/src/mesa/drivers/dri/i965/brw_blorp_blit_eu.cpp
@@ -31,7 +31,7 @@ brw_blorp_eu_emitter::brw_blorp_eu_emitter(struct brw_context 
*brw,
: mem_ctx(ralloc_context(NULL)),
  generator(brw, mem_ctx, (void *) rzalloc(mem_ctx, struct brw_wm_prog_key),
(struct brw_stage_prog_data *) rzalloc(mem_ctx, struct 
brw_wm_prog_data),
-   NULL, false, BLORP)
+   NULL, 0, false, BLORP)
 {
if (debug_flag)
   generator.enable_debug(blorp);
diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp 
b/src/mesa/drivers/dri/i965/brw_fs.cpp
index 8702ea8..7f671dc 100644
--- a/src/mesa/drivers/dri/i965/brw_fs.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
@@ -4039,7 +4039,7 @@ brw_wm_fs_emit(struct brw_context *brw,
}
 
fs_generator g(brw, mem_ctx, (void *) key, prog_data-base,
-  fp-Base, v.runtime_check_aads_emit, FS);
+  fp-Base, v.promoted_constants, v.runtime_check_aads_emit, 
FS);
 
if (unlikely(INTEL_DEBUG  DEBUG_WM)) {
   char *name;
diff --git a/src/mesa/drivers/dri/i965/brw_fs.h 
b/src/mesa/drivers/dri/i965/brw_fs.h
index 7716529..ed0bb8f 100644
--- a/src/mesa/drivers/dri/i965/brw_fs.h
+++ b/src/mesa/drivers/dri/i965/brw_fs.h
@@ -512,6 +512,8 @@ public:
bool spilled_any_registers;
 
const unsigned dispatch_width; /** 8 or 16 */
+
+   unsigned promoted_constants;
 };
 
 /**
@@ -527,6 +529,7 @@ public:
 const void *key,
 struct brw_stage_prog_data *prog_data,
 struct gl_program *fp,
+unsigned promoted_constants,
 bool runtime_check_aads_emit,
 const char *stage_abbrev);
~fs_generator();
@@ -638,6 +641,7 @@ private:
unsigned dispatch_width; /** 8 or 16 */
 
exec_list discard_halt_patches;
+   unsigned promoted_constants;
bool runtime_check_aads_emit;
bool debug_flag;
const char *shader_name;
diff --git a/src/mesa/drivers/dri/i965/brw_fs_combine_constants.cpp 
b/src/mesa/drivers/dri/i965/brw_fs_combine_constants.cpp
index 7ddb253..ebde8df 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_combine_constants.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_combine_constants.cpp
@@ -286,6 +286,7 @@ fs_visitor::opt_combine_constants()
  reg.subreg_offset = 0;
   }
}
+   promoted_constants = table.len;
 
/* Rewrite the immediate sources to refer to the new GRFs. */
for (int i = 0; i  table.len; i++) {
diff --git a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp 
b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
index e086266..e61a9d3 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
@@ -126,13 +126,15 @@ fs_generator::fs_generator(struct brw_context *brw,
const void *key,
struct brw_stage_prog_data *prog_data,
struct gl_program *prog,
+   unsigned promoted_constants,
bool runtime_check_aads_emit,
const char *stage_abbrev)
 
: brw(brw), key(key),
  prog_data(prog_data),
  prog(prog), runtime_check_aads_emit(runtime_check_aads_emit),
- debug_flag(false), stage_abbrev(stage_abbrev), mem_ctx(mem_ctx)
+ debug_flag(false), stage_abbrev(stage_abbrev), mem_ctx(mem_ctx),
+ promoted_constants(promoted_constants)
 {
ctx = brw-ctx;
 
@@ -1563,6 +1565,7 @@ fs_generator::generate_code(const cfg_t *cfg, int 
dispatch_width)
   brw_set_default_compression_control(p, BRW_COMPRESSION_COMPRESSED);
 
int start_offset = p-next_insn_offset;
+   int spill_count = 0, fill_count = 0;
int loop_count = 0;
 
struct annotation_info annotation;
@@ -1959,14 +1962,17 @@ fs_generator::generate_code(const cfg_t *cfg, int 
dispatch_width)
 
   case SHADER_OPCODE_GEN4_SCRATCH_WRITE:
 generate_scratch_write(inst, src[0]);
+ spill_count++;
 break;
 
   case SHADER_OPCODE_GEN4_SCRATCH_READ:
 generate_scratch_read(inst, dst);
+ fill_count++;
 break;
 
   case SHADER_OPCODE_GEN7_SCRATCH_READ:
 generate_scratch_read_gen7(inst, dst);
+ fill_count++;
 break;
 
   case 

Re: [Mesa-dev] [PATCH] i965/fs: Print spills:fills and number of promoted constants.

2015-03-17 Thread Jason Ekstrand
On Tue, Mar 17, 2015 at 2:09 PM, Matt Turner matts...@gmail.com wrote:
 ---
  src/mesa/drivers/dri/i965/brw_blorp_blit_eu.cpp  |  2 +-
  src/mesa/drivers/dri/i965/brw_fs.cpp |  2 +-
  src/mesa/drivers/dri/i965/brw_fs.h   |  4 
  .../drivers/dri/i965/brw_fs_combine_constants.cpp|  1 +
  src/mesa/drivers/dri/i965/brw_fs_generator.cpp   | 20 
 +---
  src/mesa/drivers/dri/i965/brw_fs_visitor.cpp |  4 ++--
  src/mesa/drivers/dri/i965/brw_vec4.cpp   |  2 +-
  7 files changed, 23 insertions(+), 12 deletions(-)

 diff --git a/src/mesa/drivers/dri/i965/brw_blorp_blit_eu.cpp 
 b/src/mesa/drivers/dri/i965/brw_blorp_blit_eu.cpp
 index f9b1737..32919b1 100644
 --- a/src/mesa/drivers/dri/i965/brw_blorp_blit_eu.cpp
 +++ b/src/mesa/drivers/dri/i965/brw_blorp_blit_eu.cpp
 @@ -31,7 +31,7 @@ brw_blorp_eu_emitter::brw_blorp_eu_emitter(struct 
 brw_context *brw,
 : mem_ctx(ralloc_context(NULL)),
   generator(brw, mem_ctx, (void *) rzalloc(mem_ctx, struct 
 brw_wm_prog_key),
 (struct brw_stage_prog_data *) rzalloc(mem_ctx, struct 
 brw_wm_prog_data),
 -   NULL, false, BLORP)
 +   NULL, 0, false, BLORP)
  {
 if (debug_flag)
generator.enable_debug(blorp);
 diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp 
 b/src/mesa/drivers/dri/i965/brw_fs.cpp
 index 8702ea8..7f671dc 100644
 --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
 +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
 @@ -4039,7 +4039,7 @@ brw_wm_fs_emit(struct brw_context *brw,
 }

 fs_generator g(brw, mem_ctx, (void *) key, prog_data-base,
 -  fp-Base, v.runtime_check_aads_emit, FS);
 +  fp-Base, v.promoted_constants, 
 v.runtime_check_aads_emit, FS);

 if (unlikely(INTEL_DEBUG  DEBUG_WM)) {
char *name;
 diff --git a/src/mesa/drivers/dri/i965/brw_fs.h 
 b/src/mesa/drivers/dri/i965/brw_fs.h
 index 7716529..ed0bb8f 100644
 --- a/src/mesa/drivers/dri/i965/brw_fs.h
 +++ b/src/mesa/drivers/dri/i965/brw_fs.h
 @@ -512,6 +512,8 @@ public:
 bool spilled_any_registers;

 const unsigned dispatch_width; /** 8 or 16 */
 +
 +   unsigned promoted_constants;
  };

  /**
 @@ -527,6 +529,7 @@ public:
  const void *key,
  struct brw_stage_prog_data *prog_data,
  struct gl_program *fp,
 +unsigned promoted_constants,
  bool runtime_check_aads_emit,
  const char *stage_abbrev);
 ~fs_generator();
 @@ -638,6 +641,7 @@ private:
 unsigned dispatch_width; /** 8 or 16 */

 exec_list discard_halt_patches;
 +   unsigned promoted_constants;
 bool runtime_check_aads_emit;
 bool debug_flag;
 const char *shader_name;
 diff --git a/src/mesa/drivers/dri/i965/brw_fs_combine_constants.cpp 
 b/src/mesa/drivers/dri/i965/brw_fs_combine_constants.cpp
 index 7ddb253..ebde8df 100644
 --- a/src/mesa/drivers/dri/i965/brw_fs_combine_constants.cpp
 +++ b/src/mesa/drivers/dri/i965/brw_fs_combine_constants.cpp
 @@ -286,6 +286,7 @@ fs_visitor::opt_combine_constants()
   reg.subreg_offset = 0;
}
 }
 +   promoted_constants = table.len;

 /* Rewrite the immediate sources to refer to the new GRFs. */
 for (int i = 0; i  table.len; i++) {
 diff --git a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp 
 b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
 index e086266..e61a9d3 100644
 --- a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
 +++ b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
 @@ -126,13 +126,15 @@ fs_generator::fs_generator(struct brw_context *brw,
 const void *key,
 struct brw_stage_prog_data *prog_data,
 struct gl_program *prog,
 +   unsigned promoted_constants,
 bool runtime_check_aads_emit,
 const char *stage_abbrev)

 : brw(brw), key(key),
   prog_data(prog_data),
   prog(prog), runtime_check_aads_emit(runtime_check_aads_emit),
 - debug_flag(false), stage_abbrev(stage_abbrev), mem_ctx(mem_ctx)
 + debug_flag(false), stage_abbrev(stage_abbrev), mem_ctx(mem_ctx),
 + promoted_constants(promoted_constants)
  {
 ctx = brw-ctx;

 @@ -1563,6 +1565,7 @@ fs_generator::generate_code(const cfg_t *cfg, int 
 dispatch_width)
brw_set_default_compression_control(p, BRW_COMPRESSION_COMPRESSED);

 int start_offset = p-next_insn_offset;
 +   int spill_count = 0, fill_count = 0;
 int loop_count = 0;

 struct annotation_info annotation;
 @@ -1959,14 +1962,17 @@ fs_generator::generate_code(const cfg_t *cfg, int 
 dispatch_width)

case SHADER_OPCODE_GEN4_SCRATCH_WRITE:
  generate_scratch_write(inst, src[0]);
 + spill_count++;
  break;

case SHADER_OPCODE_GEN4_SCRATCH_READ:
  generate_scratch_read(inst, dst);
 + fill_count++;
  

Re: [Mesa-dev] [PATCH 07/23] mesa: glGetProgramResourceLocationIndex

2015-03-17 Thread Tapani Pälli



On 03/16/2015 08:08 PM, Ilia Mirkin wrote:

On Fri, Mar 13, 2015 at 4:37 AM, Tapani Pälli tapani.pa...@intel.com wrote:

Patch adds required helper functions to shaderapi.h and
the actual implementation.

The added functionality can be tested by tests for following
functions that are refactored by later patches:

GetFragDataIndex

Signed-off-by: Tapani Pälli tapani.pa...@intel.com
---
  src/mesa/main/program_resource.c | 25 -
  src/mesa/main/shader_query.cpp   | 18 ++
  src/mesa/main/shaderapi.h|  4 
  3 files changed, 46 insertions(+), 1 deletion(-)

diff --git a/src/mesa/main/program_resource.c b/src/mesa/main/program_resource.c
index 87a0144..ae987de 100644
--- a/src/mesa/main/program_resource.c
+++ b/src/mesa/main/program_resource.c
@@ -329,9 +329,32 @@ _mesa_GetProgramResourceLocation(GLuint program, GLenum 
programInterface,
 return _mesa_program_resource_location(shProg, programInterface, name);
  }

+/**
+ * Returns output index for dual source blending.
+ */
  GLint GLAPIENTRY
  _mesa_GetProgramResourceLocationIndex(GLuint program, GLenum programInterface,
const GLchar *name)
  {
-   return -1;
+   GET_CURRENT_CONTEXT(ctx);
+   struct gl_shader_program *shProg =
+  lookup_linked_program(program, glGetProgramResourceLocationIndex);
+
+   if (!shProg || invalid_array_element_syntax(name))
+  return -1;
+
+   /* From the GL_ARB_program_interface_query spec:
+*
+* For GetProgramResourceLocationIndex, programInterface must be
+* PROGRAM_OUTPUT.
+*/


And presumably it must be a program with a fragment shader (which
might not be there for a no-rast or compute pipeline).


spec says that -1 is returned:

If program has been successfully linked but contains no fragment 
shader, no error will be generated but -1 will be returned.


this is what happens with the implementation.


+   if (programInterface != GL_PROGRAM_OUTPUT) {
+  _mesa_error(ctx, GL_INVALID_ENUM,
+  glGetProgramResourceLocationIndex (%s),
+  _mesa_lookup_enum_by_nr(programInterface));
+  return -1;
+   }
+
+   return _mesa_program_resource_location_index(shProg, programInterface,
+name);
  }
diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp
index 4ae00a6..d3264db 100644
--- a/src/mesa/main/shader_query.cpp
+++ b/src/mesa/main/shader_query.cpp
@@ -807,3 +807,21 @@ _mesa_program_resource_location(struct gl_shader_program 
*shProg,

 return program_resource_location(shProg, res, name);
  }
+
+/**
+ * Function implements following index queries:
+ *glGetFragDataIndex
+ */
+GLint
+_mesa_program_resource_location_index(struct gl_shader_program *shProg,
+  GLenum interface, const char *name)
+{
+   struct gl_program_resource *res =
+  _mesa_program_resource_find_name(shProg, interface, name);
+
+   /* Non-existent (inactive) variable. */
+   if (!res)
+  return -1;
+
+   return RESOURCE_VAR(res)-data.index;
+}
diff --git a/src/mesa/main/shaderapi.h b/src/mesa/main/shaderapi.h
index 73ebf60..5046018 100644
--- a/src/mesa/main/shaderapi.h
+++ b/src/mesa/main/shaderapi.h
@@ -248,6 +248,10 @@ extern GLint
  _mesa_program_resource_location(struct gl_shader_program *shProg,
  GLenum interface, const char *name);

+extern GLint
+_mesa_program_resource_location_index(struct gl_shader_program *shProg,
+  GLenum interface, const char *name);
+
  #ifdef __cplusplus
  }
  #endif
--
2.1.0

___
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


Re: [Mesa-dev] [PATCH 15/23] mesa: refactor GetActiveUniformsiv, use _mesa_program_resource_prop

2015-03-17 Thread Tapani Pälli



On 03/17/2015 12:38 AM, Ilia Mirkin wrote:

On Fri, Mar 13, 2015 at 4:38 AM, Tapani Pälli tapani.pa...@intel.com wrote:

Signed-off-by: Tapani Pälli tapani.pa...@intel.com
---
  src/mesa/main/uniform_query.cpp | 107 ++--
  1 file changed, 38 insertions(+), 69 deletions(-)

diff --git a/src/mesa/main/uniform_query.cpp b/src/mesa/main/uniform_query.cpp
index 9f82de9..217473a 100644
--- a/src/mesa/main/uniform_query.cpp
+++ b/src/mesa/main/uniform_query.cpp
@@ -79,6 +79,33 @@ _mesa_GetActiveUniform(GLuint program, GLuint index,
 }


8


-
-  case GL_UNIFORM_ATOMIC_COUNTER_BUFFER_INDEX:
- if (!ctx-Extensions.ARB_shader_atomic_counters)
-goto invalid_enum;
- params[i] = uni-atomic_buffer_index;
+  if (!_mesa_program_resource_prop(shProg, res, uniformIndices[i],
+   res_prop, params[i],
+   glGetActiveUniformsiv))


Will this return GL_INVALID_ENUM if res_prop == 0? If not, you need to
handle that above.


Yes, it results in INVALID_ENUM error.


With that answered or taken care of,

Reviewed-by: Ilia Mirkin imir...@alum.mit.edu


   break;
-
-  default:
- goto invalid_enum;
-  }
 }
-
-   return;
-
- invalid_enum:
-   _mesa_error(ctx, GL_INVALID_ENUM, glGetActiveUniformsiv(pname));
  }

  static struct gl_uniform_storage *
--
2.1.0

___
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


Re: [Mesa-dev] [PATCH 06/23] mesa: glGetProgramResourceLocation

2015-03-17 Thread Tapani Pälli



On 03/16/2015 07:08 PM, Ilia Mirkin wrote:

On Fri, Mar 13, 2015 at 4:37 AM, Tapani Pälli tapani.pa...@intel.com wrote:

Patch adds required helper functions to shaderapi.h and
the actual implementation.

corresponding Piglit test:
arb_program_interface_query-resource-location

The added functionality can be tested by tests for following
functions that are refactored by later patches:

GetAttribLocation
GetUniformLocation
GetFragDataLocation

Signed-off-by: Tapani Pälli tapani.pa...@intel.com
---
  src/mesa/main/program_resource.c | 81 +++-
  src/mesa/main/shader_query.cpp   | 64 +++
  src/mesa/main/shaderapi.h|  4 ++
  3 files changed, 148 insertions(+), 1 deletion(-)

diff --git a/src/mesa/main/program_resource.c b/src/mesa/main/program_resource.c
index 4fa6ac6..87a0144 100644
--- a/src/mesa/main/program_resource.c
+++ b/src/mesa/main/program_resource.c
@@ -243,11 +243,90 @@ _mesa_GetProgramResourceiv(GLuint program, GLenum 
programInterface,
  {
  }

+/**
+ * Function verifies syntax of given name for GetProgramResourceLocation
+ * and GetProgramResourceLocationIndex for the following cases:
+ *
+ * array element portion of a string passed to GetProgramResourceLocation
+ * or GetProgramResourceLocationIndex must not have, a + sign, extra
+ * leading zeroes, or whitespace.
+ *
+ * Check is written to be compatible with GL_ARB_array_of_arrays.
+ */
+static bool
+invalid_array_element_syntax(const GLchar *name)
+{
+   char *array = strrchr(name, '[');
+
+   if (!array)
+  return false;
+
+   /* No '+' or ' ' allowed anywhere. */
+   if (strchr(name, '+') || strchr(name, ' '))


I guess it'd be mildly better to do a strchr('[') and use that for the
second strchr's? You could do it like

char *first = strchr(name, '[');
char *last = strrchr(first, '[');
if (strchr(first, '+') || ... )

That way you avoid iterating over the name portion of it
unnecessarily. Probably doesn't amount to too much.


Yes, it is more optimal, will change.


+  return true;
+
+   /* Check that last array index is 0. */
+   if (array[1] == '0'  array[2] != ']')
+  return true;
+
+   return false;
+}
+
+static struct gl_shader_program *
+lookup_linked_program(GLuint program, const char *caller)
+{
+   GET_CURRENT_CONTEXT(ctx);
+   struct gl_shader_program *prog =
+  _mesa_lookup_shader_program_err(ctx, program, caller);
+
+   if (!prog)
+  return NULL;
+
+   if (prog-LinkStatus == GL_FALSE) {
+  _mesa_error(ctx, GL_INVALID_OPERATION,
+  %s(program not linked), caller);
+  return NULL;
+   }
+   return prog;
+}
+
  GLint GLAPIENTRY
  _mesa_GetProgramResourceLocation(GLuint program, GLenum programInterface,
   const GLchar *name)
  {
-   return -1;
+   GET_CURRENT_CONTEXT(ctx);
+   struct gl_shader_program *shProg =
+  lookup_linked_program(program, glGetProgramResourceLocation);
+
+   if (!shProg || invalid_array_element_syntax(name))
+  return -1;
+
+   /* Validate programInterface. */
+   switch (programInterface) {
+   case GL_UNIFORM:
+   case GL_PROGRAM_INPUT:
+   case GL_PROGRAM_OUTPUT:
+  break;
+
+   /* For reference valid cases requiring addition extension support:
+* GL_ARB_shader_subroutine
+* GL_ARB_tessellation_shader
+* GL_ARB_compute_shader
+*/
+   case GL_VERTEX_SUBROUTINE_UNIFORM:
+   case GL_TESS_CONTROL_SUBROUTINE_UNIFORM:
+   case GL_TESS_EVALUATION_SUBROUTINE_UNIFORM:
+   case GL_GEOMETRY_SUBROUTINE_UNIFORM:
+   case GL_FRAGMENT_SUBROUTINE_UNIFORM:
+   case GL_COMPUTE_SUBROUTINE_UNIFORM:
+
+   default:
+  _mesa_error(ctx, GL_INVALID_ENUM,
+  glGetProgramResourceLocation(%s %s),
+  _mesa_lookup_enum_by_nr(programInterface), name);
+   }
+
+   return _mesa_program_resource_location(shProg, programInterface, name);
  }

  GLint GLAPIENTRY
diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp
index 77a4af0..4ae00a6 100644
--- a/src/mesa/main/shader_query.cpp
+++ b/src/mesa/main/shader_query.cpp
@@ -743,3 +743,67 @@ _mesa_get_program_resource_name(struct gl_shader_program 
*shProg,
 }
 return true;
  }
+
+static GLint
+program_resource_location(struct gl_shader_program *shProg,
+  struct gl_program_resource *res, const char *name)
+{
+   unsigned index, offset;
+   int array_index = -1;


And I suppose leaving off the initializer here makes gcc complain?


Yep,warning: 'array_index' may be used uninitialized in this function


+
+   if (res-Type == GL_PROGRAM_INPUT ||
+   res-Type == GL_PROGRAM_OUTPUT) {


put all on one line?


will fix


+  array_index = array_index_of_resource(res, name);
+  if (array_index  0)
+ return -1;
+   }
+
+   switch (res-Type) {
+   case GL_PROGRAM_INPUT:
+  return RESOURCE_VAR(res)-data.location + array_index - 
VERT_ATTRIB_GENERIC0;
+   case GL_PROGRAM_OUTPUT:
+  return 

Re: [Mesa-dev] [PATCH v2 3/4] i965/state: Create separate dirty state bits for each pipeline

2015-03-17 Thread Kenneth Graunke
On Monday, March 16, 2015 10:21:31 PM Kristian Høgsberg wrote:
 On Wed, Mar 11, 2015 at 11:53 AM, Jordan Justen
 jordan.l.jus...@intel.com wrote:
  When uploading state for a pipeline, we will save changed state for
  the other pipelines.
 
  Signed-off-by: Jordan Justen jordan.l.jus...@intel.com
 
 In reviewing this again, I realize I'm not completely comfortable with
 how this is done. The overall approach is great, I'm happy to see the
 dual pipeline flags contained in brw_state_upload.c, but I think we
 need to keep brw_upload_pipeline_state() a no-op in case it fails.

Given that brw_upload_state() has always done:

   struct brw_state_flags *state = brw-state.dirty;

   state-mesa |= brw-NewGLState;
   brw-NewGLState = 0;

   state-brw |= ctx-NewDriverState;
   ctx-NewDriverState = 0;

I think there's a pretty good precedence for having it update the
driver's notion of what's dirty.  But you can still run it several
times back to back with no issues.

 I can't think of a case where it might bite us, but currently, the
 expectation is that if it fails, nothing changes and you can call it
 again after flushing the batch.  This patch changes that in two ways:
 1) we merge the pipeline brw-state.dirty into state and clear the
 pipeline state and 2) we copy the dirty state into the other pipeline.
 Suppose we call with the render pipeline and fail, flush the batch,
 then call with the compute pipeline and clear the dirty flags [1]. If
 we then go back and try to emit render state again, there are no
 longer any dirty bits and we fail to emit render state.

I'm pretty comfortable with this.  Your concerns seem to revolve around
implicitly switching pipelines in the middle of state upload.  I don't
believe that can happen (and we'd better keep it that way!).

You're always uploading state for some particular operation - drawing
(on the render pipe), or glDispatchCompute (on the compute pipe).  When
processing an operation, we first save the batch state (bytes used for
commands, number of relocations).  Then we upload state.

If we run out of batch space(*), or refer to more objects than we can
fit in the aperture, we roll back to that saved state and flush the
batch, executing everything queued except the most recent operation.

We then re-upload state for that single failed operation in a fresh
batch.  There's no pipeline switch here - when we retry that same
operation, it will dispatch to the same pipe as before.  Notably,
there are no _mesa_update_state calls, so we can never hit resolves.
(We did those at the very beginning, before even beginning to process
the operation at hand!)  It's a pretty tight loop - 1. upload state,
2. check aperture space, 3. flush, go back to step 1.

 This may be far-fetched and, as I said, I don't think we can hit this.

So we agree :)

 But I'd rather not break the contract that brw_upload_pipeline_state()
 doesn't change state in case of failure - I know I'd hate to debug
 that.
 We can just introduce a brw_clear_pipeline_dirty_bits() function
 that will be called on successful state upload and merge the state
 bits to the other pipeline and then clear the selected pipeline dirty
 flags as well as brw-state.dirty.  And we need to use a local copy of
 brw-state.dirty in brw_upload_pipeline_state() for merging in the
 pipeline dirty flags so we don't modify brw-state.dirty on error.
 
 [1] I don't see how we'd get into that, but with fast clear resolve
 using meta we end up running the render pipeline at unexpected places.

But not here - and frankly, doing resolves in the middle of state upload
would be crazy.  Resolves inherently need to set render state that the
actual draw will want to set, too.  You have to serialize them.

Which is what we do today - we handle resolves when Mesa first asks us
to draw or dispatch a compute workload, before we ever start processing
the operation itself.  The resolve will space-check-and-retry...but it
will get done.  Only then do we try the next thing, which will
space-check-and-retry independently.

Thanks for thinking about this carefully - it's easy to mess up, and we
really need to get it right.  I think we have, though.

  ---
   src/mesa/drivers/dri/i965/brw_context.h  |  1 +
   src/mesa/drivers/dri/i965/brw_state_upload.c | 42 
  ++--
   2 files changed, 35 insertions(+), 8 deletions(-)
 
  diff --git a/src/mesa/drivers/dri/i965/brw_context.h 
  b/src/mesa/drivers/dri/i965/brw_context.h
  index 91b4054..e693f50 100644
  --- a/src/mesa/drivers/dri/i965/brw_context.h
  +++ b/src/mesa/drivers/dri/i965/brw_context.h
  @@ -1101,6 +1101,7 @@ struct brw_context
  GLuint NewGLState;
  struct {
 struct brw_state_flags dirty;
  +  struct brw_state_flags pipelines[BRW_NUM_PIPELINES];
  } state;
 
  struct brw_cache cache;
  diff --git a/src/mesa/drivers/dri/i965/brw_state_upload.c 
  b/src/mesa/drivers/dri/i965/brw_state_upload.c
  index 4f21002..55a9050 100644
  --- 

[Mesa-dev] [PATCH v2] i965: Handle scratch accesses where reladdr also points to scratch space

2015-03-17 Thread Iago Toral Quiroga
This is a problem when we have IR like this:

(array_ref (var_ref temps) (swiz x (expression ivec4 bitcast_f2i
   (swiz  (array_ref (var_ref temps) (constant int (2)) ) )) )) ) )

where we are indexing an array with the result of an expression that
accesses the same array.

In this scenario, temps will be moved to scratch space and we will need
to add scratch reads/writes for all accesses to temps, however, the
current implementation does not consider the case where a reladdr pointer
(obtained by indexing into temps trough a expression) points to a register
that is also stored in scratch space (as in this case, where the expression
used to index temps access temps[2]), and thus, requires a scratch read
before it is accessed.

v2 (Francisco Jerez):
 - Handle also recursive reladdr addressing.
 - Do not memcpy dst_reg into src_reg when rewriting reladdr.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89508
---
A couple of notes for reviewers:

1. The implementation resolves recursive reladdr scratch accesses in one go
so we don't have to do multiple passes over the complete set of instructions.
This is more performant than doing something similar to what we do in
move_uniform_array_access_to_pull_constants.

2. Once we start handling recursive reladdr we are rewriting reladdr
accesses to point to the destination registers of the scratch loads. This
means that alloc.count increases and we can have a reladdr pointing to a
register location beyond  the original alloc.count, so we should take this
into account when indexing scratch_loc[] with reladdr registers.

I tested this for recursive reladdr accesses in both src and dst, including
indexing different arrays: a[b[a[b[0, etc and seems to work well.

No piglit regressions on IvyBridge.

As a side note, I also noticed that opt_array_splitting.cpp does not handle
these situations well and hits an assertion in some cases where it wrongly
assumes that an array only has constant indexing. This problem is happening
in master and is unrelated to this patch so I'll upload a bug report for that.
I mention this just in case someone wants to test the patch and hits the
problem. In my tests I had to disable that optimization pass for the more
complex cases.

 src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp | 138 +
 1 file changed, 117 insertions(+), 21 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp 
b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
index 5bf9e1b..c776c7a 100644
--- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
+++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
@@ -3391,7 +3391,8 @@ vec4_visitor::emit_scratch_write(bblock_t *block, 
vec4_instruction *inst,
 void
 vec4_visitor::move_grf_array_access_to_scratch()
 {
-   int scratch_loc[this-alloc.count];
+   int alloc_count = alloc.count;
+   int scratch_loc[alloc_count];
memset(scratch_loc, -1, sizeof(scratch_loc));
 
/* First, calculate the set of virtual GRFs that need to be punted
@@ -3400,19 +3401,29 @@ vec4_visitor::move_grf_array_access_to_scratch()
 */
foreach_block_and_inst(block, vec4_instruction, inst, cfg) {
   if (inst-dst.file == GRF  inst-dst.reladdr 
- scratch_loc[inst-dst.reg] == -1) {
-scratch_loc[inst-dst.reg] = c-last_scratch;
-c-last_scratch += this-alloc.sizes[inst-dst.reg];
+  scratch_loc[inst-dst.reg] == -1) {
+ scratch_loc[inst-dst.reg] = c-last_scratch;
+ c-last_scratch += this-alloc.sizes[inst-dst.reg];
+
+ src_reg *iter = inst-dst.reladdr;
+ while (iter-reladdr) {
+if (iter-file == GRF  scratch_loc[iter-reg] == -1) {
+   scratch_loc[iter-reg] = c-last_scratch;
+   c-last_scratch += this-alloc.sizes[iter-reg];
+}
+iter = iter-reladdr;
+ }
   }
 
   for (int i = 0 ; i  3; i++) {
-src_reg *src = inst-src[i];
-
-if (src-file == GRF  src-reladdr 
-scratch_loc[src-reg] == -1) {
-   scratch_loc[src-reg] = c-last_scratch;
-   c-last_scratch += this-alloc.sizes[src-reg];
-}
+ src_reg *iter = inst-src[i];
+ while (iter-reladdr) {
+if (iter-file == GRF  scratch_loc[iter-reg] == -1) {
+   scratch_loc[iter-reg] = c-last_scratch;
+   c-last_scratch += this-alloc.sizes[iter-reg];
+}
+iter = iter-reladdr;
+ }
   }
}
 
@@ -3422,27 +3433,112 @@ vec4_visitor::move_grf_array_access_to_scratch()
 * we're processing.
 */
foreach_block_and_inst_safe(block, vec4_instruction, inst, cfg) {
+  bool nested_reladdr;
+
   /* Set up the annotation tracking for new generated instructions. */
   base_ir = inst-ir;
   current_annotation = inst-annotation;
 
+  /* First handle scratch access on the dst */
   if (inst-dst.file == GRF  scratch_loc[inst-dst.reg] != -1) {
-emit_scratch_write(block, inst, 

Re: [Mesa-dev] [PATCH ] i965/aa: fixing anti-aliasing bug for thinnest width lines.

2015-03-17 Thread Predut, Marius
 -Original Message-
 From: ibmir...@gmail.com [mailto:ibmir...@gmail.com] On Behalf Of Ilia Mirkin
 Sent: Tuesday, March 17, 2015 6:48 PM
 To: Predut, Marius
 Cc: mesa-dev@lists.freedesktop.org
 Subject: Re: [Mesa-dev] [PATCH ] i965/aa: fixing anti-aliasing bug for
 thinnest width lines.
 
 Can you provide the output of
 
 git var -l
marius@marius-pc:~/mesa/src$ git var -l
user.email=Marius Predut marius.pre...@intel.com
user.email=marius.pre...@intel.com
user.name=Marius Predut
color.ui=auto
sendemail.smtpserver=smtp.intel.com
sendemail.signedoffbycc=no
sendemail.from=marius.pre...@intel.com
sendemail.to=marius.pre...@intel.com
sendemail.chainreplyto=false
sendemail.review_mesa.email=Marius Predut marius.pre...@intel.com
sendemail.review_mesa.name=Marius Predut
sendemail.review_mesa.smtpserver=smtp.intel.com
sendemail.review_mesa.signedoffbycc=no
sendemail.review_mesa.to=mesa-dev@lists.freedesktop.org
sendemail.review_mesa.chainreplyto=false
core.repositoryformatversion=0
core.filemode=true
core.bare=false
core.logallrefupdates=true
remote.origin.fetch=+refs/heads/*:refs/remotes/origin/*
remote.origin.url=git://anongit.freedesktop.org/git/mesa/mesa
branch.master.remote=origin
branch.master.merge=refs/heads/master
GIT_COMMITTER_IDENT=Marius Predut marius.pre...@intel.com 1426618655 +0200
GIT_AUTHOR_IDENT=Marius Predut marius.pre...@intel.com 1426618655 +0200
GIT_EDITOR=editor
GIT_PAGER=pager
 
 And the headers of the patch file you're sending (or git show
 --format=raw for the commit in question if you're not using patch
 files as intermediates)
From b84cf899ddf3b8b93b251ffcf9a082cbfe372f18 Mon Sep 17 00:00:00 2001
From: Marius Predut marius.pre...@intel.com
Date: Tue, 17 Mar 2015 19:33:21 +0200
Subject: [Mesa-dev][PATCH v1] i965/aa: fixing anti-aliasing bug for thinnest
 width lines - GEN6
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

On SNB and IVB hw, for 1 pixel line thickness or less,
 
 I suspect that GIT_AUTHOR_IDENT will differ from whatever's in the
 From: header of the patch being sent.
 

I fix this trouble by using  --from Marius Predut marius.pre...@intel.com
And without space between name and the character (if not it will not work 
:-))

 
 On Tue, Mar 17, 2015 at 12:36 PM, Predut, Marius
 marius.pre...@intel.com wrote:
  -Original Message-
  From: mesa-dev [mailto:mesa-dev-boun...@lists.freedesktop.org] On Behalf Of
 Ilia Mirkin
  Sent: Wednesday, March 11, 2015 11:09 PM
  To: Predut, Marius
  Cc: mesa-dev@lists.freedesktop.org
  Subject: Re: [Mesa-dev] [PATCH ] i965/aa: fixing anti-aliasing bug for
 thinnest width lines.
 
  On Wed, Mar 11, 2015 at 5:57 PM,  marius.pre...@intel.com wrote:
  From: Marius Predut marius.pre...@intel.com
 
  Set your email from name correctly in git and then you won't have this line
 in your git send-email results.
 
  I have this config :
  email = marius.pre...@intel.com
  name = Marius Predut
 
  but still the first line appear
 
 
  On SNB and IVB hw, for 1 pixel line thickness or less, the general
  anti-aliasing algorithm give up - garbage line is generated.
  Setting a Line Width of 0.0 specifies the rasterization of the
  “thinnest” (one-pixel-wide), non-antialiased lines.
  Lines rendered with zero Line Width are rasterized using Grid
  Intersection Quantization rules as specified by bspec section 6.3.12.1
  Zero-Width (Cosmetic) Line Rasterization.
 
  Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82668
 
  This seems like the wrong bug reference...
 
  Signed-off-by: Marius Predut marius.pre...@intel.com
  ---
   src/mesa/drivers/dri/i965/gen6_sf_state.c |   12 +---
   1 file changed, 9 insertions(+), 3 deletions(-)
 
  diff --git a/src/mesa/drivers/dri/i965/gen6_sf_state.c
  b/src/mesa/drivers/dri/i965/gen6_sf_state.c
  index f9d8d27..1bed444 100644
  --- a/src/mesa/drivers/dri/i965/gen6_sf_state.c
  +++ b/src/mesa/drivers/dri/i965/gen6_sf_state.c
  @@ -367,9 +367,15 @@ upload_sf_state(struct brw_context *brw)
 float line_width =
roundf(CLAMP(ctx-Line.Width, 0.0, ctx-Const.MaxLineWidth));
 uint32_t line_width_u3_7 = U_FIXED(line_width, 7);
  -  /* TODO: line width of 0 is not allowed when MSAA enabled */
  -  if (line_width_u3_7 == 0)
  - line_width_u3_7 = 1;
  +
  +  if (!(multisampled_fbo  ctx-Multisample.Enabled)) {
  +if (ctx-Line.SmoothFlag  ctx-Line.Width =1)
  +  line_width_u3_7 = 0;
  +  } else {
  +if (line_width_u3_7 == 0)
  +line_width_u3_7 = 1;
  +  }
  +
 dw3 |= line_width_u3_7  GEN6_SF_LINE_WIDTH_SHIFT;
  }
  if (ctx-Line.SmoothFlag) {
  --
  1.7.9.5
 
  ___
  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
  

Re: [Mesa-dev] Spec bug? Transform feedback after tessellation control shader

2015-03-17 Thread Ian Romanick
On 03/17/2015 11:33 AM, Marek Olšák wrote:
 Hi,
 
 The GL 4.5 Core spec is inconsistent. The 11.1.2.1 Output Variables
 section says that transform feedback is allowed after a tessellation
 control shader if a tessellation evaluation shader isn't present:
 
 Each program object can specify a set of output variables from one
 shader to be recorded in transform feedback mode (see section 13.2).
 The variables that can be recorded are those emitted by the first
 active shader, in order, from the following list:
 * geometry shader
 * tessellation evaluation shader
 * tessellation control shader
 * vertex shader
 
 Even glTransformFeedbackVaryings allows it:
 
 If the set of output variables to record in transform feedback mode
 is specified by TransformFeedbackVaryings, a program will fail to link
 if:
 * the count specified by TransformFeedbackVaryings is non-zero, but the
 program object has no vertex, tessellation control, tessellation evaluation, 
 or
 geometry shader;
 
 However, the 13.2 Transform Feedback section doesn't mention any
 tessellation control shader:
 
 The data captured in transform feedback mode depends on the active
 programs on each of the shader stages. If a program is active for the
 geometry shader stage, transform feedback captures the vertices of
 each primitive emitted by the geometry shader. Otherwise, if a program
 is active for the tessellation evaluation shader stage, transform
 feedback captures each primitive produced by the tessellation
 primitive generator, whose vertices are processed by the tessellation
 evaluation shader. Otherwise, transform feedback captures each
 primitive processed by the vertex shader.
 
 Also, glBeginTransformFeedback doesn't allow GL_PATCHES, which is the
 only output primitive type of the tessellation control shader:
 
 primitiveMode must be TRIANGLES, LINES, or POINTS, and specifies the
 output type of primitives that will be recorded into the buffer
 objects bound for transform feedback

And to make matters worse, the ARB_tessellation_shader spec has the
following issue.  The resolution of this issue seems to contradict the
rest of the extension spec.

(8) Should coupling of tessellation control and evaluation shaders be
required?

  RESOLVED:  No.  A tessellation control shader without an evaluation
  shader might be used in conjunction with transform feedback to generate
  regular transformed patches.  Also, if the set of patches provided by
  the application is already in a form usable by the tessellator, the
  tessellation control shader may be bypassed.  In this use case, the
  application would be required to provide default tessellation levels via
  the PatchParameterfv API, since no shader would be available to compute
  them.

  It may be useful to have a patch produced by a tessellation control
  shader to be fed directly to a geometry shader that performs some
  operation on full patches, rather than individual triangles of a
  tessellated patch.  However, such capability is not provided in this
  extension.

This definitely smells like a spec bug.  You should submit a bug in the
internal Khronos bugzilla and CC my @intel address.

 The question is: Is transform feedback really allowed after the
 tessellation control shader? I hope not, because our hardware can't do
 it.
 
 Thanks,
 
 Marek
 
 PS: The spec mentions tesellation shader twice. There is no
 tesellation shader in OpenGL even if you ignore the bad spelling.
 ___
 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


Re: [Mesa-dev] [PATCH v2 3/4] i965/state: Create separate dirty state bits for each pipeline

2015-03-17 Thread Kenneth Graunke
On Tuesday, March 17, 2015 08:20:23 AM Kristian Høgsberg wrote:
 On Tue, Mar 17, 2015 at 2:22 AM, Kenneth Graunke kenn...@whitecape.org 
 wrote:
  On Monday, March 16, 2015 10:21:31 PM Kristian Høgsberg wrote:
  On Wed, Mar 11, 2015 at 11:53 AM, Jordan Justen
  jordan.l.jus...@intel.com wrote:
[snip]
   @@ -691,8 +702,23 @@ void brw_upload_render_state(struct brw_context 
   *brw)
fprintf(stderr, \n);
  }
   }
   +
   +   /* Save all dirty state into the other pipelines */
   +   for (int i = 0; i  BRW_NUM_PIPELINES; i++) {
   +  if (i != pipeline) {
   + brw-state.pipelines[i].mesa |= state-mesa;
   + brw-state.pipelines[i].brw |= state-brw;
   +  }
   +   }
 
  When we merge this in at this point, state-mesa/brw includes dirty
  bits from the active pipelines pipeline flags. For example, suppose we
  do a bunch of render pipeline stuff, which queues up a lot of dirty
  flags for the compute pipeline. Then when we go to emit state for the
  compute pipeline we first merge all these dirty flags into
  brw-state.dirty, then merge that into the render pipeline flags. We
  only need to merge the new dirty bits, that is, the initial value of
  brw-state.dirty into the render pipeline flags. If we don't modify
  brw-state.dirty in this function and merge the flags in clear as
  described above, this problem is easy to fix.
 
  Kristian

OK, I see what you mean now - very good catch.

1. We do a render operation.  Any new dirty bits get saved in
   brw-state.pipelines[COMPUTE], so they'll happen when we
   do the next compute workload.

   This completes successfully (ignore retries).

2. We do a compute operation.

   brw-state.pipelines[COMPUTE] gets merged into brw-state.dirty.
   This includes bits that happened during #1 (and were missed on
   the compute pipe).

   This all works fine.  At the end, we save brw-state.dirty into
   brw-state.pipelines[RENDER].

3. We do another render operation.

   brw-state.pipelines[RENDER] gets merged into brw-state.dirty.
   We now have bits from #1 again, which are not necessary.

So yeah.  That's bad.  Sorry, I thought this was to handle the
hypothetical case of pipe-switch-during-retries.  But it's just
switching pipelines at all!

If I understand your suggestion correctly, you mean that
a) We should eliminate brw-state.dirty.
b) brw_upload_pipeline_state() should do:

   struct brw_state_flags *pipeline_state =
  brw-state.pipelines[pipeline];

   /* Create a local copy of the dirty state */
   struct brw_state_flags dirty;
   dirty.mesa = brw-NewGLState | pipeline_state-mesa;
   dirty.brw = ctx-NewDriverState | pipeline_state-brw;

   Use this for uploads.

c) The existing brw_clear_dirty_bits() should do:

   /* Save the dirty flags into the other pipelines */
   for (int i = 0; i  BRW_NUM_PIPELINES; i++) {
  if (i != pipeline) {
 brw-state.pipelines[i].mesa |= brw-NewGLState;
 brw-state.pipelines[i].brw |= ctx-NewDriverState;
  }
   }

   /* We've handled any bits passed to us by other pipeline uploads */
   brw-state.pipelines[pipe] = 0;

   /* We've handled the flags passed to us by Mesa */
   brw-NewGLState = 0;
   ctx-NewDriverState = 0;

Does that sound like a correct interpretation?  This does seem to solve
the problem, and I think it's a bit cleaner too.

Re-evaluating my example to verify that this approach works...

1. We do a render operation.

   The local dirty flags are bits from Mesa | saved bits.
   Nothing's saved yet, so just bits from Mesa

   When done, we save bits from Mesa to pipelines[COMPUTE].

2. We do a compute operation.

   The local dirty flags are new bits from Mesa | saved bits
   from op #1.  We use those for upload.

   When done, brw_clear_dirty_bits() saves only new bits from
   Mesa to pipelines[COMPUTE].  Notably, the bits from op #1
   are /not/ saved.

3. We do a render operation.

   We get any new bits from Mesa, plus the bits during #2.
   No bits from #1.

So...you're right - there's definitely a problem, and your suggestion
totally fixes it.  I just had to work through an example to see what
you meant.  Thanks again!

Jordan, does this make sense to you?


signature.asc
Description: This is a digitally signed message part.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] Statically linking libstdc++ and libgcc

2015-03-17 Thread Emil Velikov
On 16/03/15 23:44, Ian Romanick wrote:
 On 03/13/2015 02:32 PM, Emil Velikov wrote:
  * Allow people to static link against libgcc/libstdc++.

 Imho this should be option, disabled by default provided at configure
 time. This way builders/distributions can op-in if they choose to do so.
 
 I'm very strongly opposed to this.
 
 We already have too many build ABI options.  Each and every one of them
 has caused some level of support pain (TLS vs. non-TLS, shared glapi vs
 non-shared, etc.).  I have absolutely no interest in seeing another knob
 added.
 
Perhaps I'm missing something but how is this yet another ABI ? I'm
working on nuking the static glapi, although got distracted by other
things *cough*this thread*cough*.

As one of the few people working on the build system (props for Matt for
helping imensely), I am not at _all_ happy with us having so many
configure switches.

Although, as you've mentioned before (in a slightly different wording)
this is an evil solution, but it's a _necessary_ evil. Clearly
people are unhappy if we make this the default for obvious reasons, plus
it has a change of breaking someone's workflow. So another configure
which it is.

If your concern is about supporting such setup, that decision is for you
guys to make. Plus the check for static libstdc++/gcc_s is quite trivial
so it won't increase the time to triage bugs.

Cheers,
Emil

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 3.5/5] i965/fs: Add unit tests for Handle CMP.nz ... 0 and AND.nz ... 1 similarly in cmod propagation

2015-03-17 Thread Matt Turner
On Tue, Mar 17, 2015 at 10:30 AM, Ian Romanick i...@freedesktop.org wrote:
 From: Ian Romanick ian.d.roman...@intel.com

 This change should get squashed with the afore mentioned change.  Tests
 suggested by Matt.

 Signed-off-by: Ian Romanick ian.d.roman...@intel.com
 Cc: Matt Turner matts...@gmail.com
 ---
  .../drivers/dri/i965/test_fs_cmod_propagation.cpp  | 105 
 +
  1 files changed, 105 insertions(+), 0 deletions(-)

 diff --git a/src/mesa/drivers/dri/i965/test_fs_cmod_propagation.cpp 
 b/src/mesa/drivers/dri/i965/test_fs_cmod_propagation.cpp
 index cb92abf..8c6ff09 100644
 --- a/src/mesa/drivers/dri/i965/test_fs_cmod_propagation.cpp
 +++ b/src/mesa/drivers/dri/i965/test_fs_cmod_propagation.cpp
 @@ -449,3 +449,108 @@ TEST_F(cmod_propagation_test, 
 different_types_cmod_with_zero)
 EXPECT_EQ(BRW_OPCODE_CMP, instruction(block0, 1)-opcode);
 EXPECT_EQ(BRW_CONDITIONAL_GE, instruction(block0, 1)-conditional_mod);
  }
 +
 +TEST_F(cmod_propagation_test, andnz_one)
 +{
 +   fs_reg dest = v-vgrf(glsl_type::int_type);
 +   fs_reg src0 = v-vgrf(glsl_type::float_type);
 +   fs_reg zero(0.0f);
 +   fs_reg one(int(1));
 +
 +   v-emit(BRW_OPCODE_CMP, retype(dest, BRW_REGISTER_TYPE_F), src0, zero)
 +  -conditional_mod = BRW_CONDITIONAL_L;
 +   v-emit(BRW_OPCODE_AND, v-reg_null_f, dest, one)
 +  -conditional_mod = BRW_CONDITIONAL_NZ;
 +
 +   /* = Before =
 +* 0: cmp.l.f0(8) dest:D  src0:F  0F

We won't be emitting compares with different source and destination types since

commit 6b3a301f611c9aabc090522951eda589e8302562
Author: Matt Turner matts...@gmail.com
Date:   Wed Jan 7 11:52:05 2015 -0800

i965: Set CMP's destination type to src0's type.

so let's change this comment (I see that the actual code is already
retype()ing the dest). Same applies to the two below.

 +* 1: and.nz.f0(8)null:D  dest:D  1D
 +*
 +* = After =
 +* 0: cmp.l.f0(8) dest:D  src0:F  0F

and here.

 +*/
 +
 +   v-calculate_cfg();
 +   bblock_t *block0 = v-cfg-blocks[0];
 +
 +   EXPECT_EQ(0, block0-start_ip);
 +   EXPECT_EQ(1, block0-end_ip);
 +
 +   EXPECT_TRUE(cmod_propagation(v));
 +   EXPECT_EQ(0, block0-start_ip);
 +   EXPECT_EQ(0, block0-end_ip);
 +   EXPECT_EQ(BRW_OPCODE_CMP, instruction(block0, 0)-opcode);
 +   EXPECT_EQ(BRW_CONDITIONAL_L, instruction(block0, 0)-conditional_mod);
 +   EXPECT_TRUE(retype(dest, BRW_REGISTER_TYPE_F)
 +   .equals(instruction(block0, 0)-dst));
 +}
 +
 +TEST_F(cmod_propagation_test, andnz_non_one)
 +{
 +   fs_reg dest = v-vgrf(glsl_type::int_type);
 +   fs_reg src0 = v-vgrf(glsl_type::float_type);
 +   fs_reg zero(0.0f);
 +   fs_reg nonone(int(38));

Can just pass 38 here if you want.

 +
 +   v-emit(BRW_OPCODE_CMP, retype(dest, BRW_REGISTER_TYPE_F), src0, zero)
 +  -conditional_mod = BRW_CONDITIONAL_L;
 +   v-emit(BRW_OPCODE_AND, v-reg_null_f, dest, nonone)
 +  -conditional_mod = BRW_CONDITIONAL_NZ;
 +
 +   /* = Before =
 +* 0: cmp.l.f0(8) dest:D  src0:F  0F
 +* 1: and.nz.f0(8)null:D  dest:D  1D

s/1D/38D/

with the comments fixed,

Reviewed-by: Matt Turner matts...@gmail.com

Thanks Ian!
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH] i965/aa: fixing anti-aliasing bug for thinnest width lines - GEN6

2015-03-17 Thread Marius Predut
On SNB and IVB hw, for 1 pixel line thickness or less, the general 
anti-aliasing algorithm give up - garbage line is generated.
Setting a Line Width of 0.0 specifies the rasterization of the “thinnest” 
(one-pixel-wide), non-antialiased lines.
Lines rendered with zero Line Width are rasterized using Grid Intersection 
Quantization rules as specified by bspec section 6.3.12.1 Zero-Width (Cosmetic) 
Line Rasterization.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=28832
Signed-off-by: Marius Predut marius.pre...@intel.com
---
 src/mesa/drivers/dri/i965/gen6_sf_state.c |   12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/gen6_sf_state.c 
b/src/mesa/drivers/dri/i965/gen6_sf_state.c
index f9d8d27..1bed444 100644
--- a/src/mesa/drivers/dri/i965/gen6_sf_state.c
+++ b/src/mesa/drivers/dri/i965/gen6_sf_state.c
@@ -367,9 +367,15 @@ upload_sf_state(struct brw_context *brw)
   float line_width =
  roundf(CLAMP(ctx-Line.Width, 0.0, ctx-Const.MaxLineWidth));
   uint32_t line_width_u3_7 = U_FIXED(line_width, 7);
-  /* TODO: line width of 0 is not allowed when MSAA enabled */
-  if (line_width_u3_7 == 0)
- line_width_u3_7 = 1;
+
+  if (!(multisampled_fbo  ctx-Multisample.Enabled)) {
+if (ctx-Line.SmoothFlag  ctx-Line.Width =1)
+  line_width_u3_7 = 0;
+  } else {
+if (line_width_u3_7 == 0)
+line_width_u3_7 = 1;
+  }
+
   dw3 |= line_width_u3_7  GEN6_SF_LINE_WIDTH_SHIFT;
}
if (ctx-Line.SmoothFlag) {
-- 
1.7.9.5

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] Fwd: [PATCH 12/16] main: Added entry point for glCreateProgramPipelines

2015-03-17 Thread Laura Ekstrand
-- Forwarded message --
From: Laura Ekstrand la...@jlekstrand.net
Date: Thu, Feb 26, 2015 at 3:43 PM
Subject: Re: [Mesa-dev] [PATCH 12/16] main: Added entry point for
glCreateProgramPipelines
To: Martin Peres martin.pe...@linux.intel.com




On Mon, Feb 16, 2015 at 6:14 AM, Martin Peres martin.pe...@linux.intel.com
wrote:

 Signed-off-by: Martin Peres martin.pe...@linux.intel.com
 ---
  src/mapi/glapi/gen/ARB_direct_state_access.xml |  7 ++
  src/mesa/main/pipelineobj.c| 35
 +-
  src/mesa/main/pipelineobj.h|  3 +++
  src/mesa/main/tests/dispatch_sanity.cpp|  1 +
  4 files changed, 40 insertions(+), 6 deletions(-)

 diff --git a/src/mapi/glapi/gen/ARB_direct_state_access.xml
 b/src/mapi/glapi/gen/ARB_direct_state_access.xml
 index 99d2422..2102e82 100644
 --- a/src/mapi/glapi/gen/ARB_direct_state_access.xml
 +++ b/src/mapi/glapi/gen/ARB_direct_state_access.xml
 @@ -308,6 +308,13 @@
param name=params type=GLint * /
 /function

 +   !-- Program Pipeline object functions --
 +
 +   function name=CreateProgramPipelines offset=assign
 +  param name=n type=GLsizei /
 +  param name=pipelines type=GLuint * /
 +   /function
 +
 !-- Query object functions --

 function name=CreateQueries offset=assign
 diff --git a/src/mesa/main/pipelineobj.c b/src/mesa/main/pipelineobj.c
 index b713d95..96bf086 100644
 --- a/src/mesa/main/pipelineobj.c
 +++ b/src/mesa/main/pipelineobj.c
 @@ -498,16 +498,18 @@ _mesa_DeleteProgramPipelines(GLsizei n, const GLuint
 *pipelines)
   * \param n   Number of IDs to generate.
   * \param pipelines  pipeline of \c n locations to store the IDs.
   */
 -void GLAPIENTRY
 -_mesa_GenProgramPipelines(GLsizei n, GLuint *pipelines)
 +static void
 +create_program_pipelines(struct gl_context *ctx, GLsizei n, GLuint
 *pipelines,
 + bool dsa)
  {
 -   GET_CURRENT_CONTEXT(ctx);
 -
 +   const char *func;
 GLuint first;
 GLint i;

 +   func = dsa ? glCreateProgramPipelines : glGenProgramPipelines;
 +
 if (n  0) {
 -  _mesa_error(ctx, GL_INVALID_VALUE, glGenProgramPipelines(n0));
 +  _mesa_error(ctx, GL_INVALID_VALUE, %s (n0), func);

Minor nit: There's an extra space here ^

return;
 }

 @@ -523,16 +525,37 @@ _mesa_GenProgramPipelines(GLsizei n, GLuint
 *pipelines)

obj = _mesa_new_pipeline_object(ctx, name);
if (!obj) {
 - _mesa_error(ctx, GL_OUT_OF_MEMORY, glGenProgramPipelines);
 + _mesa_error(ctx, GL_OUT_OF_MEMORY, %s, func);
   return;
}

 +  if (dsa) {
 + /* make dsa-allocated objects behave like program objects */
 + obj-EverBound = GL_TRUE;
 +  }
 +
save_pipeline_object(ctx, obj);
pipelines[i] = first + i;
 }

  }

 +void GLAPIENTRY
 +_mesa_GenProgramPipelines(GLsizei n, GLuint *pipelines)
 +{
 +   GET_CURRENT_CONTEXT(ctx);
 +
 +   create_program_pipelines(ctx, n, pipelines, false);
 +}
 +
 +void GLAPIENTRY
 +_mesa_CreateProgramPipelines(GLsizei n, GLuint *pipelines)
 +{
 +   GET_CURRENT_CONTEXT(ctx);
 +
 +   create_program_pipelines(ctx, n, pipelines, true);
 +}
 +
  /**
   * Determine if ID is the name of an pipeline object.
   *
 diff --git a/src/mesa/main/pipelineobj.h b/src/mesa/main/pipelineobj.h
 index 7285a78..b57bcb9 100644
 --- a/src/mesa/main/pipelineobj.h
 +++ b/src/mesa/main/pipelineobj.h
 @@ -82,6 +82,9 @@ _mesa_DeleteProgramPipelines(GLsizei n, const GLuint
 *pipelines);
  extern void GLAPIENTRY
  _mesa_GenProgramPipelines(GLsizei n, GLuint *pipelines);

 +void GLAPIENTRY
 +_mesa_CreateProgramPipelines(GLsizei n, GLuint *pipelines);
 +
  extern GLboolean GLAPIENTRY
  _mesa_IsProgramPipeline(GLuint pipeline);

 diff --git a/src/mesa/main/tests/dispatch_sanity.cpp
 b/src/mesa/main/tests/dispatch_sanity.cpp
 index b65080e..cc2b267 100644
 --- a/src/mesa/main/tests/dispatch_sanity.cpp
 +++ b/src/mesa/main/tests/dispatch_sanity.cpp
 @@ -993,6 +993,7 @@ const struct function gl_core_functions_possible[] = {
 { glTextureStorage2DMultisample, 45, -1 },
 { glTextureStorage3DMultisample, 45, -1 },
 { glTextureBuffer, 45, -1 },
 +   { glCreateProgramPipelines, 45, -1 },
 { glCreateQueries, 45, -1 },
 { glGetQueryBufferObjectiv, 45, -1 },
 { glGetQueryBufferObjectuiv, 45, -1 },
 --
 2.3.0

 ___
 mesa-dev mailing list
 mesa-dev@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/mesa-dev


With that fixed,

Reviewed-by: Laura Ekstrand la...@jlekstrand.net
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] Spec bug? Transform feedback after tessellation control shader

2015-03-17 Thread Ilia Mirkin
AFAIK you can't have TCS without TES. But you can have TES without TCS.

On Tue, Mar 17, 2015 at 2:33 PM, Marek Olšák mar...@gmail.com wrote:
 Hi,

 The GL 4.5 Core spec is inconsistent. The 11.1.2.1 Output Variables
 section says that transform feedback is allowed after a tessellation
 control shader if a tessellation evaluation shader isn't present:

 Each program object can specify a set of output variables from one
 shader to be recorded in transform feedback mode (see section 13.2).
 The variables that can be recorded are those emitted by the first
 active shader, in order, from the following list:
 * geometry shader
 * tessellation evaluation shader
 * tessellation control shader
 * vertex shader

 Even glTransformFeedbackVaryings allows it:

 If the set of output variables to record in transform feedback mode
 is specified by TransformFeedbackVaryings, a program will fail to link
 if:
 * the count specified by TransformFeedbackVaryings is non-zero, but the
 program object has no vertex, tessellation control, tessellation evaluation, 
 or
 geometry shader;

 However, the 13.2 Transform Feedback section doesn't mention any
 tessellation control shader:

 The data captured in transform feedback mode depends on the active
 programs on each of the shader stages. If a program is active for the
 geometry shader stage, transform feedback captures the vertices of
 each primitive emitted by the geometry shader. Otherwise, if a program
 is active for the tessellation evaluation shader stage, transform
 feedback captures each primitive produced by the tessellation
 primitive generator, whose vertices are processed by the tessellation
 evaluation shader. Otherwise, transform feedback captures each
 primitive processed by the vertex shader.

 Also, glBeginTransformFeedback doesn't allow GL_PATCHES, which is the
 only output primitive type of the tessellation control shader:

 primitiveMode must be TRIANGLES, LINES, or POINTS, and specifies the
 output type of primitives that will be recorded into the buffer
 objects bound for transform feedback

 The question is: Is transform feedback really allowed after the
 tessellation control shader? I hope not, because our hardware can't do
 it.

 Thanks,

 Marek

 PS: The spec mentions tesellation shader twice. There is no
 tesellation shader in OpenGL even if you ignore the bad spelling.
 ___
 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


Re: [Mesa-dev] [PATCH] i965/aa: fixing anti-aliasing bug for thinnest width lines - GEN6

2015-03-17 Thread Ian Romanick
On 03/17/2015 11:29 AM, Marius Predut wrote:
 On SNB and IVB hw, for 1 pixel line thickness or less, the general 
 anti-aliasing algorithm give up - garbage line is generated.
 Setting a Line Width of 0.0 specifies the rasterization of the “thinnest” 
 (one-pixel-wide), non-antialiased lines.
 Lines rendered with zero Line Width are rasterized using Grid Intersection 
 Quantization rules as specified by bspec section 6.3.12.1 Zero-Width 
 (Cosmetic) Line Rasterization.

You'll need to re-word wrap the commit message.  You'll get the commit
message right one of these days. :)

Also... when you send out new versions of a patch, please change the
patch subject to be something like [PATCH v3]   This makes it
easier for people to know which is the latest version to review.

You should also add notes to the commit message that explain what
changed from version to version.  For example, this commit message
should have something like:

v3: Fix = used instead of == in an if-statement.  Noticed by Daniel Stone.

This is helps people know that their review comments have been applied.
 It is also important to do this when the review changes are applied and
the patch committed without re-sending to the list.  Maintaining history
like this in commit messages helps us understand code in the future.

 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=28832

There are a number of bugs that have been closed as duplicates of this
bug.  Two of these, bug #27007 and bug #60797, have test cases.  Does
this fix also fix those?

I also have a more general question:  How are you testing this?  Daniel
noticed a bug in an earlier version of the patch that piglit should have
caught.  If you're doing a full piglit run and that didn't catch the
previous assignment-instead-of-comparison bug, it would be helpful if
you could craft a test that would detect that bug.  That may be
difficult, so don't spend a huge amount of time on it.

 Signed-off-by: Marius Predut marius.pre...@intel.com
 ---
  src/mesa/drivers/dri/i965/gen6_sf_state.c |   12 +---
  1 file changed, 9 insertions(+), 3 deletions(-)
 
 diff --git a/src/mesa/drivers/dri/i965/gen6_sf_state.c 
 b/src/mesa/drivers/dri/i965/gen6_sf_state.c
 index f9d8d27..1bed444 100644
 --- a/src/mesa/drivers/dri/i965/gen6_sf_state.c
 +++ b/src/mesa/drivers/dri/i965/gen6_sf_state.c
 @@ -367,9 +367,15 @@ upload_sf_state(struct brw_context *brw)
float line_width =
   roundf(CLAMP(ctx-Line.Width, 0.0, ctx-Const.MaxLineWidth));
uint32_t line_width_u3_7 = U_FIXED(line_width, 7);
 -  /* TODO: line width of 0 is not allowed when MSAA enabled */
 -  if (line_width_u3_7 == 0)
 - line_width_u3_7 = 1;
 +
 +  if (!(multisampled_fbo  ctx-Multisample.Enabled)) {

In Mesa there are often Enabled and _Enabled fields.  The Enabled field
is the setting made by the application via the OpenGL API.  The _Enabled
field means that feature in question is actually enabled, and this may
depend on other state.

In this case, the application may enable multisampling, but
multisampling may not occur of there is not a multisample buffer.  This
means gl_multisample_attrib::Enabled would be set but
gl_multisample_attrib::_Enabled would not.  Instead of open-coding that
check, just check gl_multisample_attrib::_Enabled directly:

  if (!ctx-Multisample._Enabled) {

I actually think it's more clear if you leave the original comment and
implement this as:

  /* Line width of 0 is not allowed when MSAA enabled */
  if (ctx-Multisample._Enabled) {
 if (line_width_u3_7 == 0)
 line_width_u3_7 = 1;
  } else if (ctx-Line.SmoothFlag  ctx-Line.Width = 1.0) {
 /* For lines less than 1 pixel thick, the general
  * anti-aliasing algorithm gives up, and a garbage line is
  * generated.  Setting a Line Width of 0.0 specifies the
  * rasterization of the thinnest (one-pixel-wide),
  * non-antialiased lines.
  *
  * Lines rendered with zero Line Width are rasterized using
  * Grid Intersection Quantization rules as specified by
  * bspec section 6.3.12.1 Zero-Width (Cosmetic) Line
  * Rasterization.
  */
 line_width_u3_7 = 0;
  }

I reworded the comment a bit (from the commit message), and I changed
the Line.Width comparison to compare with 1.0.

One final question.  Does it produce better results to use 0 or 1.0?  It
sounds like using 1.0 would still enable line antialiasing, and the
resulting line shouldn't be appreciably thicker.

 +if (ctx-Line.SmoothFlag  ctx-Line.Width =1)
 +  line_width_u3_7 = 0;
 +  } else {
 +if (line_width_u3_7 == 0)
 +line_width_u3_7 = 1;
 +  }
 +
dw3 |= line_width_u3_7  GEN6_SF_LINE_WIDTH_SHIFT;
 }
 if (ctx-Line.SmoothFlag) {

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org

[Mesa-dev] Spec bug? Transform feedback after tessellation control shader

2015-03-17 Thread Marek Olšák
Hi,

The GL 4.5 Core spec is inconsistent. The 11.1.2.1 Output Variables
section says that transform feedback is allowed after a tessellation
control shader if a tessellation evaluation shader isn't present:

Each program object can specify a set of output variables from one
shader to be recorded in transform feedback mode (see section 13.2).
The variables that can be recorded are those emitted by the first
active shader, in order, from the following list:
* geometry shader
* tessellation evaluation shader
* tessellation control shader
* vertex shader

Even glTransformFeedbackVaryings allows it:

If the set of output variables to record in transform feedback mode
is specified by TransformFeedbackVaryings, a program will fail to link
if:
* the count specified by TransformFeedbackVaryings is non-zero, but the
program object has no vertex, tessellation control, tessellation evaluation, or
geometry shader;

However, the 13.2 Transform Feedback section doesn't mention any
tessellation control shader:

The data captured in transform feedback mode depends on the active
programs on each of the shader stages. If a program is active for the
geometry shader stage, transform feedback captures the vertices of
each primitive emitted by the geometry shader. Otherwise, if a program
is active for the tessellation evaluation shader stage, transform
feedback captures each primitive produced by the tessellation
primitive generator, whose vertices are processed by the tessellation
evaluation shader. Otherwise, transform feedback captures each
primitive processed by the vertex shader.

Also, glBeginTransformFeedback doesn't allow GL_PATCHES, which is the
only output primitive type of the tessellation control shader:

primitiveMode must be TRIANGLES, LINES, or POINTS, and specifies the
output type of primitives that will be recorded into the buffer
objects bound for transform feedback

The question is: Is transform feedback really allowed after the
tessellation control shader? I hope not, because our hardware can't do
it.

Thanks,

Marek

PS: The spec mentions tesellation shader twice. There is no
tesellation shader in OpenGL even if you ignore the bad spelling.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 8/9] i965/nir: Do boolean resolves on GEN = 5

2015-03-17 Thread Jason Ekstrand
---
 src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 21 +
 1 file changed, 21 insertions(+)

diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp 
b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
index 91a3f65..1ef4602 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
@@ -25,6 +25,7 @@
 #include glsl/ir_optimization.h
 #include glsl/nir/glsl_to_nir.h
 #include brw_fs.h
+#include brw_nir.h
 
 static void
 nir_optimize(nir_shader *nir)
@@ -149,6 +150,14 @@ fs_visitor::emit_nir_code()
nir_convert_from_ssa(nir);
nir_validate_shader(nir);
 
+   /* This is the last pass we run before we start emitting stuff.  It
+* determines when we need to insert boolean resolves on GEN = 5.  We
+* run it last because it stashes data in instr-pass_flags and we don't
+* want that to be squashed by other NIR passes.
+*/
+   if (brw-gen = 5)
+  brw_nir_analize_boolean_resolves(nir);
+
/* emit the arrays used for inputs and outputs - load/store intrinsics will
 * be converted to reads/writes of these arrays
 */
@@ -1263,6 +1272,18 @@ fs_visitor::nir_emit_alu(nir_alu_instr *instr)
default:
   unreachable(unhandled instruction);
}
+
+   /* If we need to do a boolean resolve, replace the result with -(x  1)
+* to convert it from junk in the top 31 bits and the actual boolean in
+* the bottom bit to the NIR standard of 0/~0.
+*/
+   if (brw-gen = 5 
+   (instr-instr.pass_flags  BRW_NIR_BOOLEAN_MASK) == 
BRW_NIR_BOOLEAN_NEEDS_RESOLVE) {
+  fs_reg masked = vgrf(glsl_type::int_type);
+  emit(AND(masked, result, fs_reg(1)));
+  masked.negate = true;
+  emit(MOV(result, masked));
+   }
 }
 
 fs_reg
-- 
2.3.2

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 0/9] Various NIR fixes

2015-03-17 Thread Jason Ekstrand
This series contains a variety of fixes for NIR.  One of them (by Matt) is
a substantial shader-db fix.  Thanks, Matt, for figuring out a much easier
way to do that.  A bunch of the rest are to get NIR working on older gens.
With this series, NIR looks good on Jenkins.

Patches 2 and 9 are to the way that we determine whether or not to use NIR.
The first makes it substantially smarter so that we can use INTEL_USE_NIR
as either a force-enable or force-disable.  The last patch (number 9)
enables NIR by default for fragment shaders on SNB+.  I'm not sure that we
actually want to do this now, but the shader-db numbers are looking pretty
good on SNB+ so I figured I'd send the patch.  On ILK-, the numbers aren't
quite so good.  I'm going to look that tonight or tomorrow.

Jason Ekstrand (8):
  i965/nir: Make our environment variable checking smarter
  i965/fs: Make emit_lrp return an fs_inst
  i965/nir: Use emit_lrp for emitting flrp
  i965/nir: Emit MUL + ADD for MAD on GEN = 5
  i965/nir: Properly set the predicate on the SEL used in min/max
  i965: Add a NIR analysis pass for determining when a boolean resolve
is needed
  i965/nir: Do boolean resolves on GEN = 5
  i965: Use NIR by default for Fragment shaders on SNB+

Matt Turner (1):
  i965/cmod_propagation: Ignore type in cmod prop if scan_inst is cmp

 src/mesa/drivers/dri/i965/Makefile.sources |   2 +
 src/mesa/drivers/dri/i965/brw_fs.cpp   |  24 ++-
 src/mesa/drivers/dri/i965/brw_fs.h |   4 +-
 .../drivers/dri/i965/brw_fs_cmod_propagation.cpp   |   9 +
 src/mesa/drivers/dri/i965/brw_fs_nir.cpp   |  34 ++-
 src/mesa/drivers/dri/i965/brw_fs_visitor.cpp   |   6 +-
 src/mesa/drivers/dri/i965/brw_nir.h|  45 
 .../dri/i965/brw_nir_analize_boolean_resolves.c| 228 +
 8 files changed, 342 insertions(+), 10 deletions(-)
 create mode 100644 src/mesa/drivers/dri/i965/brw_nir.h
 create mode 100644 src/mesa/drivers/dri/i965/brw_nir_analize_boolean_resolves.c

-- 
2.3.2

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 6/9] i965/nir: Properly set the predicate on the SEL used in min/max

2015-03-17 Thread Jason Ekstrand
---
 src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp 
b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
index 41f9ae2..91a3f65 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
@@ -1132,6 +1132,7 @@ fs_visitor::nir_emit_alu(nir_alu_instr *instr)
   } else {
  emit(CMP(reg_null_d, op[0], op[1], BRW_CONDITIONAL_L));
  inst = emit(SEL(result, op[0], op[1]));
+ inst-predicate = BRW_PREDICATE_NORMAL;
   }
   inst-saturate = instr-dest.saturate;
   break;
@@ -1145,6 +1146,7 @@ fs_visitor::nir_emit_alu(nir_alu_instr *instr)
   } else {
  emit(CMP(reg_null_d, op[0], op[1], BRW_CONDITIONAL_GE));
  inst = emit(SEL(result, op[0], op[1]));
+ inst-predicate = BRW_PREDICATE_NORMAL;
   }
   inst-saturate = instr-dest.saturate;
   break;
-- 
2.3.2

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 1/9] i965/cmod_propagation: Ignore type in cmod prop if scan_inst is cmp

2015-03-17 Thread Jason Ekstrand
From: Matt Turner matts...@gmail.com

Shader-db results for FS instructions with NIR on HSW:

total instructions in shared programs: 4186747 - 4129871 (-1.36%)
instructions in affected programs: 2438094 - 2381218 (-2.33%)
helped:13525
HURT:  0
GAINED:1
LOST:  5

Revewed-by: Jason Ekstrand jason.ekstr...@intel.com
---
 src/mesa/drivers/dri/i965/brw_fs_cmod_propagation.cpp | 9 +
 1 file changed, 9 insertions(+)

diff --git a/src/mesa/drivers/dri/i965/brw_fs_cmod_propagation.cpp 
b/src/mesa/drivers/dri/i965/brw_fs_cmod_propagation.cpp
index 1935f06..1b24358 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_cmod_propagation.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_cmod_propagation.cpp
@@ -94,6 +94,15 @@ opt_cmod_propagation_local(bblock_t *block)
 scan_inst-dst.reg_offset != inst-src[0].reg_offset)
break;
 
+if (inst-conditional_mod == BRW_CONDITIONAL_NZ 
+scan_inst-opcode == BRW_OPCODE_CMP 
+(inst-dst.type == BRW_REGISTER_TYPE_D ||
+ inst-dst.type == BRW_REGISTER_TYPE_UD)) {
+   inst-remove(block);
+   progress = true;
+   break;
+}
+
 /* This must be done before the dst.type check because the result
  * type of the AND will always be D, but the result of the CMP
  * could be anything.  The assumption is that the AND is just
-- 
2.3.2

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 7/9] i965: Add a NIR analysis pass for determining when a boolean resolve is needed

2015-03-17 Thread Jason Ekstrand
---
 src/mesa/drivers/dri/i965/Makefile.sources |   2 +
 src/mesa/drivers/dri/i965/brw_nir.h|  45 
 .../dri/i965/brw_nir_analize_boolean_resolves.c| 228 +
 3 files changed, 275 insertions(+)
 create mode 100644 src/mesa/drivers/dri/i965/brw_nir.h
 create mode 100644 src/mesa/drivers/dri/i965/brw_nir_analize_boolean_resolves.c

diff --git a/src/mesa/drivers/dri/i965/Makefile.sources 
b/src/mesa/drivers/dri/i965/Makefile.sources
index c69441b..05ebbe9 100644
--- a/src/mesa/drivers/dri/i965/Makefile.sources
+++ b/src/mesa/drivers/dri/i965/Makefile.sources
@@ -73,6 +73,8 @@ i965_FILES = \
brw_meta_util.h \
brw_misc_state.c \
brw_multisample_state.h \
+   brw_nir.h \
+   brw_nir_analize_boolean_resolves.c \
brw_object_purgeable.c \
brw_packed_float.c \
brw_performance_monitor.c \
diff --git a/src/mesa/drivers/dri/i965/brw_nir.h 
b/src/mesa/drivers/dri/i965/brw_nir.h
new file mode 100644
index 000..f79c5f1
--- /dev/null
+++ b/src/mesa/drivers/dri/i965/brw_nir.h
@@ -0,0 +1,45 @@
+/*
+ * Copyright © 2015 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the Software),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+#pragma once
+
+#include glsl/nir/nir.h
+
+#ifdef __cplusplus
+extern C {
+#endif
+
+/* Flags set in the instr-pass_flags field by i965 analysis passes */
+enum {
+   BRW_NIR_NON_BOOLEAN   = 0x0,
+   BRW_NIR_BOOLEAN_NEEDS_RESOLVE = 0x1,
+   BRW_NIR_BOOLEAN_UNRESOLVED= 0x2,
+   BRW_NIR_BOOLEAN_NO_RESOLVE= 0x3,
+   BRW_NIR_BOOLEAN_MASK  = 0x3,
+};
+
+void brw_nir_analize_boolean_resolves(nir_shader *nir);
+
+#ifdef __cplusplus
+}
+#endif
diff --git a/src/mesa/drivers/dri/i965/brw_nir_analize_boolean_resolves.c 
b/src/mesa/drivers/dri/i965/brw_nir_analize_boolean_resolves.c
new file mode 100644
index 000..7d93471
--- /dev/null
+++ b/src/mesa/drivers/dri/i965/brw_nir_analize_boolean_resolves.c
@@ -0,0 +1,228 @@
+/*
+ * Copyright © 2015 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the Software),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Authors:
+ *Jason Ekstrand ja...@jlekstrand.net
+ */
+
+#include brw_nir.h
+
+/*
+ * This file implements an analysis pass that determines when we have to do
+ * a boolean resolve on GEN = 5.  Instructions that need a boolean resolve
+ * will have the booleans portion of the instr-pass_flags field set to
+ * BRW_NIR_BOOLEAN_NEEDS_RESOLVE.
+ */
+
+static uint8_t
+get_resolve_state_for_src(nir_alu_instr *alu, unsigned src_idx)
+{
+   nir_instr *src_instr;
+   if (alu-src[src_idx].src.is_ssa) {
+  src_instr = alu-src[src_idx].src.ssa-parent_instr;
+   } else {
+  src_instr = alu-src[src_idx].src.reg.reg-parent_instr;
+   }
+
+   if (src_instr) {
+  uint8_t state = src_instr-pass_flags  BRW_NIR_BOOLEAN_MASK;
+
+  /* If the source instruction nees resolve then, from the 

[Mesa-dev] [PATCH 4/9] i965/nir: Use emit_lrp for emitting flrp

2015-03-17 Thread Jason Ekstrand
---
 src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp 
b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
index a9e75ab..5da8423 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
@@ -1239,8 +1239,7 @@ fs_visitor::nir_emit_alu(nir_alu_instr *instr)
   break;
 
case nir_op_flrp:
-  /* TODO emulate for gen  6 */
-  inst = emit(LRP(result, op[2], op[1], op[0]));
+  inst = emit_lrp(result, op[0], op[1], op[2]);
   inst-saturate = instr-dest.saturate;
   break;
 
-- 
2.3.2

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 2/9] i965/nir: Make our environment variable checking smarter

2015-03-17 Thread Jason Ekstrand
Before, we enabled NIR if you set INTEL_USE_NIR to anything which mean that
INTEL_USE_NIR=false would actually turn on NIR.  In preparation for turning
NIR on by default, this commit makes it smarter by allowing the
INTEL_USE_NIR variable to work as either a force-enable or a force-disable.
---
 src/mesa/drivers/dri/i965/brw_fs.cpp | 24 ++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp 
b/src/mesa/drivers/dri/i965/brw_fs.cpp
index 53ceb29..3d4d31a 100644
--- a/src/mesa/drivers/dri/i965/brw_fs.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
@@ -3838,6 +3838,26 @@ fs_visitor::allocate_registers()
   prog_data-total_scratch = brw_get_scratch_size(last_scratch);
 }
 
+static bool
+env_var_as_boolean(const char *var_name, bool default_value)
+{
+   const char *str = getenv(var_name);
+   if (str == NULL)
+  return default_value;
+
+   if (strcmp(str, 1) == 0 ||
+   strcasecmp(str, true) == 0 ||
+   strcasecmp(str, yes) == 0) {
+  return true;
+   } else if (strcmp(str, 0) == 0 ||
+  strcasecmp(str, false) == 0 ||
+  strcasecmp(str, no) == 0) {
+  return false;
+   } else {
+  return default_value;
+   }
+}
+
 bool
 fs_visitor::run_vs()
 {
@@ -3849,7 +3869,7 @@ fs_visitor::run_vs()
if (INTEL_DEBUG  DEBUG_SHADER_TIME)
   emit_shader_time_begin();
 
-   if (getenv(INTEL_USE_NIR) != NULL) {
+   if (env_var_as_boolean(INTEL_USE_NIR, false)) {
   emit_nir_code();
} else {
   foreach_in_list(ir_instruction, ir, shader-base.ir) {
@@ -3923,7 +3943,7 @@ fs_visitor::run_fs()
* functions called main).
*/
   if (shader) {
- if (getenv(INTEL_USE_NIR) != NULL) {
+ if (env_var_as_boolean(INTEL_USE_NIR, false)) {
 emit_nir_code();
  } else {
 foreach_in_list(ir_instruction, ir, shader-base.ir) {
-- 
2.3.2

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 3/9] i965/fs: Make emit_lrp return an fs_inst

2015-03-17 Thread Jason Ekstrand
---
 src/mesa/drivers/dri/i965/brw_fs.h   | 4 ++--
 src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 6 +++---
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_fs.h 
b/src/mesa/drivers/dri/i965/brw_fs.h
index 7716529..a520fd4 100644
--- a/src/mesa/drivers/dri/i965/brw_fs.h
+++ b/src/mesa/drivers/dri/i965/brw_fs.h
@@ -303,8 +303,8 @@ public:
fs_reg fix_math_operand(fs_reg src);
fs_inst *emit_math(enum opcode op, fs_reg dst, fs_reg src0);
fs_inst *emit_math(enum opcode op, fs_reg dst, fs_reg src0, fs_reg src1);
-   void emit_lrp(const fs_reg dst, const fs_reg x, const fs_reg y,
- const fs_reg a);
+   fs_inst *emit_lrp(const fs_reg dst, const fs_reg x, const fs_reg y,
+ const fs_reg a);
void emit_minmax(enum brw_conditional_mod conditionalmod, const fs_reg dst,
 const fs_reg src0, const fs_reg src1);
bool try_emit_saturate(ir_expression *ir);
diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp 
b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
index 6d56115..2076695 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
@@ -288,7 +288,7 @@ fs_visitor::visit(ir_dereference_array *ir)
this-result = src;
 }
 
-void
+fs_inst *
 fs_visitor::emit_lrp(const fs_reg dst, const fs_reg x, const fs_reg y,
  const fs_reg a)
 {
@@ -305,12 +305,12 @@ fs_visitor::emit_lrp(const fs_reg dst, const fs_reg x, 
const fs_reg y,
   emit(ADD(one_minus_a, negative_a, fs_reg(1.0f)));
   emit(MUL(x_times_one_minus_a, x, one_minus_a));
 
-  emit(ADD(dst, x_times_one_minus_a, y_times_a));
+  return emit(ADD(dst, x_times_one_minus_a, y_times_a));
} else {
   /* The LRP instruction actually does op1 * op0 + op2 * (1 - op0), so
* we need to reorder the operands.
*/
-  emit(LRP(dst, a, y, x));
+  return emit(LRP(dst, a, y, x));
}
 }
 
-- 
2.3.2

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 5/9] i965/nir: Emit MUL + ADD for MAD on GEN = 5

2015-03-17 Thread Jason Ekstrand
---
 src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp 
b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
index 5da8423..41f9ae2 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
@@ -1234,7 +1234,13 @@ fs_visitor::nir_emit_alu(nir_alu_instr *instr)
   break;
 
case nir_op_ffma:
-  inst = emit(MAD(result, op[2], op[1], op[0]));
+  if (brw-gen = 6) {
+ inst = emit(MAD(result, op[2], op[1], op[0]));
+  } else {
+ fs_reg temp = vgrf(glsl_type::float_type);
+ emit(MUL(temp, op[0], op[1]));
+ inst = emit(ADD(result, temp, op[2]));
+  }
   inst-saturate = instr-dest.saturate;
   break;
 
-- 
2.3.2

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 9/9] i965: Use NIR by default for Fragment shaders on SNB+

2015-03-17 Thread Jason Ekstrand
GLSL IR vs. NIR FS instructions on snb:
total instructions in shared programs: 4976784 - 4973072 (-0.07%)
instructions in affected programs: 3309521 - 3305809 (-0.11%)
helped:7161
HURT:  9839
GAINED:55
LOST:  23

GLSL IR vs. NIR FS instructions on ivb:
total instructions in shared programs: 4593938 - 4588496 (-0.12%)
instructions in affected programs: 2948822 - 2943380 (-0.18%)
helped:6878
HURT:  8801
GAINED:69
LOST:  44

GLSL IR vs. NIR FS instructions on hsw:
total instructions in shared programs: 4089459 - 4081368 (-0.20%)
instructions in affected programs: 2592474 - 2584383 (-0.31%)
helped:6492
HURT:  9403
GAINED:69
LOST:  32

GLSL IR vs. NIR FS instructions on bdw:
total instructions in shared programs: 4065050 - 4058653 (-0.16%)
instructions in affected programs: 255 - 2549158 (-0.25%)
helped:5910
HURT:  9765
GAINED:45
LOST:  53
---
 src/mesa/drivers/dri/i965/brw_fs.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp 
b/src/mesa/drivers/dri/i965/brw_fs.cpp
index 3d4d31a..42f6604 100644
--- a/src/mesa/drivers/dri/i965/brw_fs.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
@@ -3943,7 +3943,7 @@ fs_visitor::run_fs()
* functions called main).
*/
   if (shader) {
- if (env_var_as_boolean(INTEL_USE_NIR, false)) {
+ if (env_var_as_boolean(INTEL_USE_NIR, brw-gen = 6)) {
 emit_nir_code();
  } else {
 foreach_in_list(ir_instruction, ir, shader-base.ir) {
-- 
2.3.2

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 1/9] i965/cmod_propagation: Ignore type in cmod prop if scan_inst is cmp

2015-03-17 Thread Jason Ekstrand
On Tue, Mar 17, 2015 at 7:21 PM, Matt Turner matts...@gmail.com wrote:
 On Tue, Mar 17, 2015 at 7:17 PM, Jason Ekstrand ja...@jlekstrand.net wrote:
 From: Matt Turner matts...@gmail.com

 Shader-db results for FS instructions with NIR on HSW:

 total instructions in shared programs: 4186747 - 4129871 (-1.36%)
 instructions in affected programs: 2438094 - 2381218 (-2.33%)
 helped:13525
 HURT:  0
 GAINED:1
 LOST:  5

 Revewed-by: Jason Ekstrand jason.ekstr...@intel.com
 ---

 Thanks for sending this out.

 I think we should just make the subject prefix i965/fs.

How about  i965/fs:  Ignore type in cmod prop if scan_inst is cmp

--Jason
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 1/9] i965/cmod_propagation: Ignore type in cmod prop if scan_inst is cmp

2015-03-17 Thread Matt Turner
On Tue, Mar 17, 2015 at 7:17 PM, Jason Ekstrand ja...@jlekstrand.net wrote:
 From: Matt Turner matts...@gmail.com

 Shader-db results for FS instructions with NIR on HSW:

 total instructions in shared programs: 4186747 - 4129871 (-1.36%)
 instructions in affected programs: 2438094 - 2381218 (-2.33%)
 helped:13525
 HURT:  0
 GAINED:1
 LOST:  5

 Revewed-by: Jason Ekstrand jason.ekstr...@intel.com
 ---

Thanks for sending this out.

I think we should just make the subject prefix i965/fs.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 1/9] i965/cmod_propagation: Ignore type in cmod prop if scan_inst is cmp

2015-03-17 Thread Matt Turner
On Tue, Mar 17, 2015 at 7:22 PM, Jason Ekstrand ja...@jlekstrand.net wrote:
 On Tue, Mar 17, 2015 at 7:21 PM, Matt Turner matts...@gmail.com wrote:
 On Tue, Mar 17, 2015 at 7:17 PM, Jason Ekstrand ja...@jlekstrand.net wrote:
 From: Matt Turner matts...@gmail.com

 Shader-db results for FS instructions with NIR on HSW:

 total instructions in shared programs: 4186747 - 4129871 (-1.36%)
 instructions in affected programs: 2438094 - 2381218 (-2.33%)
 helped:13525
 HURT:  0
 GAINED:1
 LOST:  5

 Revewed-by: Jason Ekstrand jason.ekstr...@intel.com
 ---

 Thanks for sending this out.

 I think we should just make the subject prefix i965/fs.

 How about  i965/fs:  Ignore type in cmod prop if scan_inst is cmp

Sounds good.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [Bug 89622] Drivers/Gallium/swrast: Pixel image limited to GL_MAX_TEXTURE_SIZE

2015-03-17 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=89622

--- Comment #3 from Dan Sebald daniel.seb...@ieee.org ---
Created attachment 114410
  -- https://bugs.freedesktop.org/attachment.cgi?id=114410action=edit
Fixed image in x-dimension

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [Bug 89622] Drivers/Gallium/swrast: Pixel image limited to GL_MAX_TEXTURE_SIZE

2015-03-17 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=89622

--- Comment #4 from Dan Sebald daniel.seb...@ieee.org ---
Created attachment 114411
  -- https://bugs.freedesktop.org/attachment.cgi?id=114411action=edit
Fixed image in y-dimension

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 1/2] mesa: add void to format_array_format_table_init() declaration

2015-03-17 Thread Matt Turner
Reviewed-by: Matt Turner matts...@gmail.com
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 2/2] mesa: remove MSVC warning pragmas

2015-03-17 Thread Matt Turner
Acked-by: Matt Turner matts...@gmail.com
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [Bug 89622] Drivers/Gallium/swrast: Pixel image limited to GL_MAX_TEXTURE_SIZE

2015-03-17 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=89622

--- Comment #2 from Dan Sebald daniel.seb...@ieee.org ---
Created attachment 114409
  -- https://bugs.freedesktop.org/attachment.cgi?id=114409action=edit
Clipping in the y-dimension

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [Bug 89622] Drivers/Gallium/swrast: Pixel image limited to GL_MAX_TEXTURE_SIZE

2015-03-17 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=89622

Bug ID: 89622
   Summary: Drivers/Gallium/swrast: Pixel image limited to
GL_MAX_TEXTURE_SIZE
   Product: Mesa
   Version: git
  Hardware: Other
OS: All
Status: NEW
  Severity: normal
  Priority: medium
 Component: Other
  Assignee: mesa-dev@lists.freedesktop.org
  Reporter: daniel.seb...@ieee.org
QA Contact: mesa-dev@lists.freedesktop.org

First, I want to point out that building Mesa from scratch *without* the
options

--with-dri-drivers=nouveau --with-gallium-drivers=nouveau

produces no nouveau_dri.so library file.  There is a nouveau_vieux_dri.so, but
no nouveau_dri.so.  When I then run an app that accesses this newly built mesa
library files I see the warning message:

libGL error: unable to load driver: nouveau_dri.so
libGL error: driver pointer missing
libGL error: failed to load driver: nouveau

It's no harm of course, but if this should not be happening--given
nourveau_dri.so--wasn't built, I'll file a bug report.

OK, I filed a bug report about vertical white lines in the legacy swrast_dri.so
associated with intervals of GL_MAX_TEXTURE_SIZE in the x dimension of the
input image:

https://bugs.freedesktop.org/show_bug.cgi?id=89586

I then went to try the newer Gallium version of the software implementation and
found that a limitation is placed on the size of the input image to
GL_MAX_TEXTURE_SIZE.  This can be seen in frame buffer images I will attach,
Screenshot-incomplete_image_x-dimension.png and
Screenshot-incomplete_image_y-dimension.png where only a portion of the
gradient image appears.  The source of this limitation is in
src/mesa/state_tracker/st_cb_drawpixels.c.  Out of curiousity, I removed the
limitation as follows:

--- a/src/mesa/state_tracker/st_cb_drawpixels.c
+++ b/src/mesa/state_tracker/st_cb_drawpixels.c
@@ -1110,14 +1110,6 @@ st_DrawPixels(struct gl_context *ctx, GLint x, GLint y,

st_validate_state(st);

-   /* Limit the size of the glDrawPixels to the max texture size.
-* Strictly speaking, that's not correct but since we don't handle
-* larger images yet, this is better than crashing.
-*/
-   clippedUnpack = *unpack;
-   unpack = clippedUnpack;
-   clamp_size(st-pipe, width, height, clippedUnpack);
-
if (format == GL_DEPTH_STENCIL)
   write_stencil = write_depth = GL_TRUE;
else if (format == GL_STENCIL_INDEX)

and the result is that the image comes out fine.

So, what is the problem here?  Is it that not all systems are ensured to have
non-paged memory, and my system just happens to have it?  Can this be tested
and adjusted accordingly?  More generally, it seems that the Gallium version of
pixel zoom/draw can be done in a more straightforward way than legacy swrast is
doing.  Notice that Gallium is treating the image as blocks (i.e., the clip
affects both width and height).  It shouldn't be difficult to write a double
loop to write to the frame buffer in blocks (unless there is a problem of not
being able to select the memory page in the st_DrawPixels() routine because
that is done prior to the function call).  What role does the st-pipe play in
affecting size of writes?

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [Bug 89622] Drivers/Gallium/swrast: Pixel image limited to GL_MAX_TEXTURE_SIZE

2015-03-17 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=89622

Dan Sebald daniel.seb...@ieee.org changed:

   What|Removed |Added

 CC||daniel.seb...@ieee.org

--- Comment #1 from Dan Sebald daniel.seb...@ieee.org ---
Created attachment 114408
  -- https://bugs.freedesktop.org/attachment.cgi?id=114408action=edit
Clipping in the x-dimension

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 5/9] i965/nir: Emit MUL + ADD for MAD on GEN = 5

2015-03-17 Thread Connor Abbott
Instead of doing this, I think it would be better to add something to
nir_shader_compiler_options and do it in nir_opt_algebraic, similar to
what Eric has already done for other things. I've seen some shaders
where we transform a mul + a series of adds into a series of mad's,
which is good on gen6+ (at the very least, we reduce sched
dependencies, and we may be able to eliminate the mul) but if we do
that and then just naively lower it back we'll get worse results than
we started with. Similar thing with the previous lrp patch.

On Tue, Mar 17, 2015 at 10:17 PM, Jason Ekstrand ja...@jlekstrand.net wrote:
 ---
  src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 8 +++-
  1 file changed, 7 insertions(+), 1 deletion(-)

 diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp 
 b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
 index 5da8423..41f9ae2 100644
 --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
 +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
 @@ -1234,7 +1234,13 @@ fs_visitor::nir_emit_alu(nir_alu_instr *instr)
break;

 case nir_op_ffma:
 -  inst = emit(MAD(result, op[2], op[1], op[0]));
 +  if (brw-gen = 6) {
 + inst = emit(MAD(result, op[2], op[1], op[0]));
 +  } else {
 + fs_reg temp = vgrf(glsl_type::float_type);
 + emit(MUL(temp, op[0], op[1]));
 + inst = emit(ADD(result, temp, op[2]));
 +  }
inst-saturate = instr-dest.saturate;
break;

 --
 2.3.2

 ___
 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


Re: [Mesa-dev] [PATCH 1/4] mapi: add new _glapi_new_nop_table() and _glapi_set_nop_handler()

2015-03-17 Thread Brian Paul

Ping - on this series.

-Brian

On 03/13/2015 01:22 PM, Brian Paul wrote:

_glapi_new_nop_table() creates a new dispatch table populated with
pointers to no-op functions.

_glapi_set_nop_handler() is used to register a callback function which
will be called from each of the no-op functions.

Now we always generate a separate no-op function for each GL entrypoint.
This allows us to do proper stack clean-up for Windows __stdcall and
lets us report the actual function name in error messages.  Before this
change, for non-Windows release builds we used a single no-op function
for all entrypoints.
---
  src/mapi/glapi/glapi.h | 11 ++
  src/mapi/glapi/glapi_nop.c | 84 ++
  src/mapi/mapi_glapi.c  | 23 +
  src/mapi/table.c   | 23 ++---
  src/mapi/table.h   |  8 +
  5 files changed, 108 insertions(+), 41 deletions(-)

diff --git a/src/mapi/glapi/glapi.h b/src/mapi/glapi/glapi.h
index 8d991fb..673295b 100644
--- a/src/mapi/glapi/glapi.h
+++ b/src/mapi/glapi/glapi.h
@@ -80,6 +80,9 @@ extern C {
  #endif

  typedef void (*_glapi_proc)(void);
+
+typedef void (*_glapi_nop_handler_proc)(const char *name);
+
  struct _glapi_table;


@@ -159,6 +162,14 @@ _GLAPI_EXPORT struct _glapi_table *
  _glapi_create_table_from_handle(void *handle, const char *symbol_prefix);


+_GLAPI_EXPORT void
+_glapi_set_nop_handler(_glapi_nop_handler_proc func);
+
+/** Return pointer to new dispatch table filled with no-op functions */
+_GLAPI_EXPORT struct _glapi_table *
+_glapi_new_nop_table(unsigned num_entries);
+
+
  /** Deprecated function */
  _GLAPI_EXPORT unsigned long
  _glthread_GetID(void);
diff --git a/src/mapi/glapi/glapi_nop.c b/src/mapi/glapi/glapi_nop.c
index 628276e..0041ed5 100644
--- a/src/mapi/glapi/glapi_nop.c
+++ b/src/mapi/glapi/glapi_nop.c
@@ -30,14 +30,21 @@
   * This file defines a special dispatch table which is loaded with no-op
   * functions.
   *
- * When there's no current rendering context, calling a GL function like
- * glBegin() is a no-op.  Apps should never normally do this.  So as a
- * debugging aid, each of the no-op functions will emit a warning to
- * stderr if the MESA_DEBUG or LIBGL_DEBUG env var is set.
+ * Mesa can register a no-op handler function which will be called in
+ * the event that a no-op function is called.
+ *
+ * In the past, the dispatch table was loaded with pointers to a single
+ * no-op function.  But that broke on Windows because the GL entrypoints
+ * use __stdcall convention.  __stdcall means the callee cleans up the
+ * stack.  So one no-op function can't properly clean up the stack.  This
+ * would lead to crashes.
+ *
+ * Another benefit of unique no-op functions is we can accurately report
+ * the function's name in an error message.
   */


-
+#include string.h
  #include glapi/glapi_priv.h


@@ -51,25 +58,32 @@ _glapi_set_warning_func(_glapi_proc func)
  {
  }

-/*
- * When GLAPIENTRY is __stdcall (i.e. Windows), the stack is popped by the
- * callee making the number/type of arguments significant.
+
+/**
+ * We'll jump though this function pointer whenever a no-op function
+ * is called.
   */
-#if defined(_WIN32) || defined(DEBUG)
+static _glapi_nop_handler_proc nop_handler = NULL;
+
+
+/**
+ * Register the no-op handler call-back function.
+ */
+void
+_glapi_set_nop_handler(_glapi_nop_handler_proc func)
+{
+   nop_handler = func;
+}
+

  /**
   * Called by each of the no-op GL entrypoints.
   */
-static int
-Warn(const char *func)
+static void
+nop(const char *func)
  {
-#if defined(DEBUG)
-   if (getenv(MESA_DEBUG) || getenv(LIBGL_DEBUG)) {
-  fprintf(stderr, GL User Error: gl%s called without a rendering 
context\n,
-  func);
-   }
-#endif
-   return 0;
+   if (nop_handler)
+  nop_handler(func);
  }


@@ -79,7 +93,8 @@ Warn(const char *func)
  static GLint
  NoOpUnused(void)
  {
-   return Warn( function);
+   nop(unused GL entry point);
+   return 0;
  }

  /*
@@ -89,31 +104,28 @@ NoOpUnused(void)
  #define KEYWORD1_ALT static
  #define KEYWORD2 GLAPIENTRY
  #define NAME(func)  NoOp##func
-#define DISPATCH(func, args, msg)  Warn(#func);
-#define RETURN_DISPATCH(func, args, msg)  Warn(#func); return 0
+#define DISPATCH(func, args, msg)  nop(#func);
+#define RETURN_DISPATCH(func, args, msg)  nop(#func); return 0


  /*
   * Defines for the table of no-op entry points.
   */
  #define TABLE_ENTRY(name) (_glapi_proc) NoOp##name
+#define DISPATCH_TABLE_NAME __glapi_noop_table
+#define UNUSED_TABLE_NAME __unused_noop_functions
+
+#include glapi/glapitemp.h

-#else

-static int
-NoOpGeneric(void)
+/** Return pointer to new dispatch table filled with no-op functions */
+struct _glapi_table *
+_glapi_new_nop_table(unsigned num_entries)
  {
-   if (getenv(MESA_DEBUG) || getenv(LIBGL_DEBUG)) {
-  fprintf(stderr, GL User Error: calling GL function without a rendering 
context\n);
+   struct _glapi_table *table = malloc(num_entries * sizeof(_glapi_proc));
+   if 

[Mesa-dev] [Bug 89622] Drivers/Gallium/swrast: Pixel image limited to GL_MAX_TEXTURE_SIZE

2015-03-17 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=89622

--- Comment #5 from Ilia Mirkin imir...@alum.mit.edu ---
(In reply to Dan Sebald from comment #0)
 First, I want to point out that building Mesa from scratch *without* the
 options
 
 --with-dri-drivers=nouveau --with-gallium-drivers=nouveau
 
 produces no nouveau_dri.so library file.  There is a nouveau_vieux_dri.so,
 but no nouveau_dri.so.  When I then run an app that accesses this newly
 built mesa library files I see the warning message:
 
 libGL error: unable to load driver: nouveau_dri.so
 libGL error: driver pointer missing
 libGL error: failed to load driver: nouveau
 
 It's no harm of course, but if this should not be happening--given
 nourveau_dri.so--wasn't built, I'll file a bug report.

libGL has no idea what's built. It's told by your X server (via DRI2) that it
should look for a DRI driver called nouveau, so it looks for
nouveau_dri.so.

 OK, I filed a bug report about vertical white lines in the legacy
 swrast_dri.so associated with intervals of GL_MAX_TEXTURE_SIZE in the x
 dimension of the input image:
 
 https://bugs.freedesktop.org/show_bug.cgi?id=89586
 
 I then went to try the newer Gallium version of the software implementation
 and found that a limitation is placed on the size of the input image to
 GL_MAX_TEXTURE_SIZE.  This can be seen in frame buffer images I will attach,
 Screenshot-incomplete_image_x-dimension.png and
 Screenshot-incomplete_image_y-dimension.png where only a portion of the
 gradient image appears.  The source of this limitation is in
 src/mesa/state_tracker/st_cb_drawpixels.c.  Out of curiousity, I removed the
 limitation as follows:
 
 --- a/src/mesa/state_tracker/st_cb_drawpixels.c
 +++ b/src/mesa/state_tracker/st_cb_drawpixels.c
 @@ -1110,14 +1110,6 @@ st_DrawPixels(struct gl_context *ctx, GLint x, GLint
 y,
  
 st_validate_state(st);
  
 -   /* Limit the size of the glDrawPixels to the max texture size.
 -* Strictly speaking, that's not correct but since we don't handle
 -* larger images yet, this is better than crashing.
 -*/
 -   clippedUnpack = *unpack;
 -   unpack = clippedUnpack;
 -   clamp_size(st-pipe, width, height, clippedUnpack);
 -
 if (format == GL_DEPTH_STENCIL)
write_stencil = write_depth = GL_TRUE;
 else if (format == GL_STENCIL_INDEX)
 
 and the result is that the image comes out fine.
 
 So, what is the problem here?  Is it that not all systems are ensured to
 have non-paged memory, and my system just happens to have it?  Can this be
 tested and adjusted accordingly?  More generally, it seems that the Gallium
 version of pixel zoom/draw can be done in a more straightforward way than
 legacy swrast is doing.  Notice that Gallium is treating the image as blocks
 (i.e., the clip affects both width and height).  It shouldn't be difficult
 to write a double loop to write to the frame buffer in blocks (unless there
 is a problem of not being able to select the memory page in the
 st_DrawPixels() routine because that is done prior to the function call). 
 What role does the st-pipe play in affecting size of writes?

For those of us still getting with the program... let's say that
max_texture_size = 128 (for simplicity). Is the situation that you have a (say)
256-pixel wide image that you're feeding to glDrawPixels to be put onto a
128-wide texture, and you're using glPixelZoom(0.5) to achieve this?

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [Bug 89624] Drivers, Gallium/legacy swrast glDrawPixels differences

2015-03-17 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=89624

Bug ID: 89624
   Summary: Drivers, Gallium/legacy swrast glDrawPixels
differences
   Product: Mesa
   Version: git
  Hardware: Other
OS: All
Status: NEW
  Severity: normal
  Priority: medium
 Component: Other
  Assignee: mesa-dev@lists.freedesktop.org
  Reporter: daniel.seb...@ieee.org
QA Contact: mesa-dev@lists.freedesktop.org

In the following two bug reports:

https://bugs.freedesktop.org/show_bug.cgi?id=89586
https://bugs.freedesktop.org/show_bug.cgi?id=89622

I discussed some artifacts concerning pixels at or beyond GL_MAX_TEXTURE_SIZE
in the input image.  In the first bug report I attached a patch that fixes the
vertical lines in legacy swrast.  In the second I simply noted that Gallium
swrast limits the image, and I think it wouldn't be too difficult to correct
that.

Here I want to point out the subtle difference between legacy swrast (post
patch in bug #89586) and the Gallium swrast using images with only 20 x 20
input pixels.

For legacy swrast, the attached image
Screenshot-differences_legacy_swrast-annotated.png shows how the scaled image
can extend past the edge of the boundary in the x-dimension, or not (and it
never falls short of the boundary).  On the other hand, the image will fall
short of boundary in the y-dimension, or not (and it never extends past).  I'm
guessing this behavior has to do with xfactor  0 and yfactor  0.

For Gallium swrast, the attached image
Screenshot-differences_gallium-annotated.png illustrates that scaled image
aligns with the x-dimension edge (an it always seems to do so).  However, the
image can extend past the bottom in the y-dimension, or not (and it seems to
never fall short).

Since these two behaviors are different, as subtle as it may be, at least one
of them has a bug.  I suspect that both of them might have a flaw as far as
precisely following OpenGl standard.  It's interesting that the x-dimension for
the Gallium driver seems most consistent.

Granted, the axis box itself may not be exact, so to say exactly what the bug
is at this stage is premature, but at least the behavior of the axis box should
be the same when all that is varied is the graphics driver and not the
application binary.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [Bug 89624] Drivers, Gallium/legacy swrast glDrawPixels differences

2015-03-17 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=89624

Dan Sebald daniel.seb...@ieee.org changed:

   What|Removed |Added

 CC||daniel.seb...@ieee.org

--- Comment #1 from Dan Sebald daniel.seb...@ieee.org ---
Created attachment 114412
  -- https://bugs.freedesktop.org/attachment.cgi?id=114412action=edit
Illustration of legacy driver behavior for glDrawPixels

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH] u_primconvert: add primitive restart support

2015-03-17 Thread Dave Airlie
This add primitive restart support to the prim conversion.

This involves changing the API for the translate functions
as we need to pass the prim restart index and the original
number of indices into the translate functions.

primitive restart is support for quads, quad strips
and polygons.

This deal with the case where the actual number of output
primitives is less than the initially calculated number,
by filling the rest of the output primitives with the restart
index, the other option is to reduce the output prim number,
but that will make the generator code a bit messier.

Signed-off-by: Dave Airlie airl...@redhat.com
---
 src/gallium/auxiliary/indices/u_indices.c  |  36 ++--
 src/gallium/auxiliary/indices/u_indices.h  |   8 +-
 src/gallium/auxiliary/indices/u_indices_gen.py | 198 +++--
 src/gallium/auxiliary/indices/u_primconvert.c  |   6 +-
 src/gallium/auxiliary/indices/u_unfilled_gen.py|  20 ++-
 src/gallium/auxiliary/indices/u_unfilled_indices.c |  18 +-
 src/gallium/drivers/svga/svga_draw_elements.c  |   3 +-
 7 files changed, 202 insertions(+), 87 deletions(-)

diff --git a/src/gallium/auxiliary/indices/u_indices.c 
b/src/gallium/auxiliary/indices/u_indices.c
index 1b33f41..b20337f 100644
--- a/src/gallium/auxiliary/indices/u_indices.c
+++ b/src/gallium/auxiliary/indices/u_indices.c
@@ -27,18 +27,22 @@
 
 static void translate_memcpy_ushort( const void *in,
  unsigned start,
- unsigned nr,
+ unsigned in_nr,
+ unsigned out_nr,
+ unsigned prim_restart_idx,
  void *out )
 {
-   memcpy(out, ((short *)in)[start], nr*sizeof(short));
+   memcpy(out, ((short *)in)[start], out_nr*sizeof(short));
 }
   
 static void translate_memcpy_uint( const void *in,
unsigned start,
-   unsigned nr,
+   unsigned in_nr,
+   unsigned out_nr,
+   unsigned prim_restart_idx,
void *out )
 {
-   memcpy(out, ((int *)in)[start], nr*sizeof(int));
+   memcpy(out, ((int *)in)[start], out_nr*sizeof(int));
 }
   
 
@@ -58,6 +62,7 @@ static void translate_memcpy_uint( const void *in,
  * \param nr  number of incoming vertices
  * \param in_pv  incoming provoking vertex convention (PV_FIRST or PV_LAST)
  * \param out_pv  desired provoking vertex convention (PV_FIRST or PV_LAST)
+ * \param prim_restart  whether primitive restart is disable or enabled
  * \param out_prim  returns new PIPE_PRIM_x we'll translate to
  * \param out_index_size  returns bytes per new index value (2 or 4)
  * \param out_nr  returns number of new vertices
@@ -69,6 +74,7 @@ int u_index_translator( unsigned hw_mask,
 unsigned nr,
 unsigned in_pv,
 unsigned out_pv,
+unsigned prim_restart,
 unsigned *out_prim,
 unsigned *out_index_size,
 unsigned *out_nr,
@@ -106,68 +112,68 @@ int u_index_translator( unsigned hw_mask,
else {
   switch (prim) {
   case PIPE_PRIM_POINTS:
- *out_translate = translate[in_idx][out_idx][in_pv][out_pv][prim];
+ *out_translate = 
translate[in_idx][out_idx][in_pv][out_pv][prim_restart][prim];
  *out_prim = PIPE_PRIM_POINTS;
  *out_nr = nr;
  break;
 
   case PIPE_PRIM_LINES:
- *out_translate = translate[in_idx][out_idx][in_pv][out_pv][prim];
+ *out_translate = 
translate[in_idx][out_idx][in_pv][out_pv][prim_restart][prim];
  *out_prim = PIPE_PRIM_LINES;
  *out_nr = nr;
  break;
 
   case PIPE_PRIM_LINE_STRIP:
- *out_translate = translate[in_idx][out_idx][in_pv][out_pv][prim];
+ *out_translate = 
translate[in_idx][out_idx][in_pv][out_pv][prim_restart][prim];
  *out_prim = PIPE_PRIM_LINES;
  *out_nr = (nr - 1) * 2;
  break;
 
   case PIPE_PRIM_LINE_LOOP:
- *out_translate = translate[in_idx][out_idx][in_pv][out_pv][prim];
+ *out_translate = 
translate[in_idx][out_idx][in_pv][out_pv][prim_restart][prim];
  *out_prim = PIPE_PRIM_LINES;
  *out_nr = nr * 2;
  break;
 
   case PIPE_PRIM_TRIANGLES:
- *out_translate = translate[in_idx][out_idx][in_pv][out_pv][prim];
+ *out_translate = 
translate[in_idx][out_idx][in_pv][out_pv][prim_restart][prim];
  *out_prim = PIPE_PRIM_TRIANGLES;
  *out_nr = nr;
  break;
 
   case PIPE_PRIM_TRIANGLE_STRIP:
- *out_translate = translate[in_idx][out_idx][in_pv][out_pv][prim];
+ *out_translate = 

Re: [Mesa-dev] [PATCH 1/9] i965/cmod_propagation: Ignore type in cmod prop if scan_inst is cmp

2015-03-17 Thread Jason Ekstrand
On Tue, Mar 17, 2015 at 7:28 PM, Matt Turner matts...@gmail.com wrote:
 On Tue, Mar 17, 2015 at 7:22 PM, Jason Ekstrand ja...@jlekstrand.net wrote:
 On Tue, Mar 17, 2015 at 7:21 PM, Matt Turner matts...@gmail.com wrote:
 On Tue, Mar 17, 2015 at 7:17 PM, Jason Ekstrand ja...@jlekstrand.net 
 wrote:
 From: Matt Turner matts...@gmail.com

 Shader-db results for FS instructions with NIR on HSW:

 total instructions in shared programs: 4186747 - 4129871 (-1.36%)
 instructions in affected programs: 2438094 - 2381218 (-2.33%)
 helped:13525
 HURT:  0
 GAINED:1
 LOST:  5

 Revewed-by: Jason Ekstrand jason.ekstr...@intel.com
 ---

 Thanks for sending this out.

 I think we should just make the subject prefix i965/fs.

 How about  i965/fs:  Ignore type in cmod prop if scan_inst is cmp

 Sounds good.

Done
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] radeonsi: increase coords array for radeon_llvm_emit_prepare_cube_coords

2015-03-17 Thread Michel Dänzer
On 18.03.2015 01:53, Marek Olšák wrote:
 From: Marek Olšák marek.ol...@amd.com
 
 radeon_llvm_emit_prepare_cube_coords uses coords[4] in some cases (TXB2 etc.)
 
 Discovered by Coverity. Reported by Ilia Mirkin.
 
 Cc: mesa-sta...@lists.freedesktop.org
 ---
  src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c | 2 +-
  src/gallium/drivers/radeonsi/si_shader.c| 2 +-
  2 files changed, 2 insertions(+), 2 deletions(-)
 
 diff --git a/src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c 
 b/src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c
 index d89e2b4..1690194 100644
 --- a/src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c
 +++ b/src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c
 @@ -748,7 +748,7 @@ static void txp_fetch_args(
   const struct tgsi_full_instruction * inst = emit_data-inst;
   LLVMValueRef src_w;
   unsigned chan;
 - LLVMValueRef coords[4];
 + LLVMValueRef coords[5];
  
   emit_data-dst_type = LLVMVectorType(bld_base-base.elem_type, 4);
   src_w = lp_build_emit_fetch(bld_base, emit_data-inst, 0, TGSI_CHAN_W);
 diff --git a/src/gallium/drivers/radeonsi/si_shader.c 
 b/src/gallium/drivers/radeonsi/si_shader.c
 index de889ed..4dcf756 100644
 --- a/src/gallium/drivers/radeonsi/si_shader.c
 +++ b/src/gallium/drivers/radeonsi/si_shader.c
 @@ -1572,7 +1572,7 @@ static void tex_fetch_args(
   const struct tgsi_full_instruction * inst = emit_data-inst;
   unsigned opcode = inst-Instruction.Opcode;
   unsigned target = inst-Texture.Texture;
 - LLVMValueRef coords[4];
 + LLVMValueRef coords[5];
   LLVMValueRef address[16];
   int ref_pos;
   unsigned num_coords = tgsi_util_get_texture_coord_dim(target, ref_pos);
 

Reviewed-by: Michel Dänzer michel.daen...@amd.com


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [Bug 89624] Drivers, Gallium/legacy swrast glDrawPixels differences

2015-03-17 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=89624

--- Comment #2 from Dan Sebald daniel.seb...@ieee.org ---
Created attachment 114413
  -- https://bugs.freedesktop.org/attachment.cgi?id=114413action=edit
Illustration of Gallium driver behavior for glDrawPixels

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 1/2] mesa: add void to format_array_format_table_init() declaration

2015-03-17 Thread Brian Paul
Silences an MSVC warning where it's called from call_once().
---
 src/mesa/main/formats.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/mesa/main/formats.c b/src/mesa/main/formats.c
index 422c9dc..2bc8bca 100644
--- a/src/mesa/main/formats.c
+++ b/src/mesa/main/formats.c
@@ -388,7 +388,7 @@ array_formats_equal(const void *a, const void *b)
 }
 
 static void
-format_array_format_table_init()
+format_array_format_table_init(void)
 {
const struct gl_format_info *info;
mesa_array_format array_format;
-- 
1.9.1

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 2/2] mesa: remove MSVC warning pragmas

2015-03-17 Thread Brian Paul
Removing this block of pragmas doesn't seem to increase the number of
warning generated by MSVC.  Other than signed/unsigned comparison warnings
there's very few other warnings nowadays.
---
 src/mesa/main/compiler.h | 20 
 1 file changed, 20 deletions(-)

diff --git a/src/mesa/main/compiler.h b/src/mesa/main/compiler.h
index 5c60391..55152fd 100644
--- a/src/mesa/main/compiler.h
+++ b/src/mesa/main/compiler.h
@@ -60,26 +60,6 @@ extern C {
 #endif
 
 
-/**
- * Disable assorted warnings
- */
-#if defined(_WIN32)  !defined(__CYGWIN__)
-#  if !defined(__GNUC__) /* mingw environment */
-#pragma warning( disable : 4068 ) /* unknown pragma */
-#pragma warning( disable : 4710 ) /* function 'foo' not inlined */
-#pragma warning( disable : 4711 ) /* function 'foo' selected for automatic 
inline expansion */
-#pragma warning( disable : 4127 ) /* conditional expression is constant */
-#if defined(MESA_MINWARN)
-#  pragma warning( disable : 4244 ) /* '=' : conversion from 'const double 
' to 'float ', possible loss of data */
-#  pragma warning( disable : 4018 ) /* '' : signed/unsigned mismatch */
-#  pragma warning( disable : 4305 ) /* '=' : truncation from 'const double 
' to 'float ' */
-#  pragma warning( disable : 4550 ) /* 'function' undefined; assuming 
extern returning int */
-#  pragma warning( disable : 4761 ) /* integral size mismatch in argument; 
conversion supplied */
-#endif
-#  endif
-#endif
-
-
 /* XXX: Use standard `__func__` instead */
 #ifndef __FUNCTION__
 #  define __FUNCTION__ __func__
-- 
1.9.1

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] gallium/util: Define ffsll on OpenBSD.

2015-03-17 Thread Emil Velikov
On 17/03/15 01:25, Jonathan Gray wrote:
 On Mon, Mar 16, 2015 at 08:37:28PM +, Emil Velikov wrote:
 On 26/02/15 13:49, Jose Fonseca wrote:
 On 26/02/15 13:42, Jose Fonseca wrote:
 On 26/02/15 03:55, Jonathan Gray wrote:
 On Wed, Feb 25, 2015 at 07:09:26PM -0800, Matt Turner wrote:
 On Wed, Feb 25, 2015 at 7:03 PM, Jonathan Gray j...@jsg.id.au wrote:
 On Wed, Feb 25, 2015 at 06:53:14PM -0800, Matt Turner wrote:
 On Wed, Feb 25, 2015 at 5:37 PM, Jonathan Gray j...@jsg.id.au wrote:
 If it isn't going to be configure checks could someone merge the
 original patch in this thread?

 I committed

 commit 3492e88090d2d0c0bfbc934963b8772b45fc8880
 Author: Matt Turner matts...@gmail.com
 Date:   Fri Feb 20 18:46:43 2015 -0800

  gallium/util: Use HAVE___BUILTIN_* macros.

  Reviewed-by: Eric Anholt e...@anholt.net
  Reviewed-by: Jose Fonseca jfons...@vmware.com

 which switched over a bunch of preprocessor checks around __builtin*
 calls to use the macros defined by autotools.

 So I think cleaning it up to use __builtin_ffs* first #ifdef
 HAVE___BUILTIN_* can go forward now.

 Yes but there is no HAVE_FFSLL for constructs like

 #if !defined(HAVE_FFSLL)  defined(HAVE___BUILTIN_FFSLL)

 or is it ok to always use the builtin?

 I think the question is whether it's okay to always use the builtin if
 it's available (as opposed to libc functions). I think the answer to
 that is yes.

 So in that case how about the following?  Or is it going to break
 the android scons build?

  From cba39ba72115e57d262cb4b099c4e72106f01812 Mon Sep 17 00:00:00 2001
 From: Jonathan Gray j...@jsg.id.au
 Date: Thu, 26 Feb 2015 14:46:45 +1100
 Subject: [PATCH] gallium/util: use ffs* builtins if available

 Required to build on OpenBSD which doesn't have ffsll in libc.

 Signed-off-by: Jonathan Gray j...@jsg.id.au
 ---
   src/gallium/auxiliary/util/u_math.h | 11 ---
   1 file changed, 8 insertions(+), 3 deletions(-)

 diff --git a/src/gallium/auxiliary/util/u_math.h
 b/src/gallium/auxiliary/util/u_math.h
 index b4a65e4..5bc9b97 100644
 --- a/src/gallium/auxiliary/util/u_math.h
 +++ b/src/gallium/auxiliary/util/u_math.h
 @@ -384,9 +384,6 @@ unsigned ffs( unsigned u )

  return i;
   }
 -#elif defined(__MINGW32__) || defined(PIPE_OS_ANDROID)
 -#define ffs __builtin_ffs
 -#define ffsll __builtin_ffsll

 Scons does define HAVE___BUILTIN_FFS for mingw.

 However `git grep '\ffs\` shows ffs is used directly in many other
 places.  So I suspect this change will break them.

   #endif

   #endif /* FFS_DEFINED */
 @@ -435,7 +432,11 @@ util_last_bit_signed(int i)
   static INLINE int
   u_bit_scan(unsigned *mask)
   {
 +#if defined(HAVE___BUILTIN_FFS)
 +   int i = __builtin_ffs(*mask) - 1;
 +#else
  int i = ffs(*mask) - 1;
 +#endif
  *mask = ~(1  i);
  return i;
   }
 @@ -444,7 +445,11 @@ u_bit_scan(unsigned *mask)
   static INLINE int
   u_bit_scan64(uint64_t *mask)
   {
 +#if defined(HAVE___BUILTIN_FFSLL)
 +   int i = __builtin_ffsll(*mask) - 1;
 +#else
  int i = ffsll(*mask) - 1;
 +#endif
  *mask = ~(1llu  i);
  return i;
   }



 I think the right thing long term is to provide ffs and ffsll in
 c99_compat.h or c99_math.h for all platforms.  And let the rest of the
 code just always assume it's available somehow.


 Otherwise, let's just '#define ffs __builtin_ffs' on OpenBSD too.

 In other words, the original patch on this thread

   http://lists.freedesktop.org/archives/mesa-dev/2015-February/076071.html

 is the only patch I've seen so far that doesn't break Mingw.

 If you rather use HAVE___BUILTIN_FFSLL, then just do

 diff --git a/src/gallium/auxiliary/util/u_math.h
 b/src/gallium/auxiliary/util/u_math.h
 index 959f76e..d372cfd 100644
 --- a/src/gallium/auxiliary/util/u_math.h
 +++ b/src/gallium/auxiliary/util/u_math.h
 @@ -384,7 +384,7 @@ unsigned ffs( unsigned u )

 return i;
  }
 -#elif defined(__MINGW32__) || defined(PIPE_OS_ANDROID)
 +#elif defined(__MINGW32__) || defined(PIPE_OS_ANDROID) ||
 defined(HAVE___BUILTIN_FFSLL)
  #define ffs __builtin_ffs
  #define ffsll __builtin_ffsll
  #endif

 Jonathan

 Seems like this has ended up a longer discussion that anticipated :\
 Can you please confirm if the above works for you ?

 Thanks
 Emil
 
 It looks like that diff was mangled by the mail client and doesn't have
 the newline escaped.  It also assumes a ffsll builtin implies a ffs
 builtin is present.  So how about the following instead:
 
 diff --git a/src/gallium/auxiliary/util/u_math.h 
 b/src/gallium/auxiliary/util/u_math.h
 index 8f62cac..89c63d7 100644
 --- a/src/gallium/auxiliary/util/u_math.h
 +++ b/src/gallium/auxiliary/util/u_math.h
 @@ -383,14 +383,28 @@ unsigned ffs( unsigned u )
  
 return i;
  }
 -#elif defined(__MINGW32__) || defined(PIPE_OS_ANDROID)
 +#elif defined(__MINGW32__) || defined(PIPE_OS_ANDROID) || \
 +defined(HAVE___BUILTIN_FFS) 
  #define ffs __builtin_ffs
 -#define ffsll __builtin_ffsll
  #endif
  
  #endif /* FFS_DEFINED */
  
  /**
 + * Find 

[Mesa-dev] [RFC PATCH 1/2] glsl: Transform pow(x, 4) into (x*x)*(x*x).

2015-03-17 Thread Matt Turner
Without NIR:

total instructions in shared programs: 6190374 - 6190153 (-0.00%)
instructions in affected programs: 61126 - 60905 (-0.36%)
helped:156

With NIR:

total instructions in shared programs: 6271584 - 6271471 (-0.00%)
instructions in affected programs: 47656 - 47543 (-0.24%)
helped:113
GAINED:4
LOST:  41
---
The NIR gained/lost numbers look bad, but they're better than the next
patch alone. I wanted to send these out because Jason was doing
something that involved this.

The two shaders I poked at seemed to be doing

saturate(pow(x, 4.0f) + 1e-7f)

Just want to ensure the result is 0.0f I guess?

After changing the pow(x, 4.0f) into a pair of multiplies, we're able
to combine the second multiply with an add. Not sure why this means
we lose SIMD16, but the shaders that we lose we couldn't get SIMD16
in the non-NIR case anyway.

 src/glsl/opt_algebraic.cpp | 20 
 1 file changed, 20 insertions(+)

diff --git a/src/glsl/opt_algebraic.cpp b/src/glsl/opt_algebraic.cpp
index 69c03ea..7742f82 100644
--- a/src/glsl/opt_algebraic.cpp
+++ b/src/glsl/opt_algebraic.cpp
@@ -99,6 +99,12 @@ is_vec_two(ir_constant *ir)
 }
 
 static inline bool
+is_vec_four(ir_constant *ir)
+{
+   return (ir == NULL) ? false : ir-is_value(4.0, 4);
+}
+
+static inline bool
 is_vec_negative_one(ir_constant *ir)
 {
return (ir == NULL) ? false : ir-is_negative_one();
@@ -742,6 +748,20 @@ ir_algebraic_visitor::handle_expression(ir_expression *ir)
  return mul(x, x);
   }
 
+  if (is_vec_four(op_const[1])) {
+ ir_variable *x = new(ir) ir_variable(ir-operands[1]-type, x,
+  ir_var_temporary);
+ base_ir-insert_before(x);
+ base_ir-insert_before(assign(x, ir-operands[0]));
+
+ ir_variable *squared = new(ir) ir_variable(ir-operands[1]-type,
+squared,
+ir_var_temporary);
+ base_ir-insert_before(squared);
+ base_ir-insert_before(assign(squared, mul(x, x)));
+ return mul(squared, squared);
+  }
+
   break;
 
case ir_binop_min:
-- 
2.0.5

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [RFC PATCH 2/2] i965/fs: Allow 2-src math instructions to have immediate src1.

2015-03-17 Thread Matt Turner
Without NIR:

total instructions in shared programs: 6190153 - 6185918 (-0.07%)
instructions in affected programs: 185156 - 180921 (-2.29%)
helped:918

With NIR:

total instructions in shared programs: 6273347 - 6268409 (-0.08%)
instructions in affected programs: 205735 - 200797 (-2.40%)
helped:1171
GAINED:1
---
 src/mesa/drivers/dri/i965/brw_fs_combine_constants.cpp | 12 
 src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp  |  6 +++---
 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_fs_combine_constants.cpp 
b/src/mesa/drivers/dri/i965/brw_fs_combine_constants.cpp
index 7ddb253..714d1c2 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_combine_constants.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_combine_constants.cpp
@@ -62,9 +62,13 @@ could_coissue(const struct brw_context *brw, const fs_inst 
*inst)
  * Returns true for instructions that don't support immediate sources.
  */
 static bool
-must_promote_imm(const fs_inst *inst)
+must_promote_imm(struct brw_context *brw, const fs_inst *inst)
 {
switch (inst-opcode) {
+   case SHADER_OPCODE_POW:
+   case SHADER_OPCODE_INT_QUOTIENT:
+   case SHADER_OPCODE_INT_REMAINDER:
+  return brw-gen  8;
case BRW_OPCODE_MAD:
case BRW_OPCODE_LRP:
   return true;
@@ -207,7 +211,7 @@ fs_visitor::opt_combine_constants()
foreach_block_and_inst(block, fs_inst, inst, cfg) {
   ip++;
 
-  if (!could_coissue(brw, inst)  !must_promote_imm(inst))
+  if (!could_coissue(brw, inst)  !must_promote_imm(brw, inst))
  continue;
 
   for (int i = 0; i  inst-sources; i++) {
@@ -225,7 +229,7 @@ fs_visitor::opt_combine_constants()
 imm-block = intersection;
 imm-uses-push_tail(link(const_ctx, inst-src[i]));
 imm-uses_by_coissue += could_coissue(brw, inst);
-imm-must_promote = imm-must_promote || must_promote_imm(inst);
+imm-must_promote = imm-must_promote || must_promote_imm(brw, 
inst);
 imm-last_use_ip = ip;
  } else {
 imm = new_imm(table, const_ctx);
@@ -235,7 +239,7 @@ fs_visitor::opt_combine_constants()
 imm-uses-push_tail(link(const_ctx, inst-src[i]));
 imm-val = val;
 imm-uses_by_coissue = could_coissue(brw, inst);
-imm-must_promote = must_promote_imm(inst);
+imm-must_promote = must_promote_imm(brw, inst);
 imm-first_use_ip = ip;
 imm-last_use_ip = ip;
  }
diff --git a/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp 
b/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp
index 764741d..20b1976 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp
@@ -476,9 +476,9 @@ fs_visitor::try_constant_propagate(fs_inst *inst, acp_entry 
*entry)
   case SHADER_OPCODE_POW:
   case SHADER_OPCODE_INT_QUOTIENT:
   case SHADER_OPCODE_INT_REMAINDER:
- if (brw-gen  8)
-break;
- /* fallthrough */
+ /* Allow constant propagation into src1 regardless of generation, and
+  * let constant combining promote the constant on Gen  8.
+  */
   case BRW_OPCODE_BFI1:
   case BRW_OPCODE_ASR:
   case BRW_OPCODE_SHL:
-- 
2.0.5

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [Bug 89238] nir/nir.h, line 643: Error: In this declaration src is of an incomplete type nir_alu_src[].

2015-03-17 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=89238

Vinson Lee v...@freedesktop.org changed:

   What|Removed |Added

 Status|NEEDINFO|NEW

--- Comment #4 from Vinson Lee v...@freedesktop.org ---
(In reply to Jason Ekstrand from comment #3)
 Created attachment 114310 [details]
 Patch to fix the unsized array issue
 
 I just attached a patch that *should* fix the bug.  Let me know how it works.

nir/nir.h, line 664: Error: An array cannot have zero size unless you use the
option -features=zla.
nir/nir.h, line 722: Warning: Identifier expected instead of }.
nir/nir.h, line 813: Error: An array cannot have zero size unless you use the
option -features=zla.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [Bug 89616] bufferobj.c:1639:7: error: format not a string literal and no format arguments [-Werror=format-security]

2015-03-17 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=89616

--- Comment #1 from Laura Ekstrand la...@jlekstrand.net ---
Just pushed a commit to Mesa master that fixes this.  I looked for a while and
didn't find any other instances of the same error in the rest of my code.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [Bug 89433] GCC 4.2 does not support -Wvla

2015-03-17 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=89433

--- Comment #4 from José Fonseca jfons...@vmware.com ---
(In reply to Jonathan Gray from comment #3)
 Created attachment 114359 [details] [review]
 check if compiler supports -Werror=vla
 
 new patch that does a compile check for the flag

configure.ac is basically m4 on top of shell/bash, right?

If so the check must be done  MSVC2013_COMPAT_* vars are referred in the
MSVC2008_COMPAT_* assignment.

Otherwise looks good.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 3.5/5] i965/fs: Add unit tests for Handle CMP.nz ... 0 and AND.nz ... 1 similarly in cmod propagation

2015-03-17 Thread Ian Romanick
From: Ian Romanick ian.d.roman...@intel.com

This change should get squashed with the afore mentioned change.  Tests
suggested by Matt.

Signed-off-by: Ian Romanick ian.d.roman...@intel.com
Cc: Matt Turner matts...@gmail.com
---
 .../drivers/dri/i965/test_fs_cmod_propagation.cpp  | 105 +
 1 files changed, 105 insertions(+), 0 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/test_fs_cmod_propagation.cpp 
b/src/mesa/drivers/dri/i965/test_fs_cmod_propagation.cpp
index cb92abf..8c6ff09 100644
--- a/src/mesa/drivers/dri/i965/test_fs_cmod_propagation.cpp
+++ b/src/mesa/drivers/dri/i965/test_fs_cmod_propagation.cpp
@@ -449,3 +449,108 @@ TEST_F(cmod_propagation_test, 
different_types_cmod_with_zero)
EXPECT_EQ(BRW_OPCODE_CMP, instruction(block0, 1)-opcode);
EXPECT_EQ(BRW_CONDITIONAL_GE, instruction(block0, 1)-conditional_mod);
 }
+
+TEST_F(cmod_propagation_test, andnz_one)
+{
+   fs_reg dest = v-vgrf(glsl_type::int_type);
+   fs_reg src0 = v-vgrf(glsl_type::float_type);
+   fs_reg zero(0.0f);
+   fs_reg one(int(1));
+
+   v-emit(BRW_OPCODE_CMP, retype(dest, BRW_REGISTER_TYPE_F), src0, zero)
+  -conditional_mod = BRW_CONDITIONAL_L;
+   v-emit(BRW_OPCODE_AND, v-reg_null_f, dest, one)
+  -conditional_mod = BRW_CONDITIONAL_NZ;
+
+   /* = Before =
+* 0: cmp.l.f0(8) dest:D  src0:F  0F
+* 1: and.nz.f0(8)null:D  dest:D  1D
+*
+* = After =
+* 0: cmp.l.f0(8) dest:D  src0:F  0F
+*/
+
+   v-calculate_cfg();
+   bblock_t *block0 = v-cfg-blocks[0];
+
+   EXPECT_EQ(0, block0-start_ip);
+   EXPECT_EQ(1, block0-end_ip);
+
+   EXPECT_TRUE(cmod_propagation(v));
+   EXPECT_EQ(0, block0-start_ip);
+   EXPECT_EQ(0, block0-end_ip);
+   EXPECT_EQ(BRW_OPCODE_CMP, instruction(block0, 0)-opcode);
+   EXPECT_EQ(BRW_CONDITIONAL_L, instruction(block0, 0)-conditional_mod);
+   EXPECT_TRUE(retype(dest, BRW_REGISTER_TYPE_F)
+   .equals(instruction(block0, 0)-dst));
+}
+
+TEST_F(cmod_propagation_test, andnz_non_one)
+{
+   fs_reg dest = v-vgrf(glsl_type::int_type);
+   fs_reg src0 = v-vgrf(glsl_type::float_type);
+   fs_reg zero(0.0f);
+   fs_reg nonone(int(38));
+
+   v-emit(BRW_OPCODE_CMP, retype(dest, BRW_REGISTER_TYPE_F), src0, zero)
+  -conditional_mod = BRW_CONDITIONAL_L;
+   v-emit(BRW_OPCODE_AND, v-reg_null_f, dest, nonone)
+  -conditional_mod = BRW_CONDITIONAL_NZ;
+
+   /* = Before =
+* 0: cmp.l.f0(8) dest:D  src0:F  0F
+* 1: and.nz.f0(8)null:D  dest:D  1D
+*
+* = After =
+* (no changes)
+*/
+
+   v-calculate_cfg();
+   bblock_t *block0 = v-cfg-blocks[0];
+
+   EXPECT_EQ(0, block0-start_ip);
+   EXPECT_EQ(1, block0-end_ip);
+
+   EXPECT_FALSE(cmod_propagation(v));
+   EXPECT_EQ(0, block0-start_ip);
+   EXPECT_EQ(1, block0-end_ip);
+   EXPECT_EQ(BRW_OPCODE_CMP, instruction(block0, 0)-opcode);
+   EXPECT_EQ(BRW_CONDITIONAL_L, instruction(block0, 0)-conditional_mod);
+   EXPECT_EQ(BRW_OPCODE_AND, instruction(block0, 1)-opcode);
+   EXPECT_EQ(BRW_CONDITIONAL_NZ, instruction(block0, 1)-conditional_mod);
+}
+
+TEST_F(cmod_propagation_test, andz_one)
+{
+   fs_reg dest = v-vgrf(glsl_type::int_type);
+   fs_reg src0 = v-vgrf(glsl_type::float_type);
+   fs_reg zero(0.0f);
+   fs_reg one(int(1));
+
+   v-emit(BRW_OPCODE_CMP, retype(dest, BRW_REGISTER_TYPE_F), src0, zero)
+  -conditional_mod = BRW_CONDITIONAL_L;
+   v-emit(BRW_OPCODE_AND, v-reg_null_f, dest, one)
+  -conditional_mod = BRW_CONDITIONAL_Z;
+
+   /* = Before =
+* 0: cmp.l.f0(8) dest:D  src0:F  0F
+* 1: and.z.f0(8) null:D  dest:D  1D
+*
+* = After =
+* (no changes)
+*/
+
+   v-calculate_cfg();
+   bblock_t *block0 = v-cfg-blocks[0];
+
+   EXPECT_EQ(0, block0-start_ip);
+   EXPECT_EQ(1, block0-end_ip);
+
+   EXPECT_FALSE(cmod_propagation(v));
+   EXPECT_EQ(0, block0-start_ip);
+   EXPECT_EQ(1, block0-end_ip);
+   EXPECT_EQ(BRW_OPCODE_CMP, instruction(block0, 0)-opcode);
+   EXPECT_EQ(BRW_CONDITIONAL_L, instruction(block0, 0)-conditional_mod);
+   EXPECT_EQ(BRW_OPCODE_AND, instruction(block0, 1)-opcode);
+   EXPECT_EQ(BRW_CONDITIONAL_EQ, instruction(block0, 1)-conditional_mod);
+}
-- 
2.1.0

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH ] i965/aa: fixing anti-aliasing bug for thinnest width lines.

2015-03-17 Thread Ilia Mirkin
On Tue, Mar 17, 2015 at 2:11 PM, Predut, Marius marius.pre...@intel.com wrote:
 -Original Message-
 From: ibmir...@gmail.com [mailto:ibmir...@gmail.com] On Behalf Of Ilia Mirkin
 Sent: Tuesday, March 17, 2015 6:48 PM
 To: Predut, Marius
 Cc: mesa-dev@lists.freedesktop.org
 Subject: Re: [Mesa-dev] [PATCH ] i965/aa: fixing anti-aliasing bug for
 thinnest width lines.

 Can you provide the output of

 git var -l
 marius@marius-pc:~/mesa/src$ git var -l
 user.email=Marius Predut marius.pre...@intel.com
 user.email=marius.pre...@intel.com
 user.name=Marius Predut
 color.ui=auto
 sendemail.smtpserver=smtp.intel.com
 sendemail.signedoffbycc=no
 sendemail.from=marius.pre...@intel.com
 sendemail.to=marius.pre...@intel.com

These are the problem. Remove the .from setting. It's overriding the
defaults. Or set it to the correct thing, i.e. Marius Predut
marius.pre...@intel.com


 I suspect that GIT_AUTHOR_IDENT will differ from whatever's in the
 From: header of the patch being sent.


 I fix this trouble by using  --from Marius Predut marius.pre...@intel.com
 And without space between name and the character (if not it will not work 
 :-))

That's not a great thing to do... just remove sendmail.from and it
should all work. [And yeah, space between name and  required by
RFC822 as I recall... been a while since I've read over it though.]

  -ilia
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 2/2] i965/nir: Re-emit instructions instead of doing mov-to-flag when possible

2015-03-17 Thread Matt Turner
On Mon, Mar 16, 2015 at 9:21 PM, Jason Ekstrand ja...@jlekstrand.net wrote:
 Because of the way that NIR does conditionals, we get them in any old SSA
 value.  The actual boolean value used in the select or if is x != 0.
 Previously, we handled this by emitting a mov.nz to move the value to the
 flag register.  However, this almost always adds at least one if not two
 instructions because we have to go through the VGRF when we could be
 comparing directly to the flag and then using the flag.

 With this commit, we simply re-emit the instruction that produces the value
 when we can.  By doing so, we can use the flag directly and we trust in CSE
 to clean up for us if it can.

 Shader-db results:

 total instructions in shared programs: 4164120 - 4110511 (-1.29%)
 instructions in affected programs: 2397042 - 2343433 (-2.24%)
 helped:13167
 HURT:  31
 GAINED:4
 LOST:  4
 ---

A small change to the cmod_propagation branch accomplishes most of this --

total instructions in shared programs: 7850607 - 7793875 (-0.72%)
instructions in affected programs: 2425140 - 2368408 (-2.34%)
helped:13502
HURT:  0
GAINED:1
LOST:  5

We always emit the MOV.NZ with a D-typed destination, which
cmod_propagate won't combine with a CMP with an F-typed destination.

I don't think we ever emit MOV.NZ without NIR, so no changes without it.

Let me play with this some more.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] Spec bug? Transform feedback after tessellation control shader

2015-03-17 Thread Ian Romanick
On 03/17/2015 11:35 AM, Ilia Mirkin wrote:
 AFAIK you can't have TCS without TES. But you can have TES without TCS.

That much is for sure.  The ARB_tessellation_shader spec says the
following in the overview:

In this extension, patches may not be passed beyond
the tessellation evaluation shader, and an error is generated if an
application provides patches and the current program object contains no
tessellation evaluation shader.

 On Tue, Mar 17, 2015 at 2:33 PM, Marek Olšák mar...@gmail.com wrote:
 Hi,

 The GL 4.5 Core spec is inconsistent. The 11.1.2.1 Output Variables
 section says that transform feedback is allowed after a tessellation
 control shader if a tessellation evaluation shader isn't present:

 Each program object can specify a set of output variables from one
 shader to be recorded in transform feedback mode (see section 13.2).
 The variables that can be recorded are those emitted by the first
 active shader, in order, from the following list:
 * geometry shader
 * tessellation evaluation shader
 * tessellation control shader
 * vertex shader

 Even glTransformFeedbackVaryings allows it:

 If the set of output variables to record in transform feedback mode
 is specified by TransformFeedbackVaryings, a program will fail to link
 if:
 * the count specified by TransformFeedbackVaryings is non-zero, but the
 program object has no vertex, tessellation control, tessellation evaluation, 
 or
 geometry shader;

 However, the 13.2 Transform Feedback section doesn't mention any
 tessellation control shader:

 The data captured in transform feedback mode depends on the active
 programs on each of the shader stages. If a program is active for the
 geometry shader stage, transform feedback captures the vertices of
 each primitive emitted by the geometry shader. Otherwise, if a program
 is active for the tessellation evaluation shader stage, transform
 feedback captures each primitive produced by the tessellation
 primitive generator, whose vertices are processed by the tessellation
 evaluation shader. Otherwise, transform feedback captures each
 primitive processed by the vertex shader.

 Also, glBeginTransformFeedback doesn't allow GL_PATCHES, which is the
 only output primitive type of the tessellation control shader:

 primitiveMode must be TRIANGLES, LINES, or POINTS, and specifies the
 output type of primitives that will be recorded into the buffer
 objects bound for transform feedback

 The question is: Is transform feedback really allowed after the
 tessellation control shader? I hope not, because our hardware can't do
 it.

 Thanks,

 Marek

 PS: The spec mentions tesellation shader twice. There is no
 tesellation shader in OpenGL even if you ignore the bad spelling.
 ___
 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
 

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [Bug 89433] GCC 4.2 does not support -Wvla

2015-03-17 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=89433

--- Comment #5 from José Fonseca jfons...@vmware.com ---
... check must be done _before_ MSVC2013_COMPAT_* ...

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] egl: Remove eglQueryString virtual dispatch.

2015-03-17 Thread Chad Versace
On 03/13/2015 05:59 PM, Matt Turner wrote:

Patch is
Reviewed-by: Chad Versace chad.vers...@intel.com

 ---
 Chad, you suggested it would be nice to remove the locking from
 eglQueryString, but I don't see a way to do it. eglQueryString has
 to generate EGL_NOT_INITIALIZED if the display is valid but not
 initialized, and without locking it seems that there would be a
 race between eglInitialize and eglQueryString. Is that the case,
 or have I misunderstood something?

I still think this can be done without a lock. Currently the function
(like most other EGL functions) holds the display lock for the duration
of the function call. But eglQueryString really only needs to ensure that
(1) the display is initialized and (2) the display remains initialized
until the call returns. (#2 is important because the user could possibly
call eglTerminate in another thread, and eglTerminate has deferred destruction
semantics). I think code like this could achieve that:

  // A new function that atomically increments the display's internal refcount 
only
  // if the display is initialized. Return true if the increment succeeds.
  bool
  _eglDisplayReference(_EGLDisplay *disp);

  // Another new function that pairs with _EGLDisplayReference. Returns
  // the number of remaining references.
  uint32_t
  _eglDisplayRelease(_EGLDisplay *disp);
  
  const char *
  eglQueryString(EGLDisplay dpy, ...)
  {
 const char *string = NULL;

 _EGLDisplay *disp = _eglLookupDisplay(dpy);
 if (!_eglDisplayReference(disp)) {
// emit EGL_NOT_INITIALIZED
return NULL;
 }

 // Set 'string' in the switch statement.

 _eglDisplayRelease(disp);
 return string;
  }

To remove any races with eglTerminate, you would also need to move the 
destruction
code from egTerminate into _eglDisplayRelease and rewrite eglTerminate to
essentially just call _eglDisplayRelease.

In this scheme, lockless EGL functions would follow this pattern:
  1. Try to take a reference on the display. The reference will be held for the 
duration
 of the function call.
  2. Do the action.
  3. Release the reference and return.

I admit it's a non-trivial amount of work. But doing that work would lay some 
groundwork
for removing the display lock in other places too, even in functions that do 
real work like
eglSwapBuffers.

The bigger question is: Do we care enough to do all that work? And I think the 
answer remains
no until we find real-word uses of multithreaded EGL where lock contention 
hurts the app.
I don't know of any multithreaded EGL apps at the moment. Even so, I believe we 
should take
care when writing new EGL code to not dig ourselves into a deeper hole.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] Spec bug? Transform feedback after tessellation control shader

2015-03-17 Thread Marek Olšák
Did you mean Khronos Public Bugzilla?

Marek

On Tue, Mar 17, 2015 at 7:58 PM, Ian Romanick i...@freedesktop.org wrote:
 On 03/17/2015 11:33 AM, Marek Olšák wrote:
 Hi,

 The GL 4.5 Core spec is inconsistent. The 11.1.2.1 Output Variables
 section says that transform feedback is allowed after a tessellation
 control shader if a tessellation evaluation shader isn't present:

 Each program object can specify a set of output variables from one
 shader to be recorded in transform feedback mode (see section 13.2).
 The variables that can be recorded are those emitted by the first
 active shader, in order, from the following list:
 * geometry shader
 * tessellation evaluation shader
 * tessellation control shader
 * vertex shader

 Even glTransformFeedbackVaryings allows it:

 If the set of output variables to record in transform feedback mode
 is specified by TransformFeedbackVaryings, a program will fail to link
 if:
 * the count specified by TransformFeedbackVaryings is non-zero, but the
 program object has no vertex, tessellation control, tessellation evaluation, 
 or
 geometry shader;

 However, the 13.2 Transform Feedback section doesn't mention any
 tessellation control shader:

 The data captured in transform feedback mode depends on the active
 programs on each of the shader stages. If a program is active for the
 geometry shader stage, transform feedback captures the vertices of
 each primitive emitted by the geometry shader. Otherwise, if a program
 is active for the tessellation evaluation shader stage, transform
 feedback captures each primitive produced by the tessellation
 primitive generator, whose vertices are processed by the tessellation
 evaluation shader. Otherwise, transform feedback captures each
 primitive processed by the vertex shader.

 Also, glBeginTransformFeedback doesn't allow GL_PATCHES, which is the
 only output primitive type of the tessellation control shader:

 primitiveMode must be TRIANGLES, LINES, or POINTS, and specifies the
 output type of primitives that will be recorded into the buffer
 objects bound for transform feedback

 And to make matters worse, the ARB_tessellation_shader spec has the
 following issue.  The resolution of this issue seems to contradict the
 rest of the extension spec.

 (8) Should coupling of tessellation control and evaluation shaders be
 required?

   RESOLVED:  No.  A tessellation control shader without an evaluation
   shader might be used in conjunction with transform feedback to generate
   regular transformed patches.  Also, if the set of patches provided by
   the application is already in a form usable by the tessellator, the
   tessellation control shader may be bypassed.  In this use case, the
   application would be required to provide default tessellation levels via
   the PatchParameterfv API, since no shader would be available to compute
   them.

   It may be useful to have a patch produced by a tessellation control
   shader to be fed directly to a geometry shader that performs some
   operation on full patches, rather than individual triangles of a
   tessellated patch.  However, such capability is not provided in this
   extension.

 This definitely smells like a spec bug.  You should submit a bug in the
 internal Khronos bugzilla and CC my @intel address.

 The question is: Is transform feedback really allowed after the
 tessellation control shader? I hope not, because our hardware can't do
 it.

 Thanks,

 Marek

 PS: The spec mentions tesellation shader twice. There is no
 tesellation shader in OpenGL even if you ignore the bad spelling.
 ___
 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


[Mesa-dev] [Bug 89616] bufferobj.c:1639:7: error: format not a string literal and no format arguments [-Werror=format-security]

2015-03-17 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=89616

Bug ID: 89616
   Summary: bufferobj.c:1639:7: error: format not a string literal
and no format arguments [-Werror=format-security]
   Product: Mesa
   Version: git
  Hardware: All
OS: Linux (All)
Status: NEW
  Severity: normal
  Priority: medium
 Component: Mesa core
  Assignee: mesa-dev@lists.freedesktop.org
  Reporter: fabio@libero.it
QA Contact: mesa-dev@lists.freedesktop.org
CC: la...@jlekstrand.net

Compile error on current mesa git:
https://launchpadlibrarian.net/200474799/buildlog_ubuntu-vivid-amd64.mesa_10.6~git1503171930.579297~gd~v_BUILDING.txt.gz

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] i965/aa: fixing anti-aliasing bug for thinnest width lines - GEN6

2015-03-17 Thread Predut, Marius
 -Original Message-
 From: Ian Romanick [mailto:i...@freedesktop.org]
 Sent: Tuesday, March 17, 2015 8:27 PM
 To: Predut, Marius; mesa-dev@lists.freedesktop.org
 Subject: Re: [Mesa-dev] [PATCH] i965/aa: fixing anti-aliasing bug for thinnest
 width lines - GEN6
 
 On 03/17/2015 11:29 AM, Marius Predut wrote:
  On SNB and IVB hw, for 1 pixel line thickness or less, the general anti-
 aliasing algorithm give up - garbage line is generated.
  Setting a Line Width of 0.0 specifies the rasterization of the “thinnest”
 (one-pixel-wide), non-antialiased lines.
  Lines rendered with zero Line Width are rasterized using Grid Intersection
 Quantization rules as specified by bspec section 6.3.12.1 Zero-Width
 (Cosmetic) Line Rasterization.
 
 You'll need to re-word wrap the commit message.  You'll get the commit message
 right one of these days. :)

Yes right, also Brian Paul notice it.

 
 Also... when you send out new versions of a patch, please change the patch
 subject to be something like [PATCH v3]   This makes it easier for
 people to know which is the latest version to review.
 
 You should also add notes to the commit message that explain what changed from
 version to version.  For example, this commit message should have something
 like:
 
 v3: Fix = used instead of == in an if-statement.  Noticed by Daniel Stone.
 
 This is helps people know that their review comments have been applied.
  It is also important to do this when the review changes are applied and the
 patch committed without re-sending to the list.  Maintaining history like this
 in commit messages helps us understand code in the future.
 

Yes indeed.

  Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=28832
 
 There are a number of bugs that have been closed as duplicates of this bug.
 Two of these, bug #27007 and bug #60797, have test cases.  Does this fix also
 fix those?

Yes , was fixed.

Ok I try put all defects here.

 
 I also have a more general question:  How are you testing this?  Daniel
 noticed a bug in an earlier version of the patch that piglit should have
 caught.  If you're doing a full piglit run and that didn't catch the previous
 assignment-instead-of-comparison bug, it would be helpful if you could craft a
 test that would detect that bug.  That may be difficult, so don't spend a huge
 amount of time on it.
 

I used the piglit test /bin/line-aa-width -auto
Also I don’t observer pilgit test regressions when I run /piglit-run.py 
tests/quick.py.
I used this https://bugs.freedesktop.org/attachment.cgi?id=33930  test case and 
checked visually.
(an interrupted line rendered)
I used this https://bugs.freedesktop.org/attachment.cgi?id=8675  test case and 
checked visually.
( a triangle for witch a line is missing)

I don't know more about assignment-instead-of-comparison and Daniel noticed.


  Signed-off-by: Marius Predut marius.pre...@intel.com
  ---
   src/mesa/drivers/dri/i965/gen6_sf_state.c |   12 +---
   1 file changed, 9 insertions(+), 3 deletions(-)
 
  diff --git a/src/mesa/drivers/dri/i965/gen6_sf_state.c
  b/src/mesa/drivers/dri/i965/gen6_sf_state.c
  index f9d8d27..1bed444 100644
  --- a/src/mesa/drivers/dri/i965/gen6_sf_state.c
  +++ b/src/mesa/drivers/dri/i965/gen6_sf_state.c
  @@ -367,9 +367,15 @@ upload_sf_state(struct brw_context *brw)
 float line_width =
roundf(CLAMP(ctx-Line.Width, 0.0, ctx-Const.MaxLineWidth));
 uint32_t line_width_u3_7 = U_FIXED(line_width, 7);
  -  /* TODO: line width of 0 is not allowed when MSAA enabled */
  -  if (line_width_u3_7 == 0)
  - line_width_u3_7 = 1;
  +
  +  if (!(multisampled_fbo  ctx-Multisample.Enabled)) {
 
 In Mesa there are often Enabled and _Enabled fields.  The Enabled field is the
 setting made by the application via the OpenGL API.  The _Enabled field means
 that feature in question is actually enabled, and this may depend on other
 state.
 
 In this case, the application may enable multisampling, but multisampling may
 not occur of there is not a multisample buffer.  This means
 gl_multisample_attrib::Enabled would be set but
 gl_multisample_attrib::_Enabled would not.  Instead of open-coding that check,
 just check gl_multisample_attrib::_Enabled directly:
 
   if (!ctx-Multisample._Enabled) {
 
 I actually think it's more clear if you leave the original comment and
 implement this as:
 
   /* Line width of 0 is not allowed when MSAA enabled */
   if (ctx-Multisample._Enabled) {
  if (line_width_u3_7 == 0)
  line_width_u3_7 = 1;
   } else if (ctx-Line.SmoothFlag  ctx-Line.Width = 1.0) {
  /* For lines less than 1 pixel thick, the general
   * anti-aliasing algorithm gives up, and a garbage line is
   * generated.  Setting a Line Width of 0.0 specifies the
   * rasterization of the thinnest (one-pixel-wide),
   * non-antialiased lines.
   *
   * Lines rendered with zero Line Width are rasterized 

Re: [Mesa-dev] [PATCH 3/3] r600g: constify r600_shader_tgsi_instruction lists.

2015-03-17 Thread Marek Olšák
For the series:

Reviewed-by: Marek Olšák marek.ol...@amd.com

Marek

On Mon, Mar 16, 2015 at 3:47 PM, Emil Velikov emil.l.veli...@gmail.com wrote:
 Massive list of constant data. Annotate it as such.

 Signed-off-by: Emil Velikov emil.l.veli...@gmail.com
 ---
  src/gallium/drivers/r600/r600_shader.c | 10 +-
  1 file changed, 5 insertions(+), 5 deletions(-)

 diff --git a/src/gallium/drivers/r600/r600_shader.c 
 b/src/gallium/drivers/r600/r600_shader.c
 index acac89f..28b290a 100644
 --- a/src/gallium/drivers/r600/r600_shader.c
 +++ b/src/gallium/drivers/r600/r600_shader.c
 @@ -283,7 +283,7 @@ struct r600_shader_ctx {
 unsignedtype;
 unsignedfile_offset[TGSI_FILE_COUNT];
 unsignedtemp_reg;
 -   struct r600_shader_tgsi_instruction *inst_info;
 +   const struct r600_shader_tgsi_instruction   *inst_info;
 struct r600_bytecode*bc;
 struct r600_shader  *shader;
 struct r600_shader_src  src[4];
 @@ -316,7 +316,7 @@ struct r600_shader_tgsi_instruction {
  };

  static int emit_gs_ring_writes(struct r600_shader_ctx *ctx, bool ind);
 -static struct r600_shader_tgsi_instruction r600_shader_tgsi_instruction[], 
 eg_shader_tgsi_instruction[], cm_shader_tgsi_instruction[];
 +static const struct r600_shader_tgsi_instruction 
 r600_shader_tgsi_instruction[], eg_shader_tgsi_instruction[], 
 cm_shader_tgsi_instruction[];
  static int tgsi_helper_tempx_replicate(struct r600_shader_ctx *ctx);
  static inline void callstack_push(struct r600_shader_ctx *ctx, unsigned 
 reason);
  static void fc_pushlevel(struct r600_shader_ctx *ctx, int type);
 @@ -7270,7 +7270,7 @@ static int tgsi_umad(struct r600_shader_ctx *ctx)
 return 0;
  }

 -static struct r600_shader_tgsi_instruction r600_shader_tgsi_instruction[] = {
 +static const struct r600_shader_tgsi_instruction 
 r600_shader_tgsi_instruction[] = {
 [TGSI_OPCODE_ARL]   = { ALU_OP0_NOP, tgsi_r600_arl},
 [TGSI_OPCODE_MOV]   = { ALU_OP1_MOV, tgsi_op2},
 [TGSI_OPCODE_LIT]   = { ALU_OP0_NOP, tgsi_lit},
 @@ -7475,7 +7475,7 @@ static struct r600_shader_tgsi_instruction 
 r600_shader_tgsi_instruction[] = {
 [TGSI_OPCODE_LAST]  = { ALU_OP0_NOP, tgsi_unsupported},
  };

 -static struct r600_shader_tgsi_instruction eg_shader_tgsi_instruction[] = {
 +static const struct r600_shader_tgsi_instruction 
 eg_shader_tgsi_instruction[] = {
 [TGSI_OPCODE_ARL]   = { ALU_OP0_NOP, tgsi_eg_arl},
 [TGSI_OPCODE_MOV]   = { ALU_OP1_MOV, tgsi_op2},
 [TGSI_OPCODE_LIT]   = { ALU_OP0_NOP, tgsi_lit},
 @@ -7674,7 +7674,7 @@ static struct r600_shader_tgsi_instruction 
 eg_shader_tgsi_instruction[] = {
 [TGSI_OPCODE_LAST]  = { ALU_OP0_NOP, tgsi_unsupported},
  };

 -static struct r600_shader_tgsi_instruction cm_shader_tgsi_instruction[] = {
 +static const struct r600_shader_tgsi_instruction 
 cm_shader_tgsi_instruction[] = {
 [TGSI_OPCODE_ARL]   = { ALU_OP0_NOP, tgsi_eg_arl},
 [TGSI_OPCODE_MOV]   = { ALU_OP1_MOV, tgsi_op2},
 [TGSI_OPCODE_LIT]   = { ALU_OP0_NOP, tgsi_lit},
 --
 2.3.1

 ___
 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


[Mesa-dev] [PATCH] linker: fix varying linking if SSO program has only gs and fs

2015-03-17 Thread Tapani Pälli
Previously linker did not take in to account case where one would
have only gs and fs (with SSO), patch adds the case by refactoring
code around assign_varying_locations. This makes sure locations for
gs get populated correctly.

This was found with some of the SSO subtests of Martin's upcoming
GetProgramInterfaceiv Piglit test which passes with the patch, no
Piglit regressions.

Signed-off-by: Tapani Pälli tapani.pa...@intel.com
---
 src/glsl/linker.cpp | 32 +++-
 1 file changed, 19 insertions(+), 13 deletions(-)

diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
index 0c44677..9135ca0 100644
--- a/src/glsl/linker.cpp
+++ b/src/glsl/linker.cpp
@@ -2736,10 +2736,19 @@ link_shaders(struct gl_context *ctx, struct 
gl_shader_program *prog)
   goto done;
}
 
-   unsigned first;
-   for (first = 0; first = MESA_SHADER_FRAGMENT; first++) {
-  if (prog-_LinkedShaders[first] != NULL)
-break;
+   unsigned first, last;
+
+   first = MESA_SHADER_STAGES;
+   last = 0;
+
+   /* Determine first and last stage. */
+   for (unsigned i = 0; i  MESA_SHADER_STAGES; i++) {
+  struct gl_shader *sh = prog-_LinkedShaders[i];
+  if (!sh)
+ continue;
+  if (first == MESA_SHADER_STAGES)
+ first = i;
+  last = i;
}
 
if (num_tfeedback_decls != 0) {
@@ -2768,13 +2777,9 @@ link_shaders(struct gl_context *ctx, struct 
gl_shader_program *prog)
 * ensures that inter-shader outputs written to in an earlier stage are
 * eliminated if they are (transitively) not used in a later stage.
 */
-   int last, next;
-   for (last = MESA_SHADER_FRAGMENT; last = 0; last--) {
-  if (prog-_LinkedShaders[last] != NULL)
- break;
-   }
+   int next;
 
-   if (last = 0  last  MESA_SHADER_FRAGMENT) {
+   if (first  MESA_SHADER_FRAGMENT) {
   gl_shader *const sh = prog-_LinkedShaders[last];
 
   if (first == MESA_SHADER_GEOMETRY) {
@@ -2786,13 +2791,14 @@ link_shaders(struct gl_context *ctx, struct 
gl_shader_program *prog)
   * MESA_SHADER_GEOMETRY.
   */
  if (!assign_varying_locations(ctx, mem_ctx, prog,
-   NULL, sh,
+   NULL, prog-_LinkedShaders[first],
num_tfeedback_decls, tfeedback_decls,
prog-Geom.VerticesIn))
 goto done;
   }
 
-  if (num_tfeedback_decls != 0 || prog-SeparateShader) {
+  if (last != MESA_SHADER_FRAGMENT 
+ (num_tfeedback_decls != 0 || prog-SeparateShader)) {
  /* There was no fragment shader, but we still have to assign varying
   * locations for use by transform feedback.
   */
@@ -2814,7 +2820,7 @@ link_shaders(struct gl_context *ctx, struct 
gl_shader_program *prog)
   while (do_dead_code(sh-ir, false))
  ;
}
-   else if (first == MESA_SHADER_FRAGMENT) {
+   else if (first == MESA_SHADER_FRAGMENT  first == last) {
   /* If the program only contains a fragment shader...
*/
   gl_shader *const sh = prog-_LinkedShaders[first];
-- 
2.1.0

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [Bug 88806] nir/nir_constant_expressions.c:2754:15: error: controlling expression type 'unsigned int' not compatible with any generic association type

2015-03-17 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=88806

Jason Ekstrand ja...@jlekstrand.net changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution|--- |FIXED

--- Comment #8 from Jason Ekstrand ja...@jlekstrand.net ---
Then let's close this

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH ] i965/aa: fixing anti-aliasing bug for thinnest width lines.

2015-03-17 Thread Predut, Marius
-Original Message-
From: mesa-dev [mailto:mesa-dev-boun...@lists.freedesktop.org] On Behalf Of 
Matt Turner
Sent: Thursday, March 12, 2015 12:02 AM
To: Ilia Mirkin
Cc: mesa-dev@lists.freedesktop.org
Subject: Re: [Mesa-dev] [PATCH ] i965/aa: fixing anti-aliasing bug for thinnest 
width lines.

On Wed, Mar 11, 2015 at 2:09 PM, Ilia Mirkin imir...@alum.mit.edu wrote:
 On Wed, Mar 11, 2015 at 5:57 PM,  marius.pre...@intel.com wrote:
 From: Marius Predut marius.pre...@intel.com

 Set your email from name correctly in git and then you won't have this
 line in your git send-email results.


 On SNB and IVB hw, for 1 pixel line thickness or less,
 the general anti-aliasing algorithm give up - garbage line is generated.
 Setting a Line Width of 0.0 specifies the rasterization
 of the “thinnest” (one-pixel-wide), non-antialiased lines.
 Lines rendered with zero Line Width are rasterized using
 Grid Intersection Quantization rules as specified by
 bspec section 6.3.12.1 Zero-Width (Cosmetic) Line Rasterization.

 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82668

 This seems like the wrong bug reference...

I'm pretty sure it should be

https://bugs.freedesktop.org/show_bug.cgi?id=28832


yes it is.

thanks Matt


___
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


[Mesa-dev] [Bug 79706] [TRACKER] Mesa regression tracker

2015-03-17 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=79706
Bug 79706 depends on bug 88806, which changed state.

Bug 88806 Summary: nir/nir_constant_expressions.c:2754:15: error: controlling 
expression type 'unsigned int' not compatible with any generic association type
https://bugs.freedesktop.org/show_bug.cgi?id=88806

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution|--- |FIXED

-- 
You are receiving this mail because:
You are the assignee for the bug.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 07/23] mesa: glGetProgramResourceLocationIndex

2015-03-17 Thread Ilia Mirkin
On Tue, Mar 17, 2015 at 5:13 AM, Tapani Pälli tapani.pa...@intel.com wrote:


 On 03/16/2015 08:08 PM, Ilia Mirkin wrote:

 On Fri, Mar 13, 2015 at 4:37 AM, Tapani Pälli tapani.pa...@intel.com
 wrote:
 +/**
 + * Returns output index for dual source blending.
 + */
   GLint GLAPIENTRY
   _mesa_GetProgramResourceLocationIndex(GLuint program, GLenum
 programInterface,
 const GLchar *name)
   {
 -   return -1;
 +   GET_CURRENT_CONTEXT(ctx);
 +   struct gl_shader_program *shProg =
 +  lookup_linked_program(program,
 glGetProgramResourceLocationIndex);
 +
 +   if (!shProg || invalid_array_element_syntax(name))
 +  return -1;
 +
 +   /* From the GL_ARB_program_interface_query spec:
 +*
 +* For GetProgramResourceLocationIndex, programInterface must be
 +* PROGRAM_OUTPUT.
 +*/


 And presumably it must be a program with a fragment shader (which
 might not be there for a no-rast or compute pipeline).


 spec says that -1 is returned:

 If program has been successfully linked but contains no fragment shader,
 no error will be generated but -1 will be returned.

 this is what happens with the implementation.

Can you explain how the current implementation takes care of that?
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH ] i965/aa: fixing anti-aliasing bug for thinnest width lines - GEN7

2015-03-17 Thread marius . predut
From: Marius Predut marius.pre...@intel.com

On SNB and IVB hw, for 1 pixel line thickness or less,
the general anti-aliasing algorithm give up - garbage line is generated.
Setting a Line Width of 0.0 specifies the rasterization
of the “thinnest” (one-pixel-wide), non-antialiased lines.
Lines rendered with zero Line Width are rasterized using
Grid Intersection Quantization rules as specified by
bspec section 6.3.12.1 Zero-Width (Cosmetic) Line Rasterization.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=28832
Signed-off-by: Marius Predut marius.pre...@intel.com
---
 src/mesa/drivers/dri/i965/gen7_sf_state.c |   12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/gen7_sf_state.c 
b/src/mesa/drivers/dri/i965/gen7_sf_state.c
index c9815b0..fbad889 100644
--- a/src/mesa/drivers/dri/i965/gen7_sf_state.c
+++ b/src/mesa/drivers/dri/i965/gen7_sf_state.c
@@ -198,9 +198,15 @@ upload_sf_state(struct brw_context *brw)
   float line_width =
  roundf(CLAMP(ctx-Line.Width, 0.0, ctx-Const.MaxLineWidth));
   uint32_t line_width_u3_7 = U_FIXED(line_width, 7);
-  /* TODO: line width of 0 is not allowed when MSAA enabled */
-  if (line_width_u3_7 == 0)
- line_width_u3_7 = 1;
+
+  if (!(multisampled_fbo  ctx-Multisample.Enabled)) {
+ if (ctx-Line.SmoothFlag  ctx-Line.Width = 1 )
+line_width_u3_7 = 0;
+  } else {
+  if (line_width_u3_7 = 0)
+ line_width_u3_7 = 1;
+  }
+
   dw2 |= line_width_u3_7  GEN6_SF_LINE_WIDTH_SHIFT;
}
if (ctx-Line.SmoothFlag) {
-- 
1.7.9.5

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH ] i965/aa: fixing anti-aliasing bug for thinnest width lines - GEN7

2015-03-17 Thread Daniel Stone
Hi,

On 17 March 2015 at 16:37,  marius.pre...@intel.com wrote:
 --- a/src/mesa/drivers/dri/i965/gen7_sf_state.c
 +++ b/src/mesa/drivers/dri/i965/gen7_sf_state.c
 @@ -198,9 +198,15 @@ upload_sf_state(struct brw_context *brw)
float line_width =
   roundf(CLAMP(ctx-Line.Width, 0.0, ctx-Const.MaxLineWidth));
uint32_t line_width_u3_7 = U_FIXED(line_width, 7);
 -  /* TODO: line width of 0 is not allowed when MSAA enabled */
 -  if (line_width_u3_7 == 0)
 - line_width_u3_7 = 1;
 +
 +  if (!(multisampled_fbo  ctx-Multisample.Enabled)) {
 + if (ctx-Line.SmoothFlag  ctx-Line.Width = 1 )
 +line_width_u3_7 = 0;
 +  } else {
 +  if (line_width_u3_7 = 0)
 + line_width_u3_7 = 1;

You almost certainly meant 'if (line_width_u3 == 0)', rather than an
assignment - surprised the compiler didn't throw a warning here.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH ] i965/aa: fixing anti-aliasing bug for thinnest width lines.

2015-03-17 Thread Ilia Mirkin
Can you provide the output of

git var -l

And the headers of the patch file you're sending (or git show
--format=raw for the commit in question if you're not using patch
files as intermediates)

I suspect that GIT_AUTHOR_IDENT will differ from whatever's in the
From: header of the patch being sent.


On Tue, Mar 17, 2015 at 12:36 PM, Predut, Marius
marius.pre...@intel.com wrote:
 -Original Message-
 From: mesa-dev [mailto:mesa-dev-boun...@lists.freedesktop.org] On Behalf Of 
 Ilia Mirkin
 Sent: Wednesday, March 11, 2015 11:09 PM
 To: Predut, Marius
 Cc: mesa-dev@lists.freedesktop.org
 Subject: Re: [Mesa-dev] [PATCH ] i965/aa: fixing anti-aliasing bug for 
 thinnest width lines.

 On Wed, Mar 11, 2015 at 5:57 PM,  marius.pre...@intel.com wrote:
 From: Marius Predut marius.pre...@intel.com

 Set your email from name correctly in git and then you won't have this line 
 in your git send-email results.

 I have this config :
 email = marius.pre...@intel.com
 name = Marius Predut

 but still the first line appear


 On SNB and IVB hw, for 1 pixel line thickness or less, the general
 anti-aliasing algorithm give up - garbage line is generated.
 Setting a Line Width of 0.0 specifies the rasterization of the
 “thinnest” (one-pixel-wide), non-antialiased lines.
 Lines rendered with zero Line Width are rasterized using Grid
 Intersection Quantization rules as specified by bspec section 6.3.12.1
 Zero-Width (Cosmetic) Line Rasterization.

 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82668

 This seems like the wrong bug reference...

 Signed-off-by: Marius Predut marius.pre...@intel.com
 ---
  src/mesa/drivers/dri/i965/gen6_sf_state.c |   12 +---
  1 file changed, 9 insertions(+), 3 deletions(-)

 diff --git a/src/mesa/drivers/dri/i965/gen6_sf_state.c
 b/src/mesa/drivers/dri/i965/gen6_sf_state.c
 index f9d8d27..1bed444 100644
 --- a/src/mesa/drivers/dri/i965/gen6_sf_state.c
 +++ b/src/mesa/drivers/dri/i965/gen6_sf_state.c
 @@ -367,9 +367,15 @@ upload_sf_state(struct brw_context *brw)
float line_width =
   roundf(CLAMP(ctx-Line.Width, 0.0, ctx-Const.MaxLineWidth));
uint32_t line_width_u3_7 = U_FIXED(line_width, 7);
 -  /* TODO: line width of 0 is not allowed when MSAA enabled */
 -  if (line_width_u3_7 == 0)
 - line_width_u3_7 = 1;
 +
 +  if (!(multisampled_fbo  ctx-Multisample.Enabled)) {
 +if (ctx-Line.SmoothFlag  ctx-Line.Width =1)
 +  line_width_u3_7 = 0;
 +  } else {
 +if (line_width_u3_7 == 0)
 +line_width_u3_7 = 1;
 +  }
 +
dw3 |= line_width_u3_7  GEN6_SF_LINE_WIDTH_SHIFT;
 }
 if (ctx-Line.SmoothFlag) {
 --
 1.7.9.5

 ___
 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
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH ] i965/aa: fixing anti-aliasing bug for thinnest width lines.

2015-03-17 Thread Predut, Marius
-Original Message-
From: mesa-dev [mailto:mesa-dev-boun...@lists.freedesktop.org] On Behalf Of 
Ilia Mirkin
Sent: Wednesday, March 11, 2015 11:09 PM
To: Predut, Marius
Cc: mesa-dev@lists.freedesktop.org
Subject: Re: [Mesa-dev] [PATCH ] i965/aa: fixing anti-aliasing bug for thinnest 
width lines.

On Wed, Mar 11, 2015 at 5:57 PM,  marius.pre...@intel.com wrote:
 From: Marius Predut marius.pre...@intel.com

Set your email from name correctly in git and then you won't have this line in 
your git send-email results.

I have this config :
email = marius.pre...@intel.com
name = Marius Predut

but still the first line appear


 On SNB and IVB hw, for 1 pixel line thickness or less, the general 
 anti-aliasing algorithm give up - garbage line is generated.
 Setting a Line Width of 0.0 specifies the rasterization of the 
 “thinnest” (one-pixel-wide), non-antialiased lines.
 Lines rendered with zero Line Width are rasterized using Grid 
 Intersection Quantization rules as specified by bspec section 6.3.12.1 
 Zero-Width (Cosmetic) Line Rasterization.

 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82668

This seems like the wrong bug reference...

 Signed-off-by: Marius Predut marius.pre...@intel.com
 ---
  src/mesa/drivers/dri/i965/gen6_sf_state.c |   12 +---
  1 file changed, 9 insertions(+), 3 deletions(-)

 diff --git a/src/mesa/drivers/dri/i965/gen6_sf_state.c 
 b/src/mesa/drivers/dri/i965/gen6_sf_state.c
 index f9d8d27..1bed444 100644
 --- a/src/mesa/drivers/dri/i965/gen6_sf_state.c
 +++ b/src/mesa/drivers/dri/i965/gen6_sf_state.c
 @@ -367,9 +367,15 @@ upload_sf_state(struct brw_context *brw)
float line_width =
   roundf(CLAMP(ctx-Line.Width, 0.0, ctx-Const.MaxLineWidth));
uint32_t line_width_u3_7 = U_FIXED(line_width, 7);
 -  /* TODO: line width of 0 is not allowed when MSAA enabled */
 -  if (line_width_u3_7 == 0)
 - line_width_u3_7 = 1;
 +
 +  if (!(multisampled_fbo  ctx-Multisample.Enabled)) {
 +if (ctx-Line.SmoothFlag  ctx-Line.Width =1)
 +  line_width_u3_7 = 0;
 +  } else {
 +if (line_width_u3_7 == 0)
 +line_width_u3_7 = 1;
 +  }
 +
dw3 |= line_width_u3_7  GEN6_SF_LINE_WIDTH_SHIFT;
 }
 if (ctx-Line.SmoothFlag) {
 --
 1.7.9.5

 ___
 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
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH] radeonsi: increase coords array for radeon_llvm_emit_prepare_cube_coords

2015-03-17 Thread Marek Olšák
From: Marek Olšák marek.ol...@amd.com

radeon_llvm_emit_prepare_cube_coords uses coords[4] in some cases (TXB2 etc.)

Discovered by Coverity. Reported by Ilia Mirkin.

Cc: mesa-sta...@lists.freedesktop.org
---
 src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c | 2 +-
 src/gallium/drivers/radeonsi/si_shader.c| 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c 
b/src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c
index d89e2b4..1690194 100644
--- a/src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c
+++ b/src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c
@@ -748,7 +748,7 @@ static void txp_fetch_args(
const struct tgsi_full_instruction * inst = emit_data-inst;
LLVMValueRef src_w;
unsigned chan;
-   LLVMValueRef coords[4];
+   LLVMValueRef coords[5];
 
emit_data-dst_type = LLVMVectorType(bld_base-base.elem_type, 4);
src_w = lp_build_emit_fetch(bld_base, emit_data-inst, 0, TGSI_CHAN_W);
diff --git a/src/gallium/drivers/radeonsi/si_shader.c 
b/src/gallium/drivers/radeonsi/si_shader.c
index de889ed..4dcf756 100644
--- a/src/gallium/drivers/radeonsi/si_shader.c
+++ b/src/gallium/drivers/radeonsi/si_shader.c
@@ -1572,7 +1572,7 @@ static void tex_fetch_args(
const struct tgsi_full_instruction * inst = emit_data-inst;
unsigned opcode = inst-Instruction.Opcode;
unsigned target = inst-Texture.Texture;
-   LLVMValueRef coords[4];
+   LLVMValueRef coords[5];
LLVMValueRef address[16];
int ref_pos;
unsigned num_coords = tgsi_util_get_texture_coord_dim(target, ref_pos);
-- 
2.1.0

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] Revert main: _mesa_cube_level_complete checks NumLayers.

2015-03-17 Thread Jason Ekstrand
Reviewed-by: Jason Ekstrand jason.ekstr...@intel.com

On Thu, Mar 12, 2015 at 12:33 PM, Laura Ekstrand la...@jlekstrand.net wrote:
 This reverts commit 1ee000a0b6737d6c140d4f07b6044908b8ebfdc7.
 Failures with the GLES3 conformance suite and Synmark2 OGLHdrBloom revealed
 that this commit might be in error.  A look at the offended test in GLES3
 conformance suite, NPOT gen mipmap, suggests that NumLayers may not actually
 always be 6 for a cube complete texture with target = GL_TEXTURE_CUBE_MAP.

 Extensive testing with Piglit prior to patch review and upstreaming did not
 reveal this problem because, in the few Piglit tests that test for cube
 completeness, NumLayers = 6.  So it appears that perhaps existing test
 coverage in Piglit is inadequate.
 ---
  src/mesa/main/texobj.c | 4 
  1 file changed, 4 deletions(-)

 diff --git a/src/mesa/main/texobj.c b/src/mesa/main/texobj.c
 index a99b108..e018ab9 100644
 --- a/src/mesa/main/texobj.c
 +++ b/src/mesa/main/texobj.c
 @@ -879,10 +879,6 @@ _mesa_cube_level_complete(const struct gl_texture_object 
 *texObj,
 if (texObj-Target != GL_TEXTURE_CUBE_MAP)
return GL_FALSE;

 -   /* Make sure we have enough image planes for a cube map. */
 -   if (texObj-NumLayers  6)
 -  return GL_FALSE;
 -
 if ((level  0) || (level = MAX_TEXTURE_LEVELS))
return GL_FALSE;

 --
 2.1.0

 ___
 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


Re: [Mesa-dev] [PATCH 07/23] mesa: glGetProgramResourceLocationIndex

2015-03-17 Thread Tapani Pälli



On 03/17/2015 04:20 PM, Ilia Mirkin wrote:

On Tue, Mar 17, 2015 at 5:13 AM, Tapani Pälli tapani.pa...@intel.com wrote:



On 03/16/2015 08:08 PM, Ilia Mirkin wrote:


On Fri, Mar 13, 2015 at 4:37 AM, Tapani Pälli tapani.pa...@intel.com
wrote:

+/**
+ * Returns output index for dual source blending.
+ */
   GLint GLAPIENTRY
   _mesa_GetProgramResourceLocationIndex(GLuint program, GLenum
programInterface,
 const GLchar *name)
   {
-   return -1;
+   GET_CURRENT_CONTEXT(ctx);
+   struct gl_shader_program *shProg =
+  lookup_linked_program(program,
glGetProgramResourceLocationIndex);
+
+   if (!shProg || invalid_array_element_syntax(name))
+  return -1;
+
+   /* From the GL_ARB_program_interface_query spec:
+*
+* For GetProgramResourceLocationIndex, programInterface must be
+* PROGRAM_OUTPUT.
+*/



And presumably it must be a program with a fragment shader (which
might not be there for a no-rast or compute pipeline).



spec says that -1 is returned:

If program has been successfully linked but contains no fragment shader,
no error will be generated but -1 will be returned.

this is what happens with the implementation.


Can you explain how the current implementation takes care of that?



My earlier suggestion for this was that 
_mesa_program_resource_location_index returns -1 because resource cannot 
be found. But now I see that one could call this for any output of any 
stage and then it would likely return 0 or whatever is the uninitialized 
state of ir_variable::data.index. Will have to fix this, thanks for 
spotting this!


// Tapani
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 9/9] i965: Use NIR by default for Fragment shaders on SNB+

2015-03-17 Thread Matt Turner
Thanks. Looked through stats and at some of the regressions.

Some of the areas I noticed we were doing worse:

We generate two CMPs for discard_if; only one without NIR. I think you
had an idea about solving this.

SEL peephole interference -- we knew about this in general, but I
found an interesting failure mode:

NIR generates

(+f0) if(8) JIP: 4  UIP: 5
else(8) JIP: 3
mov(8)  g1241F-g1248,8,1F
endif(8)JIP: 6

We should of course improve dead-control-flow-elimination to remove
the else and set the if's predicate_inverse, but non-NIR generates

(+f0) sel(8)g1241Fg258,8,1F -g258,8,1F

which is probably better even than a predicated MOV since it's not a
partial write and subject to optimization restrictions. The assembly
matches the NIR exactly, so I'm assuming NIR is removing a
self-assignment in the if-then block, preventing it from turning it
into a conditional select.

Also, since the fs's SEL peephole only recognizes MOVs to the same
destination in order, it doesn't handle tropico-5/387 properly, which
already improved from 110 to 79 instructions.

I also noticed an interesting pattern in defense-grid/537 (and some
others, I think). NIR generates

mul(8)  g41Fg28,8,1F  g38,8,1F
mul(8)  g61Fg58,8,1F  g48,8,1F
mad(8)  g1271F  g64,4,1F  g34,4,1F  g24,4,1F

whereas without NIR we get:

mul(8)  g51Fg28,8,1F  g48,8,1F
mad(8)  g1271F  g54,4,1F  g34,4,1F  g54,4,1F

Again, the assembly matches the NIR, so NIR is doing something bad.

The changes to sun-temple/209 (and a lot of other sun temple shaders)
look wrong. I'm not sure if the translation from NIR is doing
something bad or what, but we should have 8 texture fetches and we
only end up with 2.


Some highlights --

Some shaders are improved because NIR recognizes exp2(log(x) * y) as
pow(x, y) when the GLSL optimizations couldn't, because its dead code
elimination is much too stupid to allow tree grafting to put the IR in
the form we expect. This accounts for a low of the high percentage
gains.

It cuts some Kerbal Space Program shaders in half (~1000 - ~500
instructions) and removes ~100 spills and ~100 fills from each. Not
sure how it actually accomplished this. Too hard to read the assembly
with all of the spills.

Code generated by tropico-5/252 is really stupid. We MOVs both before
and in an if statement that do the same thing. NIR cleans that up
easily as a side-effect of SSA.

In strike-suit-zero/156 we emit more MADs that we couldn't recognize
from the GLSL IR, since again, its DCE is terrible and we can't
combine trees.

In the-binding-of-isaac-rebirth/18, it's able to CSE some duplicate
rcp expressions in different basic blocks. I'm not sure why the
backend isn't doing this.


At this point I've looked through all of the shaders helped by 15% or more.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


  1   2   >