[Mesa-dev] [PATCH (gles3) 20/20] i965/fs/gen7: Emit code for GLSL 3.00 pack/unpack operations (v2)
eric at anholt.net
Mon Jan 21 20:35:43 PST 2013
Chad Versace <chad.versace at linux.intel.com> writes:
> On 01/21/2013 01:18 PM, Eric Anholt wrote:
>> Chad Versace <chad.versace at linux.intel.com> writes:
>>> 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,
>>> +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.
>>> + 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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Size: 197 bytes
Desc: not available
More information about the mesa-dev