Re: [PATCH] Add support for ARM embedded multilibs

[Ramana Radhakrishnan <ramana.radhakrishnan@foss.arm.com>]

On 04/11/15 08:43, Jasmin J. wrote:
> Hello Ramana!
> 
>> Thank you for your patch - In this case before you make any more
>> changes to this patch - comparing your patch and Terry's patch here
>> https://gcc.gnu.org/ml/gcc-patches/2014-05/msg00729.html shows no real
>> differences
> As I wrote in the patch, it is a port from the embedded-4_9-branch and it
> is exactly what Terry did. I didn't know this patch. I extracted it from
> the branch.

> 
> All other stuff required to build a gcc for the ARM embedded CPUs are already
> on gcc trunk. This last piece is missing to get rid of the embedded-X_X-branch!
> I would love to see it in mainline GCC.

CC'ing Tejas as he now looks after this branch internally at ARM. There are usually features on the embedded-X_X-branch used to create releases that may not be on an FSF release branch.

> 
>> I would like to ask if  you have a copyright assignment on file with the FSF
> I will do that.

So, you don't have one ? In which case it may make more sense for Tejas to deal with this given he can handle it under ARM's copyright assignment. If you make changes to this without a copyright assignment on file and submit it, it may be difficult to review this from a copyright position.

> 
>> How was your patch tested
> The patch is old an doesn't contain any real GCC change, but support for the
> library building process only. This was used since 15 months by many people
> and I used it to build an ARM embedded compiler based on gcc-5-branch.

Changing the way in which you build GCC is a real change to GCC that affects many developers. Not testing your change by building and checking that it does what it says on the tin is unacceptable.

> 
>> see for example how I added t-aprofile to the backend and the kind of
>> testing it underwent
> I will look on that, if it is really required.
> 
>>>  * configure.ac (with_multilib_list): Export for being used in arm
>>>    embedded multilib fragment.
>>>  * Makefile.in (with_multilib_list): Import for being used in
>>>    multilib fragment.
>>
>> This is already being used in config.gcc - why do you need this
>> additional hunk ?
> To be hones, I ported the patch and checked if it works. I will analyse it more
> detailed, if this is really required.
> 
>> The t-rmprofile file will need updating to newer values for -mcpu and
>> march as well as comments up top to explain what multilibs are being
>> built .
> Where can I find them?

gcc/doc/invoke.texi should document the rest.. I think mapping all the remaining -mcpu=cortex-a* cores and -mcpu=cortex-m* cores in there would be a sensible first step.


regards
Ramana


> 
> BR
>    Jasmin
>