This is the mail archive of the
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: Wed, 2 Mar 2016 12:22:32 +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>
* Alan Modra <firstname.lastname@example.org> [2016-03-02 18:33:41 +1030]:
> On Wed, Mar 02, 2016 at 12:22:58AM +0000, Andrew Burgess wrote:
> > Ping!
> > * Andrew Burgess <email@example.com> [2016-02-14 00:44:15 +0000]:
> > > In commit b6518b3 (Fix compile time warnings generated when compiling
> > > with clang.) the generated fr30-ibld.c file was modified, but the
> > > underlying fr30.cpu file, from which it is generated was not.
> > >
> > > This commit updates the fr30.cpu file in such a way that the C code
> > > generated in fr30-ibld.c avoids the undefined behaviour that b6518b3 was
> > > trying to avoid, while also being something that CGEN can handle without
> > > any changes.
> Sorry, I had this email queued up to answer in my inbox, and then
> totally forgot about it. I didn't see it again until I'd worked my
> way through my email backlog.
> > > * fr30.cpu (f-m4): Adjust extract handler to avoid shifting a
> > > negative value (undefined behaviour in C).
> This is fine, but I've already committed a slightly different patch,
> in a quest to have no local patches to all the generated files in
> opcodes. See https://sourceware.org/ml/cgen/2016-q1/msg00002.html
> and https://sourceware.org/ml/cgen/2016-q1/msg00003.html
OK, I see, commit 62de1c630f16c21418464727692bcd29e23ef1b0.
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.
Do you think it would be worth updating the comment to better explain
what's going on.... feel free to take anything from my patch if that