Re: [Patch, AArch64, ILP32] 5/5 Improve the debugging experience with the generated file

[Yufeng Zhang <Yufeng dot Zhang at arm dot com>]

Hi David,

Thanks for the review.

On 06/21/13 19:52, David Daney wrote:
> On 06/21/2013 11:08 AM, Yufeng Zhang wrote:
> > diff --git a/bfd/Makefile.am b/bfd/Makefile.am
> > index 19b3710..0e69d12 100644
> > --- a/bfd/Makefile.am
> > +++ b/bfd/Makefile.am
> > @@ -917,12 +917,14 @@ elf64-target.h : elfxx-target.h
> >
> >    elf32-aarch64.c : elfnn-aarch64.c
> >    	rm -f elf32-aarch64.c
> > -	sed -e s/NN/32/g<  $(srcdir)/elfnn-aarch64.c>  elf32-aarch64.new
> > +	echo "#line 1 \"$(srcdir)/elfnn-aarch64.c\"">  elf32-aarch64.new
> > +	sed -e s/NN/32/g<  $(srcdir)/elfnn-aarch64.c>>  elf32-aarch64.new
> >    	mv -f elf32-aarch64.new elf32-aarch64.c
> >
> >    elf64-aarch64.c : elfnn-aarch64.c
> >    	rm -f elf64-aarch64.c
> > -	sed -e s/NN/64/g<  $(srcdir)/elfnn-aarch64.c>  elf64-aarch64.new
> > +	echo "#line 1 \"$(srcdir)/elfnn-aarch64.c\"">  elf64-aarch64.new
> > +	sed -e s/NN/64/g<  $(srcdir)/elfnn-aarch64.c>>  elf64-aarch64.new
> >    	mv -f elf64-aarch64.new elf64-aarch64.c
> >
> >    elf32-ia64.c : elfnn-ia64.c
>
> I really hate this pattern of running the file through sed and
> synthesizing the #line.
>
> I know this was built up out of several of the other patches, but I am
> commenting on it here.

I understand that you dislike the way of generating c files through 
'sed'; there were similar comments received during the internal patch 
review.  I had considered the cpp macro based approach but didn't find 
it particular helpful in parameterizing the large number of different 
structure names and functions names that contain elfNN or ElfNN (where 
NN is either 64 or 32); it will make the source code less readable by 
defining more macros.

Another argument is that the use of 'sed' is an existing technique in 
bfd.  elf32-target.h and elf64-target.h are generated from 
elfxx-target.h via 'sed', so is the backend file generation in the IA64 
port.

> Can you instead move the duplicated code to a file to be included
> twice, parameterizing it with one or more preprocessor macros?
>
> I.E.:
>
> .
> .
> .
> #define AARCH64_ABI 32
> #include "elfnn-arch64.c"
> #undef AARCH64_ABI
> #define AARCH64_ABI 64
> #include "elfnn-arch64.c"
> .
> .
> .

It looks to me that due to the way bfd is structured it is not feasible 
to have the ELF32 and ELF64 support defined in the same file.  If we 
really want to go after the cpp macro based approach, we can have two 
different files include elfnn-aarch64.c with different macro definitions.


Regards,
Yufeng