diff mbox

[GCC/ARM,ping] Optional -mthumb for Thumb only targets

Message ID 3c8a46c4-9865-00d4-b953-87f8d5ac9c38@foss.arm.com
State New
Headers show

Commit Message

Thomas Preudhomme Nov. 18, 2016, 4:51 p.m. UTC
On 11/11/16 14:35, Kyrill Tkachov wrote:
>
> On 08/11/16 13:36, Thomas Preudhomme wrote:
>> Ping?
>>
>> Best regards,
>>
>> Thomas
>>
>> On 25/10/16 18:07, Thomas Preudhomme wrote:
>>> Hi,
>>>
>>> Currently when a user compiles for a thumb-only target (such as Cortex-M
>>> processors) without specifying the -mthumb option GCC throws the error "target
>>> CPU does not support ARM mode". This is suboptimal from a usability point of
>>> view: the -mthumb could be deduced from the -march or -mcpu option when there is
>>> no ambiguity.
>>>
>>> This patch implements this behavior by extending the DRIVER_SELF_SPECS to
>>> automatically append -mthumb to the command line for thumb-only targets. It does
>>> so by checking the last -march option if any is given or the last -mcpu option
>>> otherwise. There is no ordering issue because conflicting -mcpu and -march is
>>> already handled.
>>>
>>> Note that the logic cannot be implemented in function arm_option_override
>>> because we need to provide the modified command line to the GCC driver for
>>> finding the right multilib path and the function arm_option_override is executed
>>> too late for that effect.
>>>
>>> ChangeLog entries are as follow:
>>>
>>> *** gcc/ChangeLog ***
>>>
>>> 2016-10-18  Terry Guo  <terry.guo@arm.com>
>>>             Thomas Preud'homme <thomas.preudhomme@arm.com>
>>>
>>>         PR target/64802
>>>         * common/config/arm/arm-common.c (arm_target_thumb_only): New function.
>>>         * config/arm/arm-opts.h: Include arm-flags.h.
>>>         (struct arm_arch_core_flag): Define.
>>>         (arm_arch_core_flags): Define.
>>>         * config/arm/arm-protos.h: Include arm-flags.h.
>>>         (FL_NONE, FL_ANY, FL_CO_PROC, FL_ARCH3M, FL_MODE26, FL_MODE32,
>>>         FL_ARCH4, FL_ARCH5, FL_THUMB, FL_LDSCHED, FL_STRONG, FL_ARCH5E,
>>>         FL_XSCALE, FL_ARCH6, FL_VFPV2, FL_WBUF, FL_ARCH6K, FL_THUMB2, FL_NOTM,
>>>         FL_THUMB_DIV, FL_VFPV3, FL_NEON, FL_ARCH7EM, FL_ARCH7, FL_ARM_DIV,
>>>         FL_ARCH8, FL_CRC32, FL_SMALLMUL, FL_NO_VOLATILE_CE, FL_IWMMXT,
>>>         FL_IWMMXT2, FL_ARCH6KZ, FL2_ARCH8_1, FL2_ARCH8_2, FL2_FP16INST,
>>>         FL_TUNE, FL_FOR_ARCH2, FL_FOR_ARCH3, FL_FOR_ARCH3M, FL_FOR_ARCH4,
>>>         FL_FOR_ARCH4T, FL_FOR_ARCH5, FL_FOR_ARCH5T, FL_FOR_ARCH5E,
>>>         FL_FOR_ARCH5TE, FL_FOR_ARCH5TEJ, FL_FOR_ARCH6, FL_FOR_ARCH6J,
>>>         FL_FOR_ARCH6K, FL_FOR_ARCH6Z, FL_FOR_ARCH6ZK, FL_FOR_ARCH6KZ,
>>>         FL_FOR_ARCH6T2, FL_FOR_ARCH6M, FL_FOR_ARCH7, FL_FOR_ARCH7A,
>>>         FL_FOR_ARCH7VE, FL_FOR_ARCH7R, FL_FOR_ARCH7M, FL_FOR_ARCH7EM,
>>>         FL_FOR_ARCH8A, FL2_FOR_ARCH8_1A, FL2_FOR_ARCH8_2A, FL_FOR_ARCH8M_BASE,
>>>         FL_FOR_ARCH8M_MAIN, arm_feature_set, ARM_FSET_MAKE,
>>>         ARM_FSET_MAKE_CPU1, ARM_FSET_MAKE_CPU2, ARM_FSET_CPU1, ARM_FSET_CPU2,
>>>         ARM_FSET_EMPTY, ARM_FSET_ANY, ARM_FSET_HAS_CPU1, ARM_FSET_HAS_CPU2,
>>>         ARM_FSET_HAS_CPU, ARM_FSET_ADD_CPU1, ARM_FSET_ADD_CPU2,
>>>         ARM_FSET_DEL_CPU1, ARM_FSET_DEL_CPU2, ARM_FSET_UNION, ARM_FSET_INTER,
>>>         ARM_FSET_XOR, ARM_FSET_EXCLUDE, ARM_FSET_IS_EMPTY,
>>>         ARM_FSET_CPU_SUBSET): Move to ...
>>>         * config/arm/arm-flags.h: This new file.
>>>         * config/arm/arm.h (TARGET_MODE_SPEC_FUNCTIONS): Define.
>>>         (EXTRA_SPEC_FUNCTIONS): Add TARGET_MODE_SPEC_FUNCTIONS to its value.
>>>         (TARGET_MODE_SPECS): Define.
>>>         (DRIVER_SELF_SPECS): Add TARGET_MODE_SPECS to its value.
>>>
>>>
>>> *** gcc/testsuite/ChangeLog ***
>>>
>>> 2016-10-11  Thomas Preud'homme <thomas.preudhomme@arm.com>
>>>
>>>         PR target/64802
>>>         * gcc.target/arm/optional_thumb-1.c: New test.
>>>         * gcc.target/arm/optional_thumb-2.c: New test.
>>>         * gcc.target/arm/optional_thumb-3.c: New test.
>>>
>>>
>>> No regression when running the testsuite for -mcpu=cortex-m0 -mthumb,
>>> -mcpu=cortex-m0 -marm and -mcpu=cortex-a8 -marm
>>>
>>> Is this ok for trunk?
>>>
>
> This looks like a useful usability improvement.
> This is ok after a bootstrap on an arm-none-linux-gnueabihf target.
>
> Sorry for the delay,
> Kyrill

I've rebased the patch on top of the arm_feature_set type consistency fix [1] 
and committed it. The committed patch is in attachment for reference.

[1] https://gcc.gnu.org/ml/gcc-patches/2016-11/msg01680.html

Best regards,

Thomas

Comments

Christophe Lyon Nov. 21, 2016, 8:51 a.m. UTC | #1
Hi Thomas,


On 18 November 2016 at 17:51, Thomas Preudhomme
<thomas.preudhomme@foss.arm.com> wrote:
> On 11/11/16 14:35, Kyrill Tkachov wrote:
>>
>>
>> On 08/11/16 13:36, Thomas Preudhomme wrote:
>>>
>>> Ping?
>>>
>>> Best regards,
>>>
>>> Thomas
>>>
>>> On 25/10/16 18:07, Thomas Preudhomme wrote:
>>>>
>>>> Hi,
>>>>
>>>> Currently when a user compiles for a thumb-only target (such as Cortex-M
>>>> processors) without specifying the -mthumb option GCC throws the error
>>>> "target
>>>> CPU does not support ARM mode". This is suboptimal from a usability
>>>> point of
>>>> view: the -mthumb could be deduced from the -march or -mcpu option when
>>>> there is
>>>> no ambiguity.
>>>>
>>>> This patch implements this behavior by extending the DRIVER_SELF_SPECS
>>>> to
>>>> automatically append -mthumb to the command line for thumb-only targets.
>>>> It does
>>>> so by checking the last -march option if any is given or the last -mcpu
>>>> option
>>>> otherwise. There is no ordering issue because conflicting -mcpu and
>>>> -march is
>>>> already handled.
>>>>
>>>> Note that the logic cannot be implemented in function
>>>> arm_option_override
>>>> because we need to provide the modified command line to the GCC driver
>>>> for
>>>> finding the right multilib path and the function arm_option_override is
>>>> executed
>>>> too late for that effect.
>>>>
>>>> ChangeLog entries are as follow:
>>>>
>>>> *** gcc/ChangeLog ***
>>>>
>>>> 2016-10-18  Terry Guo  <terry.guo@arm.com>
>>>>             Thomas Preud'homme <thomas.preudhomme@arm.com>
>>>>
>>>>         PR target/64802
>>>>         * common/config/arm/arm-common.c (arm_target_thumb_only): New
>>>> function.
>>>>         * config/arm/arm-opts.h: Include arm-flags.h.
>>>>         (struct arm_arch_core_flag): Define.
>>>>         (arm_arch_core_flags): Define.
>>>>         * config/arm/arm-protos.h: Include arm-flags.h.
>>>>         (FL_NONE, FL_ANY, FL_CO_PROC, FL_ARCH3M, FL_MODE26, FL_MODE32,
>>>>         FL_ARCH4, FL_ARCH5, FL_THUMB, FL_LDSCHED, FL_STRONG, FL_ARCH5E,
>>>>         FL_XSCALE, FL_ARCH6, FL_VFPV2, FL_WBUF, FL_ARCH6K, FL_THUMB2,
>>>> FL_NOTM,
>>>>         FL_THUMB_DIV, FL_VFPV3, FL_NEON, FL_ARCH7EM, FL_ARCH7,
>>>> FL_ARM_DIV,
>>>>         FL_ARCH8, FL_CRC32, FL_SMALLMUL, FL_NO_VOLATILE_CE, FL_IWMMXT,
>>>>         FL_IWMMXT2, FL_ARCH6KZ, FL2_ARCH8_1, FL2_ARCH8_2, FL2_FP16INST,
>>>>         FL_TUNE, FL_FOR_ARCH2, FL_FOR_ARCH3, FL_FOR_ARCH3M,
>>>> FL_FOR_ARCH4,
>>>>         FL_FOR_ARCH4T, FL_FOR_ARCH5, FL_FOR_ARCH5T, FL_FOR_ARCH5E,
>>>>         FL_FOR_ARCH5TE, FL_FOR_ARCH5TEJ, FL_FOR_ARCH6, FL_FOR_ARCH6J,
>>>>         FL_FOR_ARCH6K, FL_FOR_ARCH6Z, FL_FOR_ARCH6ZK, FL_FOR_ARCH6KZ,
>>>>         FL_FOR_ARCH6T2, FL_FOR_ARCH6M, FL_FOR_ARCH7, FL_FOR_ARCH7A,
>>>>         FL_FOR_ARCH7VE, FL_FOR_ARCH7R, FL_FOR_ARCH7M, FL_FOR_ARCH7EM,
>>>>         FL_FOR_ARCH8A, FL2_FOR_ARCH8_1A, FL2_FOR_ARCH8_2A,
>>>> FL_FOR_ARCH8M_BASE,
>>>>         FL_FOR_ARCH8M_MAIN, arm_feature_set, ARM_FSET_MAKE,
>>>>         ARM_FSET_MAKE_CPU1, ARM_FSET_MAKE_CPU2, ARM_FSET_CPU1,
>>>> ARM_FSET_CPU2,
>>>>         ARM_FSET_EMPTY, ARM_FSET_ANY, ARM_FSET_HAS_CPU1,
>>>> ARM_FSET_HAS_CPU2,
>>>>         ARM_FSET_HAS_CPU, ARM_FSET_ADD_CPU1, ARM_FSET_ADD_CPU2,
>>>>         ARM_FSET_DEL_CPU1, ARM_FSET_DEL_CPU2, ARM_FSET_UNION,
>>>> ARM_FSET_INTER,
>>>>         ARM_FSET_XOR, ARM_FSET_EXCLUDE, ARM_FSET_IS_EMPTY,
>>>>         ARM_FSET_CPU_SUBSET): Move to ...
>>>>         * config/arm/arm-flags.h: This new file.
>>>>         * config/arm/arm.h (TARGET_MODE_SPEC_FUNCTIONS): Define.
>>>>         (EXTRA_SPEC_FUNCTIONS): Add TARGET_MODE_SPEC_FUNCTIONS to its
>>>> value.
>>>>         (TARGET_MODE_SPECS): Define.
>>>>         (DRIVER_SELF_SPECS): Add TARGET_MODE_SPECS to its value.
>>>>
>>>>
>>>> *** gcc/testsuite/ChangeLog ***
>>>>
>>>> 2016-10-11  Thomas Preud'homme <thomas.preudhomme@arm.com>
>>>>
>>>>         PR target/64802
>>>>         * gcc.target/arm/optional_thumb-1.c: New test.
>>>>         * gcc.target/arm/optional_thumb-2.c: New test.
>>>>         * gcc.target/arm/optional_thumb-3.c: New test.
>>>>
>>>>
>>>> No regression when running the testsuite for -mcpu=cortex-m0 -mthumb,
>>>> -mcpu=cortex-m0 -marm and -mcpu=cortex-a8 -marm
>>>>
>>>> Is this ok for trunk?
>>>>
>>
>> This looks like a useful usability improvement.
>> This is ok after a bootstrap on an arm-none-linux-gnueabihf target.
>>
>> Sorry for the delay,
>> Kyrill
>
>
> I've rebased the patch on top of the arm_feature_set type consistency fix
> [1] and committed it. The committed patch is in attachment for reference.
>
> [1] https://gcc.gnu.org/ml/gcc-patches/2016-11/msg01680.html
>

Since this commit (242597), I've noticed that:
- the 2 new tests optional_thumb-1.c and optional_thumb-2.c fail
if GCC was configured --with-mode=arm. The error message is:
cc1: error: target CPU does not support ARM mode

- on armeb --with-mode=arm, gcc.dg/vect/pr64252.c fails at execution

See: http://people.linaro.org/~christophe.lyon/cross-validation/gcc/trunk/242597/report-build-info.html

Christophe


> Best regards,
>
> Thomas
Thomas Preudhomme Nov. 21, 2016, 2:16 p.m. UTC | #2
On 21/11/16 08:51, Christophe Lyon wrote:
> Hi Thomas,

Hi Christophe,

>
>
> On 18 November 2016 at 17:51, Thomas Preudhomme
> <thomas.preudhomme@foss.arm.com> wrote:
>> On 11/11/16 14:35, Kyrill Tkachov wrote:
>>>
>>>
>>> On 08/11/16 13:36, Thomas Preudhomme wrote:
>>>>
>>>> Ping?
>>>>
>>>> Best regards,
>>>>
>>>> Thomas
>>>>
>>>> On 25/10/16 18:07, Thomas Preudhomme wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> Currently when a user compiles for a thumb-only target (such as Cortex-M
>>>>> processors) without specifying the -mthumb option GCC throws the error
>>>>> "target
>>>>> CPU does not support ARM mode". This is suboptimal from a usability
>>>>> point of
>>>>> view: the -mthumb could be deduced from the -march or -mcpu option when
>>>>> there is
>>>>> no ambiguity.
>>>>>
>>>>> This patch implements this behavior by extending the DRIVER_SELF_SPECS
>>>>> to
>>>>> automatically append -mthumb to the command line for thumb-only targets.
>>>>> It does
>>>>> so by checking the last -march option if any is given or the last -mcpu
>>>>> option
>>>>> otherwise. There is no ordering issue because conflicting -mcpu and
>>>>> -march is
>>>>> already handled.
>>>>>
>>>>> Note that the logic cannot be implemented in function
>>>>> arm_option_override
>>>>> because we need to provide the modified command line to the GCC driver
>>>>> for
>>>>> finding the right multilib path and the function arm_option_override is
>>>>> executed
>>>>> too late for that effect.
>>>>>
>>>>> ChangeLog entries are as follow:
>>>>>
>>>>> *** gcc/ChangeLog ***
>>>>>
>>>>> 2016-10-18  Terry Guo  <terry.guo@arm.com>
>>>>>             Thomas Preud'homme <thomas.preudhomme@arm.com>
>>>>>
>>>>>         PR target/64802
>>>>>         * common/config/arm/arm-common.c (arm_target_thumb_only): New
>>>>> function.
>>>>>         * config/arm/arm-opts.h: Include arm-flags.h.
>>>>>         (struct arm_arch_core_flag): Define.
>>>>>         (arm_arch_core_flags): Define.
>>>>>         * config/arm/arm-protos.h: Include arm-flags.h.
>>>>>         (FL_NONE, FL_ANY, FL_CO_PROC, FL_ARCH3M, FL_MODE26, FL_MODE32,
>>>>>         FL_ARCH4, FL_ARCH5, FL_THUMB, FL_LDSCHED, FL_STRONG, FL_ARCH5E,
>>>>>         FL_XSCALE, FL_ARCH6, FL_VFPV2, FL_WBUF, FL_ARCH6K, FL_THUMB2,
>>>>> FL_NOTM,
>>>>>         FL_THUMB_DIV, FL_VFPV3, FL_NEON, FL_ARCH7EM, FL_ARCH7,
>>>>> FL_ARM_DIV,
>>>>>         FL_ARCH8, FL_CRC32, FL_SMALLMUL, FL_NO_VOLATILE_CE, FL_IWMMXT,
>>>>>         FL_IWMMXT2, FL_ARCH6KZ, FL2_ARCH8_1, FL2_ARCH8_2, FL2_FP16INST,
>>>>>         FL_TUNE, FL_FOR_ARCH2, FL_FOR_ARCH3, FL_FOR_ARCH3M,
>>>>> FL_FOR_ARCH4,
>>>>>         FL_FOR_ARCH4T, FL_FOR_ARCH5, FL_FOR_ARCH5T, FL_FOR_ARCH5E,
>>>>>         FL_FOR_ARCH5TE, FL_FOR_ARCH5TEJ, FL_FOR_ARCH6, FL_FOR_ARCH6J,
>>>>>         FL_FOR_ARCH6K, FL_FOR_ARCH6Z, FL_FOR_ARCH6ZK, FL_FOR_ARCH6KZ,
>>>>>         FL_FOR_ARCH6T2, FL_FOR_ARCH6M, FL_FOR_ARCH7, FL_FOR_ARCH7A,
>>>>>         FL_FOR_ARCH7VE, FL_FOR_ARCH7R, FL_FOR_ARCH7M, FL_FOR_ARCH7EM,
>>>>>         FL_FOR_ARCH8A, FL2_FOR_ARCH8_1A, FL2_FOR_ARCH8_2A,
>>>>> FL_FOR_ARCH8M_BASE,
>>>>>         FL_FOR_ARCH8M_MAIN, arm_feature_set, ARM_FSET_MAKE,
>>>>>         ARM_FSET_MAKE_CPU1, ARM_FSET_MAKE_CPU2, ARM_FSET_CPU1,
>>>>> ARM_FSET_CPU2,
>>>>>         ARM_FSET_EMPTY, ARM_FSET_ANY, ARM_FSET_HAS_CPU1,
>>>>> ARM_FSET_HAS_CPU2,
>>>>>         ARM_FSET_HAS_CPU, ARM_FSET_ADD_CPU1, ARM_FSET_ADD_CPU2,
>>>>>         ARM_FSET_DEL_CPU1, ARM_FSET_DEL_CPU2, ARM_FSET_UNION,
>>>>> ARM_FSET_INTER,
>>>>>         ARM_FSET_XOR, ARM_FSET_EXCLUDE, ARM_FSET_IS_EMPTY,
>>>>>         ARM_FSET_CPU_SUBSET): Move to ...
>>>>>         * config/arm/arm-flags.h: This new file.
>>>>>         * config/arm/arm.h (TARGET_MODE_SPEC_FUNCTIONS): Define.
>>>>>         (EXTRA_SPEC_FUNCTIONS): Add TARGET_MODE_SPEC_FUNCTIONS to its
>>>>> value.
>>>>>         (TARGET_MODE_SPECS): Define.
>>>>>         (DRIVER_SELF_SPECS): Add TARGET_MODE_SPECS to its value.
>>>>>
>>>>>
>>>>> *** gcc/testsuite/ChangeLog ***
>>>>>
>>>>> 2016-10-11  Thomas Preud'homme <thomas.preudhomme@arm.com>
>>>>>
>>>>>         PR target/64802
>>>>>         * gcc.target/arm/optional_thumb-1.c: New test.
>>>>>         * gcc.target/arm/optional_thumb-2.c: New test.
>>>>>         * gcc.target/arm/optional_thumb-3.c: New test.
>>>>>
>>>>>
>>>>> No regression when running the testsuite for -mcpu=cortex-m0 -mthumb,
>>>>> -mcpu=cortex-m0 -marm and -mcpu=cortex-a8 -marm
>>>>>
>>>>> Is this ok for trunk?
>>>>>
>>>
>>> This looks like a useful usability improvement.
>>> This is ok after a bootstrap on an arm-none-linux-gnueabihf target.
>>>
>>> Sorry for the delay,
>>> Kyrill
>>
>>
>> I've rebased the patch on top of the arm_feature_set type consistency fix
>> [1] and committed it. The committed patch is in attachment for reference.
>>
>> [1] https://gcc.gnu.org/ml/gcc-patches/2016-11/msg01680.html
>>
>
> Since this commit (242597), I've noticed that:
> - the 2 new tests optional_thumb-1.c and optional_thumb-2.c fail
> if GCC was configured --with-mode=arm. The error message is:
> cc1: error: target CPU does not support ARM mode

I need to skip the test when GCC is built with --with-mode= but we do not have a 
directive for that. I'll see if I can add one.

>
> - on armeb --with-mode=arm, gcc.dg/vect/pr64252.c fails at execution
>
> See: http://people.linaro.org/~christophe.lyon/cross-validation/gcc/trunk/242597/report-build-info.html

I cannot reproduce that last issue. I've built r242658 for armeb-none-eabi and 
it passes fine in qemu-armeb. I'll try building a linux toolchain.

Best regards,

Thomas
Christophe Lyon Nov. 21, 2016, 3:21 p.m. UTC | #3
On 21 November 2016 at 15:16, Thomas Preudhomme
<thomas.preudhomme@foss.arm.com> wrote:
> On 21/11/16 08:51, Christophe Lyon wrote:
>>
>> Hi Thomas,
>
>
> Hi Christophe,
>
>
>>
>>
>> On 18 November 2016 at 17:51, Thomas Preudhomme
>> <thomas.preudhomme@foss.arm.com> wrote:
>>>
>>> On 11/11/16 14:35, Kyrill Tkachov wrote:
>>>>
>>>>
>>>>
>>>> On 08/11/16 13:36, Thomas Preudhomme wrote:
>>>>>
>>>>>
>>>>> Ping?
>>>>>
>>>>> Best regards,
>>>>>
>>>>> Thomas
>>>>>
>>>>> On 25/10/16 18:07, Thomas Preudhomme wrote:
>>>>>>
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> Currently when a user compiles for a thumb-only target (such as
>>>>>> Cortex-M
>>>>>> processors) without specifying the -mthumb option GCC throws the error
>>>>>> "target
>>>>>> CPU does not support ARM mode". This is suboptimal from a usability
>>>>>> point of
>>>>>> view: the -mthumb could be deduced from the -march or -mcpu option
>>>>>> when
>>>>>> there is
>>>>>> no ambiguity.
>>>>>>
>>>>>> This patch implements this behavior by extending the DRIVER_SELF_SPECS
>>>>>> to
>>>>>> automatically append -mthumb to the command line for thumb-only
>>>>>> targets.
>>>>>> It does
>>>>>> so by checking the last -march option if any is given or the last
>>>>>> -mcpu
>>>>>> option
>>>>>> otherwise. There is no ordering issue because conflicting -mcpu and
>>>>>> -march is
>>>>>> already handled.
>>>>>>
>>>>>> Note that the logic cannot be implemented in function
>>>>>> arm_option_override
>>>>>> because we need to provide the modified command line to the GCC driver
>>>>>> for
>>>>>> finding the right multilib path and the function arm_option_override
>>>>>> is
>>>>>> executed
>>>>>> too late for that effect.
>>>>>>
>>>>>> ChangeLog entries are as follow:
>>>>>>
>>>>>> *** gcc/ChangeLog ***
>>>>>>
>>>>>> 2016-10-18  Terry Guo  <terry.guo@arm.com>
>>>>>>             Thomas Preud'homme <thomas.preudhomme@arm.com>
>>>>>>
>>>>>>         PR target/64802
>>>>>>         * common/config/arm/arm-common.c (arm_target_thumb_only): New
>>>>>> function.
>>>>>>         * config/arm/arm-opts.h: Include arm-flags.h.
>>>>>>         (struct arm_arch_core_flag): Define.
>>>>>>         (arm_arch_core_flags): Define.
>>>>>>         * config/arm/arm-protos.h: Include arm-flags.h.
>>>>>>         (FL_NONE, FL_ANY, FL_CO_PROC, FL_ARCH3M, FL_MODE26, FL_MODE32,
>>>>>>         FL_ARCH4, FL_ARCH5, FL_THUMB, FL_LDSCHED, FL_STRONG,
>>>>>> FL_ARCH5E,
>>>>>>         FL_XSCALE, FL_ARCH6, FL_VFPV2, FL_WBUF, FL_ARCH6K, FL_THUMB2,
>>>>>> FL_NOTM,
>>>>>>         FL_THUMB_DIV, FL_VFPV3, FL_NEON, FL_ARCH7EM, FL_ARCH7,
>>>>>> FL_ARM_DIV,
>>>>>>         FL_ARCH8, FL_CRC32, FL_SMALLMUL, FL_NO_VOLATILE_CE, FL_IWMMXT,
>>>>>>         FL_IWMMXT2, FL_ARCH6KZ, FL2_ARCH8_1, FL2_ARCH8_2,
>>>>>> FL2_FP16INST,
>>>>>>         FL_TUNE, FL_FOR_ARCH2, FL_FOR_ARCH3, FL_FOR_ARCH3M,
>>>>>> FL_FOR_ARCH4,
>>>>>>         FL_FOR_ARCH4T, FL_FOR_ARCH5, FL_FOR_ARCH5T, FL_FOR_ARCH5E,
>>>>>>         FL_FOR_ARCH5TE, FL_FOR_ARCH5TEJ, FL_FOR_ARCH6, FL_FOR_ARCH6J,
>>>>>>         FL_FOR_ARCH6K, FL_FOR_ARCH6Z, FL_FOR_ARCH6ZK, FL_FOR_ARCH6KZ,
>>>>>>         FL_FOR_ARCH6T2, FL_FOR_ARCH6M, FL_FOR_ARCH7, FL_FOR_ARCH7A,
>>>>>>         FL_FOR_ARCH7VE, FL_FOR_ARCH7R, FL_FOR_ARCH7M, FL_FOR_ARCH7EM,
>>>>>>         FL_FOR_ARCH8A, FL2_FOR_ARCH8_1A, FL2_FOR_ARCH8_2A,
>>>>>> FL_FOR_ARCH8M_BASE,
>>>>>>         FL_FOR_ARCH8M_MAIN, arm_feature_set, ARM_FSET_MAKE,
>>>>>>         ARM_FSET_MAKE_CPU1, ARM_FSET_MAKE_CPU2, ARM_FSET_CPU1,
>>>>>> ARM_FSET_CPU2,
>>>>>>         ARM_FSET_EMPTY, ARM_FSET_ANY, ARM_FSET_HAS_CPU1,
>>>>>> ARM_FSET_HAS_CPU2,
>>>>>>         ARM_FSET_HAS_CPU, ARM_FSET_ADD_CPU1, ARM_FSET_ADD_CPU2,
>>>>>>         ARM_FSET_DEL_CPU1, ARM_FSET_DEL_CPU2, ARM_FSET_UNION,
>>>>>> ARM_FSET_INTER,
>>>>>>         ARM_FSET_XOR, ARM_FSET_EXCLUDE, ARM_FSET_IS_EMPTY,
>>>>>>         ARM_FSET_CPU_SUBSET): Move to ...
>>>>>>         * config/arm/arm-flags.h: This new file.
>>>>>>         * config/arm/arm.h (TARGET_MODE_SPEC_FUNCTIONS): Define.
>>>>>>         (EXTRA_SPEC_FUNCTIONS): Add TARGET_MODE_SPEC_FUNCTIONS to its
>>>>>> value.
>>>>>>         (TARGET_MODE_SPECS): Define.
>>>>>>         (DRIVER_SELF_SPECS): Add TARGET_MODE_SPECS to its value.
>>>>>>
>>>>>>
>>>>>> *** gcc/testsuite/ChangeLog ***
>>>>>>
>>>>>> 2016-10-11  Thomas Preud'homme <thomas.preudhomme@arm.com>
>>>>>>
>>>>>>         PR target/64802
>>>>>>         * gcc.target/arm/optional_thumb-1.c: New test.
>>>>>>         * gcc.target/arm/optional_thumb-2.c: New test.
>>>>>>         * gcc.target/arm/optional_thumb-3.c: New test.
>>>>>>
>>>>>>
>>>>>> No regression when running the testsuite for -mcpu=cortex-m0 -mthumb,
>>>>>> -mcpu=cortex-m0 -marm and -mcpu=cortex-a8 -marm
>>>>>>
>>>>>> Is this ok for trunk?
>>>>>>
>>>>
>>>> This looks like a useful usability improvement.
>>>> This is ok after a bootstrap on an arm-none-linux-gnueabihf target.
>>>>
>>>> Sorry for the delay,
>>>> Kyrill
>>>
>>>
>>>
>>> I've rebased the patch on top of the arm_feature_set type consistency fix
>>> [1] and committed it. The committed patch is in attachment for reference.
>>>
>>> [1] https://gcc.gnu.org/ml/gcc-patches/2016-11/msg01680.html
>>>
>>
>> Since this commit (242597), I've noticed that:
>> - the 2 new tests optional_thumb-1.c and optional_thumb-2.c fail
>> if GCC was configured --with-mode=arm. The error message is:
>> cc1: error: target CPU does not support ARM mode
>
>
> I need to skip the test when GCC is built with --with-mode= but we do not
> have a directive for that. I'll see if I can add one.
>
Since the test adds -march=armv6m, shouldn't you patch implicitly add -mthumb?

>>
>> - on armeb --with-mode=arm, gcc.dg/vect/pr64252.c fails at execution
>>
>> See:
>> http://people.linaro.org/~christophe.lyon/cross-validation/gcc/trunk/242597/report-build-info.html
>
>
> I cannot reproduce that last issue. I've built r242658 for armeb-none-eabi
> and it passes fine in qemu-armeb. I'll try building a linux toolchain.
>

Indeed, I see it passing at r242646 (it failed between 242597 and
242632 included).
It was fixed between 242632 and 242646 in my logs....

Sorry for the noise.


> Best regards,
>
> Thomas
Thomas Preudhomme Nov. 21, 2016, 6:11 p.m. UTC | #4
On 21/11/16 15:21, Christophe Lyon wrote:
> On 21 November 2016 at 15:16, Thomas Preudhomme
> <thomas.preudhomme@foss.arm.com> wrote:
>> On 21/11/16 08:51, Christophe Lyon wrote:
>>>
>>> Hi Thomas,
>>
>>
>> Hi Christophe,
>>
>>
>>>
>>>
>>> On 18 November 2016 at 17:51, Thomas Preudhomme
>>> <thomas.preudhomme@foss.arm.com> wrote:
>>>>
>>>> On 11/11/16 14:35, Kyrill Tkachov wrote:
>>>>>
>>>>>
>>>>>
>>>>> On 08/11/16 13:36, Thomas Preudhomme wrote:
>>>>>>
>>>>>>
>>>>>> Ping?
>>>>>>
>>>>>> Best regards,
>>>>>>
>>>>>> Thomas
>>>>>>
>>>>>> On 25/10/16 18:07, Thomas Preudhomme wrote:
>>>>>>>
>>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> Currently when a user compiles for a thumb-only target (such as
>>>>>>> Cortex-M
>>>>>>> processors) without specifying the -mthumb option GCC throws the error
>>>>>>> "target
>>>>>>> CPU does not support ARM mode". This is suboptimal from a usability
>>>>>>> point of
>>>>>>> view: the -mthumb could be deduced from the -march or -mcpu option
>>>>>>> when
>>>>>>> there is
>>>>>>> no ambiguity.
>>>>>>>
>>>>>>> This patch implements this behavior by extending the DRIVER_SELF_SPECS
>>>>>>> to
>>>>>>> automatically append -mthumb to the command line for thumb-only
>>>>>>> targets.
>>>>>>> It does
>>>>>>> so by checking the last -march option if any is given or the last
>>>>>>> -mcpu
>>>>>>> option
>>>>>>> otherwise. There is no ordering issue because conflicting -mcpu and
>>>>>>> -march is
>>>>>>> already handled.
>>>>>>>
>>>>>>> Note that the logic cannot be implemented in function
>>>>>>> arm_option_override
>>>>>>> because we need to provide the modified command line to the GCC driver
>>>>>>> for
>>>>>>> finding the right multilib path and the function arm_option_override
>>>>>>> is
>>>>>>> executed
>>>>>>> too late for that effect.
>>>>>>>
>>>>>>> ChangeLog entries are as follow:
>>>>>>>
>>>>>>> *** gcc/ChangeLog ***
>>>>>>>
>>>>>>> 2016-10-18  Terry Guo  <terry.guo@arm.com>
>>>>>>>             Thomas Preud'homme <thomas.preudhomme@arm.com>
>>>>>>>
>>>>>>>         PR target/64802
>>>>>>>         * common/config/arm/arm-common.c (arm_target_thumb_only): New
>>>>>>> function.
>>>>>>>         * config/arm/arm-opts.h: Include arm-flags.h.
>>>>>>>         (struct arm_arch_core_flag): Define.
>>>>>>>         (arm_arch_core_flags): Define.
>>>>>>>         * config/arm/arm-protos.h: Include arm-flags.h.
>>>>>>>         (FL_NONE, FL_ANY, FL_CO_PROC, FL_ARCH3M, FL_MODE26, FL_MODE32,
>>>>>>>         FL_ARCH4, FL_ARCH5, FL_THUMB, FL_LDSCHED, FL_STRONG,
>>>>>>> FL_ARCH5E,
>>>>>>>         FL_XSCALE, FL_ARCH6, FL_VFPV2, FL_WBUF, FL_ARCH6K, FL_THUMB2,
>>>>>>> FL_NOTM,
>>>>>>>         FL_THUMB_DIV, FL_VFPV3, FL_NEON, FL_ARCH7EM, FL_ARCH7,
>>>>>>> FL_ARM_DIV,
>>>>>>>         FL_ARCH8, FL_CRC32, FL_SMALLMUL, FL_NO_VOLATILE_CE, FL_IWMMXT,
>>>>>>>         FL_IWMMXT2, FL_ARCH6KZ, FL2_ARCH8_1, FL2_ARCH8_2,
>>>>>>> FL2_FP16INST,
>>>>>>>         FL_TUNE, FL_FOR_ARCH2, FL_FOR_ARCH3, FL_FOR_ARCH3M,
>>>>>>> FL_FOR_ARCH4,
>>>>>>>         FL_FOR_ARCH4T, FL_FOR_ARCH5, FL_FOR_ARCH5T, FL_FOR_ARCH5E,
>>>>>>>         FL_FOR_ARCH5TE, FL_FOR_ARCH5TEJ, FL_FOR_ARCH6, FL_FOR_ARCH6J,
>>>>>>>         FL_FOR_ARCH6K, FL_FOR_ARCH6Z, FL_FOR_ARCH6ZK, FL_FOR_ARCH6KZ,
>>>>>>>         FL_FOR_ARCH6T2, FL_FOR_ARCH6M, FL_FOR_ARCH7, FL_FOR_ARCH7A,
>>>>>>>         FL_FOR_ARCH7VE, FL_FOR_ARCH7R, FL_FOR_ARCH7M, FL_FOR_ARCH7EM,
>>>>>>>         FL_FOR_ARCH8A, FL2_FOR_ARCH8_1A, FL2_FOR_ARCH8_2A,
>>>>>>> FL_FOR_ARCH8M_BASE,
>>>>>>>         FL_FOR_ARCH8M_MAIN, arm_feature_set, ARM_FSET_MAKE,
>>>>>>>         ARM_FSET_MAKE_CPU1, ARM_FSET_MAKE_CPU2, ARM_FSET_CPU1,
>>>>>>> ARM_FSET_CPU2,
>>>>>>>         ARM_FSET_EMPTY, ARM_FSET_ANY, ARM_FSET_HAS_CPU1,
>>>>>>> ARM_FSET_HAS_CPU2,
>>>>>>>         ARM_FSET_HAS_CPU, ARM_FSET_ADD_CPU1, ARM_FSET_ADD_CPU2,
>>>>>>>         ARM_FSET_DEL_CPU1, ARM_FSET_DEL_CPU2, ARM_FSET_UNION,
>>>>>>> ARM_FSET_INTER,
>>>>>>>         ARM_FSET_XOR, ARM_FSET_EXCLUDE, ARM_FSET_IS_EMPTY,
>>>>>>>         ARM_FSET_CPU_SUBSET): Move to ...
>>>>>>>         * config/arm/arm-flags.h: This new file.
>>>>>>>         * config/arm/arm.h (TARGET_MODE_SPEC_FUNCTIONS): Define.
>>>>>>>         (EXTRA_SPEC_FUNCTIONS): Add TARGET_MODE_SPEC_FUNCTIONS to its
>>>>>>> value.
>>>>>>>         (TARGET_MODE_SPECS): Define.
>>>>>>>         (DRIVER_SELF_SPECS): Add TARGET_MODE_SPECS to its value.
>>>>>>>
>>>>>>>
>>>>>>> *** gcc/testsuite/ChangeLog ***
>>>>>>>
>>>>>>> 2016-10-11  Thomas Preud'homme <thomas.preudhomme@arm.com>
>>>>>>>
>>>>>>>         PR target/64802
>>>>>>>         * gcc.target/arm/optional_thumb-1.c: New test.
>>>>>>>         * gcc.target/arm/optional_thumb-2.c: New test.
>>>>>>>         * gcc.target/arm/optional_thumb-3.c: New test.
>>>>>>>
>>>>>>>
>>>>>>> No regression when running the testsuite for -mcpu=cortex-m0 -mthumb,
>>>>>>> -mcpu=cortex-m0 -marm and -mcpu=cortex-a8 -marm
>>>>>>>
>>>>>>> Is this ok for trunk?
>>>>>>>
>>>>>
>>>>> This looks like a useful usability improvement.
>>>>> This is ok after a bootstrap on an arm-none-linux-gnueabihf target.
>>>>>
>>>>> Sorry for the delay,
>>>>> Kyrill
>>>>
>>>>
>>>>
>>>> I've rebased the patch on top of the arm_feature_set type consistency fix
>>>> [1] and committed it. The committed patch is in attachment for reference.
>>>>
>>>> [1] https://gcc.gnu.org/ml/gcc-patches/2016-11/msg01680.html
>>>>
>>>
>>> Since this commit (242597), I've noticed that:
>>> - the 2 new tests optional_thumb-1.c and optional_thumb-2.c fail
>>> if GCC was configured --with-mode=arm. The error message is:
>>> cc1: error: target CPU does not support ARM mode
>>
>>
>> I need to skip the test when GCC is built with --with-mode= but we do not
>> have a directive for that. I'll see if I can add one.
>>
> Since the test adds -march=armv6m, shouldn't you patch implicitly add -mthumb?

Precisely not, since the purpose is to test that GCC deduce the -mthumb 
automatically when the target is Thumb-only. The thing is the test should only 
be run when no -mthumb or -marm is passed and --with-mode=* is equivalent to 
passing -mthumb or -marm.

>
>>>
>>> - on armeb --with-mode=arm, gcc.dg/vect/pr64252.c fails at execution
>>>
>>> See:
>>> http://people.linaro.org/~christophe.lyon/cross-validation/gcc/trunk/242597/report-build-info.html
>>
>>
>> I cannot reproduce that last issue. I've built r242658 for armeb-none-eabi
>> and it passes fine in qemu-armeb. I'll try building a linux toolchain.
>>
>
> Indeed, I see it passing at r242646 (it failed between 242597 and
> 242632 included).
> It was fixed between 242632 and 242646 in my logs....
>
> Sorry for the noise.

Good to know.

Best regards,

Thomas
diff mbox

Patch

diff --git a/gcc/common/config/arm/arm-common.c b/gcc/common/config/arm/arm-common.c
index f3b674339a50460d55920ca8d26275a550bbbc1e..473417a2e5f04488197c27ead2b65680bddec274 100644
--- a/gcc/common/config/arm/arm-common.c
+++ b/gcc/common/config/arm/arm-common.c
@@ -98,6 +98,29 @@  arm_rewrite_mcpu (int argc, const char **argv)
   return arm_rewrite_selected_cpu (argv[argc - 1]);
 }
 
+/* Called by the driver to check whether the target denoted by current
+   command line options is a Thumb-only target.  ARGV is an array of
+   -march and -mcpu values (ie. it contains the rhs after the equal
+   sign) and we use the last one of them to make a decision.  The
+   number of elements in ARGV is given in ARGC.  */
+const char *
+arm_target_thumb_only (int argc, const char **argv)
+{
+  unsigned int opt;
+
+  if (argc)
+    {
+      for (opt = 0; opt < (ARRAY_SIZE (arm_arch_core_flags)); opt++)
+	if ((strcmp (argv[argc - 1], arm_arch_core_flags[opt].name) == 0)
+	    && !ARM_FSET_HAS_CPU1(arm_arch_core_flags[opt].flags, FL_NOTM))
+	  return "-mthumb";
+
+      return NULL;
+    }
+  else
+    return NULL;
+}
+
 #undef ARM_CPU_NAME_LENGTH
 
 
diff --git a/gcc/config/arm/arm-flags.h b/gcc/config/arm/arm-flags.h
new file mode 100644
index 0000000000000000000000000000000000000000..7ce059bcf87b162d2ea77a0d1322edb25d04676c
--- /dev/null
+++ b/gcc/config/arm/arm-flags.h
@@ -0,0 +1,211 @@ 
+/* Flags used to identify the presence of processor capabilities.
+
+   Copyright (C) 2016 Free Software Foundation, Inc.
+   Contributed by ARM Ltd.
+
+   This file is part of GCC.
+
+   GCC is free software; you can redistribute it and/or modify it
+   under the terms of the GNU General Public License as published
+   by the Free Software Foundation; either version 3, or (at your
+   option) any later version.
+
+   GCC is distributed in the hope that it will be useful, but WITHOUT
+   ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+   or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public
+   License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with GCC; see the file COPYING3.  If not see
+   <http://www.gnu.org/licenses/>.  */
+
+#ifndef GCC_ARM_FLAGS_H
+#define GCC_ARM_FLAGS_H
+
+/* Flags used to identify the presence of processor capabilities.  */
+
+/* Bit values used to identify processor capabilities.  */
+#define FL_NONE	      (0U)		/* No flags.  */
+#define FL_ANY	      (0xffffffffU)	/* All flags.  */
+#define FL_CO_PROC    (1U << 0)		/* Has external co-processor bus.  */
+#define FL_ARCH3M     (1U << 1)		/* Extended multiply.  */
+#define FL_MODE26     (1U << 2)		/* 26-bit mode support.  */
+#define FL_MODE32     (1U << 3)		/* 32-bit mode support.  */
+#define FL_ARCH4      (1U << 4)		/* Architecture rel 4.  */
+#define FL_ARCH5      (1U << 5)		/* Architecture rel 5.  */
+#define FL_THUMB      (1U << 6)		/* Thumb aware.  */
+#define FL_LDSCHED    (1U << 7)		/* Load scheduling necessary.  */
+#define FL_STRONG     (1U << 8)		/* StrongARM.  */
+#define FL_ARCH5E     (1U << 9)		/* DSP extensions to v5.  */
+#define FL_XSCALE     (1U << 10)	/* XScale.  */
+/* spare	      (1U << 11) */
+#define FL_ARCH6      (1U << 12)	/* Architecture rel 6.  Adds
+					   media instructions.  */
+#define FL_VFPV2      (1U << 13)	/* Vector Floating Point V2.  */
+#define FL_WBUF	      (1U << 14)	/* Schedule for write buffer ops.
+					   Note: ARM6 & 7 derivatives only.  */
+#define FL_ARCH6K     (1U << 15)	/* Architecture rel 6 K extensions.  */
+#define FL_THUMB2     (1U << 16)	/* Thumb-2.  */
+#define FL_NOTM	      (1U << 17)	/* Instructions not present in the 'M'
+					   profile.  */
+#define FL_THUMB_DIV  (1U << 18)	/* Hardware divide (Thumb mode).  */
+#define FL_VFPV3      (1U << 19)	/* Vector Floating Point V3.  */
+#define FL_NEON       (1U << 20)	/* Neon instructions.  */
+#define FL_ARCH7EM    (1U << 21)	/* Instructions present in the ARMv7E-M
+					   architecture.  */
+#define FL_ARCH7      (1U << 22)	/* Architecture 7.  */
+#define FL_ARM_DIV    (1U << 23)	/* Hardware divide (ARM mode).  */
+#define FL_ARCH8      (1U << 24)	/* Architecture 8.  */
+#define FL_CRC32      (1U << 25)	/* ARMv8 CRC32 instructions.  */
+#define FL_SMALLMUL   (1U << 26)	/* Small multiply supported.  */
+#define FL_NO_VOLATILE_CE  (1U << 27)	/* No volatile memory in IT block.  */
+
+#define FL_IWMMXT     (1U << 29)	/* XScale v2 or "Intel Wireless MMX
+					   technology".  */
+#define FL_IWMMXT2    (1U << 30)	/* "Intel Wireless MMX2
+					    technology".  */
+#define FL_ARCH6KZ    (1U << 31)	/* ARMv6KZ architecture.  */
+
+#define FL2_ARCH8_1   (1U << 0)		/* Architecture 8.1.  */
+#define FL2_ARCH8_2   (1U << 1)		/* Architecture 8.2.  */
+#define FL2_FP16INST  (1U << 2)		/* FP16 Instructions for ARMv8.2 and
+					   later.  */
+
+/* Flags that only effect tuning, not available instructions.  */
+#define FL_TUNE		(FL_WBUF | FL_VFPV2 | FL_STRONG | FL_LDSCHED \
+			 | FL_CO_PROC)
+
+#define FL_FOR_ARCH2		FL_NOTM
+#define FL_FOR_ARCH3		(FL_FOR_ARCH2 | FL_MODE32)
+#define FL_FOR_ARCH3M		(FL_FOR_ARCH3 | FL_ARCH3M)
+#define FL_FOR_ARCH4		(FL_FOR_ARCH3M | FL_ARCH4)
+#define FL_FOR_ARCH4T		(FL_FOR_ARCH4 | FL_THUMB)
+#define FL_FOR_ARCH5		(FL_FOR_ARCH4 | FL_ARCH5)
+#define FL_FOR_ARCH5T		(FL_FOR_ARCH5 | FL_THUMB)
+#define FL_FOR_ARCH5E		(FL_FOR_ARCH5 | FL_ARCH5E)
+#define FL_FOR_ARCH5TE		(FL_FOR_ARCH5E | FL_THUMB)
+#define FL_FOR_ARCH5TEJ		FL_FOR_ARCH5TE
+#define FL_FOR_ARCH6		(FL_FOR_ARCH5TE | FL_ARCH6)
+#define FL_FOR_ARCH6J		FL_FOR_ARCH6
+#define FL_FOR_ARCH6K		(FL_FOR_ARCH6 | FL_ARCH6K)
+#define FL_FOR_ARCH6Z		FL_FOR_ARCH6
+#define FL_FOR_ARCH6ZK		FL_FOR_ARCH6K
+#define FL_FOR_ARCH6KZ		(FL_FOR_ARCH6K | FL_ARCH6KZ)
+#define FL_FOR_ARCH6T2		(FL_FOR_ARCH6 | FL_THUMB2)
+#define FL_FOR_ARCH6M		(FL_FOR_ARCH6 & ~FL_NOTM)
+#define FL_FOR_ARCH7		((FL_FOR_ARCH6T2 & ~FL_NOTM) | FL_ARCH7)
+#define FL_FOR_ARCH7A		(FL_FOR_ARCH7 | FL_NOTM | FL_ARCH6K)
+#define FL_FOR_ARCH7VE		(FL_FOR_ARCH7A | FL_THUMB_DIV | FL_ARM_DIV)
+#define FL_FOR_ARCH7R		(FL_FOR_ARCH7A | FL_THUMB_DIV)
+#define FL_FOR_ARCH7M		(FL_FOR_ARCH7 | FL_THUMB_DIV)
+#define FL_FOR_ARCH7EM		(FL_FOR_ARCH7M | FL_ARCH7EM)
+#define FL_FOR_ARCH8A		(FL_FOR_ARCH7VE | FL_ARCH8)
+#define FL2_FOR_ARCH8_1A	FL2_ARCH8_1
+#define FL2_FOR_ARCH8_2A	(FL2_FOR_ARCH8_1A | FL2_ARCH8_2)
+#define FL_FOR_ARCH8M_BASE	(FL_FOR_ARCH6M | FL_ARCH8 | FL_THUMB_DIV)
+#define FL_FOR_ARCH8M_MAIN	(FL_FOR_ARCH7M | FL_ARCH8)
+
+/* There are too many feature bits to fit in a single word so the set of cpu and
+   fpu capabilities is a structure.  A feature set is created and manipulated
+   with the ARM_FSET macros.  */
+
+typedef struct
+{
+  unsigned cpu[2];
+} arm_feature_set;
+
+
+/* Initialize a feature set.  */
+
+#define ARM_FSET_MAKE(CPU1,CPU2) { { (CPU1), (CPU2) } }
+
+#define ARM_FSET_MAKE_CPU1(CPU1) ARM_FSET_MAKE ((CPU1), (FL_NONE))
+#define ARM_FSET_MAKE_CPU2(CPU2) ARM_FSET_MAKE ((FL_NONE), (CPU2))
+
+/* Accessors.  */
+
+#define ARM_FSET_CPU1(S) ((S).cpu[0])
+#define ARM_FSET_CPU2(S) ((S).cpu[1])
+
+/* Useful combinations.  */
+
+#define ARM_FSET_EMPTY ARM_FSET_MAKE (FL_NONE, FL_NONE)
+#define ARM_FSET_ANY ARM_FSET_MAKE (FL_ANY, FL_ANY)
+
+/* Tests for a specific CPU feature.  */
+
+#define ARM_FSET_HAS_CPU1(A, F)  \
+  (((A).cpu[0] & ((unsigned long)(F))) == ((unsigned long)(F)))
+#define ARM_FSET_HAS_CPU2(A, F)  \
+  (((A).cpu[1] & ((unsigned long)(F))) == ((unsigned long)(F)))
+#define ARM_FSET_HAS_CPU(A, F1, F2)				\
+  (ARM_FSET_HAS_CPU1 ((A), (F1)) && ARM_FSET_HAS_CPU2 ((A), (F2)))
+
+/* Add a feature to a feature set.  */
+
+#define ARM_FSET_ADD_CPU1(DST, F)		\
+  do {						\
+    (DST).cpu[0] |= (F);			\
+  } while (0)
+
+#define ARM_FSET_ADD_CPU2(DST, F)		\
+  do {						\
+    (DST).cpu[1] |= (F);			\
+  } while (0)
+
+/* Remove a feature from a feature set.  */
+
+#define ARM_FSET_DEL_CPU1(DST, F)		\
+  do {						\
+    (DST).cpu[0] &= ~(F);			\
+  } while (0)
+
+#define ARM_FSET_DEL_CPU2(DST, F)		\
+  do {						\
+    (DST).cpu[1] &= ~(F);			\
+  } while (0)
+
+/* Union of feature sets.  */
+
+#define ARM_FSET_UNION(DST,F1,F2)		\
+  do {						\
+    (DST).cpu[0] = (F1).cpu[0] | (F2).cpu[0];	\
+    (DST).cpu[1] = (F1).cpu[1] | (F2).cpu[1];	\
+  } while (0)
+
+/* Intersection of feature sets.  */
+
+#define ARM_FSET_INTER(DST,F1,F2)		\
+  do {						\
+    (DST).cpu[0] = (F1).cpu[0] & (F2).cpu[0];	\
+    (DST).cpu[1] = (F1).cpu[1] & (F2).cpu[1];	\
+  } while (0)
+
+/* Exclusive disjunction.  */
+
+#define ARM_FSET_XOR(DST,F1,F2)				\
+  do {							\
+    (DST).cpu[0] = (F1).cpu[0] ^ (F2).cpu[0];		\
+    (DST).cpu[1] = (F1).cpu[1] ^ (F2).cpu[1];		\
+  } while (0)
+
+/* Difference of feature sets: F1 excluding the elements of F2.  */
+
+#define ARM_FSET_EXCLUDE(DST,F1,F2)		\
+  do {						\
+    (DST).cpu[0] = (F1).cpu[0] & ~(F2).cpu[0];	\
+    (DST).cpu[1] = (F1).cpu[1] & ~(F2).cpu[1];	\
+  } while (0)
+
+/* Test for an empty feature set.  */
+
+#define ARM_FSET_IS_EMPTY(A)		\
+  (!((A).cpu[0]) && !((A).cpu[1]))
+
+/* Tests whether the cpu features of A are a subset of B.  */
+
+#define ARM_FSET_CPU_SUBSET(A,B)					\
+  ((((A).cpu[0] & (B).cpu[0]) == (A).cpu[0])				\
+   && (((A).cpu[1] & (B).cpu[1]) == (A).cpu[1]))
+
+#endif /* GCC_ARM_FLAGS_H */
diff --git a/gcc/config/arm/arm-opts.h b/gcc/config/arm/arm-opts.h
index 9fa8113f279bbb19519423e1b66b936c3a1c5c3d..6971534a562e7b37a67d9a20f40f29b5f00dd9bc 100644
--- a/gcc/config/arm/arm-opts.h
+++ b/gcc/config/arm/arm-opts.h
@@ -25,6 +25,8 @@ 
 #ifndef ARM_OPTS_H
 #define ARM_OPTS_H
 
+#include "arm-flags.h"
+
 /* The various ARM cores.  */
 enum processor_type
 {
@@ -77,4 +79,24 @@  enum arm_tls_type {
   TLS_GNU,
   TLS_GNU2
 };
+
+struct arm_arch_core_flag
+{
+  const char *const name;
+  const arm_feature_set flags;
+};
+
+static const struct arm_arch_core_flag arm_arch_core_flags[] =
+{
+#undef ARM_CORE
+#define ARM_CORE(NAME, X, IDENT, ARCH, FLAGS, COSTS) \
+  {NAME, FLAGS},
+#include "arm-cores.def"
+#undef ARM_CORE
+#undef ARM_ARCH
+#define ARM_ARCH(NAME, CORE, ARCH, FLAGS) \
+  {NAME, FLAGS},
+#include "arm-arches.def"
+#undef ARM_ARCH
+};
 #endif
diff --git a/gcc/config/arm/arm-protos.h b/gcc/config/arm/arm-protos.h
index 5cee7718554886982f535da2e9baa5015da609e4..68e9beaa2612ca5aff7a835123690aabc2c600ad 100644
--- a/gcc/config/arm/arm-protos.h
+++ b/gcc/config/arm/arm-protos.h
@@ -22,6 +22,8 @@ 
 #ifndef GCC_ARM_PROTOS_H
 #define GCC_ARM_PROTOS_H
 
+#include "arm-flags.h"
+
 extern enum unwind_info_type arm_except_unwind_info (struct gcc_options *);
 extern int use_return_insn (int, rtx);
 extern bool use_simple_return_p (void);
@@ -348,192 +350,6 @@  extern void arm_cpu_cpp_builtins (struct cpp_reader *);
 
 extern bool arm_is_constant_pool_ref (rtx);
 
-/* Flags used to identify the presence of processor capabilities.  */
-
-/* Bit values used to identify processor capabilities.  */
-#define FL_NONE	      (0U)		/* No flags.  */
-#define FL_ANY	      (0xffffffffU)	/* All flags.  */
-#define FL_CO_PROC    (1U << 0)		/* Has external co-processor bus.  */
-#define FL_ARCH3M     (1U << 1)		/* Extended multiply.  */
-#define FL_MODE26     (1U << 2)		/* 26-bit mode support.  */
-#define FL_MODE32     (1U << 3)		/* 32-bit mode support.  */
-#define FL_ARCH4      (1U << 4)		/* Architecture rel 4.  */
-#define FL_ARCH5      (1U << 5)		/* Architecture rel 5.  */
-#define FL_THUMB      (1U << 6)		/* Thumb aware.  */
-#define FL_LDSCHED    (1U << 7)		/* Load scheduling necessary.  */
-#define FL_STRONG     (1U << 8)		/* StrongARM.  */
-#define FL_ARCH5E     (1U << 9)		/* DSP extensions to v5.  */
-#define FL_XSCALE     (1U << 10)	/* XScale.  */
-/* spare	      (1U << 11) */
-#define FL_ARCH6      (1U << 12)	/* Architecture rel 6.  Adds
-					   media instructions.  */
-#define FL_VFPV2      (1U << 13)	/* Vector Floating Point V2.  */
-#define FL_WBUF	      (1U << 14)	/* Schedule for write buffer ops.
-					   Note: ARM6 & 7 derivatives only.  */
-#define FL_ARCH6K     (1U << 15)	/* Architecture rel 6 K extensions.  */
-#define FL_THUMB2     (1U << 16)	/* Thumb-2.  */
-#define FL_NOTM	      (1U << 17)	/* Instructions not present in the 'M'
-					   profile.  */
-#define FL_THUMB_DIV  (1U << 18)	/* Hardware divide (Thumb mode).  */
-#define FL_VFPV3      (1U << 19)	/* Vector Floating Point V3.  */
-#define FL_NEON       (1U << 20)	/* Neon instructions.  */
-#define FL_ARCH7EM    (1U << 21)	/* Instructions present in the ARMv7E-M
-					   architecture.  */
-#define FL_ARCH7      (1U << 22)	/* Architecture 7.  */
-#define FL_ARM_DIV    (1U << 23)	/* Hardware divide (ARM mode).  */
-#define FL_ARCH8      (1U << 24)	/* Architecture 8.  */
-#define FL_CRC32      (1U << 25)	/* ARMv8 CRC32 instructions.  */
-#define FL_SMALLMUL   (1U << 26)	/* Small multiply supported.  */
-#define FL_NO_VOLATILE_CE  (1U << 27)	/* No volatile memory in IT block.  */
-
-#define FL_IWMMXT     (1U << 29)	/* XScale v2 or "Intel Wireless MMX
-					   technology".  */
-#define FL_IWMMXT2    (1U << 30)	/* "Intel Wireless MMX2
-					    technology".  */
-#define FL_ARCH6KZ    (1U << 31)	/* ARMv6KZ architecture.  */
-
-#define FL2_ARCH8_1   (1U << 0)		/* Architecture 8.1.  */
-#define FL2_ARCH8_2   (1U << 1)		/* Architecture 8.2.  */
-#define FL2_FP16INST  (1U << 2)		/* FP16 Instructions for ARMv8.2 and
-					   later.  */
-
-/* Flags that only effect tuning, not available instructions.  */
-#define FL_TUNE		(FL_WBUF | FL_VFPV2 | FL_STRONG | FL_LDSCHED \
-			 | FL_CO_PROC)
-
-#define FL_FOR_ARCH2		FL_NOTM
-#define FL_FOR_ARCH3		(FL_FOR_ARCH2 | FL_MODE32)
-#define FL_FOR_ARCH3M		(FL_FOR_ARCH3 | FL_ARCH3M)
-#define FL_FOR_ARCH4		(FL_FOR_ARCH3M | FL_ARCH4)
-#define FL_FOR_ARCH4T		(FL_FOR_ARCH4 | FL_THUMB)
-#define FL_FOR_ARCH5		(FL_FOR_ARCH4 | FL_ARCH5)
-#define FL_FOR_ARCH5T		(FL_FOR_ARCH5 | FL_THUMB)
-#define FL_FOR_ARCH5E		(FL_FOR_ARCH5 | FL_ARCH5E)
-#define FL_FOR_ARCH5TE		(FL_FOR_ARCH5E | FL_THUMB)
-#define FL_FOR_ARCH5TEJ		FL_FOR_ARCH5TE
-#define FL_FOR_ARCH6		(FL_FOR_ARCH5TE | FL_ARCH6)
-#define FL_FOR_ARCH6J		FL_FOR_ARCH6
-#define FL_FOR_ARCH6K		(FL_FOR_ARCH6 | FL_ARCH6K)
-#define FL_FOR_ARCH6Z		FL_FOR_ARCH6
-#define FL_FOR_ARCH6ZK		FL_FOR_ARCH6K
-#define FL_FOR_ARCH6KZ		(FL_FOR_ARCH6K | FL_ARCH6KZ)
-#define FL_FOR_ARCH6T2		(FL_FOR_ARCH6 | FL_THUMB2)
-#define FL_FOR_ARCH6M		(FL_FOR_ARCH6 & ~FL_NOTM)
-#define FL_FOR_ARCH7		((FL_FOR_ARCH6T2 & ~FL_NOTM) | FL_ARCH7)
-#define FL_FOR_ARCH7A		(FL_FOR_ARCH7 | FL_NOTM | FL_ARCH6K)
-#define FL_FOR_ARCH7VE		(FL_FOR_ARCH7A | FL_THUMB_DIV | FL_ARM_DIV)
-#define FL_FOR_ARCH7R		(FL_FOR_ARCH7A | FL_THUMB_DIV)
-#define FL_FOR_ARCH7M		(FL_FOR_ARCH7 | FL_THUMB_DIV)
-#define FL_FOR_ARCH7EM		(FL_FOR_ARCH7M | FL_ARCH7EM)
-#define FL_FOR_ARCH8A		(FL_FOR_ARCH7VE | FL_ARCH8)
-#define FL2_FOR_ARCH8_1A	FL2_ARCH8_1
-#define FL2_FOR_ARCH8_2A	(FL2_FOR_ARCH8_1A | FL2_ARCH8_2)
-#define FL_FOR_ARCH8M_BASE	(FL_FOR_ARCH6M | FL_ARCH8 | FL_THUMB_DIV)
-#define FL_FOR_ARCH8M_MAIN	(FL_FOR_ARCH7M | FL_ARCH8)
-
-/* There are too many feature bits to fit in a single word so the set of cpu and
-   fpu capabilities is a structure.  A feature set is created and manipulated
-   with the ARM_FSET macros.  */
-
-typedef struct
-{
-  unsigned cpu[2];
-} arm_feature_set;
-
-
-/* Initialize a feature set.  */
-
-#define ARM_FSET_MAKE(CPU1,CPU2) { { (CPU1), (CPU2) } }
-
-#define ARM_FSET_MAKE_CPU1(CPU1) ARM_FSET_MAKE ((CPU1), (FL_NONE))
-#define ARM_FSET_MAKE_CPU2(CPU2) ARM_FSET_MAKE ((FL_NONE), (CPU2))
-
-/* Accessors.  */
-
-#define ARM_FSET_CPU1(S) ((S).cpu[0])
-#define ARM_FSET_CPU2(S) ((S).cpu[1])
-
-/* Useful combinations.  */
-
-#define ARM_FSET_EMPTY ARM_FSET_MAKE (FL_NONE, FL_NONE)
-#define ARM_FSET_ANY ARM_FSET_MAKE (FL_ANY, FL_ANY)
-
-/* Tests for a specific CPU feature.  */
-
-#define ARM_FSET_HAS_CPU1(A, F)  \
-  (((A).cpu[0] & ((unsigned long)(F))) == ((unsigned long)(F)))
-#define ARM_FSET_HAS_CPU2(A, F)  \
-  (((A).cpu[1] & ((unsigned long)(F))) == ((unsigned long)(F)))
-#define ARM_FSET_HAS_CPU(A, F1, F2)				\
-  (ARM_FSET_HAS_CPU1 ((A), (F1)) && ARM_FSET_HAS_CPU2 ((A), (F2)))
-
-/* Add a feature to a feature set.  */
-
-#define ARM_FSET_ADD_CPU1(DST, F)		\
-  do {						\
-    (DST).cpu[0] |= (F);			\
-  } while (0)
-
-#define ARM_FSET_ADD_CPU2(DST, F)		\
-  do {						\
-    (DST).cpu[1] |= (F);			\
-  } while (0)
-
-/* Remove a feature from a feature set.  */
-
-#define ARM_FSET_DEL_CPU1(DST, F)		\
-  do {						\
-    (DST).cpu[0] &= ~(F);			\
-  } while (0)
-
-#define ARM_FSET_DEL_CPU2(DST, F)		\
-  do {						\
-    (DST).cpu[1] &= ~(F);			\
-  } while (0)
-
-/* Union of feature sets.  */
-
-#define ARM_FSET_UNION(DST,F1,F2)		\
-  do {						\
-    (DST).cpu[0] = (F1).cpu[0] | (F2).cpu[0];	\
-    (DST).cpu[1] = (F1).cpu[1] | (F2).cpu[1];	\
-  } while (0)
-
-/* Intersection of feature sets.  */
-
-#define ARM_FSET_INTER(DST,F1,F2)		\
-  do {						\
-    (DST).cpu[0] = (F1).cpu[0] & (F2).cpu[0];	\
-    (DST).cpu[1] = (F1).cpu[1] & (F2).cpu[1];	\
-  } while (0)
-
-/* Exclusive disjunction.  */
-
-#define ARM_FSET_XOR(DST,F1,F2)				\
-  do {							\
-    (DST).cpu[0] = (F1).cpu[0] ^ (F2).cpu[0];		\
-    (DST).cpu[1] = (F1).cpu[1] ^ (F2).cpu[1];		\
-  } while (0)
-
-/* Difference of feature sets: F1 excluding the elements of F2.  */
-
-#define ARM_FSET_EXCLUDE(DST,F1,F2)		\
-  do {						\
-    (DST).cpu[0] = (F1).cpu[0] & ~(F2).cpu[0];	\
-    (DST).cpu[1] = (F1).cpu[1] & ~(F2).cpu[1];	\
-  } while (0)
-
-/* Test for an empty feature set.  */
-
-#define ARM_FSET_IS_EMPTY(A)		\
-  (!((A).cpu[0]) && !((A).cpu[1]))
-
-/* Tests whether the cpu features of A are a subset of B.  */
-
-#define ARM_FSET_CPU_SUBSET(A,B)					\
-  ((((A).cpu[0] & (B).cpu[0]) == (A).cpu[0])				\
-   && (((A).cpu[1] & (B).cpu[1]) == (A).cpu[1]))
-
 /* The bits in this mask specify which
    instructions we are allowed to generate.  */
 extern arm_feature_set insn_flags;
diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h
index c8d74623814884fbdbcaa7cb4546f7319dbbaac4..7ad0fbf0d803cfc9015b78ffdb4e8d2f94f43d1e 100644
--- a/gcc/config/arm/arm.h
+++ b/gcc/config/arm/arm.h
@@ -2250,13 +2250,18 @@  extern const char *arm_rewrite_mcpu (int argc, const char **argv);
    "   :%{march=*:-march=%*}}"					\
    BIG_LITTLE_SPEC
 
+extern const char *arm_target_thumb_only (int argc, const char **argv);
+#define TARGET_MODE_SPEC_FUNCTIONS					\
+  { "target_mode_check", arm_target_thumb_only },
+
 /* -mcpu=native handling only makes sense with compiler running on
    an ARM chip.  */
 #if defined(__arm__)
 extern const char *host_detect_local_cpu (int argc, const char **argv);
 # define EXTRA_SPEC_FUNCTIONS						\
   { "local_cpu_detect", host_detect_local_cpu },			\
-  BIG_LITTLE_CPU_SPEC_FUNCTIONS
+  BIG_LITTLE_CPU_SPEC_FUNCTIONS						\
+  TARGET_MODE_SPEC_FUNCTIONS
 
 # define MCPU_MTUNE_NATIVE_SPECS					\
    " %{march=native:%<march=native %:local_cpu_detect(arch)}"		\
@@ -2264,10 +2269,21 @@  extern const char *host_detect_local_cpu (int argc, const char **argv);
    " %{mtune=native:%<mtune=native %:local_cpu_detect(tune)}"
 #else
 # define MCPU_MTUNE_NATIVE_SPECS ""
-# define EXTRA_SPEC_FUNCTIONS BIG_LITTLE_CPU_SPEC_FUNCTIONS
+# define EXTRA_SPEC_FUNCTIONS						\
+	BIG_LITTLE_CPU_SPEC_FUNCTIONS					\
+	TARGET_MODE_SPEC_FUNCTIONS
 #endif
 
-#define DRIVER_SELF_SPECS MCPU_MTUNE_NATIVE_SPECS
+/* Automatically add -mthumb for Thumb-only targets if mode isn't specified
+   via the configuration option --with-mode or via the command line. The
+   function target_mode_check is called to do the check with either:
+   - an array of -march values if any is given;
+   - an array of -mcpu values if any is given;
+   - an empty array.  */
+#define TARGET_MODE_SPECS						\
+  " %{!marm:%{!mthumb:%:target_mode_check(%{march=*:%*;mcpu=*:%*;:})}}"
+
+#define DRIVER_SELF_SPECS MCPU_MTUNE_NATIVE_SPECS TARGET_MODE_SPECS
 #define TARGET_SUPPORTS_WIDE_INT 1
 
 /* For switching between functions with different target attributes.  */
diff --git a/gcc/testsuite/gcc.target/arm/optional_thumb-1.c b/gcc/testsuite/gcc.target/arm/optional_thumb-1.c
new file mode 100644
index 0000000000000000000000000000000000000000..23df62887ba4aaa1d8717a34ecda9a40246f0552
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/optional_thumb-1.c
@@ -0,0 +1,7 @@ 
+/* { dg-do compile } */
+/* { dg-skip-if "-marm/-mthumb/-march/-mcpu given" { *-*-*} { "-marm" "-mthumb" "-march=*" "-mcpu=*" } } */
+/* { dg-options "-march=armv6-m" } */
+
+/* Check that -mthumb is not needed when compiling for a Thumb-only target.  */
+
+int foo;
diff --git a/gcc/testsuite/gcc.target/arm/optional_thumb-2.c b/gcc/testsuite/gcc.target/arm/optional_thumb-2.c
new file mode 100644
index 0000000000000000000000000000000000000000..4bd53a45eca97e62dd3b86d5a1a66c5ca21e7aad
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/optional_thumb-2.c
@@ -0,0 +1,7 @@ 
+/* { dg-do compile } */
+/* { dg-skip-if "-marm/-mthumb/-march/-mcpu given" { *-*-*} { "-marm" "-mthumb" "-march=*" "-mcpu=*" } } */
+/* { dg-options "-mcpu=cortex-m4" } */
+
+/* Check that -mthumb is not needed when compiling for a Thumb-only target.  */
+
+int foo;
diff --git a/gcc/testsuite/gcc.target/arm/optional_thumb-3.c b/gcc/testsuite/gcc.target/arm/optional_thumb-3.c
new file mode 100644
index 0000000000000000000000000000000000000000..f1fd5c8840b191e600c20a7817c611bb9bb645df
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/optional_thumb-3.c
@@ -0,0 +1,9 @@ 
+/* { dg-do compile } */
+/* { dg-require-effective-target arm_cortex_m } */
+/* { dg-skip-if "-mthumb given" { *-*-*} { "-mthumb" } } */
+/* { dg-options "-marm" } */
+/* { dg-error "target CPU does not support ARM mode" "missing error with -marm on Thumb-only targets" { target *-*-*} 0 } */
+
+/* Check that -marm gives an error when compiling for a Thumb-only target.  */
+
+int foo;