[Mesa-dev] [PATCH 2/3] i965/vec4: use byte_offset() instead of offset()
Francisco Jerez
currojerez at riseup.net
Wed Oct 26 20:13:26 UTC 2016
Iago Toral Quiroga <itoral at igalia.com> writes:
> In a later patch we want to change the semantics of offset() to be in terms
> of SIMD width and scalar channels so it is consistent with the definition
> of the same helper in the scalar backend. However, some uses of offset()
> in the vec4 backend do not operate naturally in terms of these
> semantics. In these cases it is more natural to use the byte_offset() helper
> instead.
> ---
> src/mesa/drivers/dri/i965/brw_vec4_cse.cpp | 10 ++++++----
> .../drivers/dri/i965/brw_vec4_dead_code_eliminate.cpp | 16 +++++++++++-----
> src/mesa/drivers/dri/i965/brw_vec4_live_variables.cpp | 13 +++++++++----
> src/mesa/drivers/dri/i965/brw_vec4_tcs.cpp | 3 ++-
> src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp | 2 +-
> 5 files changed, 29 insertions(+), 15 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_cse.cpp b/src/mesa/drivers/dri/i965/brw_vec4_cse.cpp
> index 1b91db9..bf7e4a4 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_cse.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_cse.cpp
> @@ -182,8 +182,9 @@ vec4_visitor::opt_cse_local(bblock_t *block)
> NULL), inst->dst.type);
>
> for (unsigned i = 0; i < regs_written(entry->generator); ++i) {
> - vec4_instruction *copy = MOV(offset(entry->generator->dst, i),
> - offset(entry->tmp, i));
> + vec4_instruction *copy =
> + MOV(byte_offset(entry->generator->dst, i * REG_SIZE),
> + byte_offset(entry->tmp, i * REG_SIZE));
> copy->force_writemask_all =
> entry->generator->force_writemask_all;
> entry->generator->insert_after(block, copy);
> @@ -197,8 +198,9 @@ vec4_visitor::opt_cse_local(bblock_t *block)
> assert(inst->dst.type == entry->tmp.type);
>
> for (unsigned i = 0; i < regs_written(inst); ++i) {
> - vec4_instruction *copy = MOV(offset(inst->dst, i),
> - offset(entry->tmp, i));
> + vec4_instruction *copy =
> + MOV(byte_offset(inst->dst, i * REG_SIZE),
> + byte_offset(entry->tmp, i * REG_SIZE));
> copy->force_writemask_all = inst->force_writemask_all;
> inst->insert_before(block, copy);
These last two hunks seem rather suspect... They're emitting a MOV
instruction for each register written even though you'd need one for
each SIMD-wide vector written. Seems like the loop upper bound will
need to be changed to 'inst->size_written / component_size' and the MOV
arguments will need to be changed back to use offset() for DF support.
That said this keeps the current behavior unchanged so it looks okay to
me for the moment:
Reviewed-by: Francisco Jerez <currojerez at riseup.net>
> }
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_dead_code_eliminate.cpp b/src/mesa/drivers/dri/i965/brw_vec4_dead_code_eliminate.cpp
> index 50706a9..916a3fb 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_dead_code_eliminate.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_dead_code_eliminate.cpp
> @@ -61,8 +61,9 @@ vec4_visitor::dead_code_eliminate()
> if (inst->dst.file == VGRF) {
> for (unsigned i = 0; i < regs_written(inst); i++) {
> for (int c = 0; c < 4; c++)
> - result_live[c] |= BITSET_TEST(
> - live, var_from_reg(alloc, offset(inst->dst, i), c));
> + result_live[c] |= BITSET_TEST(live,
> + var_from_reg(alloc,
> + byte_offset(inst->dst, i * REG_SIZE), c));
> }
> } else {
> for (unsigned c = 0; c < 4; c++)
> @@ -113,8 +114,11 @@ vec4_visitor::dead_code_eliminate()
> for (unsigned i = 0; i < regs_written(inst); i++) {
> for (int c = 0; c < 4; c++) {
> if (inst->dst.writemask & (1 << c)) {
> - BITSET_CLEAR(live, var_from_reg(alloc,
> - offset(inst->dst, i), c));
> + BITSET_CLEAR(live,
> + var_from_reg(alloc,
> + byte_offset(inst->dst,
> + i * REG_SIZE),
> + c));
> }
> }
> }
> @@ -135,7 +139,9 @@ vec4_visitor::dead_code_eliminate()
> for (unsigned j = 0; j < regs_read(inst, i); j++) {
> for (int c = 0; c < 4; c++) {
> BITSET_SET(live, var_from_reg(alloc,
> - offset(inst->src[i], j), c));
> + byte_offset(inst->src[i],
> + j * REG_SIZE),
> + c));
> }
> }
> }
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_live_variables.cpp b/src/mesa/drivers/dri/i965/brw_vec4_live_variables.cpp
> index 20344ed..b70d6c2 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_live_variables.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_live_variables.cpp
> @@ -79,7 +79,9 @@ vec4_live_variables::setup_def_use()
> for (unsigned j = 0; j < regs_read(inst, i); j++) {
> for (int c = 0; c < 4; c++) {
> const unsigned v =
> - var_from_reg(alloc, offset(inst->src[i], j), c);
> + var_from_reg(alloc,
> + byte_offset(inst->src[i], j * REG_SIZE),
> + c);
> if (!BITSET_TEST(bd->def, v))
> BITSET_SET(bd->use, v);
> }
> @@ -103,7 +105,8 @@ vec4_live_variables::setup_def_use()
> for (int c = 0; c < 4; c++) {
> if (inst->dst.writemask & (1 << c)) {
> const unsigned v =
> - var_from_reg(alloc, offset(inst->dst, i), c);
> + var_from_reg(alloc,
> + byte_offset(inst->dst, i * REG_SIZE), c);
> if (!BITSET_TEST(bd->use, v))
> BITSET_SET(bd->def, v);
> }
> @@ -260,7 +263,8 @@ vec4_visitor::calculate_live_intervals()
> for (unsigned j = 0; j < regs_read(inst, i); j++) {
> for (int c = 0; c < 4; c++) {
> const unsigned v =
> - var_from_reg(alloc, offset(inst->src[i], j), c);
> + var_from_reg(alloc,
> + byte_offset(inst->src[i], j * REG_SIZE), c);
> start[v] = MIN2(start[v], ip);
> end[v] = ip;
> }
> @@ -273,7 +277,8 @@ vec4_visitor::calculate_live_intervals()
> for (int c = 0; c < 4; c++) {
> if (inst->dst.writemask & (1 << c)) {
> const unsigned v =
> - var_from_reg(alloc, offset(inst->dst, i), c);
> + var_from_reg(alloc,
> + byte_offset(inst->dst, i * REG_SIZE), c);
> start[v] = MIN2(start[v], ip);
> end[v] = ip;
> }
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_tcs.cpp b/src/mesa/drivers/dri/i965/brw_vec4_tcs.cpp
> index 498fb7c..95f5c33 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_tcs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_tcs.cpp
> @@ -240,7 +240,8 @@ vec4_tcs_visitor::emit_urb_write(const src_reg &value,
> inst = emit(TCS_OPCODE_SET_OUTPUT_URB_OFFSETS, dst_reg(message),
> brw_imm_ud(writemask), indirect_offset);
> inst->force_writemask_all = true;
> - inst = emit(MOV(offset(dst_reg(retype(message, value.type)), 1), value));
> + inst = emit(MOV(byte_offset(dst_reg(retype(message, value.type)), REG_SIZE),
> + value));
> inst->force_writemask_all = true;
>
> inst = emit(TCS_OPCODE_URB_WRITE, dst_null_f(), message);
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> index 3e785bc..814d624 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> @@ -781,7 +781,7 @@ vec4_visitor::emit_pull_constant_load_reg(dst_reg dst,
> else
> emit(pull);
>
> - dst_reg index_reg = retype(offset(dst_reg(header), 1),
> + dst_reg index_reg = retype(byte_offset(dst_reg(header), REG_SIZE),
> offset_reg.type);
> pull = MOV(writemask(index_reg, WRITEMASK_X), offset_reg);
>
> --
> 2.7.4
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20161026/70739502/attachment.sig>
More information about the mesa-dev
mailing list