On 04/24/2014 08:40 AM, Matt Turner wrote: > On Wed, Apr 23, 2014 at 11:19 PM, Kenneth Graunke <[email protected]> > 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.
[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.
But, if you don't like the suggestion, feel free to drop it. It's just
a variable name. :)
signature.asc
Description: OpenPGP digital signature
_______________________________________________ mesa-dev mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/mesa-dev
