[Mesa-dev] [PATCH 8/9] gallivm: use undef integer in broadcast.

[Jose Fonseca]

Dave,

The series looks great.

Just this one patch seems off: LLVMTypeOf(scalar) should always match bld->elem_type, and it would only match bld->int_elem_type if the type itself is an integer. Therefore it seems that the caller is using an integer scalar with a floating point build context when it shouldn't.

Either the caller uses a integer build context, or we add a new lp_build_broadcast_scalar_int() function making it explicit that we're broadcasting not the element themselves but an (integer) mask.

(The rationale for not doing type guessing at these low level helper functions is that it ends up making harder to find issues: by forgiving types mismatches one is simply delaying bugs or masking them, instead of finding them earlier.)

Jose

----- Original Message -----
> From: Dave Airlie <airlied at redhat.com>
> 
> This adds an undefined int type and uses it for broadcasting
> instead of the float type.
> 
> Signed-off-by: Dave Airlie <airlied at redhat.com>
> ---
>  src/gallium/auxiliary/gallivm/lp_bld_swizzle.c |   15
>  +++++++++++----
>  src/gallium/auxiliary/gallivm/lp_bld_type.c    |    1 +
>  src/gallium/auxiliary/gallivm/lp_bld_type.h    |    3 +++
>  3 files changed, 15 insertions(+), 4 deletions(-)
> 
> diff --git a/src/gallium/auxiliary/gallivm/lp_bld_swizzle.c
> b/src/gallium/auxiliary/gallivm/lp_bld_swizzle.c
> index 7169360..80608db 100644
> --- a/src/gallium/auxiliary/gallivm/lp_bld_swizzle.c
> +++ b/src/gallium/auxiliary/gallivm/lp_bld_swizzle.c
> @@ -84,10 +84,17 @@ lp_build_broadcast_scalar(struct lp_build_context
> *bld,
>        struct lp_type i32_vec_type = lp_type_int_vec(32);
>        i32_vec_type.length = type.length;
>  
> -      res = LLVMBuildInsertElement(builder, bld->undef, scalar,
> -
>                                   lp_build_const_int32(bld->gallivm,
> 0), "");
> -      res = LLVMBuildShuffleVector(builder, res, bld->undef,
> -
>                                   lp_build_const_int_vec(bld->gallivm,
> i32_vec_type, 0), "");
> +      if (LLVMTypeOf(scalar) == bld->int_elem_type) {
> +         res = LLVMBuildInsertElement(builder, bld->undef_int,
> scalar,
> +
>                                      lp_build_const_int32(bld->gallivm,
> 0), "");
> +         res = LLVMBuildShuffleVector(builder, res, bld->undef_int,
> +
>                                      lp_build_const_int_vec(bld->gallivm,
> i32_vec_type, 0), "");
> +      } else {
> +         res = LLVMBuildInsertElement(builder, bld->undef, scalar,
> +
>                                      lp_build_const_int32(bld->gallivm,
> 0), "");
> +         res = LLVMBuildShuffleVector(builder, res, bld->undef,
> +
>                                      lp_build_const_int_vec(bld->gallivm,
> i32_vec_type, 0), "");
> +      }
>  #else
>        /* XXX: The above path provokes a bug in LLVM 2.6 */
>        unsigned i;
> diff --git a/src/gallium/auxiliary/gallivm/lp_bld_type.c
> b/src/gallium/auxiliary/gallivm/lp_bld_type.c
> index 413e69b..1972dd6 100644
> --- a/src/gallium/auxiliary/gallivm/lp_bld_type.c
> +++ b/src/gallium/auxiliary/gallivm/lp_bld_type.c
> @@ -408,6 +408,7 @@ lp_build_context_init(struct lp_build_context
> *bld,
>     }
>  
>     bld->undef = LLVMGetUndef(bld->vec_type);
> +   bld->undef_int = LLVMGetUndef(bld->int_vec_type);
>     bld->zero = LLVMConstNull(bld->vec_type);
>     bld->one = lp_build_one(gallivm, type);
>  }
> diff --git a/src/gallium/auxiliary/gallivm/lp_bld_type.h
> b/src/gallium/auxiliary/gallivm/lp_bld_type.h
> index f11a190..267c18f 100644
> --- a/src/gallium/auxiliary/gallivm/lp_bld_type.h
> +++ b/src/gallium/auxiliary/gallivm/lp_bld_type.h
> @@ -143,6 +143,9 @@ struct lp_build_context
>     /** Same as lp_build_undef(type) */
>     LLVMValueRef undef;
>  
> +   /** Same as lp_build_undef(int_vec_type) */
> +   LLVMValueRef undef_int;
> +
>     /** Same as lp_build_zero(type) */
>     LLVMValueRef zero;
>  
> --
> 1.7.7.6
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>