On 23 Feb 2015 10:40, James Bowman wrote:
> > > 2014-02-03  James Bowman  <james.bowman@ftdichip.com>
> > > 
> > >         * gdb/Makefile.in, gdb/configure.tgt: FT32 target added
> > >         * sim/configure.tgt: FT32 target added
> > >         * sim/configure: Regenerated
> > >         * sim/ft32/configure: Regenerated
> > >         * gdb/ft32-tdep.c,h: Support FT32
> > >         * sim/ft32/*: FT32 simulator
> > 
> > notes:
> >  - ChangeLog entries are split up across dirs
> 
> Do you mean I should not split? Organize in some other way?

see my patch here as an example:
https://sourceware.org/ml/gdb-patches/2014-03/msg00173.html

> > where's the testsuite man ? :)  it should be trivial to start one with .s
> > files  -- just look at sim/testsuite/sim/.  otherwise there's no way to keep
> > regressions from slipping in.
> 
> Good idea. We are currently running the gcc testsuite on the simulator
> as our regression test.  Would it be OK if I defer, and add a sim
> testsuite after this submit?

that's what everyone says ;).  the trouble with having no sim testsuite is that 
people (like me) only use the sim testsuite to verify common/arch changes don't 
break things.  you don't need a comphrensive one, just one or two basic things.  
that way we at least know the sim isn't completely hosed.

> > > +/* Use an invalid address value as 'not available' marker.  */
> > > +enum { REG_UNAVAIL = (CORE_ADDR) -1 };
> 
> This is actually the value -1 being cast. I have rewritten the line to
> make this clearer.

ah sorry about that

> > since you're a new port, you should start with SIM_AC_OPTION_WARNINGS enabled.  
> > obviously that also means cleaning up all the warnings generated in the ft32/ 
> > subdir once you do :).
> 
> Done - it now compiles without warnings. It would be nice to be able to
> build it with warnings as errors.

the functionality is in place, but it's disabled atm because the sim code has a 
lot of legacy that needs cleaning.  although might be good to bite that bullet 
now and uncomment the lines in common/acinclude.m4 that disables it.

> > > --- /dev/null
> > > +++ b/sim/ft32/interp.c
> > 
> > ideally you'd switch to sim-reg.o in your Makefile's SIM_OBJS ... that'll 
> > provide these entry points.  that would require also enabling sim-model.o 
> > & SIM_AC_OPTION_DEFAULT_MODEL support, but i don't think that'd be too hard.
> > if you look at bfin/machs.c and start at "sim_machs", i think you should be 
> > able to track it down easily enough.
> 
> This change is causing me some trouble.  OK to defer it until after the
> main submit?

mach/model support can wait

i'll follow up once i review the actual code ;)
-mike