[Mesa-dev] [PATCH 07/38] i965/vec4: Take into account all instruction fields in CSE instructions_match().

Matt Turner mattst88 at gmail.com
Fri Jun 5 14:20:57 PDT 2015


On Fri, Jun 5, 2015 at 2:10 PM, Francisco Jerez <currojerez at riseup.net> wrote:
> Matt Turner <mattst88 at gmail.com> writes:
>
>> On Thu, Jun 4, 2015 at 9:04 AM, Francisco Jerez <currojerez at riseup.net> wrote:
>>> Most of these fields affect the behaviour of the instruction so it
>>> could actually break the program if we CSE a pair of otherwise
>>> matching instructions with different values of these fields.
>>> ---
>>>  src/mesa/drivers/dri/i965/brw_vec4_cse.cpp | 8 ++++++++
>>>  1 file changed, 8 insertions(+)
>>>
>>> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_cse.cpp b/src/mesa/drivers/dri/i965/brw_vec4_cse.cpp
>>> index 9147c3c..c9fe0ce 100644
>>> --- a/src/mesa/drivers/dri/i965/brw_vec4_cse.cpp
>>> +++ b/src/mesa/drivers/dri/i965/brw_vec4_cse.cpp
>>> @@ -114,8 +114,16 @@ instructions_match(vec4_instruction *a, vec4_instruction *b)
>>>  {
>>>     return a->opcode == b->opcode &&
>>>            a->saturate == b->saturate &&
>>> +          a->predicate == b->predicate &&
>>> +          a->predicate_inverse == b->predicate_inverse &&
>>>            a->conditional_mod == b->conditional_mod &&
>>> +          a->flag_subreg == b->flag_subreg &&
>>>            a->dst.type == b->dst.type &&
>>> +          a->offset == b->offset &&
>>> +          a->mlen == b->mlen &&
>>> +          a->base_mrf == b->base_mrf &&
>>> +          a->header_size == b->header_size &&
>>> +          a->shadow_compare == b->shadow_compare &&
>>
>> I guess this is fine, but it's really just dead code. We don't CSE
>> textures in the vec4 backend.
>
> We don't today, but we're not that far from supporting send-from-GRF
> texturing and some load-payload-like instruction in the vec4 back-end,

Ah, I didn't think about that. Okay, that makes sense.

> and when we do, it seems like a nice trap for the future to have a
> function called "instructions_match()" that only checks if half of the
> instruction matches...
>
> That said, I should probably have made it more clear in the commit
> message that this doesn't fix any bug that I'm aware of -- I mostly
> copy-pasted the commit message from the commit that does the same in the
> FS back-end, which fixes an actual bug.


More information about the mesa-dev mailing list