Iago Toral Quiroga <ito...@igalia.com> writes: > So we can access it in the vec4 backend to handle byte offsets into > registers.
This change has deep implications in the meaning of the vec4 register objects because the representation of register offsets is now split between 'reg_offset' and 'subreg_offset', and there are a *lot* of places that directly rely on the current representation of register offsets, just grep the VEC4 back-end for 'reg_offset' -- Every occurrence is now suspect of being inaccurate because the offset of a register is no longer guaranteed to be 32B-aligned, so comparing reg_offsets alone to find out whether two registers are equivalent or whether they overlap is no longer sufficient. Do you *really* need to make this representation change for FP64? Or is there some way around it? ISTR that in the big VEC4 FP64 series you ended up using backend_reg::subnr for this (which is just an undercover version of subreg_offset so the same caveat applies :P) when you had to address both halves of a single-precision register during SIMD lowering (Can you remind me what the exact use-case was for that?). To address the ZW components of a double-precision vector this seems less of a requirement because you can just use the logical (i.e. 64-bit-based) swizzles at the IR level and only translate to physical swizzles+offsets+strides during codegen time. If the answer is that you definitely need this change, I think I'm going to help you out with this because the change is going to be massive and involve auditing the whole VEC4 back-end. I think there are two lessons to learn from the FS back-end: - Having the register offset split between two variables has been an endless source of bugs, because it's just too tempting to only take one of them into account and ignore the other. - It wouldn't be substantially more difficult to change reg_offset to be expressed in byte units instead, even if it involves going through every occurrence of reg_offset in the back-end, because adding a separate subreg_offset field invalidates every ocurrence of reg_offset in the back-end anyway. > --- > src/mesa/drivers/dri/i965/brw_ir_fs.h | 6 ------ > src/mesa/drivers/dri/i965/brw_shader.h | 6 ++++++ > 2 files changed, 6 insertions(+), 6 deletions(-) > > diff --git a/src/mesa/drivers/dri/i965/brw_ir_fs.h > b/src/mesa/drivers/dri/i965/brw_ir_fs.h > index f214483..00fbace 100644 > --- a/src/mesa/drivers/dri/i965/brw_ir_fs.h > +++ b/src/mesa/drivers/dri/i965/brw_ir_fs.h > @@ -52,12 +52,6 @@ public: > /** Smear a channel of the reg to all channels. */ > fs_reg &set_smear(unsigned subreg); > > - /** > - * Offset in bytes from the start of the register. Values up to a > - * backend_reg::reg_offset unit are valid. > - */ > - int subreg_offset; > - > /** Register region horizontal stride */ > uint8_t stride; > }; > diff --git a/src/mesa/drivers/dri/i965/brw_shader.h > b/src/mesa/drivers/dri/i965/brw_shader.h > index e61c080..ae23830 100644 > --- a/src/mesa/drivers/dri/i965/brw_shader.h > +++ b/src/mesa/drivers/dri/i965/brw_shader.h > @@ -75,6 +75,12 @@ struct backend_reg : private brw_reg > */ > uint16_t reg_offset; > > + /** > + * Offset in bytes from the start of the register. Values up to a > + * backend_reg::reg_offset unit are valid. > + */ > + uint16_t subreg_offset; > + > using brw_reg::type; > using brw_reg::file; > using brw_reg::negate; > -- > 2.7.4
signature.asc
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
