Re: [PATCH] Add word-sized conversion functions to libgcc2

[Richard Sandiford <richard at codesourcery dot com>]

Adam Nemet <anemet@caviumnetworks.com> writes:
>   2. I don't generate the functions Roger has added but basically keep
>   them around.  This I think is easy to fix too but just adding the
>   names into LIB2FUNCS_OPT.  I just wanted to minimize my changes so
>   that I could apply the same patch to 4.1 as well.

To be honest, I was hoping to do the same with mine.  Roger's patch
was applied to 4.1 after it branched, and the bug it triggered is a
regression from previous releases, so I was hoping the fact that my
patch reverts his would not make it any less suitable.  It's Mark's
call of course.

>   3. I extend the old unfortunate design which apparently is very
>   prone to error.
>
>   This I think is the decisive argument either way and I think the
>   question that we need to answer is whether your design is a step in
>   the right direction.
>
>   The other possible argument that my change is less intrusive is
>   probably not a very strong one.  I read Richard's patch and I think
>   it is very easy to see that on other targets it falls back on the
>   existing behavior so I think the solution is at least as safe as
>   mine is.

Thanks.

>   But getting back to the design question, I think we all agree that
>   the current design of libgcc2.c and how it works for 64-bit targets
>   is broken and needs to be revised.  The basic idea of your patch is
>   to add an optional format to how lib2funcs are requested.  The
>   format is <objfile>:<func>:<wordsize>.  The question is whether this
>   is required in the long run interface-wise and whether just
>   specifying e.g. fixunsdfdi would not hold enough information for
>   libgcc2.c to generate the right functionality.  Note that even with
>   this patch we still have misnamed object files for all other
>   DI-functions.

In my original message, I tried to cover the last sentence with:

    Longer term, it would be nicer to:

      (1) Name all object files after the functions they implement,
          not just the conversion functions we're dealing with here.

      (2) Replace "si" and "di" in the L* names with something
          more neutral (perhaps "WW" and "DD", like I used as above.)

I did not want to lump those changes into the current patch because
it would affect the patch's suitability for 4.1.

>   If we need to keep word size "changeable" across different libgcc2.c
>   complations then I agree that Richard's patch is a step in the right
>   direction if not then I am not sure why go through the hassle of
>   generalizing something that should really go away.
>
>   Now I can see that Mark has already approved the patch but I would
>   still appreciate a reply clarifying this last issue.

To answer the last three paragraphs more broadly: the point of both your
patch and mine is to allow one libgcc2.c function template to be used to
generate more than one function.  That's not something we do currently.
And I think reusing templates (rather than copying them) _is_ the right
approach.  Other libraries tend to take a similar approach in similar
situations.

So to requote what you said above:

>   The basic idea of your patch is to add an optional format to how
>   lib2funcs are requested.  The format is <objfile>:<func>:<wordsize>.

[The <objfile>:<func>:<wordsize> format is just an implementation detail
that's local to mklibgcc.in.  The basic idea isn't really the format as
such, it's the fact that we're getting more help from libgcc.mk.]

>   The question is whether this is required in the long run
>   interface-wise and whether just specifying e.g. fixunsdfdi would not
>   hold enough information for libgcc2.c to generate the right
>   functionality.

Your question appears to be whether libgcc2.c should have the smarts to
work out on its own how to implement fixunsdfdi, or whether it relies on
external information (provided in my patch by libgcc.mk).  I tend to
prefer the latter because shell scripts are more expressive than the C
preprocessor.  If we put the logic in libgcc2.c itself, then I think
every function defined by libgcc2.c would have something similar the
#ifdef blocks in your patch:

    #ifdef L_fixunsdfdi2
      #define TEMPLATE_fixunsdfDD2
      #define DDSIZE 64
    #endif

This would have to be repeated for {fix,fixuns,float,floatun}x{sf,df,tf,xf};
i.e. 16 combinations, and that's only a subset of the functions that
libgcc2.c provides.  Getting help from an external shell script allows
us to use things like "for" loops to do the mechanical stuff.

Richard