[Mesa-dev] [PATCH 2/3] r600g: replace trans/vector-only instruction lists with ranges

[Vadim Girlin]

On Mon, 2012-01-23 at 14:20 +0100, Christian König wrote:
> On 22.01.2012 17:24, Dave Airlie wrote:
> > 2012/1/22 Christian König<deathsimple at vodafone.de>:
> >> On 22.01.2012 16:46, Dave Airlie wrote:
> >>> 2012/1/22 Christian König<deathsimple at vodafone.de>:
> >>>> Sorry, but that looks really ugly and pretty much unmaintainable, cause
> >>>> you
> >>>> constantly need to lookup the meaning of the values.
> >>>>
> >>>> Also I haven't looked into the docs (but going to do so tomorrow), but
> >>>> I'm
> >>>> pretty sure that those ranges aren't 100% correct.
> >>> It was the docs that the ranges came from, and we keep forgetting to
> >>> add things to these lookup functions.
> >>
> >> Really? Where? I asked around when I coded this in the first place if it
> >> could be simplified by using ranges, but never got an clear answer on this
> >> topic.
> >>
> >> When I now look into the AMD docs they mostly seems to use tables for opcode
> >> attributes, so I assumed that they are spread around in the opcode range.
> >>
> >> If this isn't the case then this indeed seems to be an good idea.
> > Its in the Evergreen ISA docs (it might only be evergreen they managed this).
> >
> > ALU_WORD1_OP2
> > ALU_INST
> > [17:7]
> > enum(11)
> > Instruction. The top three bits of this field must be zero. Gaps in
> > opcode values
> > are not marked in the list below. See Chapter 7 for descriptions of each
> > instruction. Opcodes 0..95 can be used in either the Vector or Trans unit.
> > Opcodes 128..159 are Trans only. Opcodes 160..255 are vector only.
> Those ranges are even better than the table based documentation! The 
> tables contained an quite ugly bug which lead to this comment in the 
> original code:
> 
> /* Note that FLT_TO_INT_* instructions are vector-only instructions
>   * on Evergreen, despite what the documentation says. FLT_TO_INT
>   * can do both vector and scalar. */
> 
> Very interesting, but I couldn't such ranges for R6xx and R7xx. Alex do 
> you remember where you got that original documentation from?
> 
> Anyway I would still suggest to not use magic numbers, let's define the 
> beginning and end of ranges in r600_opcode.h instead, and then use those 
> defines inside the code.

I'm not sure which names we could choose for that, and if this will make
things more clear and readable. I think at least for me it's much easier
to compare some opcode against the numeric ranges, than to look for
names/values in the additional lists/headers/etc.

Vadim

> 
> Christian.