This is the mail archive of the binutils@sourceware.org mailing list for the binutils 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: Update binutils for Itanium SDM 2.2


On Wed, 2006-02-15 at 16:18, H. J. Lu wrote:
> This patch supports SDM 2.2 and 2 new instructions, vmsw.0 and vmsw.1.

It looks OK except for the handling of the new DV rule 17.  You made no
apparent attempt to handle this correctly.  This code needs to look
something like the existing PSR code for note == 7, except that you will
have a fixed mask including the 3 fields that matter instead of checking
that two insns use the same mask.  Also, there is a complication here in
that mov.i psr= insns take a general register as input.  Hence, you
won't know what fields are being modified, and can't tell for sure
whether there is a DV conflict.  Thus these will have to be "may
violate" instead of "violate" conflicts.  ssm instructions take an
immediate as input, and hence you can check the mask bits for them. 
Thus these will always be "violate" conflicts.  I doubt that you will be
able to get this right unless you take the time to write some testcases,
some that trigger rule 17, and some that don't, for both mov-to-psr and
ssm, and then step through the assembler to see how this all works.

Also, there is a table in the comment before specify_resource that
documents what all of the note values mean.  You did not add 17 to this
list.  This is needed because the .tbl files don't define what the
various extra rules mean.  So the sources are not readable without this.

If you can't figure out how to implement the rule 17 correctly, then
file a bug report, and I can take a look at this.  If you think the
current code is the right approach, then at the very least there should
be a ??? comment pointing out the cases that we don't handle correctly
so this isn't forgotten.   That is, we must document that we don't
verify that one of the 3 relevant fields is being modified.  Right now
the code is emitting more DV warnings than it should, which is at least
safe, but we can do better.

Otherwise the rest of this looks good to me.  I didn't bother to verify
the .tbl files.  I am assuming you are updating them from the master
copies inside Intel.  Some of the clz and shrp4 changes are curious, I'm
surprised we never noticed these omissions before.

If you want to check in this patch and then fix the rule 17 related
problems with a follow on patch, I'm OK with that.
-- 
Jim Wilson, GNU Tools Support, http://www.specifix.com


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