Re: [PATCH] [ARC] Improve filtering instructions while disassembling.

[Andrew Burgess <andrew dot burgess at embecosm dot com>]

* Claudiu Zissulescu <Claudiu.Zissulescu@synopsys.com> [2016-07-18 16:46:56 +0000]:

> > Hmm, so how do you ensure that the user does not attempt to link an
> > application intended for one particular ARC variant with a library
> > built for a different ARC variant ?  Or is this something which is
> > not currently addressed, but which the ABI notes are intended to
> > address ?
> 
> That is the current problem, which we try to avoid using the .note. At this moment, we make a very raw selection between various architectures ARC6xx, ARC7xx, ARCHS, and ARCEM based on e_flags and friends. However, even added .note will not fully solve the issue as there may be programmers tring to link objects which are incompatible. For example, libraries obtained with Synopsys payed compiler with gnu build object. Hence, though .note seems to solve all the problems it doesn't until all ARC elf producing/consuming tools are aligned to this standard. Thus, my quest for standardization among all ARC tools free or not.
> 
> > Umm, so does that mean that the meaning of ARC opcodes can be changed
> > on a per-customer basis ?  Isn't that going to make correct disassembly
> > almost impossible if the tool has not been customised for that particular
> > customer ?  (Sorry about the pun).
> 
> That is possible, although some of our customers are hiring people like Andrew to place their custom ops/ or custom configuration into mainline and correct this deficiency.  Others are just happy with extinstruction pseudo-op and friends to add support for their custom insns.
> 
> > Also - with multilibs you have to select the desired CPU variant using
> > a command line switch when compiling right ?  So a [multilib experienced]
> > user will already be familiar with the idea of selecting a cpu, and adding
> > an option to objdump (to override a default set up by the --with-cpu) will
> > hardly be a difficulty for them.
> 
> From gcc point of view, they are used to pass the options combinations to the driver, but sometime we teach them to build the toolchain only for a particular configuration. Hence, everything works without the switches. It really depends on how much a customer wants to be involved in gnu building process. Like, I just want to use a prebuild toolchain versus I want to have my toolchain to match my custom configuration. 

Claudiu,

There is something that maybe you could quantify that would help (I
think) understand more about this patch.

If we consider two arc extensions, lets call them A and B, then we
could imagine their opcode spaces as being like this:

    AAAAAAAAAAAAAAA
        BBBBBBBBBBBBBBB
        |-Overlap-|

The algorithm in your patch relies on spotting an instruction from
either 'A' or 'B' in the non-overlapping region, before spotting an
instruction from the overlapping region (right?)

If the first thing we spot is an instruction from the overlapping
region then we have no way to know if we should pick 'A' or 'B', so we
just pick one based on code order (say 'A').

After that if we see a 'B' from the non-overlapping region then (I
think) we'll get a '.word ...' or some other immediate value form in
the output (right?).

So my question would be, for the extensions that you have visibility
of, which you know overlap, what is the degree of overlap?  If the
overlap is high then we'll likely be wrong ~50% of the time.  If the
degree of overlap is low, then we'll be right more often.

I also wonder if the patch could be improved such that if the first
thing we spot is an overlap then maybe we can signal this in some way
(to indicate that we are guessing at the decode)?  Or maybe as Nick
suggested earlier, error in that case with a message indicating that a
command line flag is needed?

Also, I wonder if we first see an overlap, and then see a
non-overlapping instruction from the sub-set we didn't choose (imagine
we guessed 'A', but then see a non-overlapping 'B') maybe we should
error, or at least print a warning to confess that we likely got it
wrong.

In general I understand, and am sympathetic to your desire to handle
legacy, or non-free toolchains, my concern here is primarily that I
would rather see the fuller meta-data driven solution implemented
first, and then add the fuzzy/guess based solution as a later pass.

I have not looked at your revised patch in detail yet (sorry, I was
crazy busy today), but over this patch I would like to see the
deficiencies in the algorithm (discussed above) mentioned more clearly
code comments around you filtering code.  The original patch (I felt)
gave the impression that this change would "do-the-right-thing", when
I think it's more "have-a-guess-at-what-the-right-thing-is".

I hope this helps to make my concerns clearer.

Many thanks,
Andrew