<div dir="ltr">HI Ilia <div class="gmail_extra"> <div class="gmail_quote">2015-08-14 1:31 GMT-03:00 Ilia Mirkin <<a href="mailto:imirkin@alum.mit.edu" target="_blank">imirkin@alum.mit.edu</a>>: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Fri, Aug 14, 2015 at 12:25 AM, Marcos Souza <<a href="mailto:marcos.souza.org@gmail.com">marcos.souza.org@gmail.com</a>> wrote: > Hi Ilia, > > 2015-08-14 1:02 GMT-03:00 Ilia Mirkin <<a href="mailto:imirkin@alum.mit.edu">imirkin@alum.mit.edu</a>>: >> >> On Thu, Aug 13, 2015 at 11:55 PM, Marcos Souza >> <<a href="mailto:marcos.souza.org@gmail.com">marcos.souza.org@gmail.com</a>> wrote: >> > Hi Ilia, >> > >> > So i found the point here it addrs that double brackets, and the >> > following >> > patch solves it, but this is a right solution? If someone could guide me >> > here, I could fix it :) >> > >> > diff --git a/src/gallium/auxiliary/tgsi/tgsi_dump.c >> > b/src/gallium/auxiliary/tgsi/tgsi_dump.c >> > index 8ceb5b4..046471e 100644 >> > --- a/src/gallium/auxiliary/tgsi/tgsi_dump.c >> > +++ b/src/gallium/auxiliary/tgsi/tgsi_dump.c >> > @@ -302,10 +302,14 @@ iter_declaration( >> > TXT("[]"); >> > } >> > >> > - if (decl->Declaration.Dimension) { >> >> The issue is that the declaration is getting a dimension set by the >> parser, which in turn is causing it to print funny. It shouldn't be >> getting a dimension in the first place for those. > > > The following patch fix the problem, is it the right place to put it? I don't think so. Just glanced at the code, look at parse_register_dcl /* for geometry shader we don't really care about * the first brackets it's always the size of the * input primitive. so we want to declare just * the index relevant to the semantics which is in * the second bracket */ if (ctx->processor == TGSI_PROCESSOR_GEOMETRY && *file == TGSI_FILE_INPUT) { brackets[0] = brackets[1]; *num_brackets = 1; } Basically you need to extend this logic to similarly exclude (a) tess ctrl inputs and outputs (b) tess eval inputs Technically you need to exclude patch/tessinner/tessouter from that, but in practice they won't have an extra set of brackets either. <div class="HOEnZb"><div class="h5"> </div></div></blockquote><div> </div><div>Sorry for flooding the list, but I'm relaly excited about it :) </div><div>So, this is the change you asked. It also solved the problem: diff --git a/src/gallium/auxiliary/tgsi/tgsi_text.c b/src/gallium/auxiliary/tgsi/tgsi_text.c index 8647e4e..95c1daf 100644 --- a/src/gallium/auxiliary/tgsi/tgsi_text.c +++ b/src/gallium/auxiliary/tgsi/tgsi_text.c @@ -684,7 +684,12 @@ parse_register_dcl( * input primitive. so we want to declare just * the index relevant to the semantics which is in * the second bracket */ - if (ctx->processor == TGSI_PROCESSOR_GEOMETRY && *file == TGSI_FILE_INPUT) { + + /* similarly from tessalation */ + int exclude = (ctx->processor == TGSI_PROCESSOR_TESS_EVAL && *file == TGSI_FILE_INPUT) || + (ctx->processor == TGSI_PROCESSOR_TESS_CTRL && (*file == TGSI_FILE_INPUT || + *file == TGSI_FILE_OUTPUT)); + if ((ctx->processor == TGSI_PROCESSOR_GEOMETRY && *file == TGSI_FILE_INPUT) || exclude) { brackets[0] = brackets[1]; *num_brackets = 1; } else { </div><div>What do you think Ilia? </div><div>Thanks! </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5"> > > diff --git a/src/gallium/auxiliary/tgsi/tgsi_text.c > b/src/gallium/auxiliary/tgsi/tgsi_text.c > index 8647e4e..f734d58 100644 > --- a/src/gallium/auxiliary/tgsi/tgsi_text.c > +++ b/src/gallium/auxiliary/tgsi/tgsi_text.c > @@ -1185,7 +1185,10 @@ static boolean parse_declaration( struct > translate_ctx *ctx ) > decl.Range.First = brackets[1].first; > decl.Range.Last = brackets[1].last; > > - decl.Declaration.Dimension = 1; > + if (!(ctx->processor == TGSI_PROCESSOR_TESS_CTRL || > + ctx->processor == TGSI_PROCESSOR_TESS_EVAL)) > + decl.Declaration.Dimension = 1; > + > decl.Dim.Index2D = brackets[0].first; > } > > > >> >> >> > - CHR('['); >> > - SID(decl->Dim.Index2D); >> > - CHR(']'); >> > + /* FIXME: patched version could have tree dimensions?? */ >> > + if (patch && (iter->processor.Processor == TGSI_PROCESSOR_TESS_CTRL >> > || >> > + iter->processor.Processor == TGSI_PROCESSOR_TESS_EVAL)) { >> > + if (decl->Declaration.Dimension) { >> > + CHR('['); >> > + SID(decl->Dim.Index2D); >> > + CHR(']'); >> > + } >> > } >> > >> > After this patch, tess_eval output is the same before and after, but >> > tess_ctrl is a little different: >> > [marcos@x mesa]$ diff tess_ctrl tess_ctrl_new >> > 29c29 >> > < 15: LRP OUT[ADDR[1].x][3], TEMP[1].xxxx, TEMP[3], TEMP[2] >> > --- >> >> 15: LRP OUT[0][3], TEMP[1].xxxx, TEMP[3], TEMP[2] >> > 40c40 >> > < 26: MOV OUT[ADDR[1].x][2], TEMP[0] >> > --- >> >> 26: MOV OUT[0][2], TEMP[0] >> > >> > I'll try to investigate and send a new patch in the weekend. >> > >> > Thanks for all help Ilia and others! >> > >> > 2015-08-13 18:43 GMT-03:00 Ilia Mirkin <<a href="mailto:imirkin@alum.mit.edu">imirkin@alum.mit.edu</a>>: >> >> >> >> [mesa-dev readded, please don't drop CC's] >> >> >> >> I found it by feeding the shader to nouveau_compiler with >> >> NV50_PROG_DEBUG=1 set, which dumps the input tgsi. Those two should >> >> match up. >> >> >> >> On Thu, Aug 13, 2015 at 5:39 PM, Marcos Paulo de souza >> >> <<a href="mailto:marcos.souza.org@gmail.com">marcos.souza.org@gmail.com</a>> wrote: >> >> > Hi Ilia, >> >> > >> >> > So, how can I test it? Do I need to especify some patameter to verify >> >> > this >> >> > type of problem? >> >> > >> >> > Thanks for the quick revision! >> >> > >> >> > >> >> > Em 13-08-2015 16:03, Ilia Mirkin escreveu: >> >> >> >> >> >> Hi Macros, >> >> >> >> >> >> Looks like it's not parsed in exactly right. It will parse something >> >> >> like >> >> >> >> >> >> TESS_EVAL >> >> >> PROPERTY TES_PRIM_MODE 7 >> >> >> PROPERTY TES_SPACING 2 >> >> >> PROPERTY TES_VERTEX_ORDER_CW 0 >> >> >> PROPERTY TES_POINT_MODE 0 >> >> >> DCL IN[][0], GENERIC[0] >> >> >> DCL IN[][1], GENERIC[1] >> >> >> >> >> >> as >> >> >> >> >> >> TESS_EVAL >> >> >> PROPERTY TES_PRIM_MODE 7 >> >> >> PROPERTY TES_SPACING 2 >> >> >> PROPERTY TES_VERTEX_ORDER_CW 0 >> >> >> PROPERTY TES_POINT_MODE 0 >> >> >> DCL IN[][0][0], GENERIC[0] >> >> >> DCL IN[][0][1], GENERIC[1] >> >> >> >> >> >> Perhaps the same issue happens for geometry shaders, but that >> >> >> doesn't >> >> >> make it right :) You might have to look at the printing logic to get >> >> >> a >> >> >> better understanding of what's going wrong. >> >> >> >> >> >> Also you should send patches to nouveau separately from patches to >> >> >> the >> >> >> rest of the infra. Ideally this would have been 2 patches, e.g. >> >> >> >> >> >> tgsi: set implicit array size for tess stages >> >> >> nouveau: recognize tess stages in nouveau_compiler >> >> >> >> >> >> or something like that. >> >> >> >> >> >> On Wed, Aug 12, 2015 at 9:25 PM, Marcos Paulo de Souza >> >> >> <<a href="mailto:marcos.souza.org@gmail.com">marcos.souza.org@gmail.com</a>> wrote: >> >> >>> >> >> >>> From: Marcos Paulo de Souza <<a href="mailto:marcos.souza.org@gmail.com">marcos.souza.org@gmail.com</a>> >> >> >>> >> >> >>> Signed-off-by: Marcos Paulo de Souza <<a href="http://marcos.souza.org" rel="noreferrer" target="_blank">marcos.souza.org</a>> >> >> >>> Suggested-by: Ilia Mirkin <<a href="mailto:imirkin@alum.mit.edu">imirkin@alum.mit.edu</a>> >> >> >>> --- >> >> >>> src/gallium/auxiliary/tgsi/tgsi_text.c | 6 +++++- >> >> >>> src/gallium/drivers/nouveau/nouveau_compiler.c | 4 ++++ >> >> >>> 2 files changed, 9 insertions(+), 1 deletion(-) >> >> >>> >> >> >>> diff --git a/src/gallium/auxiliary/tgsi/tgsi_text.c >> >> >>> b/src/gallium/auxiliary/tgsi/tgsi_text.c >> >> >>> index a6675c5..8647e4e 100644 >> >> >>> --- a/src/gallium/auxiliary/tgsi/tgsi_text.c >> >> >>> +++ b/src/gallium/auxiliary/tgsi/tgsi_text.c >> >> >>> @@ -259,7 +259,7 @@ struct translate_ctx >> >> >>> struct tgsi_token *tokens_end; >> >> >>> struct tgsi_header *header; >> >> >>> unsigned processor : 4; >> >> >>> - int implied_array_size : 5; >> >> >>> + int implied_array_size : 6; >> >> >>> unsigned num_immediates; >> >> >>> }; >> >> >>> >> >> >>> @@ -1623,6 +1623,10 @@ static boolean translate( struct >> >> >>> translate_ctx >> >> >>> *ctx ) >> >> >>> if (!parse_header( ctx )) >> >> >>> return FALSE; >> >> >>> >> >> >>> + if (ctx->processor == TGSI_PROCESSOR_TESS_CTRL || >> >> >>> + ctx->processor == TGSI_PROCESSOR_TESS_EVAL) >> >> >>> + ctx->implied_array_size = 32 ; >> >> >>> + >> >> >>> while (*ctx->cur != '\0') { >> >> >>> uint label_val = 0; >> >> >>> if (!eat_white( &ctx->cur )) { >> >> >>> diff --git a/src/gallium/drivers/nouveau/nouveau_compiler.c >> >> >>> b/src/gallium/drivers/nouveau/nouveau_compiler.c >> >> >>> index 8660498..495450b 100644 >> >> >>> --- a/src/gallium/drivers/nouveau/nouveau_compiler.c >> >> >>> +++ b/src/gallium/drivers/nouveau/nouveau_compiler.c >> >> >>> @@ -190,6 +190,10 @@ main(int argc, char *argv[]) >> >> >>> type = PIPE_SHADER_GEOMETRY; >> >> >>> else if (!strncmp(text, "COMP", 4)) >> >> >>> type = PIPE_SHADER_COMPUTE; >> >> >>> + else if (!strncmp(text, "TESS_CTRL", 9)) >> >> >>> + type = PIPE_SHADER_TESS_CTRL; >> >> >>> + else if (!strncmp(text, "TESS_EVAL", 9)) >> >> >>> + type = PIPE_SHADER_TESS_EVAL; >> >> >>> else { >> >> >>> _debug_printf("Unrecognized TGSI header\n"); >> >> >>> return 1; >> >> >>> -- >> >> >>> 2.4.3 >> >> >>> >> >> >>> _______________________________________________ >> >> >>> mesa-dev mailing list >> >> >>> <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> >> >> >>> <a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> >> >> > >> >> > >> > >> > >> > >> > >> > -- >> > Att, >> > >> > Marcos Paulo de Souza >> > Github: <a href="https://github.com/marcosps/" rel="noreferrer" target="_blank">https://github.com/marcosps/</a> > > > > > -- > Att, > > Marcos Paulo de Souza > Github: <a href="https://github.com/marcosps/" rel="noreferrer" target="_blank">https://github.com/marcosps/</a> </div></div></blockquote></div> -- <div class="gmail_signature"><div dir="ltr">Att, Marcos Paulo de Souza Github: <a href="https://github.com/marcosps/" target="_blank">https://github.com/marcosps/</a> </div></div> </div></div>