[Mesa-dev] [PATCH] i965: Fix bitcast operations with negate (ceil)

[Jason Ekstrand]

On Wed, Dec 17, 2014 at 1:24 AM, Iago Toral Quiroga <itoral at igalia.com>
wrote:
>
> Commit 0ae9ca12a8 put source modifiers out of the bitcast operations
> by adding a MOV operation that would handle them separately. It missed
> the case of ceil though, which negates both its source and destination
> operands.
>
> Fixes the following 42 dEQP tests:
> dEQP-GLES3.functional.shaders.builtin_functions.common.ceil.*_vertex
> dEQP-GLES3.functional.shaders.builtin_functions.common.ceil.*_fragment
> dEQP-GLES3.functional.shaders.builtin_functions.precision.ceil._*vertex.*
> dEQP-GLES3.functional.shaders.builtin_functions.precision.ceil._*fragment.*
> ---
>  src/mesa/drivers/dri/i965/brw_fs_visitor.cpp   | 12 ++++++++----
>  src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp | 12 ++++++++----
>  2 files changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> index b4f8f37..4526b75 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> @@ -889,10 +889,14 @@ fs_visitor::visit(ir_expression *ir)
>     case ir_unop_trunc:
>        emit(RNDZ(this->result, op[0]));
>        break;
> -   case ir_unop_ceil:
> -      op[0].negate = !op[0].negate;
> -      emit(RNDD(this->result, op[0]));
> -      this->result.negate = true;
> +   case ir_unop_ceil: {
> +         fs_reg tmp = fs_reg(this, ir->type);
> +         op[0].negate = !op[0].negate;
> +         emit(MOV(tmp, op[0]));
>

Do we really need this first mov?  I don't think we do as we can just use a
source modifier on RNDD.  It won't affect the quality of the final code
because copy propagation should clean it up, but more direct codegen is
nicer.


> +         emit(RNDD(tmp, tmp));
> +         tmp.negate = true;
> +         emit(MOV(this->result, tmp));
> +      }
>        break;
>     case ir_unop_floor:
>        emit(RNDD(this->result, op[0]));
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> index 73fff75..7c4a3e1 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> @@ -1649,10 +1649,14 @@ vec4_visitor::visit(ir_expression *ir)
>     case ir_unop_trunc:
>        emit(RNDZ(result_dst, op[0]));
>        break;
> -   case ir_unop_ceil:
> -      op[0].negate = !op[0].negate;
> -      inst = emit(RNDD(result_dst, op[0]));
> -      this->result.negate = true;
> +   case ir_unop_ceil: {
> +         src_reg tmp = src_reg(this, ir->type);
> +         op[0].negate = !op[0].negate;
> +         emit(MOV(dst_reg(tmp), op[0]));
> +         emit(RNDD(dst_reg(tmp), tmp));
> +         tmp.negate = true;
> +         emit(MOV(result_dst, tmp));
> +      }
>        break;
>     case ir_unop_floor:
>        inst = emit(RNDD(result_dst, op[0]));
> --
> 1.9.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20141217/86152818/attachment.html>