[Mesa-dev] [PATCH 0/2] i965/vec4: Change SEL and MOV types as needed to propagate source modifiers

[Alejandro Piñeiro]

On 22/09/15 08:38, Thomas Helland wrote:
>
> 16. sep. 2015 21.47 skrev "Alejandro Piñeiro" <apinheiro at igalia.com
> <mailto:apinheiro at igalia.com>>:
> >
> > On the review of the patch "i965/nir/vec4: fill the type of the dst
> > and src when loading an uniform" Jason Ekstrand suggested to change
> > the optimization pass in order to allow the copy propagation with
> > MOVs even if there is a type mismatch, as was done on the fs path,
> > instead of fixing the type for MOV instructions.[1]
> >
> > So using commit 472ef9 as reference I implemented the equivalent
> > for the vec4 case. But that only worked if it was the current
> > instruction the MOV with default types. It didn't fixed the shader-db
> > instruction count regression I was working on, that was when it was
> > the from instruction the MOV with default types. Or in other words,
> > it didn't cover this case:
> >
> >    1: mov vgrf1.0:UD, u0.xyzw:UD
> >    2: add vgrf2.0:F, vgrf0.xyzw:F, -vgrf1.xyzw:F
> >
> > So I extended the same idea by checking too against the from
> > instruction. In order to do that, I needed to also track
> > the vec4_instructions on the copy_entry struct.
> >
> > Submitting two patches because I think that it will be easier
> > to review in this way. But if this solutions is approved, I
> > think that it could be better to push them squashed on just
> > one patch.
> >
> > Shader-db results for vec4 programs on Haswell:
> > total instructions in shared programs: 1746280 -> 1732159 (-0.81%)
> > instructions in affected programs:     760595 -> 746474 (-1.86%)
> > helped:                                6132
> > HURT:                                  0
> > GAINED:                                0
> > LOST:                                  0
> >
> >
> > [1]
> http://lists.freedesktop.org/archives/mesa-dev/2015-September/094555.html
> >
> > Alejandro Piñeiro (2):
> >   i965/vec4: Change types as needed to propagate source modifiers using
> >     current instruction
> >   i965/vec4: Change types as needed to propagate source modifiers using
> >     from instruction
> >
> >  .../drivers/dri/i965/brw_vec4_copy_propagation.cpp | 45
> ++++++++++++++++++++--
> >  1 file changed, 41 insertions(+), 4 deletions(-)
> >
>
> I don't know if it is of much use, but I implemented a function along
> these lines
> for my value range propagation pass, but that works in NIR.
> It tries to infer the type based on the users or the sources.
> It lives in [1] as get_alu_type_from_sources / dest.
> Just thought I'd mention it. It's not perfect, but I think it should
> be quite clear how I wanted it to work. Maybe it's off use, maybe not,
> just thought I'd point you to it.
>

Thanks for pointing this. In any case, we don't need to infer types on
this case. Take a look to the patch I originally proposed to solve the
issues this patch solve:
http://lists.freedesktop.org/archives/mesa-dev/2015-September/094547.html

It is possible to fill the proper types without any inference. But the
point of Jason Ekstrand on his review:
http://lists.freedesktop.org/archives/mesa-dev/2015-September/094547.html

Is that it is possible to do the copy propagation regardless the type,
because on several specific cases they are not preventing us to do the
optimization.

Again, thanks for the suggestion

> [1]
> https://github.com/thohel/mesa/blob/gsoc-final-range-prop/src/glsl/nir/nir_opt_value_range.c
>

-- 
Alejandro Piñeiro (apinheiro at igalia.com)

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150922/915ae243/attachment.html>