[Mesa-dev] [PATCH 15/53] intel/fs: Set up FB write message headers in the visitor
Francisco Jerez
currojerez at riseup.net
Sat May 26 18:27:28 UTC 2018
Jason Ekstrand <jason at jlekstrand.net> writes:
> Doing instruction header setup in the generator is aweful for a number
> of reasons. For one, we can't schedule the header setup at all. For
> another, it means lots of implied writes which the instruction scheduler
> and other passes can't properly read about. The second isn't a huge
> problem for FB writes since they always happen at the end. We made a
> similar change to sampler handling in ff4726077d86.
> ---
> src/intel/compiler/brw_fs.cpp | 103 ++++++++++++++++++++++++++------
> src/intel/compiler/brw_fs_generator.cpp | 66 --------------------
> 2 files changed, 86 insertions(+), 83 deletions(-)
>
> diff --git a/src/intel/compiler/brw_fs.cpp b/src/intel/compiler/brw_fs.cpp
> index 3e9ccc4..5276a66 100644
> --- a/src/intel/compiler/brw_fs.cpp
> +++ b/src/intel/compiler/brw_fs.cpp
> @@ -3247,7 +3247,18 @@ fs_visitor::emit_repclear_shader()
> write->mlen = 1;
> } else {
> assume(key->nr_color_regions > 0);
> +
> + struct brw_reg header =
> + retype(brw_message_reg(base_mrf), BRW_REGISTER_TYPE_UD);
> + bld.exec_all().group(16, 0)
> + .MOV(header, retype(brw_vec8_grf(0, 0), BRW_REGISTER_TYPE_UD));
> +
> for (int i = 0; i < key->nr_color_regions; ++i) {
> + if (i > 0) {
> + bld.exec_all().group(1, 0)
> + .MOV(component(header, 2), brw_imm_ud(i));
> + }
> +
> write = bld.emit(FS_OPCODE_REP_FB_WRITE);
> write->saturate = key->clamp_fragment_color;
> write->base_mrf = base_mrf;
> @@ -3972,25 +3983,83 @@ lower_fb_write_logical_send(const fs_builder &bld, fs_inst *inst,
> int header_size = 2, payload_header_size;
> unsigned length = 0;
>
> - /* From the Sandy Bridge PRM, volume 4, page 198:
> - *
> - * "Dispatched Pixel Enables. One bit per pixel indicating
> - * which pixels were originally enabled when the thread was
> - * dispatched. This field is only required for the end-of-
> - * thread message and on all dual-source messages."
> - */
> - if (devinfo->gen >= 6 &&
> - (devinfo->is_haswell || devinfo->gen >= 8 || !prog_data->uses_kill) &&
> - color1.file == BAD_FILE &&
> - key->nr_color_regions == 1) {
> - header_size = 0;
> - }
> + if (devinfo->gen < 6) {
> + /* For gen4-5, we always have a header consisting of g0 and g1. We have
> + * an implied MOV from g0,g1 to the start of the message. The MOV from
> + * g0 is handled by the hardware and the MOV from g1 is provided by the
> + * generator. This is required because, on gen4-5, the generator may
> + * generate two write messages with different message lengths in order
> + * to handle AA data properly.
> + *
> + * Also, since the pixel mask goes in the g0 portion of the message and
> + * since render target writes are the last thing in the shader, we write
> + * the pixel mask directly into g0 and it will get copied as part of the
> + * implied write.
> + */
> + if (prog_data->uses_kill) {
> + bld.exec_all().group(1, 0)
> + .MOV(retype(brw_vec1_grf(0, 0), BRW_REGISTER_TYPE_UW),
> + brw_flag_reg(0, 1));
> + }
> +
> + assert(length == 0);
> + length = 2;
> + } else if ((devinfo->gen <= 7 && !devinfo->is_haswell &&
> + prog_data->uses_kill) ||
> + color1.file != BAD_FILE ||
> + key->nr_color_regions > 1) {
> + /* From the Sandy Bridge PRM, volume 4, page 198:
> + *
> + * "Dispatched Pixel Enables. One bit per pixel indicating
> + * which pixels were originally enabled when the thread was
> + * dispatched. This field is only required for the end-of-
> + * thread message and on all dual-source messages."
> + */
> + const fs_builder ubld = bld.exec_all().group(8, 0);
> +
> + /* The header starts off as g0 and g1 */
> + fs_reg header = ubld.vgrf(BRW_REGISTER_TYPE_UD, 2);
> + ubld.group(16, 0).MOV(header, retype(brw_vec8_grf(0, 0),
> + BRW_REGISTER_TYPE_UD));
> +
> + uint32_t g00_bits = 0;
> +
> + /* Set "Source0 Alpha Present to RenderTarget" bit in message
> + * header.
> + */
> + if (inst->target > 0 && key->replicate_alpha)
> + g00_bits |= 1 << 11;
> +
> + /* Set computes stencil to render target */
> + if (prog_data->computed_stencil)
> + g00_bits |= 1 << 14;
> +
> + if (g00_bits) {
> + /* OR extra bits into g0.0 */
> + ubld.group(1, 0).OR(component(header, 0),
> + retype(brw_vec1_grf(0, 0),
> + BRW_REGISTER_TYPE_UD),
> + brw_imm_ud(g00_bits));
> + }
> +
> + /* Set the render target index for choosing BLEND_STATE. */
> + if (inst->target > 0) {
> + ubld.group(1, 0).MOV(component(header, 2), brw_imm_ud(inst->target));
> + }
> +
> + if (prog_data->uses_kill) {
> + ubld.group(1, 0).MOV(retype(component(header, 15),
> + BRW_REGISTER_TYPE_UW),
> + brw_flag_reg(0, 1));
> + }
The original patch from my branch implemented the header setup in a
helper function called setup_fb_write_header() which allowed it to share
the code among emit_repclear_shader() and lower_fb_write_logical_send().
I don't think there is any benefit from open-coding the header setup in
the already complex enough callers, seems like an obfuscation to me
(particularly of the emit_repclear_shader() codepath with PATCH 52 in
mind).
>
> - if (header_size != 0) {
> - assert(header_size == 2);
> - /* Allocate 2 registers for a header */
> - length += 2;
> + assert(length == 0);
> + sources[0] = header;
> + sources[1] = horiz_offset(header, 8);
> + length = 2;
> }
> + assert(length == 0 || length == 2);
> + header_size = length;
>
> if (payload.aa_dest_stencil_reg) {
> sources[length] = fs_reg(VGRF, bld.shader->alloc.allocate(1));
> diff --git a/src/intel/compiler/brw_fs_generator.cpp b/src/intel/compiler/brw_fs_generator.cpp
> index 8cc9d31..1a2b961 100644
> --- a/src/intel/compiler/brw_fs_generator.cpp
> +++ b/src/intel/compiler/brw_fs_generator.cpp
> @@ -307,9 +307,6 @@ fs_generator::fire_fb_write(fs_inst *inst,
> void
> fs_generator::generate_fb_write(fs_inst *inst, struct brw_reg payload)
> {
> - struct brw_wm_prog_data *prog_data = brw_wm_prog_data(this->prog_data);
> - const brw_wm_prog_key * const key = (brw_wm_prog_key * const) this->key;
> -
> if (devinfo->gen < 8 && !devinfo->is_haswell) {
> brw_set_default_predicate_control(p, BRW_PREDICATE_NONE);
> }
> @@ -320,69 +317,6 @@ fs_generator::generate_fb_write(fs_inst *inst, struct brw_reg payload)
> if (inst->base_mrf >= 0)
> payload = brw_message_reg(inst->base_mrf);
>
> - /* Header is 2 regs, g0 and g1 are the contents. g0 will be implied
> - * move, here's g1.
> - */
> - if (inst->header_size != 0) {
> - brw_push_insn_state(p);
> - brw_set_default_mask_control(p, BRW_MASK_DISABLE);
> - brw_set_default_exec_size(p, BRW_EXECUTE_1);
> - brw_set_default_predicate_control(p, BRW_PREDICATE_NONE);
> - brw_set_default_compression_control(p, BRW_COMPRESSION_NONE);
> - brw_set_default_flag_reg(p, 0, 0);
> -
> - /* On HSW, the GPU will use the predicate on SENDC, unless the header is
> - * present.
> - */
> - if (prog_data->uses_kill) {
> - struct brw_reg pixel_mask;
> -
> - if (devinfo->gen >= 6)
> - pixel_mask = retype(brw_vec1_grf(1, 7), BRW_REGISTER_TYPE_UW);
> - else
> - pixel_mask = retype(brw_vec1_grf(0, 0), BRW_REGISTER_TYPE_UW);
> -
> - brw_MOV(p, pixel_mask, brw_flag_reg(0, 1));
> - }
> -
> - if (devinfo->gen >= 6) {
> - brw_push_insn_state(p);
> - brw_set_default_exec_size(p, BRW_EXECUTE_16);
> - brw_set_default_compression_control(p, BRW_COMPRESSION_COMPRESSED);
> - brw_MOV(p,
> - retype(payload, BRW_REGISTER_TYPE_UD),
> - retype(brw_vec8_grf(0, 0), BRW_REGISTER_TYPE_UD));
> - brw_pop_insn_state(p);
> -
> - if (inst->target > 0 && key->replicate_alpha) {
> - /* Set "Source0 Alpha Present to RenderTarget" bit in message
> - * header.
> - */
> - brw_OR(p,
> - vec1(retype(payload, BRW_REGISTER_TYPE_UD)),
> - vec1(retype(brw_vec8_grf(0, 0), BRW_REGISTER_TYPE_UD)),
> - brw_imm_ud(0x1 << 11));
> - }
> -
> - if (inst->target > 0) {
> - /* Set the render target index for choosing BLEND_STATE. */
> - brw_MOV(p, retype(vec1(suboffset(payload, 2)),
> - BRW_REGISTER_TYPE_UD),
> - brw_imm_ud(inst->target));
> - }
> -
> - /* Set computes stencil to render target */
> - if (prog_data->computed_stencil) {
> - brw_OR(p,
> - vec1(retype(payload, BRW_REGISTER_TYPE_UD)),
> - vec1(retype(brw_vec8_grf(0, 0), BRW_REGISTER_TYPE_UD)),
> - brw_imm_ud(0x1 << 14));
> - }
> - }
> -
> - brw_pop_insn_state(p);
> - }
> -
> if (!runtime_check_aads_emit) {
> fire_fb_write(inst, payload, implied_header, inst->mlen);
> } else {
> --
> 2.5.0.400.gff86faf
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 227 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180526/f551cf32/attachment.sig>
More information about the mesa-dev
mailing list