This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: PING: Re: [PATCH] cpu/opcodes: Sync up fr30.cpu and generated opcodes files
- From: Andrew Burgess <andrew dot burgess at embecosm dot com>
- To: Alan Modra <amodra at gmail dot com>
- Cc: binutils at sourceware dot org
- Date: Thu, 3 Mar 2016 08:36:01 +0000
- Subject: Re: PING: Re: [PATCH] cpu/opcodes: Sync up fr30.cpu and generated opcodes files
- Authentication-results: sourceware.org; auth=none
- References: <26ed35ad6217386ff606faa9b3020d58cc6dbfd0 dot 1455410562 dot git dot andrew dot burgess at embecosm dot com> <20160302002258 dot GV9275 at embecosm dot com> <20160302080341 dot GD10657 at bubble dot grove dot modra dot org> <20160302122232 dot GY9275 at embecosm dot com> <20160303022841 dot GA11933 at bubble dot grove dot modra dot org>
* Alan Modra <amodra@gmail.com> [2016-03-03 12:58:41 +1030]:
> On Wed, Mar 02, 2016 at 12:22:32PM +0000, Andrew Burgess wrote:
> > I just wonder, in my patch you'll notice I also updated the comment
> > just before the line in cpu/fr30.cpu, after your commit I don't think
> > that the comment makes sense any more. I replaced the comment with
> > one which I felt explained what was going one. I'll be honest, I'm
> > not sure that I would immediately understand your use of -16.
>
> I don't see any difference between "x & (-1 << 4)", "x & -16", or
> "x & ~15" on 2's complement arithmetic. The first expression is the
> original code, the second my change, the third yours. So if the
> comment was correct before my change (or yours for that matter), it is
> correct afterwards. That's why I left the comment alone.
>
> On looking into it a bit deeper, I'd say the comment is quite wrong to
> say anything about 64 bit hosts. That isn't the issue here.
> Something like the following is more likely to be correct.
>
> * fr30.cpu (f.m4): Replace bogus comment with a better guess
> at what is really going on.
>
> diff --git a/cpu/fr30.cpu b/cpu/fr30.cpu
> index 0124647..84c32cf 100644
> --- a/cpu/fr30.cpu
> +++ b/cpu/fr30.cpu
> @@ -159,10 +159,12 @@
> (dnf f-u4c "4 bit 0 extended" () 12 4)
> (df f-i4 "4 bit sign extended" () 8 4 INT #f #f)
> (df f-m4 "4 bit minus extended" () 8 4 UINT
> + ; ??? This field takes a value in the range [-16,-1] but there
> + ; doesn't seem a way to tell CGEN that. Use an unsigned field and
> + ; disable range checks on insertion by masking. Restore the sign
> + ; on extraction. CGEN generated documentation for insns that use
> + ; this field will be wrong.
> ((value pc) (and WI value (const #xf)))
> - ; ??? On a 64 bit host this doesn't get completely sign extended
> - ; if the value is recorded in a long, as it is during extraction.
> - ; Various fixes exist, pick one.
> ((value pc) (or WI value (const -16)))
> )
> (dnf f-u8 "8 bit unsigned" () 8 8)
I think that's better.
Thanks,
Andrew