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 <kenn...@whitecape.org>
> >>> Cc: mesa-sta...@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.
>
> BRAhh, 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.
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
