On 15 Aug 2016 10:22, Nick Clifton wrote:
> Hi Mike,
> > diff --git a/sim/aarch64/memory.h b/sim/aarch64/memory.h
> > -extern void         mem_add_blk (sim_cpu *, uint64_t, char *, uint64_t, bfd_boolean);
> 
> (I have no problem with this part of the patch, but just to note
> that it is nothing to do with unifying symbol table handling...)

true, but it was collateral damage.  by removing the (now unused &
unneeded) bfd.h include from this header, this prototype broke (as
it uses bfd_boolean).  so i had to drop it as well.

> > +int
> > +trace_load_symbols (SIM_DESC sd)
> > +{
> 
> Wouldn't it make sense for trace_load_symbols to also remove useless
> symbols and then sort the table ?  Isn't this something that all 
> sims will want ?  [The code for remove_useless_symbols in aarch64/interp.c
> is basically generic, not aarch64 specific].

i hadn't fully digested this bit of logic ... i was focusing on the bits
the core already handled, and were requiring custom clean ups/callbacks
in the non-common code.  you're right though that we could update the
core to include this code for the benefit of all.

> > +bfd_vma
> > +trace_sym_value (SIM_DESC sd, const char *name)
> > +{
> > [...]
> > +  for (i = 0; i < STATE_PROG_SYMS_COUNT (sd); ++i)
> > +    if (strcmp (asymbols[i]->name, name) == 0)
> > +      return bfd_asymbol_value (asymbols[i]);
> 
> If there was a flag to say that the symbol table was sorted, then
> this lookup could be done using bsearch().
> 
> 
> So basically as far as I can see there is nothing wrong with the patch.
> Certainly not from an AArch64 of MSP430 point of view.

thanks.  i'll merge this as-is, and add your ideas to my TODO list :).
-mike