[Mesa-dev] [PATCH] glsl: Remove CSE pass.

[Kenneth Graunke]

On Saturday, October 03, 2015 10:57:38 AM Matt Turner wrote:
> With NIR, it actually hurts things.
> 
> total instructions in shared programs: 6529329 -> 6528888 (-0.01%)
> instructions in affected programs:     14833 -> 14392 (-2.97%)
> helped:                                299
> HURT:                                  1
> 
> In all affected programs I inspected (including the single hurt one) the
> pass CSE'd some multiplies and caused some reassociation (e.g., caused
> (A * B) * C to be A * (B * C)) when the original intermediate result was
> reused elsewhere.
> ---
>  src/glsl/Makefile.sources       |   1 -
>  src/glsl/glsl_parser_extras.cpp |   1 -
>  src/glsl/ir_optimization.h      |   1 -
>  src/glsl/opt_cse.cpp            | 472 ----------------------------------------
>  4 files changed, 475 deletions(-)
>  delete mode 100644 src/glsl/opt_cse.cpp

FWIW, I'm in favor of removing this pass.
Acked-by: Kenneth Graunke <kenneth at whitecape.org>

In general, I think it makes a lot of sense to introduce a boolean for
"I don't want GLSL IR optimizations, NIR/LLVM/NVC/whatever is going to
take care of that for me later."...and put more and more passes behind
it.

But, this pass has serious problems.  One example is that, given two
large identical expression trees, it will start at the bottom up, and
CSE two operations.  But the new CSE temporaries won't be marked read
only...and the pass bails on non-read-only things.  So it then won't
be able to CSE the rest of the tree.  Shoots itself in the foot.

The other thing is that the primary motivation for the pass was to
optimize repeated texturing with the same coordinates.  The fact that it
only handles constant/read-only coordinates really limits this - who
does repeated texturing with identical *constant* coordinates?  The old
Unigine Tropics demo, of course...but I doubt anybody cares about
running that on old hardware.  (Their newer stuff is more reasonable,
thankfully...)

I'd be in favor of keeping a solid CSE pass behind a flag, but this is
not that.  Let's drop it.

--Ken
-------------- 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/20151005/e18f0a35/attachment.sig>