[Mesa-dev] [PATCH v2 1/3] i965/fs: Use the source type when looking for UD negations in copy prop

[Matt Turner]

On Fri, Apr 3, 2015 at 2:06 PM, Jason Ekstrand <jason at jlekstrand.net> wrote:
> There can be problems with floats and conditional modifiers when
> copy-propagating a negated UD source.  Previously, we checked the source to
> be copied for the negate and then checked the source being propagated to for
> the type.  This isn't quite what we want because we are really just looking
> for negated UD sources.  A check later in the file ensures that both ends
> of the propagate have the right type so it works.  However, if we relax the
> restriction that both ends of the propagation have the same type, it ends
> up causing us to bail early in cases we don't want.
> ---
>  src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp b/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp
> index 764741d..e8d092c 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp
> @@ -307,7 +307,7 @@ fs_visitor::try_copy_propagate(fs_inst *inst, int arg, acp_entry *entry)
>      * instead. See also resolve_ud_negate() and comment in
>      * fs_generator::generate_code.
>      */
> -   if (inst->src[arg].type == BRW_REGISTER_TYPE_UD &&
> +   if (entry->src.type == BRW_REGISTER_TYPE_UD &&
>         entry->src.negate)
>        return false;
>
> --

I think the claim is that we're mistakenly preventing something like

   mov tmp:D, -src:D
   cmp ..., tmp:UD, ...

from being handled by copy propagation? If it were propagated, it
would turn into

   cmp ..., -src:UD, ...

(By the way, an example like this in the commit message would make
review a lot easier)

Same page so far?

As far as I can tell that transformation would be wrong. As I
understand it, negating a UD value generates a value that potentially
needs 33 bits to represent, and the comparison handles that temporary
value properly, but if you negate and copy to a register (like the mov
is doing) you lose the top bit.