<div dir="ltr"> On Dec 10, 2015 6:58 AM, "Francisco Jerez" <<a href="mailto:currojerez@riseup.net" target="_blank">currojerez@riseup.net</a>> wrote: > > Jason Ekstrand <<a href="mailto:jason@jlekstrand.net" target="_blank">jason@jlekstrand.net</a>> writes: > > > We aren't using it anymore. > > It seems useful to me to be able to represent indirect access as part of > any instruction source or destination register. > > The following: > > | mov_indirect g0, g1, a0 > | foo g2, g0 > > and the converse case with indirect destination offset (which you don't > seem to represent currently) can be implemented by the hardware more > efficiently using a single instruction in certain cases. The current IR > is able to represent what the hardware can do, but supporting the > MOV_INDIRECT instruction only would force us to keep the indirection > separate from the instruction that uses it, so it seems like a less > expressive representation to me than the current approach, unless you're > willing to add _INDIRECT variants of most hardware opcodes. Yes and, mostly, no. Yes, you can put an indirect on almost anything but it has substantial restrictions: 1) Destination indirects must be uniform (I'm only 95% sure this is the case) 2) We only have 8 address subregisters on HSW and prior and 16 on BDW which means: a) All indirects must be uniform OR b) We must be on Broadwell, have only two indirects, and split the instruction OR c) We can only have one indirect (and still split the instruction on HSW)So, yes, "everthing can be uniform", but not in real life. The reladdr abstraction, while maybe ok for a high-level IR, doesn't accurately represent the hardware at all because it can't represent any of the restrictions without piles of helper functions and checks. The MOV_INDIRECT instruction, on the other hand, is something we can always do because it falls in the "one indirect" case above. The reladdr abstraction also suffers from the recursive reladdr problem which MOV_INDIRECT neatly side-steps by design. We can't even use reladdr past initial NIR -> FS stage because none of the optimization passes know what to do with it (and no, I don't want to add reladdr support to any of them).Yes, removing reladdr makes the IR "less expressive", but we're not really using that expressiveness nor is reladdr the abstraction we want. If we want to start taking more advantage of relative addressing in the future, we can come up with a better abstraction then.--Jason > > --- > > src/mesa/drivers/dri/i965/brw_fs.cpp | 7 +------ > > src/mesa/drivers/dri/i965/brw_ir_fs.h | 5 +---- > > 2 files changed, 2 insertions(+), 10 deletions(-) > > > > diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp > > index 7cc03c5..786c5fb 100644 > > --- a/src/mesa/drivers/dri/i965/brw_fs.cpp > > +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp > > @@ -433,7 +433,6 @@ fs_reg::fs_reg(struct ::brw_reg reg) : > > { > > this->reg_offset = 0; > > this->subreg_offset = 0; > > - this->reladdr = NULL; > > this->stride = 1; > > if (this->file == IMM && > > (this->type != BRW_REGISTER_TYPE_V && > > @@ -448,7 +447,6 @@ fs_reg::equals(const fs_reg &r) const > > { > > return (this->backend_reg::equals(r) && > > subreg_offset == r.subreg_offset && > > - !reladdr && !r.reladdr && > > stride == r.stride); > > } > > > > @@ -4716,9 +4714,7 @@ fs_visitor::dump_instruction(backend_instruction *be_inst, FILE *file) > > break; > > case UNIFORM: > > fprintf(file, "u%d", inst->src[i].nr + inst->src[i].reg_offset); > > - if (inst->src[i].reladdr) { > > - fprintf(file, "+reladdr"); > > - } else if (inst->src[i].subreg_offset) { > > + if (inst->src[i].subreg_offset) { > > fprintf(file, "+%d.%d", inst->src[i].reg_offset, > > inst->src[i].subreg_offset); > > } > > @@ -4829,7 +4825,6 @@ fs_visitor::get_instruction_generating_reg(fs_inst *start, > > { > > if (end == start || > > end->is_partial_write() || > > - reg.reladdr || > > !reg.equals(end->dst)) { > > return NULL; > > } else { > > diff --git a/src/mesa/drivers/dri/i965/brw_ir_fs.h b/src/mesa/drivers/dri/i965/brw_ir_fs.h > > index c3eec2e..e4f20f4 100644 > > --- a/src/mesa/drivers/dri/i965/brw_ir_fs.h > > +++ b/src/mesa/drivers/dri/i965/brw_ir_fs.h > > @@ -58,8 +58,6 @@ public: > > */ > > int subreg_offset; > > > > - fs_reg *reladdr; > > - > > /** Register region horizontal stride */ > > uint8_t stride; > > }; > > @@ -136,8 +134,7 @@ component(fs_reg reg, unsigned idx) > > static inline bool > > is_uniform(const fs_reg &reg) > > { > > - return (reg.stride == 0 || reg.is_null()) && > > - (!reg.reladdr || is_uniform(*reg.reladdr)); > > + return (reg.stride == 0 || reg.is_null()); > > } > > > > /** > > -- > > 2.5.0.400.gff86faf > > > > _______________________________________________ > > mesa-dev mailing list > > <a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a> > > <a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev" target="_blank">http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </div>