[Mesa-dev] [PATCH 2/2] st/glsl_to_tgsi: fix 64-bit integer bit shifts

[Marek Olšák]

On Thu, Mar 30, 2017 at 5:29 PM, Nicolai Hähnle <nhaehnle at gmail.com> wrote:
> From: Nicolai Hähnle <nicolai.haehnle at amd.com>
>
> Fix a bug that was caused by a type mismatch in the shfit count between
> GLSL and TGSI. I briefly considered adjusting the TGSI semantics, but
> since both LLVM and AMD GCN require both arguments to be of the same type,
> it makes more sense to keep TGSI as-is -- it reflects the underlying
> implementation better.
>
> I'm also sending out piglit tests that expose this error.
> ---
>  src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 20 +++++++++++++++-----
>  1 file changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> index 31c14ed..6ef41f2 100644
> --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> @@ -2095,27 +2095,37 @@ glsl_to_tgsi_visitor::visit_expression(ir_expression* ir, st_src_reg *op)
>        if (native_integers) {
>           emit_asm(ir, TGSI_OPCODE_NOT, result_dst, op[0]);
>           break;
>        }
>     case ir_unop_u2f:
>        if (native_integers) {
>           emit_asm(ir, TGSI_OPCODE_U2F, result_dst, op[0]);
>           break;
>        }
>     case ir_binop_lshift:
> -      if (native_integers) {
> -         emit_asm(ir, TGSI_OPCODE_SHL, result_dst, op[0], op[1]);
> -         break;
> -      }
>     case ir_binop_rshift:
>        if (native_integers) {
> -         emit_asm(ir, TGSI_OPCODE_ISHR, result_dst, op[0], op[1]);
> +         unsigned opcode = ir->operation == ir_binop_lshift ? TGSI_OPCODE_SHL
> +                                                            : TGSI_OPCODE_ISHR;
> +         st_src_reg count;
> +
> +         if (glsl_base_type_is_64bit(op[0].type)) {
> +            /* GLSL shift operations have 32-bit shift counts, but TGSI uses
> +             * 64 bits.
> +             */
> +            count = get_temp(glsl_type::u64vec(4));

Does count really have to have 4 u64 components?

Marek