On Wed, Nov 25, 2015 at 5:36 AM, Juan A. Suarez Romero
<jasua...@igalia.com> wrote:
>
> When checking output VS in glsl-mat-from-int-ctor-03 piglit, I got the
> following
> (part of) code.
>
> mov(8) g19<1>.xyzF g6<4,4,1>.xyzzD { align16
> 1Q };
> dp4(8) g115<1>.wF g4<4,4,1>F g2.4<0,4,1>F { align16
> NoDDChk 1Q };
> cmp.nz.f0(8) null<1>F g11<4,4,1>.xyzzF g19<4,4,1>.xyzzF {
> align16 1Q switch };
> cmp.nz.f0(8) null<1>D g7<4,4,1>D 0D { align16
> 1Q switch };
> (+f0.any4h) mov(8) g21<1>.xUD 0xffffffffUD { align16
> 1Q };
>
> Clearly the first cmp can be removed because the result is overwritten by the
> second one.
>
> Investigating why this is not happening, saw that in brw_vec4, after running
> opt_vector_float(), we are running optimizations just once, instead of in a
> loop
> until no progress happens.
>
> Not sure if there is a reason to keep it separated from the previous
> loop. Tracking back the code seems that originally it wasn't added because
> opt_vector_float() wasn't written as an optimization, and no optimizations
> were
> done after running it. But later someone suggested to run some optimizations
> if
> opt_vector_float() success, which made to add a conditional. In final commits,
> that opt_vector_float() was converted in a true optimiztaion, but still kept
> out
> of the loop. At this point I'm not sure if there was a good reason (no
> explanation found) or just to make the less changes in code. Maybe someone can
> bring light here.
>
> So merged those optimizations inside the previous loop (second commit). But
> this
> made some piglit tests to never end. Checking about this, saw that some
> optimizations were been reverted by others (specifically CSE reverted by
> copy-propagation). So I added a minor change in CSE (first commit) that
> prevents
> apply it when the common expression is just an immediate (it saves nothing,
> and
> adds a new instruction).
Nice analysis. That's exactly why I did not run opt_vector_float() as
part of the optimization loop! :)
I'm surprised that none of my commit messages explained why it was
done this way. Sorry for that -- I must have explained it on the
mailing list but not recorded it in the commit messages.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
