On Mon, Feb 11, 2019 at 8:44 PM Jason Ekstrand <ja...@jlekstrand.net> wrote:
>
> This fixes a bug in runscape where we were optimizing x >> 16 to an
> extract and then negating and converting to float.  The NIR to fs pass
> was dropping the negate on the floor breaking a geometry shader and
> causing it to render nothing.
>
> Fixes: 1f862e923cb "i965/fs: Optimize float conversions of byte/word..."
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=109601
> Cc: Matt Turner <matts...@gmail.com>
> ---
>  src/intel/compiler/brw_fs_nir.cpp | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/src/intel/compiler/brw_fs_nir.cpp 
> b/src/intel/compiler/brw_fs_nir.cpp
> index b80f4351b49..204640ac726 100644
> --- a/src/intel/compiler/brw_fs_nir.cpp
> +++ b/src/intel/compiler/brw_fs_nir.cpp
> @@ -510,6 +510,11 @@ fs_visitor::optimize_extract_to_float(nir_alu_instr 
> *instr,
>         src0->op != nir_op_extract_i8 && src0->op != nir_op_extract_i16)
>        return false;
>
> +   /* If either opcode has source modifiers, bail. */
> +   if (instr->src[0].abs || instr->src[0].negate ||
> +        src0->src[0].abs || src0->src[0].negate)

You've done something weird here to vertically align things. I don't
know if I like it, but if you're doing to indent the 'src0' so that
the rest aligns then at least be consistent and align both 'src0's :)

> +      return false;
> +

We could handle this better by allowing the optimization if we're
extracting the high byte/word. In that case I think we could safely
apply the source mod. We should do that separately, but it might be
nice to leave a FINISHME here so we'll remember.

Reviewed-by: Matt Turner <matts...@gmail.com>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to