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? > 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 _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev