From: Dave Brolley <brolley@redhat.com>
To: Doug Evans <dje@transmeta.com>
Cc: cgen@sources.redhat.com
Subject: Re: [patch][rfa] Ordering insns in hash chain for cgen disassemblers
Date: Thu, 15 Nov 2001 09:21:00 -0000 [thread overview]
Message-ID: <3BF3F9AE.8070907@redhat.com> (raw)
Message-ID: <20011115092100.ptaN_wIT735qEt_Z-T_z4QdsZWMrJyFjwYJVMPcN1_E@z> (raw)
In-Reply-To: <15346.55680.817467.321067@casey.transmeta.com>
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 :-)
next prev parent reply other threads:[~2001-11-15 9:21 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2001-10-05 13:20 Dave Brolley
2001-10-09 12:09 ` [patch][rfa] Ordering insns in hash chain for cgen disassemblers -- committed Dave Brolley
2001-11-14 12:09 ` Dave Brolley
2001-10-09 16:29 ` [patch][rfa] Ordering insns in hash chain for cgen disassemblers Doug Evans
2001-10-10 2:11 ` Dave Brolley [this message]
2001-10-10 3:33 ` Doug Evans
2001-10-10 6:33 ` Dave Brolley
2001-11-15 10:11 ` Dave Brolley
2001-11-15 9:52 ` Doug Evans
[not found] ` <15348.168.750723.846784.cygnus.local.cgen@casey.transmeta.com>
2001-10-13 2:39 ` Frank Ch. Eigler
2001-11-15 12:42 ` Frank Ch. Eigler
2001-11-15 9:21 ` Dave Brolley
2001-11-14 12:52 ` Doug Evans
2001-11-13 11:46 ` Dave Brolley
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=3BF3F9AE.8070907@redhat.com \
--to=brolley@redhat.com \
--cc=cgen@sources.redhat.com \
--cc=dje@transmeta.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).