[Mesa-dev] [PATCH 04/12] i965/fs: Pass a BAD_FILE header source to LOAD_PAYLOAD in emit_texture_gen7().

[Francisco Jerez]

Jason Ekstrand <jason at jlekstrand.net> writes:

> On Wed, Jul 22, 2015 at 12:43 AM, Francisco Jerez <currojerez at riseup.net> wrote:
>> Jason Ekstrand <jason at jlekstrand.net> writes:
>>
>>> On Sat, Jul 18, 2015 at 7:34 AM, Francisco Jerez <currojerez at riseup.net> wrote:
>>>> So that it's left uninitialized by LOAD_PAYLOAD, we only need to
>>>> reserve space for it in the message since it will be initialized
>>>> implicitly by the generator.
>>>> ---
>>>>  src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 7 ++++---
>>>>  1 file changed, 4 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
>>>> index 522e13e..89fcc49 100644
>>>> --- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
>>>> +++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
>>>> @@ -473,8 +473,9 @@ fs_visitor::emit_texture_gen7(ir_texture_opcode op, fs_reg dst,
>>>>     if (op == ir_tg4 || offset_value.file != BAD_FILE ||
>>>>         is_high_sampler(devinfo, sampler)) {
>>>>        /* For general texture offsets (no txf workaround), we need a header to
>>>> -       * put them in.  Note that for SIMD16 we're making space for two actual
>>>> -       * hardware registers here, so the emit will have to fix up for this.
>>>> +       * put them in.  Note that we're only reserving space for it in the
>>>> +       * message payload as it will be initialized implicitly by the
>>>> +       * generator.
>>>
>>> Mind splitting this in two?  Fixing the comment above is really
>>> unrelated to passing BAD_FILE below.
>>
>> It seemed like the sentence I changed was trying to explain what the
>> line of code I changed was doing.  It may sound unrelated because the
>> comment was completely misleading (or just outdated), but it can only
>> refer to the three lines of code below...
>
> It does refer to the three lines of code below, but the fact that it's
> bogus has nothing to do with the change below.  It's been wrong for
> probably 8 months and I've just never noticed.  Register allocation
> used to, in SIMD16 mode, work in terms of pairs of physical registers.
> In the texturing code, we needed a 1-register header so in SIMD16, we
> got 2 registers and then the generator knew to put the header in the
> second half.  About 8 months ago, I changed RA to work in terms of
> single hardware registers even on SIMD16 and this comment has been
> stale ever since.

Alright...  I'll drop the comment from this patch and change it as a
follow-up.

>
>>> --Jason
>>>
>>>>         *
>>>>         * * ir4_tg4 needs to place its channel select in the header,
>>>>         * for interaction with ARB_texture_swizzle
>>>> @@ -483,7 +484,7 @@ fs_visitor::emit_texture_gen7(ir_texture_opcode op, fs_reg dst,
>>>>         * need to offset the Sampler State Pointer in the header.
>>>>         */
>>>>        header_size = 1;
>>>> -      sources[0] = fs_reg(GRF, alloc.allocate(1), BRW_REGISTER_TYPE_UD);
>>>> +      sources[0] = fs_reg();
>>>>        length++;
>>>>     }
>>>>
>>>> --
>>>> 2.4.3
>>>>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150722/b3640c7f/attachment-0001.sig>