Matt Turner <matts...@gmail.com> writes:
> On Thu, Jun 4, 2015 at 9:04 AM, Francisco Jerez <curroje...@riseup.net> wrote:
>> ---
>> src/mesa/drivers/dri/i965/brw_fs_cse.cpp | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs_cse.cpp
>> b/src/mesa/drivers/dri/i965/brw_fs_cse.cpp
>> index 3ddd17c..822a6a3 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs_cse.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_fs_cse.cpp
>> @@ -206,9 +206,10 @@ create_copy_instr(fs_visitor *v, fs_inst *inst, fs_reg
>> src, bool negate)
>> copy = v->LOAD_PAYLOAD(inst->dst, payload, sources, header_size);
>> } else {
>> copy = v->MOV(inst->dst, src);
>> - copy->force_writemask_all = inst->force_writemask_all;
>> copy->src[0].negate = negate;
>> }
>> + copy->force_writemask_all = inst->force_writemask_all;
>> + copy->force_sechalf = inst->force_sechalf;
>
> I thought we resolved not to allow force_writemask_all/force_sechalf
> on load_payload instructions?
>
I don't see why not, it's easy to come up with a well-behaved
interpretation of those flags for LOAD_PAYLOAD (apply them to all
payload MOVs it's lowered to), it's going to be useful, and, well,
except for this bug it's already working.> I see in lower_load_payload that we often do > > mov->force_writemask_all = inst->force_writemask_all; > > where inst is the load_payload, as if its force_writemask_all might be > set, but I don't see any places where we're setting it. I also don't > see any uses of force_sechalf with load_payload. Will > lower_load_payload do the "right thing" with these set? What is the > "right thing"? > Yes, it does the right thing already. > We've clearly got a bug in that we're dropping force_sechalf on the > MOV, but what problem are you fixing with the LOAD_PAYLOAD? > Presumably something later will emit a texture or an atomic with > force_sechalf and if we drop force_sechalf during CSE we'll break > something? That's the kind of thing the commit message needs to say. It turns out that, yes, image load/store will need LOAD_PAYLOAD instructions with force_sechalf set, but I didn't expect it to need any further explanation, it seemed obviously incorrect to respect the execution controls of the generating instruction *only* if the generating instruction wasn't of certain type.
signature.asc
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
