Chad Versace <chad.vers...@linux.intel.com> writes:
> On 01/21/2013 01:18 PM, Eric Anholt wrote:
>> Chad Versace <chad.vers...@linux.intel.com> writes:
>>> ir->remove();
>>> diff --git a/src/mesa/drivers/dri/i965/brw_fs_emit.cpp
>>> b/src/mesa/drivers/dri/i965/brw_fs_emit.cpp
>>> index 324e665..0ff296c 100644
>>> --- a/src/mesa/drivers/dri/i965/brw_fs_emit.cpp
>>> +++ b/src/mesa/drivers/dri/i965/brw_fs_emit.cpp
>>> @@ -923,6 +923,34 @@ fs_generator::generate_set_global_offset(fs_inst *inst,
>>> }
>>>
>>> void
>>> +fs_generator::generate_unpack_half_2x16_split_y(fs_inst *inst,
>>> + struct brw_reg dst,
>>> + struct brw_reg src)
>>> +{
>>> + assert(intel->gen >= 7);
>>> +
>>> + /* src has the form of unpackHalf2x16's input:
>>> + *
>>> + * w z y x
>>> + * |undef|undef|undef|0xhhhhllll|
>>
>> I don't understand what "w z y x" mean here. I thought src was just a
>> scalar uint (one uint per pixel).
>
> You're right. I had vec4 on the mind when I wrote the comment. How about
> changing it to this?
>
> /* Each channel of src has the form of unpackHalf2x16's input: 0xhhhhllll.
> * We wish to access only the "hhhh" bits of each channel, and hence
> * must access the register with a 16 bit subregister offset. To do so, we
> * must halve the size of the source data type from UD to UW and compensate
> * by doubling the stride.
> */Looks good! >>> + assert(src.type == BRW_REGISTER_TYPE_UD); >>> + src.type = BRW_REGISTER_TYPE_UW; >>> + if (src.vstride > 0) >>> + ++src.vstride; >>> + if (src.hstride > 0) >>> + ++src.hstride; >> >> This ++hstride/++vstride looks totally wrong to me. We use a vstride of >> 8 in 16-wide for temporaries -- doesn't this totally trash that? >> >> I think what you want here is an unconditional *= 2 for both values. > > This indeed does *= 2, from the hardware's perspective. My mistake for > not providing an explanation. > > src.hstride, before the adjustment, is one of the BRE_HORIZONTAL_STRIDE_* > values, which are defined as > > #define BRW_HORIZONTAL_STRIDE_0 0 > #define BRW_HORIZONTAL_STRIDE_1 1 > #define BRW_HORIZONTAL_STRIDE_2 2 > #define BRW_HORIZONTAL_STRIDE_4 3 Oh, right. I'd love to switch which type of value brw_reg uses some day -- the enums are irritating to work with, and I've made this particular mistake more than once.
pgpi5VLFHBOGw.pgp
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
