Am 22.04.2013 16:43, schrieb [email protected]: > From: José Fonseca <[email protected]> > > TEMP is not the only register file that accept unsigned. OUT too. > > Actually, what determines the appropriate type of the destination value is > not the opcode, but rather the register. > > Also cleanup/simplify code. Add a few more asserts, but also make > code more robust by handling graceful if assert fails. > > This fixes segfault / assertion in the included vert-uadd.sh graw shader. > --- > src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c | 127 > ++++++++------------- > src/gallium/tests/graw/vertex-shader/vert-uadd.sh | 9 ++ > 2 files changed, 59 insertions(+), 77 deletions(-) > create mode 100755 src/gallium/tests/graw/vertex-shader/vert-uadd.sh > > diff --git a/src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c > b/src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c > index c48c6e9..467d395 100644 > --- a/src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c > +++ b/src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c > @@ -569,7 +569,7 @@ static void lp_exec_default(struct lp_exec_mask *mask, > } > > > -/* stores val into an address pointed to by dst. > +/* stores val into an address pointed to by dst_ptr. > * mask->exec_mask is used to figure out which bits of val > * should be stored into the address > * (0 means don't store this bit, 1 means do store). > @@ -578,10 +578,14 @@ static void lp_exec_mask_store(struct lp_exec_mask > *mask, > struct lp_build_context *bld_store, > LLVMValueRef pred, > LLVMValueRef val, > - LLVMValueRef dst) > + LLVMValueRef dst_ptr) > { > LLVMBuilderRef builder = mask->bld->gallivm->builder; > > + assert(lp_check_value(bld_store->type, val)); > + assert(LLVMGetTypeKind(LLVMTypeOf(dst_ptr)) == LLVMPointerTypeKind); > + assert(LLVMGetElementType(LLVMTypeOf(dst_ptr)) == LLVMTypeOf(val)); > + > /* Mix the predicate and execution mask */ > if (mask->has_mask) { > if (pred) { > @@ -592,16 +596,13 @@ static void lp_exec_mask_store(struct lp_exec_mask > *mask, > } > > if (pred) { > - LLVMValueRef real_val, dst_val; > - > - dst_val = LLVMBuildLoad(builder, dst, ""); > - real_val = lp_build_select(bld_store, > - pred, > - val, dst_val); > + LLVMValueRef res, dst; > > - LLVMBuildStore(builder, real_val, dst); > + dst = LLVMBuildLoad(builder, dst_ptr, ""); > + res = lp_build_select(bld_store, pred, val, dst); > + LLVMBuildStore(builder, res, dst_ptr); > } else > - LLVMBuildStore(builder, val, dst); > + LLVMBuildStore(builder, val, dst_ptr); > } > > static void lp_exec_mask_call(struct lp_exec_mask *mask, > @@ -1312,54 +1313,38 @@ emit_store_chan( > LLVMValueRef value) > { > struct lp_build_tgsi_soa_context * bld = lp_soa_context(bld_base); > - struct gallivm_state *gallivm = bld->bld_base.base.gallivm; > + struct gallivm_state *gallivm = bld_base->base.gallivm; > LLVMBuilderRef builder = gallivm->builder; > const struct tgsi_full_dst_register *reg = &inst->Dst[index]; > + struct lp_build_context *float_bld = &bld_base->base; > + struct lp_build_context *int_bld = &bld_base->int_bld; > struct lp_build_context *uint_bld = &bld_base->uint_bld; > LLVMValueRef indirect_index = NULL; > - struct lp_build_context *bld_store; > enum tgsi_opcode_type dtype = > tgsi_opcode_infer_dst_type(inst->Instruction.Opcode); > > - switch (dtype) { > - default: > - case TGSI_TYPE_FLOAT: > - case TGSI_TYPE_UNTYPED: > - bld_store = &bld_base->base; > - break; > - case TGSI_TYPE_UNSIGNED: > - bld_store = &bld_base->uint_bld; > - break; > - case TGSI_TYPE_SIGNED: > - bld_store = &bld_base->int_bld; > - break; > - case TGSI_TYPE_DOUBLE: > - case TGSI_TYPE_VOID: > - assert(0); > - bld_store = NULL; > - break; > - } > - > - /* If the destination is untyped then the source can be anything, > - * but LLVM won't like if the types don't match so lets cast > - * to the correct destination type as expected by LLVM. */ > - if (dtype == TGSI_TYPE_UNTYPED && > - !lp_check_vec_type(bld_store->type, LLVMTypeOf(value))) { > - value = LLVMBuildBitCast(builder, value, bld_store->vec_type, > - "src_casted"); > - } > - > + /* > + * Apply saturation. > + * > + * It is always assumed to be float. > + */ > switch( inst->Instruction.Saturate ) { > case TGSI_SAT_NONE: > break; > > case TGSI_SAT_ZERO_ONE: > - value = lp_build_max(&bld->bld_base.base, value, > bld->bld_base.base.zero); > - value = lp_build_min(&bld->bld_base.base, value, > bld->bld_base.base.one); > + assert(dtype == TGSI_TYPE_FLOAT || > + dtype == TGSI_TYPE_UNTYPED); > + value = LLVMBuildBitCast(builder, value, float_bld->vec_type, ""); > + value = lp_build_max(float_bld, value, float_bld->zero); > + value = lp_build_min(float_bld, value, float_bld->one); > break; > > case TGSI_SAT_MINUS_PLUS_ONE: > - value = lp_build_max(&bld->bld_base.base, value, > lp_build_const_vec(bld->bld_base.base.gallivm, bld->bld_base.base.type, > -1.0)); > - value = lp_build_min(&bld->bld_base.base, value, > bld->bld_base.base.one); > + assert(dtype == TGSI_TYPE_FLOAT || > + dtype == TGSI_TYPE_UNTYPED); > + value = LLVMBuildBitCast(builder, value, float_bld->vec_type, ""); > + value = lp_build_max(float_bld, value, lp_build_const_vec(gallivm, > float_bld->type, -1.0)); > + value = lp_build_min(float_bld, value, float_bld->one); > break; > > default: > @@ -1373,16 +1358,19 @@ emit_store_chan( > ®->Indirect); > } else { > assert(reg->Register.Index <= > - > bld->bld_base.info->file_max[reg->Register.File]); > + bld_base->info->file_max[reg->Register.File]); > } > > switch( reg->Register.File ) { > case TGSI_FILE_OUTPUT: > + /* Outputs are always stored as floats */ > + value = LLVMBuildBitCast(builder, value, float_bld->vec_type, ""); > + > if (reg->Register.Indirect) { > LLVMValueRef chan_vec = > lp_build_const_int_vec(gallivm, uint_bld->type, chan_index); > LLVMValueRef length_vec = > - lp_build_const_int_vec(gallivm, uint_bld->type, > bld->bld_base.base.type.length); > + lp_build_const_int_vec(gallivm, uint_bld->type, > float_bld->type.length); > LLVMValueRef index_vec; /* indexes into the temp registers */ > LLVMValueRef outputs_array; > LLVMValueRef pixel_offsets; > @@ -1391,7 +1379,7 @@ emit_store_chan( > > /* build pixel offset vector: {0, 1, 2, 3, ...} */ > pixel_offsets = uint_bld->undef; > - for (i = 0; i < bld->bld_base.base.type.length; i++) { > + for (i = 0; i < float_bld->type.length; i++) { > LLVMValueRef ii = lp_build_const_int32(gallivm, i); > pixel_offsets = LLVMBuildInsertElement(builder, pixel_offsets, > ii, ii, ""); > @@ -1415,17 +1403,20 @@ emit_store_chan( > else { > LLVMValueRef out_ptr = lp_get_output_ptr(bld, reg->Register.Index, > chan_index); > - lp_exec_mask_store(&bld->exec_mask, bld_store, pred, value, > out_ptr); > + lp_exec_mask_store(&bld->exec_mask, float_bld, pred, value, > out_ptr); > } > break; > > case TGSI_FILE_TEMPORARY: > + /* Temporaries are always stored as floats */ > + value = LLVMBuildBitCast(builder, value, float_bld->vec_type, ""); > + > if (reg->Register.Indirect) { > LLVMValueRef chan_vec = > lp_build_const_int_vec(gallivm, uint_bld->type, chan_index); > LLVMValueRef length_vec = > lp_build_const_int_vec(gallivm, uint_bld->type, > - bld->bld_base.base.type.length); > + float_bld->type.length); > LLVMValueRef index_vec; /* indexes into the temp registers */ > LLVMValueRef temps_array; > LLVMValueRef pixel_offsets; > @@ -1434,7 +1425,7 @@ emit_store_chan( > > /* build pixel offset vector: {0, 1, 2, 3, ...} */ > pixel_offsets = uint_bld->undef; > - for (i = 0; i < bld->bld_base.base.type.length; i++) { > + for (i = 0; i < float_bld->type.length; i++) { > LLVMValueRef ii = lp_build_const_int32(gallivm, i); > pixel_offsets = LLVMBuildInsertElement(builder, pixel_offsets, > ii, ii, ""); > @@ -1457,42 +1448,24 @@ emit_store_chan( > } > else { > LLVMValueRef temp_ptr; > - > - switch (dtype) { > - case TGSI_TYPE_UNSIGNED: > - case TGSI_TYPE_SIGNED: { > - LLVMTypeRef itype = bld_base->int_bld.vec_type; > - LLVMTypeRef ivtype = LLVMPointerType(itype, 0); > - LLVMValueRef tint_ptr = lp_get_temp_ptr_soa(bld, > reg->Register.Index, > - chan_index); > - LLVMValueRef temp_value_ptr; > - > - temp_ptr = LLVMBuildBitCast(builder, tint_ptr, ivtype, ""); > - temp_value_ptr = LLVMBuildBitCast(builder, value, itype, ""); > - value = temp_value_ptr; > - break; > - } > - default: > - case TGSI_TYPE_FLOAT: > - case TGSI_TYPE_UNTYPED: > - temp_ptr = lp_get_temp_ptr_soa(bld, reg->Register.Index, > - chan_index); > - break; > - } > - > - lp_exec_mask_store(&bld->exec_mask, bld_store, pred, value, > temp_ptr); > + temp_ptr = lp_get_temp_ptr_soa(bld, reg->Register.Index, > + chan_index); > + lp_exec_mask_store(&bld->exec_mask, float_bld, pred, value, > temp_ptr); > } > break; > > case TGSI_FILE_ADDRESS: > assert(dtype == TGSI_TYPE_SIGNED); > - assert(LLVMTypeOf(value) == bld_base->base.int_vec_type); > - lp_exec_mask_store(&bld->exec_mask, bld_store, pred, value, > + assert(LLVMTypeOf(value) == int_bld->vec_type); > + value = LLVMBuildBitCast(builder, value, int_bld->vec_type, ""); > + lp_exec_mask_store(&bld->exec_mask, int_bld, pred, value, > bld->addr[reg->Register.Index][chan_index]); > break; > > case TGSI_FILE_PREDICATE: > - lp_exec_mask_store(&bld->exec_mask, bld_store, pred, value, > + assert(LLVMTypeOf(value) == float_bld->vec_type); > + value = LLVMBuildBitCast(builder, value, float_bld->vec_type, ""); > + lp_exec_mask_store(&bld->exec_mask, float_bld, pred, value, > bld->preds[reg->Register.Index][chan_index]); > break; > > diff --git a/src/gallium/tests/graw/vertex-shader/vert-uadd.sh > b/src/gallium/tests/graw/vertex-shader/vert-uadd.sh > new file mode 100755 > index 0000000..d2a7a1b > --- /dev/null > +++ b/src/gallium/tests/graw/vertex-shader/vert-uadd.sh > @@ -0,0 +1,9 @@ > +VERT > +DCL IN[0] > +DCL IN[1] > +DCL OUT[0], GENERIC[0] > +DCL OUT[1], GENERIC[1] > +IMM[0] INT32 {1, 0, 0, 0} > +MOV OUT[0], IN[0] > +UADD OUT[1].x, IN[1].xxxx, IMM[0].xxxx > +END >
Looks good. I think there might be a warning about unused dtype though in release builds if I see that right. Reviewed-by: Roland Scheidegger <[email protected]> _______________________________________________ mesa-dev mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/mesa-dev
