[Mesa-dev] [PATCH 03/20] i965/fs: Implement lowering of logical surface instructions.
Francisco Jerez
currojerez at riseup.net
Wed Jul 22 10:16:44 PDT 2015
Jason Ekstrand <jason at jlekstrand.net> writes:
> On Wed, Jul 22, 2015 at 10:02 AM, Francisco Jerez <currojerez at riseup.net> wrote:
>> Jason Ekstrand <jason at jlekstrand.net> writes:
>>
>>> On Tue, Jul 21, 2015 at 9:38 AM, Francisco Jerez <currojerez at riseup.net> wrote:
>>>> ---
>>>> src/mesa/drivers/dri/i965/brw_fs.cpp | 63 +++++++++++++++++++++++++++++++-----
>>>> 1 file changed, 55 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp
>>>> index a996676..0b0c5e1 100644
>>>> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
>>>> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
>>>> @@ -3881,6 +3881,20 @@ lower_sampler_logical_send(const fs_builder &bld, fs_inst *inst, opcode op)
>>>> }
>>>> }
>>>>
>>>> +/**
>>>> + * Initialize the header present in some typed and untyped surface
>>>> + * messages.
>>>> + */
>>>> +static fs_reg
>>>> +emit_surface_header(const fs_builder &bld, const fs_reg &sample_mask)
>>>> +{
>>>> + fs_builder ubld = bld.exec_all().group(8, 0);
>>>> + const fs_reg dst = ubld.vgrf(BRW_REGISTER_TYPE_UD);
>>>> + ubld.MOV(dst, fs_reg(0));
>>>> + ubld.MOV(component(dst, 7), sample_mask);
>>>> + return dst;
>>>> +}
>>>> +
>>>> static void
>>>> lower_surface_logical_send(const fs_builder &bld, fs_inst *inst, opcode op,
>>>> const fs_reg &sample_mask)
>>>> @@ -3889,10 +3903,43 @@ lower_surface_logical_send(const fs_builder &bld, fs_inst *inst, opcode op,
>>>> const fs_reg &addr = inst->src[0];
>>>> const fs_reg &src = inst->src[1];
>>>> const fs_reg &surface = inst->src[2];
>>>> - const fs_reg &dims = inst->src[3];
>>>> + const UNUSED fs_reg &dims = inst->src[3];
>>>> const fs_reg &arg = inst->src[4];
>>>>
>>>> - assert(!"Not implemented");
>>>> + /* Calculate the total number of components of the payload. */
>>>> + const unsigned addr_sz = inst->components_read(0);
>>>> + const unsigned src_sz = inst->components_read(1);
>>>> + const unsigned header_sz = (sample_mask.file == BAD_FILE ? 0 : 1);
>>>> + const unsigned sz = header_sz + addr_sz + src_sz;
>>>> +
>>>> + /* Allocate space for the payload. */
>>>> + fs_reg *const components = new fs_reg[sz];
>>>> + const fs_reg payload = bld.vgrf(BRW_REGISTER_TYPE_UD, sz);
>>>> + unsigned n = 0;
>>>> +
>>>> + /* Construct the payload. */
>>>> + if (header_sz)
>>>> + components[n++] = emit_surface_header(bld, sample_mask);
>>>> +
>>>> + for (unsigned i = 0; i < addr_sz; i++)
>>>> + components[n++] = offset(addr, bld, i);
>>>> +
>>>> + for (unsigned i = 0; i < src_sz; i++)
>>>> + components[n++] = offset(src, bld, i);
>>>> +
>>>> + bld.LOAD_PAYLOAD(payload, components, sz, header_sz);
>>>> +
>>>> + /* Update the original instruction. */
>>>> + inst->opcode = op;
>>>> + inst->mlen = header_sz + (addr_sz + src_sz) * inst->exec_size / 8;
>>>> + inst->header_size = header_sz;
>>>> +
>>>> + inst->src[0] = payload;
>>>> + inst->src[1] = surface;
>>>> + inst->src[2] = arg;
>>>> + inst->resize_sources(3);
>>>> +
>>>> + delete[] components;
>>>
>>> I don't think this is correct. Unless something has changed, the
>>> fs_inst constructor that takes an array doesn't make a copy so you
>>> have to ralloc the array and *not* delete it.
>>>
>> It is, I fixed the fs_inst constructor some time ago to take the array
>> by value (e.g. so allocation on the stack and re-use of the same array
>> is possible for the caller).
>
> Probably a good idea. In that case, would you mind using a vla
> instead of new/delete?
>
I'd rather use new/delete (or the C++ standard library). VLAs are a
non-standard feature only guaranteed to be available in C99, elsewhere
they are an extension with far from universal compiler support -- The
requirement was in fact again dropped from the C11 standard and the
functionally-similar C++ feature was also voted out by the committee
during the standardization of C++14.
I'd rather stick to the standard...
>>>> }
>>>>
>>>> bool
>>>> @@ -3965,37 +4012,37 @@ fs_visitor::lower_logical_sends()
>>>> case SHADER_OPCODE_UNTYPED_SURFACE_READ_LOGICAL:
>>>> lower_surface_logical_send(ibld, inst,
>>>> SHADER_OPCODE_UNTYPED_SURFACE_READ,
>>>> - fs_reg());
>>>> + fs_reg(0xffff));
>>>> break;
>>>>
>>>> case SHADER_OPCODE_UNTYPED_SURFACE_WRITE_LOGICAL:
>>>> lower_surface_logical_send(ibld, inst,
>>>> SHADER_OPCODE_UNTYPED_SURFACE_WRITE,
>>>> - fs_reg());
>>>> + ibld.sample_mask_reg());
>>>> break;
>>>>
>>>> case SHADER_OPCODE_UNTYPED_ATOMIC_LOGICAL:
>>>> lower_surface_logical_send(ibld, inst,
>>>> SHADER_OPCODE_UNTYPED_ATOMIC,
>>>> - fs_reg());
>>>> + ibld.sample_mask_reg());
>>>> break;
>>>>
>>>> case SHADER_OPCODE_TYPED_SURFACE_READ_LOGICAL:
>>>> lower_surface_logical_send(ibld, inst,
>>>> SHADER_OPCODE_TYPED_SURFACE_READ,
>>>> - fs_reg());
>>>> + fs_reg(0xffff));
>>>> break;
>>>>
>>>> case SHADER_OPCODE_TYPED_SURFACE_WRITE_LOGICAL:
>>>> lower_surface_logical_send(ibld, inst,
>>>> SHADER_OPCODE_TYPED_SURFACE_WRITE,
>>>> - fs_reg());
>>>> + ibld.sample_mask_reg());
>>>> break;
>>>>
>>>> case SHADER_OPCODE_TYPED_ATOMIC_LOGICAL:
>>>> lower_surface_logical_send(ibld, inst,
>>>> SHADER_OPCODE_TYPED_ATOMIC,
>>>> - fs_reg());
>>>> + ibld.sample_mask_reg());
>>>> break;
>>>>
>>>> default:
>>>> --
>>>> 2.4.3
>>>>
>>>> _______________________________________________
>>>> mesa-dev mailing list
>>>> mesa-dev at lists.freedesktop.org
>>>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- 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/3a70c0fc/attachment.sig>
More information about the mesa-dev
mailing list