Re: [Mesa-dev] [RFC] i965: Factor out descriptor building for indirect send messages
Pohjolainen, Topi topi.pohjolai...@intel.com writes: On Tue, Mar 10, 2015 at 11:07:26PM +0200, Francisco Jerez wrote: Pohjolainen, Topi topi.pohjolai...@intel.com writes: On Mon, Mar 09, 2015 at 12:43:08PM +0200, Francisco Jerez wrote: Pohjolainen, Topi topi.pohjolai...@intel.com writes: On Sat, Mar 07, 2015 at 04:15:08PM +0200, Francisco Jerez wrote: Topi Pohjolainen topi.pohjolai...@intel.com writes: The original patch from Curro was based on something that is not present in the master yet. This patch tries to mimick the logic on top master. I wanted to see if could separate the building of the descriptor instruction from building of the send instruction. This logic now allows the caller to construct any kind of sequence of instructions filling in the descriptor before giving it to the send instruction builder. This is only compile tested. Curro, how would you feel about this sort of approach? I apologise for muddying the waters again but I wasn't entirely comfortable with the logic and wanted to try to something else. I believe patch number five should go nicely on top of this as the descriptor instruction could be followed by (or preceeeded by) any additional instructions modifying the descriptor register before the actual send instruction. Topi, the goal I had in mind with PATCH 01 was to refactor a commonly recurring pattern. In terms of the functions defined in this patch my example from yesterday [1] would now look like: | if (index.file == BRW_IMMEDIATE_VALUE) { | | uint32_t surf_index = index.dw1.ud; | | brw_inst *send = brw_next_insn(p, BRW_OPCODE_SEND); | brw_set_dest(p, send, retype(dst, BRW_REGISTER_TYPE_UW)); | brw_set_src0(p, send, offset); | brw_set_sampler_message(p, send, | surf_index, | 0, /* LD message ignores sampler unit */ | GEN5_SAMPLER_MESSAGE_SAMPLE_LD, | rlen, | mlen, | false, /* no header */ | simd_mode, | 0); | | brw_mark_surface_used(prog_data, surf_index); | | } else { | | struct brw_reg addr = vec1(retype(brw_address_reg(0), BRW_REGISTER_TYPE_UD)); | | brw_push_insn_state(p); | brw_set_default_mask_control(p, BRW_MASK_DISABLE); | brw_set_default_access_mode(p, BRW_ALIGN_1); | | /* a0.0 = surf_index 0xff */ | brw_inst *insn_and = brw_next_insn(p, BRW_OPCODE_AND); | brw_inst_set_exec_size(p-brw, insn_and, BRW_EXECUTE_1); | brw_set_dest(p, insn_and, addr); | brw_set_src0(p, insn_and, vec1(retype(index, BRW_REGISTER_TYPE_UD))); | brw_set_src1(p, insn_and, brw_imm_ud(0x0ff)); | | | /* a0.0 |= descriptor */ | brw_inst *descr_inst = brw_build_indirect_message_descr(p, addr, addr); | brw_set_sampler_message(p, descr_inst, | 0 /* surface */, | 0 /* sampler */, | GEN5_SAMPLER_MESSAGE_SAMPLE_LD, | rlen /* rlen */, | mlen /* mlen */, | false /* header */, | simd_mode, | 0); | | /* dst = send(offset, a0.0) */ | brw_send_indirect_message(p, BRW_SFID_SAMPLER, dst, offset, addr); | | brw_pop_insn_state(p); | | /* visitor knows more than we do about the surface limit required, | * so has already done marking. | */ | } Which I think could also be written as follows. Or am I missing something again? static brw_inst * brw_build_surface_index_descr(struct brw_compile *p, struct brw_reg dst, index) { brw_set_default_mask_control(p, BRW_MASK_DISABLE); brw_set_default_access_mode(p, BRW_ALIGN_1); /* a0.0 = surf_index 0xff */ brw_inst *insn_and = brw_next_insn(p, BRW_OPCODE_AND); brw_inst_set_exec_size(p-brw, insn_and, BRW_EXECUTE_1); brw_set_dest(p, insn_and, addr); brw_set_src0(p, insn_and, vec1(retype(index, BRW_REGISTER_TYPE_UD))); brw_set_src1(p, insn_and, brw_imm_ud(0x0ff)); /* a0.0 |= descriptor */ brw_inst *descr_inst = brw_build_indirect_message_descr(p, addr, addr); } ... brw_inst *descr_inst; if (index.file == BRW_IMMEDIATE_VALUE) { descr = brw_next_insn(p, BRW_OPCODE_SEND); brw_set_dest(p, send, retype(dst, BRW_REGISTER_TYPE_UW)); brw_set_src0(p, send, offset); brw_mark_surface_used(prog_data, surf_index); }
Re: [Mesa-dev] [RFC] i965: Factor out descriptor building for indirect send messages
On Tue, Mar 10, 2015 at 11:07:26PM +0200, Francisco Jerez wrote: Pohjolainen, Topi topi.pohjolai...@intel.com writes: On Mon, Mar 09, 2015 at 12:43:08PM +0200, Francisco Jerez wrote: Pohjolainen, Topi topi.pohjolai...@intel.com writes: On Sat, Mar 07, 2015 at 04:15:08PM +0200, Francisco Jerez wrote: Topi Pohjolainen topi.pohjolai...@intel.com writes: The original patch from Curro was based on something that is not present in the master yet. This patch tries to mimick the logic on top master. I wanted to see if could separate the building of the descriptor instruction from building of the send instruction. This logic now allows the caller to construct any kind of sequence of instructions filling in the descriptor before giving it to the send instruction builder. This is only compile tested. Curro, how would you feel about this sort of approach? I apologise for muddying the waters again but I wasn't entirely comfortable with the logic and wanted to try to something else. I believe patch number five should go nicely on top of this as the descriptor instruction could be followed by (or preceeeded by) any additional instructions modifying the descriptor register before the actual send instruction. Topi, the goal I had in mind with PATCH 01 was to refactor a commonly recurring pattern. In terms of the functions defined in this patch my example from yesterday [1] would now look like: | if (index.file == BRW_IMMEDIATE_VALUE) { | | uint32_t surf_index = index.dw1.ud; | | brw_inst *send = brw_next_insn(p, BRW_OPCODE_SEND); | brw_set_dest(p, send, retype(dst, BRW_REGISTER_TYPE_UW)); | brw_set_src0(p, send, offset); | brw_set_sampler_message(p, send, | surf_index, | 0, /* LD message ignores sampler unit */ | GEN5_SAMPLER_MESSAGE_SAMPLE_LD, | rlen, | mlen, | false, /* no header */ | simd_mode, | 0); | | brw_mark_surface_used(prog_data, surf_index); | | } else { | | struct brw_reg addr = vec1(retype(brw_address_reg(0), BRW_REGISTER_TYPE_UD)); | | brw_push_insn_state(p); | brw_set_default_mask_control(p, BRW_MASK_DISABLE); | brw_set_default_access_mode(p, BRW_ALIGN_1); | | /* a0.0 = surf_index 0xff */ | brw_inst *insn_and = brw_next_insn(p, BRW_OPCODE_AND); | brw_inst_set_exec_size(p-brw, insn_and, BRW_EXECUTE_1); | brw_set_dest(p, insn_and, addr); | brw_set_src0(p, insn_and, vec1(retype(index, BRW_REGISTER_TYPE_UD))); | brw_set_src1(p, insn_and, brw_imm_ud(0x0ff)); | | | /* a0.0 |= descriptor */ | brw_inst *descr_inst = brw_build_indirect_message_descr(p, addr, addr); | brw_set_sampler_message(p, descr_inst, | 0 /* surface */, | 0 /* sampler */, | GEN5_SAMPLER_MESSAGE_SAMPLE_LD, | rlen /* rlen */, | mlen /* mlen */, | false /* header */, | simd_mode, | 0); | | /* dst = send(offset, a0.0) */ | brw_send_indirect_message(p, BRW_SFID_SAMPLER, dst, offset, addr); | | brw_pop_insn_state(p); | | /* visitor knows more than we do about the surface limit required, | * so has already done marking. | */ | } Which I think could also be written as follows. Or am I missing something again? static brw_inst * brw_build_surface_index_descr(struct brw_compile *p, struct brw_reg dst, index) { brw_set_default_mask_control(p, BRW_MASK_DISABLE); brw_set_default_access_mode(p, BRW_ALIGN_1); /* a0.0 = surf_index 0xff */ brw_inst *insn_and = brw_next_insn(p, BRW_OPCODE_AND); brw_inst_set_exec_size(p-brw, insn_and, BRW_EXECUTE_1); brw_set_dest(p, insn_and, addr); brw_set_src0(p, insn_and, vec1(retype(index, BRW_REGISTER_TYPE_UD))); brw_set_src1(p, insn_and, brw_imm_ud(0x0ff)); /* a0.0 |= descriptor */ brw_inst *descr_inst = brw_build_indirect_message_descr(p, addr, addr); } ... brw_inst *descr_inst; if (index.file == BRW_IMMEDIATE_VALUE) { descr = brw_next_insn(p, BRW_OPCODE_SEND); brw_set_dest(p, send, retype(dst, BRW_REGISTER_TYPE_UW)); brw_set_src0(p, send, offset); brw_mark_surface_used(prog_data, surf_index); } else { struct brw_reg addr =
Re: [Mesa-dev] [RFC] i965: Factor out descriptor building for indirect send messages
On Mon, Mar 09, 2015 at 12:43:08PM +0200, Francisco Jerez wrote: Pohjolainen, Topi topi.pohjolai...@intel.com writes: On Sat, Mar 07, 2015 at 04:15:08PM +0200, Francisco Jerez wrote: Topi Pohjolainen topi.pohjolai...@intel.com writes: The original patch from Curro was based on something that is not present in the master yet. This patch tries to mimick the logic on top master. I wanted to see if could separate the building of the descriptor instruction from building of the send instruction. This logic now allows the caller to construct any kind of sequence of instructions filling in the descriptor before giving it to the send instruction builder. This is only compile tested. Curro, how would you feel about this sort of approach? I apologise for muddying the waters again but I wasn't entirely comfortable with the logic and wanted to try to something else. I believe patch number five should go nicely on top of this as the descriptor instruction could be followed by (or preceeeded by) any additional instructions modifying the descriptor register before the actual send instruction. Topi, the goal I had in mind with PATCH 01 was to refactor a commonly recurring pattern. In terms of the functions defined in this patch my example from yesterday [1] would now look like: | if (index.file == BRW_IMMEDIATE_VALUE) { | | uint32_t surf_index = index.dw1.ud; | | brw_inst *send = brw_next_insn(p, BRW_OPCODE_SEND); | brw_set_dest(p, send, retype(dst, BRW_REGISTER_TYPE_UW)); | brw_set_src0(p, send, offset); | brw_set_sampler_message(p, send, | surf_index, | 0, /* LD message ignores sampler unit */ | GEN5_SAMPLER_MESSAGE_SAMPLE_LD, | rlen, | mlen, | false, /* no header */ | simd_mode, | 0); | | brw_mark_surface_used(prog_data, surf_index); | | } else { | | struct brw_reg addr = vec1(retype(brw_address_reg(0), BRW_REGISTER_TYPE_UD)); | | brw_push_insn_state(p); | brw_set_default_mask_control(p, BRW_MASK_DISABLE); | brw_set_default_access_mode(p, BRW_ALIGN_1); | | /* a0.0 = surf_index 0xff */ | brw_inst *insn_and = brw_next_insn(p, BRW_OPCODE_AND); | brw_inst_set_exec_size(p-brw, insn_and, BRW_EXECUTE_1); | brw_set_dest(p, insn_and, addr); | brw_set_src0(p, insn_and, vec1(retype(index, BRW_REGISTER_TYPE_UD))); | brw_set_src1(p, insn_and, brw_imm_ud(0x0ff)); | | | /* a0.0 |= descriptor */ | brw_inst *descr_inst = brw_build_indirect_message_descr(p, addr, addr); | brw_set_sampler_message(p, descr_inst, | 0 /* surface */, | 0 /* sampler */, | GEN5_SAMPLER_MESSAGE_SAMPLE_LD, | rlen /* rlen */, | mlen /* mlen */, | false /* header */, | simd_mode, | 0); | | /* dst = send(offset, a0.0) */ | brw_send_indirect_message(p, BRW_SFID_SAMPLER, dst, offset, addr); | | brw_pop_insn_state(p); | | /* visitor knows more than we do about the surface limit required, | * so has already done marking. | */ | } Which I think could also be written as follows. Or am I missing something again? static brw_inst * brw_build_surface_index_descr(struct brw_compile *p, struct brw_reg dst, index) { brw_set_default_mask_control(p, BRW_MASK_DISABLE); brw_set_default_access_mode(p, BRW_ALIGN_1); /* a0.0 = surf_index 0xff */ brw_inst *insn_and = brw_next_insn(p, BRW_OPCODE_AND); brw_inst_set_exec_size(p-brw, insn_and, BRW_EXECUTE_1); brw_set_dest(p, insn_and, addr); brw_set_src0(p, insn_and, vec1(retype(index, BRW_REGISTER_TYPE_UD))); brw_set_src1(p, insn_and, brw_imm_ud(0x0ff)); /* a0.0 |= descriptor */ brw_inst *descr_inst = brw_build_indirect_message_descr(p, addr, addr); } ... brw_inst *descr_inst; if (index.file == BRW_IMMEDIATE_VALUE) { descr = brw_next_insn(p, BRW_OPCODE_SEND); brw_set_dest(p, send, retype(dst, BRW_REGISTER_TYPE_UW)); brw_set_src0(p, send, offset); brw_mark_surface_used(prog_data, surf_index); } else { struct brw_reg addr = vec1(retype(brw_address_reg(0), BRW_REGISTER_TYPE_UD)); brw_push_insn_state(p); brw_build_surface_index_descr(p, addr, index); /* dst = send(offset, a0.0) */ descr_inst = brw_send_indirect_message(p,
Re: [Mesa-dev] [RFC] i965: Factor out descriptor building for indirect send messages
Pohjolainen, Topi topi.pohjolai...@intel.com writes: On Mon, Mar 09, 2015 at 12:43:08PM +0200, Francisco Jerez wrote: Pohjolainen, Topi topi.pohjolai...@intel.com writes: On Sat, Mar 07, 2015 at 04:15:08PM +0200, Francisco Jerez wrote: Topi Pohjolainen topi.pohjolai...@intel.com writes: The original patch from Curro was based on something that is not present in the master yet. This patch tries to mimick the logic on top master. I wanted to see if could separate the building of the descriptor instruction from building of the send instruction. This logic now allows the caller to construct any kind of sequence of instructions filling in the descriptor before giving it to the send instruction builder. This is only compile tested. Curro, how would you feel about this sort of approach? I apologise for muddying the waters again but I wasn't entirely comfortable with the logic and wanted to try to something else. I believe patch number five should go nicely on top of this as the descriptor instruction could be followed by (or preceeeded by) any additional instructions modifying the descriptor register before the actual send instruction. Topi, the goal I had in mind with PATCH 01 was to refactor a commonly recurring pattern. In terms of the functions defined in this patch my example from yesterday [1] would now look like: | if (index.file == BRW_IMMEDIATE_VALUE) { | | uint32_t surf_index = index.dw1.ud; | | brw_inst *send = brw_next_insn(p, BRW_OPCODE_SEND); | brw_set_dest(p, send, retype(dst, BRW_REGISTER_TYPE_UW)); | brw_set_src0(p, send, offset); | brw_set_sampler_message(p, send, | surf_index, | 0, /* LD message ignores sampler unit */ | GEN5_SAMPLER_MESSAGE_SAMPLE_LD, | rlen, | mlen, | false, /* no header */ | simd_mode, | 0); | | brw_mark_surface_used(prog_data, surf_index); | | } else { | | struct brw_reg addr = vec1(retype(brw_address_reg(0), BRW_REGISTER_TYPE_UD)); | | brw_push_insn_state(p); | brw_set_default_mask_control(p, BRW_MASK_DISABLE); | brw_set_default_access_mode(p, BRW_ALIGN_1); | | /* a0.0 = surf_index 0xff */ | brw_inst *insn_and = brw_next_insn(p, BRW_OPCODE_AND); | brw_inst_set_exec_size(p-brw, insn_and, BRW_EXECUTE_1); | brw_set_dest(p, insn_and, addr); | brw_set_src0(p, insn_and, vec1(retype(index, BRW_REGISTER_TYPE_UD))); | brw_set_src1(p, insn_and, brw_imm_ud(0x0ff)); | | | /* a0.0 |= descriptor */ | brw_inst *descr_inst = brw_build_indirect_message_descr(p, addr, addr); | brw_set_sampler_message(p, descr_inst, | 0 /* surface */, | 0 /* sampler */, | GEN5_SAMPLER_MESSAGE_SAMPLE_LD, | rlen /* rlen */, | mlen /* mlen */, | false /* header */, | simd_mode, | 0); | | /* dst = send(offset, a0.0) */ | brw_send_indirect_message(p, BRW_SFID_SAMPLER, dst, offset, addr); | | brw_pop_insn_state(p); | | /* visitor knows more than we do about the surface limit required, | * so has already done marking. | */ | } Which I think could also be written as follows. Or am I missing something again? static brw_inst * brw_build_surface_index_descr(struct brw_compile *p, struct brw_reg dst, index) { brw_set_default_mask_control(p, BRW_MASK_DISABLE); brw_set_default_access_mode(p, BRW_ALIGN_1); /* a0.0 = surf_index 0xff */ brw_inst *insn_and = brw_next_insn(p, BRW_OPCODE_AND); brw_inst_set_exec_size(p-brw, insn_and, BRW_EXECUTE_1); brw_set_dest(p, insn_and, addr); brw_set_src0(p, insn_and, vec1(retype(index, BRW_REGISTER_TYPE_UD))); brw_set_src1(p, insn_and, brw_imm_ud(0x0ff)); /* a0.0 |= descriptor */ brw_inst *descr_inst = brw_build_indirect_message_descr(p, addr, addr); } ... brw_inst *descr_inst; if (index.file == BRW_IMMEDIATE_VALUE) { descr = brw_next_insn(p, BRW_OPCODE_SEND); brw_set_dest(p, send, retype(dst, BRW_REGISTER_TYPE_UW)); brw_set_src0(p, send, offset); brw_mark_surface_used(prog_data, surf_index); } else { struct brw_reg addr = vec1(retype(brw_address_reg(0), BRW_REGISTER_TYPE_UD)); brw_push_insn_state(p); brw_build_surface_index_descr(p, addr, index); /* dst = send(offset, a0.0) */
Re: [Mesa-dev] [RFC] i965: Factor out descriptor building for indirect send messages
Pohjolainen, Topi topi.pohjolai...@intel.com writes: On Sat, Mar 07, 2015 at 04:15:08PM +0200, Francisco Jerez wrote: Topi Pohjolainen topi.pohjolai...@intel.com writes: The original patch from Curro was based on something that is not present in the master yet. This patch tries to mimick the logic on top master. I wanted to see if could separate the building of the descriptor instruction from building of the send instruction. This logic now allows the caller to construct any kind of sequence of instructions filling in the descriptor before giving it to the send instruction builder. This is only compile tested. Curro, how would you feel about this sort of approach? I apologise for muddying the waters again but I wasn't entirely comfortable with the logic and wanted to try to something else. I believe patch number five should go nicely on top of this as the descriptor instruction could be followed by (or preceeeded by) any additional instructions modifying the descriptor register before the actual send instruction. Topi, the goal I had in mind with PATCH 01 was to refactor a commonly recurring pattern. In terms of the functions defined in this patch my example from yesterday [1] would now look like: | if (index.file == BRW_IMMEDIATE_VALUE) { | | uint32_t surf_index = index.dw1.ud; | | brw_inst *send = brw_next_insn(p, BRW_OPCODE_SEND); | brw_set_dest(p, send, retype(dst, BRW_REGISTER_TYPE_UW)); | brw_set_src0(p, send, offset); | brw_set_sampler_message(p, send, | surf_index, | 0, /* LD message ignores sampler unit */ | GEN5_SAMPLER_MESSAGE_SAMPLE_LD, | rlen, | mlen, | false, /* no header */ | simd_mode, | 0); | | brw_mark_surface_used(prog_data, surf_index); | | } else { | | struct brw_reg addr = vec1(retype(brw_address_reg(0), BRW_REGISTER_TYPE_UD)); | | brw_push_insn_state(p); | brw_set_default_mask_control(p, BRW_MASK_DISABLE); | brw_set_default_access_mode(p, BRW_ALIGN_1); | | /* a0.0 = surf_index 0xff */ | brw_inst *insn_and = brw_next_insn(p, BRW_OPCODE_AND); | brw_inst_set_exec_size(p-brw, insn_and, BRW_EXECUTE_1); | brw_set_dest(p, insn_and, addr); | brw_set_src0(p, insn_and, vec1(retype(index, BRW_REGISTER_TYPE_UD))); | brw_set_src1(p, insn_and, brw_imm_ud(0x0ff)); | | | /* a0.0 |= descriptor */ | brw_inst *descr_inst = brw_build_indirect_message_descr(p, addr, addr); | brw_set_sampler_message(p, descr_inst, | 0 /* surface */, | 0 /* sampler */, | GEN5_SAMPLER_MESSAGE_SAMPLE_LD, | rlen /* rlen */, | mlen /* mlen */, | false /* header */, | simd_mode, | 0); | | /* dst = send(offset, a0.0) */ | brw_send_indirect_message(p, BRW_SFID_SAMPLER, dst, offset, addr); | | brw_pop_insn_state(p); | | /* visitor knows more than we do about the surface limit required, | * so has already done marking. | */ | } Which I think could also be written as follows. Or am I missing something again? static brw_inst * brw_build_surface_index_descr(struct brw_compile *p, struct brw_reg dst, index) { brw_set_default_mask_control(p, BRW_MASK_DISABLE); brw_set_default_access_mode(p, BRW_ALIGN_1); /* a0.0 = surf_index 0xff */ brw_inst *insn_and = brw_next_insn(p, BRW_OPCODE_AND); brw_inst_set_exec_size(p-brw, insn_and, BRW_EXECUTE_1); brw_set_dest(p, insn_and, addr); brw_set_src0(p, insn_and, vec1(retype(index, BRW_REGISTER_TYPE_UD))); brw_set_src1(p, insn_and, brw_imm_ud(0x0ff)); /* a0.0 |= descriptor */ brw_inst *descr_inst = brw_build_indirect_message_descr(p, addr, addr); } ... brw_inst *descr_inst; if (index.file == BRW_IMMEDIATE_VALUE) { descr = brw_next_insn(p, BRW_OPCODE_SEND); brw_set_dest(p, send, retype(dst, BRW_REGISTER_TYPE_UW)); brw_set_src0(p, send, offset); brw_mark_surface_used(prog_data, surf_index); } else { struct brw_reg addr = vec1(retype(brw_address_reg(0), BRW_REGISTER_TYPE_UD)); brw_push_insn_state(p); brw_build_surface_index_descr(p, addr, index); /* dst = send(offset, a0.0) */ descr_inst = brw_send_indirect_message(p, BRW_SFID_SAMPLER, dst, offset, addr); brw_pop_insn_state(p); } uint32_t surf_index = index.file == BRW_IMMEDIATE_VALUE ? index.dw1.ud : 0;
Re: [Mesa-dev] [RFC] i965: Factor out descriptor building for indirect send messages
On Sat, Mar 07, 2015 at 04:15:08PM +0200, Francisco Jerez wrote: Topi Pohjolainen topi.pohjolai...@intel.com writes: The original patch from Curro was based on something that is not present in the master yet. This patch tries to mimick the logic on top master. I wanted to see if could separate the building of the descriptor instruction from building of the send instruction. This logic now allows the caller to construct any kind of sequence of instructions filling in the descriptor before giving it to the send instruction builder. This is only compile tested. Curro, how would you feel about this sort of approach? I apologise for muddying the waters again but I wasn't entirely comfortable with the logic and wanted to try to something else. I believe patch number five should go nicely on top of this as the descriptor instruction could be followed by (or preceeeded by) any additional instructions modifying the descriptor register before the actual send instruction. Topi, the goal I had in mind with PATCH 01 was to refactor a commonly recurring pattern. In terms of the functions defined in this patch my example from yesterday [1] would now look like: | if (index.file == BRW_IMMEDIATE_VALUE) { | | uint32_t surf_index = index.dw1.ud; | | brw_inst *send = brw_next_insn(p, BRW_OPCODE_SEND); | brw_set_dest(p, send, retype(dst, BRW_REGISTER_TYPE_UW)); | brw_set_src0(p, send, offset); | brw_set_sampler_message(p, send, | surf_index, | 0, /* LD message ignores sampler unit */ | GEN5_SAMPLER_MESSAGE_SAMPLE_LD, | rlen, | mlen, | false, /* no header */ | simd_mode, | 0); | | brw_mark_surface_used(prog_data, surf_index); | | } else { | | struct brw_reg addr = vec1(retype(brw_address_reg(0), BRW_REGISTER_TYPE_UD)); | | brw_push_insn_state(p); | brw_set_default_mask_control(p, BRW_MASK_DISABLE); | brw_set_default_access_mode(p, BRW_ALIGN_1); | | /* a0.0 = surf_index 0xff */ | brw_inst *insn_and = brw_next_insn(p, BRW_OPCODE_AND); | brw_inst_set_exec_size(p-brw, insn_and, BRW_EXECUTE_1); | brw_set_dest(p, insn_and, addr); | brw_set_src0(p, insn_and, vec1(retype(index, BRW_REGISTER_TYPE_UD))); | brw_set_src1(p, insn_and, brw_imm_ud(0x0ff)); | | | /* a0.0 |= descriptor */ | brw_inst *descr_inst = brw_build_indirect_message_descr(p, addr, addr); | brw_set_sampler_message(p, descr_inst, | 0 /* surface */, | 0 /* sampler */, | GEN5_SAMPLER_MESSAGE_SAMPLE_LD, | rlen /* rlen */, | mlen /* mlen */, | false /* header */, | simd_mode, | 0); | | /* dst = send(offset, a0.0) */ | brw_send_indirect_message(p, BRW_SFID_SAMPLER, dst, offset, addr); | | brw_pop_insn_state(p); | | /* visitor knows more than we do about the surface limit required, | * so has already done marking. | */ | } Which I think could also be written as follows. Or am I missing something again? static brw_inst * brw_build_surface_index_descr(struct brw_compile *p, struct brw_reg dst, index) { brw_set_default_mask_control(p, BRW_MASK_DISABLE); brw_set_default_access_mode(p, BRW_ALIGN_1); /* a0.0 = surf_index 0xff */ brw_inst *insn_and = brw_next_insn(p, BRW_OPCODE_AND); brw_inst_set_exec_size(p-brw, insn_and, BRW_EXECUTE_1); brw_set_dest(p, insn_and, addr); brw_set_src0(p, insn_and, vec1(retype(index, BRW_REGISTER_TYPE_UD))); brw_set_src1(p, insn_and, brw_imm_ud(0x0ff)); /* a0.0 |= descriptor */ brw_inst *descr_inst = brw_build_indirect_message_descr(p, addr, addr); } ... brw_inst *descr_inst; if (index.file == BRW_IMMEDIATE_VALUE) { descr = brw_next_insn(p, BRW_OPCODE_SEND); brw_set_dest(p, send, retype(dst, BRW_REGISTER_TYPE_UW)); brw_set_src0(p, send, offset); brw_mark_surface_used(prog_data, surf_index); } else { struct brw_reg addr = vec1(retype(brw_address_reg(0), BRW_REGISTER_TYPE_UD)); brw_push_insn_state(p); brw_build_surface_index_descr(p, addr, index); /* dst = send(offset, a0.0) */ descr_inst = brw_send_indirect_message(p, BRW_SFID_SAMPLER, dst, offset, addr); brw_pop_insn_state(p); } uint32_t surf_index = index.file == BRW_IMMEDIATE_VALUE ? index.dw1.ud : 0; brw_set_sampler_message(p, descr_inst, surf_index,
Re: [Mesa-dev] [RFC] i965: Factor out descriptor building for indirect send messages
Topi Pohjolainen topi.pohjolai...@intel.com writes: The original patch from Curro was based on something that is not present in the master yet. This patch tries to mimick the logic on top master. I wanted to see if could separate the building of the descriptor instruction from building of the send instruction. This logic now allows the caller to construct any kind of sequence of instructions filling in the descriptor before giving it to the send instruction builder. This is only compile tested. Curro, how would you feel about this sort of approach? I apologise for muddying the waters again but I wasn't entirely comfortable with the logic and wanted to try to something else. I believe patch number five should go nicely on top of this as the descriptor instruction could be followed by (or preceeeded by) any additional instructions modifying the descriptor register before the actual send instruction. Topi, the goal I had in mind with PATCH 01 was to refactor a commonly recurring pattern. In terms of the functions defined in this patch my example from yesterday [1] would now look like: | if (index.file == BRW_IMMEDIATE_VALUE) { | | uint32_t surf_index = index.dw1.ud; | | brw_inst *send = brw_next_insn(p, BRW_OPCODE_SEND); | brw_set_dest(p, send, retype(dst, BRW_REGISTER_TYPE_UW)); | brw_set_src0(p, send, offset); | brw_set_sampler_message(p, send, | surf_index, | 0, /* LD message ignores sampler unit */ | GEN5_SAMPLER_MESSAGE_SAMPLE_LD, | rlen, | mlen, | false, /* no header */ | simd_mode, | 0); | | brw_mark_surface_used(prog_data, surf_index); | | } else { | | struct brw_reg addr = vec1(retype(brw_address_reg(0), BRW_REGISTER_TYPE_UD)); | | brw_push_insn_state(p); | brw_set_default_mask_control(p, BRW_MASK_DISABLE); | brw_set_default_access_mode(p, BRW_ALIGN_1); | | /* a0.0 = surf_index 0xff */ | brw_inst *insn_and = brw_next_insn(p, BRW_OPCODE_AND); | brw_inst_set_exec_size(p-brw, insn_and, BRW_EXECUTE_1); | brw_set_dest(p, insn_and, addr); | brw_set_src0(p, insn_and, vec1(retype(index, BRW_REGISTER_TYPE_UD))); | brw_set_src1(p, insn_and, brw_imm_ud(0x0ff)); | | | /* a0.0 |= descriptor */ | brw_inst *descr_inst = brw_build_indirect_message_descr(p, addr, addr); | brw_set_sampler_message(p, descr_inst, | 0 /* surface */, | 0 /* sampler */, | GEN5_SAMPLER_MESSAGE_SAMPLE_LD, | rlen /* rlen */, | mlen /* mlen */, | false /* header */, | simd_mode, | 0); | | /* dst = send(offset, a0.0) */ | brw_send_indirect_message(p, BRW_SFID_SAMPLER, dst, offset, addr); | | brw_pop_insn_state(p); | | /* visitor knows more than we do about the surface limit required, | * so has already done marking. | */ | } I agree that this would be a modest improvement, but it still requires the caller to handle the immediate and indirect descriptor cases explicitly, which is what I wanted to avoid because the exact same mechanism would still have to be repeated in each of the 11 indirect message sends (counting the new surface opcodes). If you look at my PATCH 11-12, you won't find any explicit handling for indirect surface indices, the code for emitting each of the typed or untyped surface opcodes looks like: | void | brw_typed_surface_read(/* ... */) | { |const struct brw_context *brw = p-brw; |const unsigned sfid = /* ... */; |struct brw_inst *insn = brw_send_indirect_surface_message( | p, sfid, dst, payload, surface, msg_length, | brw_surface_payload_size(p, num_channels, |brw-gen = 8 || brw-is_haswell, false), | true); | |brw_set_dp_typed_surface_read_message( | p, insn, num_channels); | } Which works regardless of whether surface is an immediate or a register thanks to brw_send_indirect_message(). [1] http://lists.freedesktop.org/archives/mesa-dev/2015-March/078746.html Signed-off-by: Topi Pohjolainen topi.pohjolai...@intel.com --- src/mesa/drivers/dri/i965/brw_eu.h | 17 +-- src/mesa/drivers/dri/i965/brw_eu_emit.c | 53 -- src/mesa/drivers/dri/i965/brw_fs_generator.cpp | 58 ++-- src/mesa/drivers/dri/i965/brw_vec4_generator.cpp | 38 4 files changed, 75 insertions(+), 91 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_eu.h b/src/mesa/drivers/dri/i965/brw_eu.h index 736c54b..82570f5 100644 --- a/src/mesa/drivers/dri/i965/brw_eu.h +++
[Mesa-dev] [RFC] i965: Factor out descriptor building for indirect send messages
The original patch from Curro was based on something that is not present in the master yet. This patch tries to mimick the logic on top master. I wanted to see if could separate the building of the descriptor instruction from building of the send instruction. This logic now allows the caller to construct any kind of sequence of instructions filling in the descriptor before giving it to the send instruction builder. This is only compile tested. Curro, how would you feel about this sort of approach? I apologise for muddying the waters again but I wasn't entirely comfortable with the logic and wanted to try to something else. I believe patch number five should go nicely on top of this as the descriptor instruction could be followed by (or preceeeded by) any additional instructions modifying the descriptor register before the actual send instruction. Signed-off-by: Topi Pohjolainen topi.pohjolai...@intel.com --- src/mesa/drivers/dri/i965/brw_eu.h | 17 +-- src/mesa/drivers/dri/i965/brw_eu_emit.c | 53 -- src/mesa/drivers/dri/i965/brw_fs_generator.cpp | 58 ++-- src/mesa/drivers/dri/i965/brw_vec4_generator.cpp | 38 4 files changed, 75 insertions(+), 91 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_eu.h b/src/mesa/drivers/dri/i965/brw_eu.h index 736c54b..82570f5 100644 --- a/src/mesa/drivers/dri/i965/brw_eu.h +++ b/src/mesa/drivers/dri/i965/brw_eu.h @@ -205,11 +205,6 @@ void brw_set_sampler_message(struct brw_compile *p, unsigned simd_mode, unsigned return_format); -void brw_set_indirect_send_descriptor(struct brw_compile *p, - brw_inst *insn, - unsigned sfid, - struct brw_reg descriptor); - void brw_set_dp_read_message(struct brw_compile *p, brw_inst *insn, unsigned binding_table_index, @@ -242,6 +237,18 @@ void brw_urb_WRITE(struct brw_compile *p, unsigned offset, unsigned swizzle); +struct brw_inst * +brw_build_indirect_message_descr(struct brw_compile *p, + struct brw_reg dst, + struct brw_reg src); + +struct brw_inst * +brw_send_indirect_message(struct brw_compile *p, + unsigned sfid, + struct brw_reg dst, + struct brw_reg payload, + struct brw_reg desc); + void brw_ff_sync(struct brw_compile *p, struct brw_reg dest, unsigned msg_reg_nr, diff --git a/src/mesa/drivers/dri/i965/brw_eu_emit.c b/src/mesa/drivers/dri/i965/brw_eu_emit.c index 6f29468..cd0d199 100644 --- a/src/mesa/drivers/dri/i965/brw_eu_emit.c +++ b/src/mesa/drivers/dri/i965/brw_eu_emit.c @@ -772,21 +772,6 @@ brw_set_sampler_message(struct brw_compile *p, } } -void brw_set_indirect_send_descriptor(struct brw_compile *p, - brw_inst *insn, - unsigned sfid, - struct brw_reg descriptor) -{ - /* Only a0.0 may be used as SEND's descriptor operand. */ - assert(descriptor.file == BRW_ARCHITECTURE_REGISTER_FILE); - assert(descriptor.type == BRW_REGISTER_TYPE_UD); - assert(descriptor.nr == BRW_ARF_ADDRESS); - assert(descriptor.subnr == 0); - - brw_set_message_descriptor(p, insn, sfid, 0, 0, false, false); - brw_set_src1(p, insn, descriptor); -} - static void gen7_set_dp_scratch_message(struct brw_compile *p, brw_inst *inst, @@ -2495,6 +2480,44 @@ void brw_urb_WRITE(struct brw_compile *p, swizzle); } +struct brw_inst * +brw_build_indirect_message_descr(struct brw_compile *p, + struct brw_reg dst, + struct brw_reg src) +{ + assert(dst.type == BRW_REGISTER_TYPE_UD); + assert(src.type == BRW_REGISTER_TYPE_UD); + + brw_push_insn_state(p); + brw_set_default_access_mode(p, BRW_ALIGN_1); + brw_set_default_mask_control(p, BRW_MASK_DISABLE); + brw_set_default_predicate_control(p, BRW_PREDICATE_NONE); + + struct brw_inst *setup = brw_OR(p, dst, src, brw_imm_ud(0)); + + brw_pop_insn_state(p); + + return setup; +} + +struct brw_inst * +brw_send_indirect_message(struct brw_compile *p, + unsigned sfid, + struct brw_reg dst, + struct brw_reg payload, + struct brw_reg desc) +{ + const struct brw_context *brw = p-brw; + struct brw_inst *send = next_insn(p, BRW_OPCODE_SEND); + + brw_set_src1(p, send, desc); + brw_set_dest(p, send, retype(dst, BRW_REGISTER_TYPE_UD)); + brw_set_src0(p, send, retype(payload, BRW_REGISTER_TYPE_UD)); +