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 <airl...@redhat.com>
> 
> This adds an undefined int type and uses it for broadcasting
> instead of the float type.
> 
> Signed-off-by: Dave Airlie <airl...@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@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to