[Mesa-dev] [PATCH] i965/fs: Remove try_replace_with_sel().

[Kenneth Graunke]

On Friday, November 21, 2014 10:23:43 AM Matt Turner wrote:
> On Tue, Nov 11, 2014 at 9:41 AM, Matt Turner <mattst88 at gmail.com> wrote:
> > The rest of our backend optimizations have replaced the need for this
> > since it was written.
> >
> > instructions in affected programs:     30626 -> 30564 (-0.20%)
> >
> > Hurts a small number of CSGO shaders by one instruction, but helps even
> > more. Hurts two by a larger number because of something I noticed when I
> > first wrote the SEL peephole: try_replace_with_sel() operates on
> > instructions before we've demoted uniforms to pull constants. So code
> > like
> >
> >    var.x = ( -abs(r6.w) >= 0.0 ) ? pc[82].x : r9.x;
> >    var.y = ( -abs(r6.w) >= 0.0 ) ? pc[82].y : r9.y;
> >    var.z = ( -abs(r6.w) >= 0.0 ) ? pc[82].z : r9.z;
> >    var.w = ( -abs(r6.w) >= 0.0 ) ? pc[82].w : r9.w;
> >
> > where pc[82] gets demoted to a pull constant, we end up emitting a
> > send(4) instruction to load pc[82] each time, and since they're in
> > different basic blocks because we mishandle the ternary operator in this
> > case we can't combine them. Once we handle this common ternary pattern
> > better the problem will go away.
> > ---
> 
> Thoughts?

I don't know...if I'm reading the above text correctly, this doesn't look
compelling.  Your argument for deleting this is "it's not necessary anymore",
but you go on to undermine that by saying that it hurts a few shaders, and
even quadrouples the number of pull loads in a few cases...

Sure, it'll get fixed if we handle ternary operations better...but we haven't
yet...so...

I'm pretty confused.  Maybe I'm misreading your justification...
-------------- 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: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20141204/cd45b3c0/attachment.sig>