Iago Toral <ito...@igalia.com> writes: > On Wed, 2016-09-07 at 18:48 -0700, Francisco Jerez wrote: >> This fixes regs_written() and regs_read() to return a more accurate >> value when the padding left between components due to a stride value >> greater than one causes the region bounds given by size_written or >> size_read to overflow into the next register. This could become a >> problem in optimization passes that keep track of dataflow using >> fixed-size arrays with register granularity, because the overflow >> register (not actually accessed by the region) may not have been >> allocated at all which could lead to undefined memory access. > > ah, nice catch! I am curious: did you find any cases where this was > creating a real problem or just something you noticed by inspection? >
Yeah, it definitely fixed real problems in a bunch of FP64 tests (I
forgot which ones but I could dig it up if you're interested) which
would have regressed from the next commits that fix regs_written and
regs_read to take into account misalignment -- The only reason this
wasn't a problem before is that we weren't rounding up the register
count correctly in most places...
>> An alternative to this would be to subtract the trailing padding
>> already during the calculation of fs_inst::size_read and
>> ::size_written, but that would break code that currently assumes that
>> ::size_read and _written are whole multiples of the component size,
>> and would be hard to maintain looking forward because size_written is
>> assigned from a bunch of different places.
>
> Yeah, this would be a more problematic solution.
>
>> ---
>> src/mesa/drivers/dri/i965/brw_ir_fs.h | 22 ++++++++++++++++++++--
>> 1 file changed, 20 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_ir_fs.h
>> b/src/mesa/drivers/dri/i965/brw_ir_fs.h
>> index 4ac9baa..0be67b7 100644
>> --- a/src/mesa/drivers/dri/i965/brw_ir_fs.h
>> +++ b/src/mesa/drivers/dri/i965/brw_ir_fs.h
>> @@ -192,6 +192,20 @@ reg_offset(const fs_reg &r)
>> }
>>
>> /**
>> + * Return the amount of padding in bytes left unused between
>> individual
>> + * components of register \p r due to a (horizontal) stride value
>> greater than
>> + * one, or zero if components are tightly packed in the register
>> file.
>> + */
>> +static inline unsigned
>> +reg_padding(const fs_reg &r)
>> +{
>> + const unsigned stride = ((r.file != ARF && r.file != FIXED_GRF) ?
>> r.stride :
>> + r.hstride == 0 ? 0 :
>> + 1 << (r.hstride - 1));
>> + return (MAX2(1, stride) - 1) * type_sz(r.type);
>> +}
>> +
>> +/**
>> * Return whether the register region starting at \p r and spanning
>> \p dr
>> * bytes could potentially overlap the register region starting at
>> \p s and
>> * spanning \p ds bytes.
>> @@ -423,7 +437,9 @@ regs_written(const fs_inst *inst)
>> {
>> /* XXX - Take into account register-misaligned offsets correctly.
>> */
>> assert(inst->dst.file != UNIFORM && inst->dst.file != IMM);
>> - return DIV_ROUND_UP(inst->size_written, REG_SIZE);
>> + return DIV_ROUND_UP(inst->size_written -
>> + MIN2(inst->size_written, reg_padding(inst-
>> >dst)),
>> + REG_SIZE);
>> }
>>
>> /**
>> @@ -438,7 +454,9 @@ regs_read(const fs_inst *inst, unsigned i)
>> /* XXX - Take into account register-misaligned offsets correctly.
>> */
>> const unsigned reg_size =
>> inst->src[i].file == UNIFORM || inst->src[i].file == IMM ? 4 :
>> REG_SIZE;
>> - return DIV_ROUND_UP(inst->size_read(i), reg_size);
>> + return DIV_ROUND_UP(inst->size_read(i) -
>> + MIN2(inst->size_read(i), reg_padding(inst-
>> >src[i])),
>> + reg_size);
>> }
>>
>> #endif
signature.asc
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
