[Mesa-dev] llvmpipe lp_build_context

[Jose Fonseca]

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 at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>