Jason Ekstrand <ja...@jlekstrand.net> writes: > On Dec 10, 2015 6:58 AM, "Francisco Jerez" <curroje...@riseup.net> wrote: >> >> Jason Ekstrand <ja...@jlekstrand.net> 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: > Yes, I'm aware of the restrictions of indirect addressing on Gen hardware. The fact that reladdr can represent addressing modes which aren't directly implemented in the hardware doesn't invalidate the abstraction, nor implies that optimization and translation passes must be made aware of such restrictions. It just means that our abstraction is a superset of the hardware indirect addressing scheme, which is fine given a lowering pass to convert indirect addressing into a legal form.
Your proposal instead goes the opposite direction and replaces the
preexisting abstraction with a new abstraction which can only represent
a tiny subset of the functionality of hardware instructions, which I
don't think is acceptable for a low-level IR.
> 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:
>
That's the kind of thing that SIMD lowering would be able to take care
of easily.
> 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 reladdr abstraction can represent the full flexibility of the
hardware, while MOV_INDIRECT cannot, that makes reladdr a more accurate
low-level representation of the program. For that reason I'd argue the
exact opposite: MOV_INDIRECT would be a great abstraction and a welcome
simplification for a high-level IR (e.g. NIR), in which nothing is lost
by sacrificing the expressiveness of individual instructions in favour
of instruction composition, because the back-end can always combine
multiple high-level instructions into a single low-level instruction
where the hardware ISA allows, as long as the low-level IR is able to
represent the full functionality of hardware instructions (IOW the
mapping between low-level IR instructions and hardware instructions is
surjective), which is further from being the case after this series.
> 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.
I don't think the reladdr "problem" requires any fundamentally more
difficult logic in optimization passes than what you need in order to
handle MOV_INDIRECT instructions reasonably.
> 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 ®)
>> > {
>> > - 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
>> > mesa-dev@lists.freedesktop.org
>> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
signature.asc
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
