On Thu, Apr 24, 2014 at 10:30 AM, Kenneth Graunke <kenn...@whitecape.org> wrote:
> On 04/24/2014 08:40 AM, Matt Turner wrote:
>> On Wed, Apr 23, 2014 at 11:19 PM, Kenneth Graunke <kenn...@whitecape.org>
>> wrote:
>>> On 04/18/2014 11:56 AM, Matt Turner wrote:
>>>> ---
>>>> src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 135
>>>> +++++++++++++++------------
>>>> 1 file changed, 73 insertions(+), 62 deletions(-)
>>>>
>>>> diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
>>>> b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
>>>> index 2aa3acd..91bbe0a 100644
>>>> --- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
>>>> +++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
>>>> @@ -1257,8 +1257,11 @@ fs_visitor::emit_texture_gen7(ir_texture *ir,
>>>> fs_reg dst, fs_reg coordinate,
>>>> int reg_width = dispatch_width / 8;
>>>> bool header_present = false;
>>>>
>>>> - fs_reg payload = fs_reg(this, glsl_type::float_type);
>>>> - fs_reg next = payload;
>>>> + fs_reg *sources = ralloc_array(mem_ctx, fs_reg,
>>>> MAX_SAMPLER_MESSAGE_SIZE);
>>>> + for (int i = 0; i < MAX_SAMPLER_MESSAGE_SIZE; i++) {
>>>> + sources[i] = fs_reg(this, glsl_type::float_type);
>>>> + }
>>>> + int length = 0;
>>>>
>>>> if (ir->op == ir_tg4 || (ir->offset && ir->op != ir_txf) || sampler >=
>>>> 16) {
>>>> /* For general texture offsets (no txf workaround), we need a
>>>> header to
>>>> @@ -1272,12 +1275,13 @@ fs_visitor::emit_texture_gen7(ir_texture *ir,
>>>> fs_reg dst, fs_reg coordinate,
>>>> * need to offset the Sampler State Pointer in the header.
>>>> */
>>>> header_present = true;
>>>> - next.reg_offset++;
>>>> + sources[length] = reg_undef;
>>>> + length++;
>>>> }
>>>
>>> So...if you don't take the header_present = true path...then it looks
>>> like the next thing (say, shadow_c) will land in sources[0], rather than
>>> leaving sources[0] as BAD_FILE and putting the first part of the payload
>>> in sources[1].
>>>
>>> Is sources[0] special and reserved for the header or isn't it?
>>
>> It's the header if there is a header, and it's a regular source if
>> there isn't a header. We can't make it always a header, because if the
>> header doesn't exist we'll end up asking the register allocator for a
>> set of registers one too large.
>
> That seems reasonable. But, if those are the semantics, I don't see any
> reason for the lowering pass to handle src[0] specially. It could just
> loop from i = 0; i < inst->sources. BAD_FILE should never happen.
Yeah, I think you're right.
> [snip]
>>>> @@ -1403,40 +1407,44 @@ fs_visitor::emit_texture_gen7(ir_texture *ir,
>>>> fs_reg dst, fs_reg coordinate,
>>>> /* Set up the coordinate (except for cases where it was done above) */
>>>> if (ir->coordinate && !coordinate_done) {
>>>> for (int i = 0; i < ir->coordinate->type->vector_elements; i++) {
>>>> - emit(MOV(next, coordinate));
>>>> + emit(MOV(sources[length], coordinate));
>>>> coordinate.reg_offset++;
>>>> - next.reg_offset++;
>>>> + length++;
>>>> }
>>>> }
>>>>
>>>> + fs_reg src_payload = fs_reg(this, glsl_type::float_type);
>>>> + virtual_grf_sizes[src_payload.reg] = length;
>>>> + emit(SHADER_OPCODE_LOAD_PAYLOAD, src_payload, sources, length);
>>>> +
>>>
>>> I think I would prefer to see this called "payload", "msg_payload" or
>>> simply "message". There are enough things called src/sources/...
>>
>> Right. I don't know, I thought "load the sources into the source
>> payload" read pretty well. All of the other names suggest that they're
>> already a contiguous set of registers.
>
> But src_payload basically /is/ a contiguous set of registers. At least,
> it's a larger-than-size-1 VGRF which will be allocated to contiguous
> registers. This assembles an array of disjoint source registers into a
> proper contiguous message.
Oh, I thought you were talking about the 'sources' variable. Yeah,
changing the src_payload name is no problem.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
