Hi Dave,
Thanks for looking at this.
The gist is there, but I'd prefer that the ambiguity of "ints encoded as
floats" stayed contained inside lp_bld_tgsi_* and never bleeds into
lp_bld_arith or other generic LLVM emiting helper functions. That is, when
lp_bld_arith and other functions are called, the types should be sanitized and
consistent with values, i.e, float typed lp_build_context with float values;
integer typed lp_build_context with int values; etc.
Let's take TGSI_OPCODE_UADD for example. IMO, the clean way to do this is:
- Have an auxiliary function emit_fetch_int(), which wraps/bases upon
emit_fetch() but bitcasts the results from float to an integer before returning.
I'm not sure what semantics were chosen for source absolute/negate modifiers
-- if they are allowed and should be carried as integers then emit_fetch_int()
must take the task of implementing it by itself (ie., 3 functions will be
needed: e.g., emit_fetch_int, emit_fetch_float, emit_fetch_common, being
absolute/negation done in the former 2, and the swizzling done in the later).
- Likewise, have an auxiliary function emit_store_int(), which wraps/bases upon
emit_store(), but bitcasts the parameters from integer to floats before
invoking. Again, saturation probably needs to be done separately (so that
arithmetic is carried out integers and not floats).
- The opcode case clause should be responsible for emitting the appropriate
float<->int bitcasts:
bool int_dst = FALSE;
[...]
case TGSI_OPCODE_UADD:
FOR_EACH_DST0_ENABLED_CHANNEL( inst, chan_index ) {
src0 = emit_fetch_int( bld, inst, 0, chan_index );
^^^
src1 = emit_fetch_int( bld, inst, 1, chan_index );
^^^
dst0[chan_index] = lp_build_add(&bld->uint_bld, src0, src1);
^^^^^^^^
int_dst = TRUE; // this could be also stored tgsi_opcode_info
}
break;
[...]
"^^^^" marks the places that change vs TGSI_OPCODE_UADD.
Then the bottom of emit_instruction() should be changed:
if (info->num_dst) {
LLVMValueRef pred[NUM_CHANNELS];
emit_fetch_predicate( bld, inst, pred );
FOR_EACH_DST0_ENABLED_CHANNEL( inst, chan_index ) {
if (int_dst) {
emit_store_int( bld, inst, 0, chan_index, pred[chan_index],
dst0[chan_index]);
else {
emit_store( bld, inst, 0, chan_index, pred[chan_index],
dst0[chan_index]);
}
}
}
And so on.
That is, lp_bld_arit.c functions such as lp_build_add() are be confident that
passed the values and types are consistent, and all assertions to test that
should be preserved.
I hope this makes sense.
Jose
----- Original Message -----
> Hi,
>
> So I've been playing a bit more with integers in gallivm, but I'm not
> 100% sure how the original design though about incorporating them.
>
> The lp_build_contexts seems to take a base type of a float and have
> some sort of int types associated it with it, should I add more to
> this struct for uints?
>
> In my current hacking around, I've created a new build context called
> uint_bld that goes alongside the base one, but I'm getting the
> feeling
> this isn't the correct direction.
>
> http://cgit.freedesktop.org/~airlied/mesa/log/?h=llvmpipe-int-work
>
> has my current attempt at getting the opcodes in place, mainly
> removing asserts and trying to add ints.
>
> Dave.
> _______________________________________________
> 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
