On 21/11/16 15:21, Christophe Lyon wrote:
On 21 November 2016 at 15:16, Thomas Preudhomme
<thomas.preudho...@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.preudho...@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....@arm.com>
            Thomas Preud'homme <thomas.preudho...@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.preudho...@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

Reply via email to