Re: [PATCH, GCC/ARM 2/2] Allow combination of aprofile and rmprofile multilibs

[Ramana Radhakrishnan <ramana dot gcc at googlemail dot com>]

On Thu, Oct 13, 2016 at 4:35 PM, Thomas Preudhomme
<thomas.preudhomme@foss.arm.com> wrote:
> Hi ARM maintainers,
>
> This patchset aims at adding multilib support for R and M profile ARM
> architectures and allowing it to be built alongside multilib for A profile
> ARM architectures. This specific patch is concerned with the latter. The
> patch works by moving the bits shared by both aprofile and rmprofile
> multilib build (variable initilization as well as ISA and float ABI to build
> multilib for) to a new t-multilib file. Then, based on which profile was
> requested in --with-multilib-list option, that files includes t-aprofile
> and/or t-rmprofile where the architecture and FPU to build the multilib for
> are specified.
>
> Unfortunately the duplication of CPU to A profile architectures could not be
> avoided because substitution due to MULTILIB_MATCHES are not transitive.
> Therefore, mapping armv7-a to armv7 for rmprofile multilib build does not
> have the expected effect. Two patches were written to allow this using 2
> different approaches but I decided against it because this is not the right
> solution IMO. See caveats below for what I believe is the correct approach.
>
>
> *** combined build caveats ***
>
> As the documentation in this patch warns, there is a few caveats to using a
> combined multilib build due to the way the multilib framework works.
>
> 1) For instance, when using only rmprofile the combination of options
> -mthumb -march=armv7 -mfpu=neon the thumb/-march=armv7 multilib but in a
> combined multilib build the default multilib would be used. This is because
> in the rmprofile build -mfpu=neon is not specified in MULTILIB_OPTION and
> thus the option is ignored when considering MULTILIB_REQUIRED entries.
>
> 2) Another issue is the fact that aprofile and rmprofile multilib build have
> some conflicting requirements in terms of how to map options for which no
> multilib is built to another option. (i) A first example of this is the
> difference of CPU to architecture mapping mentionned above: rmprofile
> multilib build needs A profile CPUs and architectures to be mapped down to
> ARMv7 so that one of the v7-ar multilib gets chosen in such a case but
> aprofile needs A profile architectures to stand on their own because
> multilibs are built for several architectures.
>
> (ii) Another example of this is that in aprofile multilib build no multilib
> is built with -mfpu=fpv5-d16 but some multilibs are built with
> -mfpu=fpv4-d16. Therefore, aprofile defines a match rule to map fpv5-d16
> onto fpv4-d16. However, rmprofile multilib profile *does* build some
> multilibs with -mfpu=fpv5-d16. This has the consequence that when building
> for -mthumb -march=armv7e-m -mfpu=fpv5-d16 -mfloat-abi=hard the default
> multilib is chosen because this is rewritten into -mthumb -march=armv7e-m
> -mfpu=fpv5-d16 -mfloat-abi=hard and there is no multilib for that.
>
> Both of these issues could be handled by using MULTILIB_REUSE instead of
> MULTILIB_MATCHES but this would require a large set of rules. I believe
> instead the right approach is to create a new mechanism to inform GCC on how
> options can be down mapped _when no multilib can be found_ which would
> require a smaller set of rules and would make it explicit that the options
> are not equivalent. A patch will be posted to this effect at a later time.

I think this needs to be handled along with Richard's rewrite of the
options . I wouldn't lose too much sleep over it (after all fpv5-d16
is really an Mprofile option) and its going to be quite rare to use it
in that form.

This is OK now that we are in stage1.


Ramana


>
> ChangeLog entry is as follows:
>
>
> *** gcc/ChangeLog ***
>
> 2016-10-03  Thomas Preud'homme  <thomas.preudhomme@arm.com>
>
>         * config.gcc: Allow combinations of aprofile and rmprofile values
> for
>         --with-multilib-list.
>         * config/arm/t-multilib: New file.
>         * config/arm/t-aprofile: Remove initialization of MULTILIB_*
>         variables.  Remove setting of ISA and floating-point ABI in
>         MULTILIB_OPTIONS and MULTILIB_DIRNAMES.  Set architecture and FPU in
>         MULTI_ARCH_OPTS_A and MULTI_ARCH_DIRS_A rather than MULTILIB_OPTIONS
>         and MULTILIB_DIRNAMES respectively.  Add comment to introduce all
>         matches.  Add architecture matches for marvel-pj4 and
> generic-armv7-a
>         CPU options.
>         * config/arm/t-rmprofile: Likewise except for the matches changes.
>         * doc/install.texi (--with-multilib-list): Document the combination
> of
>         aprofile and rmprofile values and warn about pitfalls in doing that.
>
>
> Testing:
>
> * "tree install/lib/gcc/arm-none-eabi/7.0.0" is the same before and after
> the patchset for both aprofile and rmprofile
> * "tree install/lib/gcc/arm-none-eabi/7.0.0" is the same for
> aprofile,rmprofile and rmprofile,aprofile
> * default spec (gcc -dumpspecs) is the same for aprofile,rmprofile and
> rmprofile,aprofile
>
> * Difference in --print-multi-directory between aprofile or rmprofile and
> aprofile,rmprofile for all combination of ISA (ARM/Thumb), architecture,
> CPU, FPU and float ABI is as expected (best multilib for the combination is
> chosen), modulo the caveat mentionned above and the new marvel-pj4 and
> generic-armv7-a CPU to architecture mapping.
>
>
> Is this ok for master?
>
> Best regards,
>
> Thomas