Jason Ekstrand <ja...@jlekstrand.net> writes: > Soon we will start using the builder to explicitly set all the execution > sizes. We could make a 32-wide builder, but the builder asserts that we > never grow it which is usually a reasonable assumption. Sinc this one > instruction is a bit of an odd-ball, we just set the exec_size explicitly. > > Reviewed-by: Iago Toral Quiroga <ito...@igalia.com> > > v2: Explicitly new the fs_inst instead of using the builder and setting > exec_size after the fact. > > v3: Set force_writemask_all with the builder instead of directly. The > builder over-writes it if we set it manually. Also, if we don't have > force_writemask_all in the builder it will assert-fail on SIMD32.
It seems to me that it would be useful to be able to create instructions using the builder with execution size higher than the default as long as force_writemask_all is set, as this isn't the first time I've found myself wanting that feature -- In fact before your series it was possible to do it by using a destination register of the correct width. How about we change the builder to allow it? (Patch attached) > --- > src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp > b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp > index 9a4bad6..8976c25 100644 > --- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp > +++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp > @@ -1357,10 +1357,12 @@ fs_visitor::emit_interpolation_setup_gen6() > */ > fs_reg int_pixel_xy(GRF, alloc.allocate(dispatch_width / 8), > BRW_REGISTER_TYPE_UW, dispatch_width * 2); > - abld.exec_all() > - .ADD(int_pixel_xy, > - fs_reg(stride(suboffset(g1_uw, 4), 1, 4, 0)), > - fs_reg(brw_imm_v(0x11001010))); > + fs_inst *add = > + new (mem_ctx) fs_inst(BRW_OPCODE_ADD, dispatch_width * 2, > + int_pixel_xy, > + fs_reg(stride(suboffset(g1_uw, 4), 1, 4, 0)), > + fs_reg(brw_imm_v(0x11001010))); > + abld.exec_all().emit(add); > > this->pixel_x = vgrf(glsl_type::float_type); > this->pixel_y = vgrf(glsl_type::float_type); > -- > 2.4.3 > > _______________________________________________ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
From 09f6cb08cd9951d8618dea7360aa7619cc806988 Mon Sep 17 00:00:00 2001
From: Francisco Jerez <curroje...@riseup.net>
Date: Tue, 30 Jun 2015 15:15:44 +0300
Subject: [PATCH] i965/fs: Relax fs_builder channel group assertion when
force_writemask_all is on.
This assertion was meant to catch code inadvertently escaping the
control flow jail determined by the group of channel enable signals
selected by some caller, however it seems useful to be able to
increase the default execution size as long as force_writemask_all is
enabled, because force_writemask_all is an explicit indication that
there is no longer a one-to-one correspondence between channels and
SIMD components so the restriction doesn't apply.
In addition reorder the calls to fs_builder::group and ::exec_all in a
couple of places to make sure that we don't temporarily break this
invariant in the future for instructions with exec_size higher than
the dispatch width.
---
src/mesa/drivers/dri/i965/brw_fs.cpp | 4 ++--
src/mesa/drivers/dri/i965/brw_fs_builder.h | 4 ++--
src/mesa/drivers/dri/i965/brw_fs_cse.cpp | 4 ++--
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp
index 8658554..430710c 100644
--- a/src/mesa/drivers/dri/i965/brw_fs.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
@@ -2963,8 +2963,8 @@ fs_visitor::lower_load_payload()
assert(inst->dst.file == MRF || inst->dst.file == GRF);
assert(inst->saturate == false);
- const fs_builder ibld = bld.group(inst->exec_size, inst->force_sechalf)
- .exec_all(inst->force_writemask_all)
+ const fs_builder ibld = bld.exec_all(inst->force_writemask_all)
+ .group(inst->exec_size, inst->force_sechalf)
.at(block, inst);
fs_reg dst = inst->dst;
diff --git a/src/mesa/drivers/dri/i965/brw_fs_builder.h b/src/mesa/drivers/dri/i965/brw_fs_builder.h
index 58ac598..012180f 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_builder.h
+++ b/src/mesa/drivers/dri/i965/brw_fs_builder.h
@@ -99,8 +99,8 @@ namespace brw {
fs_builder
group(unsigned n, unsigned i) const
{
- assert(n <= dispatch_width() &&
- i < dispatch_width() / n);
+ assert(force_writemask_all ||
+ (n <= dispatch_width() && i < dispatch_width() / n));
fs_builder bld = *this;
bld._dispatch_width = n;
bld._group += i * n;
diff --git a/src/mesa/drivers/dri/i965/brw_fs_cse.cpp b/src/mesa/drivers/dri/i965/brw_fs_cse.cpp
index 70f0217..0aa700e 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_cse.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_cse.cpp
@@ -180,8 +180,8 @@ create_copy_instr(const fs_builder &bld, fs_inst *inst, fs_reg src, bool negate)
{
int written = inst->regs_written;
int dst_width = inst->dst.width / 8;
- const fs_builder ubld = bld.group(inst->exec_size, inst->force_sechalf)
- .exec_all(inst->force_writemask_all);
+ const fs_builder ubld = bld.exec_all(inst->force_writemask_all)
+ .group(inst->exec_size, inst->force_sechalf);
fs_inst *copy;
if (written > dst_width) {
--
2.4.3
signature.asc
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
