On 06/23/2016 03:30 PM, Ilia Mirkin wrote: > On Thu, Jun 23, 2016 at 6:07 PM, Ian Romanick <i...@freedesktop.org> wrote: >> On 06/16/2016 12:12 PM, Ilia Mirkin wrote: >>> On Thu, Jun 16, 2016 at 3:06 PM, Ian Romanick <i...@freedesktop.org> wrote: >>>> From: Ian Romanick <ian.d.roman...@intel.com> >>>> >>>> Outputs from the vertex shader need to be able to match >>>> per-vertex-arrayed inputs of later stages. Acomplish this by stripping >>>> one level of arrayness from the names and types of outputs going to a >>>> per-vertex-arrayed stage. >>>> >>>> v2: Add missing checks for TESS_EVAL->GEOMETRY. Noticed by Timothy >>>> Arceri. >>>> >>>> Signed-off-by: Ian Romanick <ian.d.roman...@intel.com> >>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96358 >>>> Cc: "12.0" <mesa-sta...@lists.freedesktop.org> >>>> Cc: Gregory Hainaut <gregory.hain...@gmail.com> >>>> Cc: Ilia Mirkin <imir...@alum.mit.edu> >>>> --- >>>> src/mesa/main/shader_query.cpp | 100 >>>> +++++++++++++++++++++++++++++++++++++---- >>>> 1 file changed, 92 insertions(+), 8 deletions(-) >>>> >>>> diff --git a/src/mesa/main/shader_query.cpp >>>> b/src/mesa/main/shader_query.cpp >>>> index 5956ce4..a6a0a2f 100644 >>>> --- a/src/mesa/main/shader_query.cpp >>>> +++ b/src/mesa/main/shader_query.cpp >>>> @@ -1385,13 +1385,26 @@ _mesa_get_program_resourceiv(struct >>>> gl_shader_program *shProg, >>>> >>>> static bool >>>> validate_io(struct gl_shader_program *producer, >>>> - struct gl_shader_program *consumer) >>>> + struct gl_shader_program *consumer, >>>> + gl_shader_stage producer_stage, >>>> + gl_shader_stage consumer_stage) >>>> { >>>> if (producer == consumer) >>>> return true; >>>> >>>> + const bool nonarray_stage_to_array_stage = >>>> + (producer_stage == MESA_SHADER_VERTEX && >>>> + (consumer_stage == MESA_SHADER_GEOMETRY || >>>> + consumer_stage == MESA_SHADER_TESS_CTRL || >>>> + consumer_stage == MESA_SHADER_TESS_EVAL)) || >>>> + (producer_stage == MESA_SHADER_TESS_EVAL && >>>> + consumer_stage == MESA_SHADER_GEOMETRY); >>> >>> Since TCS -> GEOM is not possible, isn't the only way that array -> >>> array can happen is TCS -> TES? IOW, couldn't this just be >>> >>> producer != TCS && (consumer == TCS || consumer == TES || consumer == GS) >>> >>> Your call whether you want to rewrite it. [No comment on the rest of >>> the patch...] >> >> I had though of another (slightly less) clever way to do this, but I >> decided against it because it was less clear. >> >> Anyway, I tried your suggestion. Here's the assembly 'diff >> --side-by-side'. Mine is on the left, yours is on the right. >> >> mov 0x4(%rax),%eax mov 0x4(%rax),%eax >> cmp %rbp,%rbx cmp %rbp,%rbx >> je 374c | je 3744 >> test %eax,%eax | movb $0x0,0x1f(%rsp) >> jne 357b | cmp $0x1,%eax >> lea -0x1(%rdx),%ecx | je 357f >> movb $0x1,0x1f(%rsp) | sub $0x1,%edx >> cmp $0x2,%ecx | cmp $0x2,%edx >> jbe 358d | setbe 0x1f(%rsp) >> cmp $0x2,%eax < >> sete %cl < >> cmp $0x3,%edx < >> sete %al < >> and %eax,%ecx < >> mov %cl,0x1f(%rsp) < >> >> I don't know that we'd be able to measure a performance difference. >> Maybe having one less branch that may not predict well is good? *shrug* > > A little intense that you looked at the asm... my argument was mostly > for the sake of simplicity and readability. If you don't think my > version is easier to understand, then keep yours.
Heh... I just thought it was due diligence. :) > -ilia > _______________________________________________ > mesa-stable mailing list > mesa-sta...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-stable > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev