[Mesa-dev] [PATCH 05/19] i965/fs: Loop from 0 to inst->sources, not 0 to 3.
Ian Romanick
idr at freedesktop.org
Mon Jun 2 15:48:23 PDT 2014
On 05/31/2014 10:11 PM, Kenneth Graunke wrote:
> On Tuesday, May 27, 2014 06:47:36 PM Matt Turner wrote:
> [snip]
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp
> b/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp
>> index 069b60f..a1aff21 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp
>> @@ -353,7 +353,7 @@ fs_visitor::try_constant_propagate(fs_inst *inst,
> acp_entry *entry)
>> if (entry->src.file != IMM)
>> return false;
>>
>> - for (int i = 2; i >= 0; i--) {
>> + for (int i = inst->sources - 1; i >= 0; i--) {
>> if (inst->src[i].file != entry->dst.file ||
>> inst->src[i].reg != entry->dst.reg ||
>> inst->src[i].reg_offset != entry->dst.reg_offset ||
>
> I am glad to see that you're using a signed integer type here. For
> BRW_OPCODE_NOP, SHADER_OPCODE_PLACEHOLDER_HALT, and so on, inst->sources could
> conceievably be 0. (Today, you set it to 3 for some reason, but I could see
> someone coming along and "fixing" that later...)
>
> If you had used unsigned types, this would translate into i = 4294967296; i >=
> 0; i--, which would take forever. With signed types, it works, and no one
> should get bitten by that bug.
Consistency of types is good. Some folks build with additional
warnings, and the flood of
main/bufferobj.c:2855:18: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
can hide legit bugs. :(
> Patches 1-8 are:
> Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
> (and you can ignore my comment on patch 4 - I'm happy to see that done in
> later patches - no need to block this any further on trivialities)
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140602/952c004e/attachment.sig>
More information about the mesa-dev
mailing list