This is the mail archive of the
binutils@sources.redhat.com
mailing list for the binutils project.
Re: [PATCH] ia64 bundling issues
- From: James E Wilson <wilson at specifixinc dot com>
- To: Jan Beulich <JBeulich at novell dot com>
- Cc: binutils at sources dot redhat dot com
- Date: Wed, 26 Jan 2005 18:55:12 -0800
- Subject: Re: [PATCH] ia64 bundling issues
- References: <s1f4ca67.023@emea1-mh.id2.novell.com>
On Mon, 2005-01-24 at 02:14, Jan Beulich wrote:
> * config/tc-ia64.c (emit_one_bundle): Snapshot manual bundling
> state
> before actually using it. Don't generate an error in manual
> bundling
> ...
This looks mostly OK.
Your comments don't follow GNU coding conventions, which require that
they contain full sentences that start with a capital letter and end
with a period followed by two spaces. I realize that we already have a
number of improper comments, but we shouldn't add more of them.
Also, the ChangeLog entry should normally say what changes were made to
the code, not why the change was made. Info about why changes were made
belong in comments in the source code, not in the ChangeLog file. This
ChangeLog rule isn't always rigorously enforced though and what you
have is probably OK, but it would be nice if more of this stuff was
actually comments in the source code.
> + || (user_template >= 0
> + && (template ^ required_template) > 1)))
I believe this part of the patch is wrong. In the object file encoding
of templates, the low order bit is used to differentiate between whether
or not there is a stop bit at the end of the template. I think this is
what you are trying to check here. However, in gas internals, we have
our own encoding of templates which drops this low bit, as we don't care
whether there is a stop bit at the end or not. See the ia64_templ_desc
table in opcodes/ia64-opc.c, and note that for instance MFI is 6 and MMF
is 7. I think the test you want here is
(user_template >= 0 && template == required_template)
--
Jim Wilson, GNU Tools Support, http://www.SpecifixInc.com