[Mesa-stable] [Mesa-dev] [PATCH 2/5] glsl: Optionally lower TES gl_PatchVerticesIn to a uniform.

[Kenneth Graunke]

On Thursday, June 9, 2016 9:00:43 AM PDT Alejandro Piñeiro wrote:
> On 09/06/16 05:50, Kenneth Graunke wrote:
> > On Wednesday, June 8, 2016 7:45:16 PM PDT Alejandro Piñeiro wrote:
> >> On 02/06/16 23:09, Kenneth Graunke wrote:
> >>> i965 has no special hardware for this, so we need to pass this value in
> >>> as a uniform (unless the TES is linked against a TCS, in which case the
> >>> linker can just replace this with a constant).
> >>>
> >>> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> >>> Cc: mesa-stable at lists.freedesktop.org
> >>> ---
> >>>  src/compiler/glsl/linker.cpp      | 20 +++++++++++++++++---
> >>>  src/mesa/main/mtypes.h            |  1 +
> >>>  src/mesa/program/prog_statevars.c |  7 +++++++
> >>>  src/mesa/program/prog_statevars.h |  1 +
> >>>  4 files changed, 26 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp
> >>> index 9e65590..7c7dbfe 100644
> >>> --- a/src/compiler/glsl/linker.cpp
> >>> +++ b/src/compiler/glsl/linker.cpp
> >>> @@ -72,6 +72,7 @@
> >>>  #include "ir.h"
> >>>  #include "program.h"
> >>>  #include "program/hash_table.h"
> >>> +#include "program/prog_instruction.h"
> >>>  #include "linker.h"
> >>>  #include "link_varyings.h"
> >>>  #include "ir_optimization.h"
> >>> @@ -2485,7 +2486,7 @@ resize_tes_inputs(struct gl_context *ctx,
> >>>        ir->accept(&input_resize_visitor);
> >>>     }
> >>>  
> >>> -   if (tcs) {
> >>> +   if (tcs || ctx->Const.LowerTESPatchVerticesIn) {
> >>>        /* Convert the gl_PatchVerticesIn system value into a constant, since
> >>>         * the value is known at this point.
> >> Now this comment became obsolete as can be converted to a constant or a
> >> uniform. Perhaps it would be better to just remove the comment.
> >>
> >>>         */
> >>> @@ -2494,9 +2495,22 @@ resize_tes_inputs(struct gl_context *ctx,
> >>>           if (var && var->data.mode == ir_var_system_value &&
> >>>               var->data.location == SYSTEM_VALUE_VERTICES_IN) {
> >>>              void *mem_ctx = ralloc_parent(var);
> >>> -            var->data.mode = ir_var_auto;
> >>>              var->data.location = 0;
> >>> -            var->constant_value = new(mem_ctx) ir_constant(num_vertices);
> >>> +            var->data.explicit_location = false;
> >> Nitpick: With this change you are also changing slightly the option that
> >> was present before (use a constant) in addition to add the option to
> >> lower to an uniform. Not sure if it is worth to be mentioned though,
> >> feel free to ignore.
> > I am?  Before this patch, there was no option.  If the program being
> > linked contained a TCS, we would turn it into a constant.  If not, we
> > would leave it as a system value.
> >
> > With this patch, my intention was to do the same behavior when no
> > options were set.  If ctx->Const.LowerTESPatchVerticesIn is set, we
> > turn it into a constant if there's a linked TCS (as before), and turn
> > it into a uniform otherwise (instead of a system value).
> >
> > Did I miss something?
> 
> Yes, the behaviour without options set would be the same. The slight
> change I was mentioning, is that on the line Im pointing, you are
> setting data.explicit location to false on both cases (uniform and
> constant). Before this patch, that only have the constant option,
> data.explicit_location was not assigned.
> 
> BR

Ahh, good catch!  Right, that is a change.

I guess I accidentally left explicit_location set on the auto variable
in my earlier constant-only code.  Presumably this worked out because
we never looked at explicit_location for local variables, as it isn't
meaningful.  It seems desirable to clear it in both cases.

Thanks for catching this.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.freedesktop.org/archives/mesa-stable/attachments/20160615/78800cf8/attachment.sig>