This is the mail archive of the cgen@sources.redhat.com mailing list for the CGEN project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [patch][rfa] Ordering insns in hash chain for cgen disassemblers


Doug Evans wrote:

>
>Setting aside the state of the implementation of ifield assertions
>(since I don't remember what it is), why wouldn't an ifield assertion
>work here?
>
Yes, I specified them fully and found that they are ignored  :-(

>I'm not saying I disapprove of the patch in principle, just curious
>whether you tried that and the results of it.
>
>Studying your patch though, I have some questions.
>Are you assuming both insns are on the same hash chain?
>[If not, never mind of course.]
>If so, is that guaranteed?
>
Nope it's not guaranteed. If two such insns end up on different hash 
chains, then even better. The ambiguity would be removed at a higher 
level. The solution presented by my patch removes dependence on any 
particular hash function by allowing the insns to be resolved even if 
they do end up on the same hash chain.

>Is basing the sort strictly on the number of decodable bit sufficient?
>
I think that basing the sort on the number of decodable bits is 
sufficient becase the decodable bits of the more general insn will be a 
proper subset of the decodable bits if the other. Thus the number of 
decodable bits will be less than that of the less general insn. Sorting 
based on the number of decodable bits will therefore guarantee the 
relative ordering  between the conflicting insns.

>Also, style nit: I hate the absence of a blank line between the comment
>introducing the function and the function definition.
>
Feel free to add one if you like.

>Another style note while I'm on it.  Boy do I hate the proliferation
>of "pretty aligned code".
>
>int                         a;
>some_long_type_definition_t b;
>
I assume you're referring to previous code I may have written? I didn't 
do this in this patch, although I personally do like this style.

>(or see the static fn decls at the top of cgen-dis.c)
>
That was there already.

>puhleeze. :-(
>
Thank you  :-)



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]