| Message ID | 1410814919-11523-4-git-send-email-thomas.petazzoni@free-electrons.com |
|---|---|
| State | Superseded |
| Headers | show |
Hi Thomas, On Mon, Sep 15, 2014 at 11:01 PM, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote: > In commit 88cf3bb91792c9c04586e14f293d89a6e0c13e1d > ("arch/Config.in.arm: Use armv6k for arm1136jf-s rev1"), Benoît > Thébaudeau added separate options for the revision 0 and revision 1 of > the ARM1136JF-S processor, so that different -march values could be > used (armv6j for revision 0, armv6k for revision 1). > > However, this is preventing the removal of the BR2_GCC_TARGET_ARCH > option, which we need to do to give only the CPU type to gcc, and let > it decide the architecture variant that matches. This is because this > story of revision 0 vs. revision 1 is the only case where -mcpu > doesn't fully define the CPU. > > Moreover, a quick test with gcc shows that -march=armv6j > -mcpu=arm1136jf-s is accepted, while -march=armv6k -mcpu=arm1136jf-s > makes gcc complain: " warning: switch -mcpu=arm1136jf-s conflicts with > -march=armv6k switch". Yes, because -mcpu=arm1136jf-s implies -march=armv6j. But how about keeping -march and -mtune, and removing -mcpu, instead of the contrary? This would give more flexibility, and full control over the CPU optimization. > In addition, gcc 5 will apparently no longer allow to pass all of > --with-arch, --with-cpu and --with-tune, so we will anyway have to > rely only on one of them. What combinations will no longer be supported? Only -mcpu with -march and -mtune? [...] Best regards, Benoît
On 2014-09-15 23:01 +0200, Thomas Petazzoni spake thusly: > In commit 88cf3bb91792c9c04586e14f293d89a6e0c13e1d > ("arch/Config.in.arm: Use armv6k for arm1136jf-s rev1"), Benoît > Thébaudeau added separate options for the revision 0 and revision 1 of > the ARM1136JF-S processor, so that different -march values could be > used (armv6j for revision 0, armv6k for revision 1). > > However, this is preventing the removal of the BR2_GCC_TARGET_ARCH > option, which we need to do to give only the CPU type to gcc, and let > it decide the architecture variant that matches. This is because this > story of revision 0 vs. revision 1 is the only case where -mcpu > doesn't fully define the CPU. > > Moreover, a quick test with gcc shows that -march=armv6j > -mcpu=arm1136jf-s is accepted, while -march=armv6k -mcpu=arm1136jf-s > makes gcc complain: " warning: switch -mcpu=arm1136jf-s conflicts with > -march=armv6k switch". > > In addition, gcc 5 will apparently no longer allow to pass all of > --with-arch, --with-cpu and --with-tune, so we will anyway have to > rely only on one of them. > > As a consequence, this commit basically reverts > 88cf3bb91792c9c04586e14f293d89a6e0c13e1d and provides only one option > for ARM1136JF-S. If the two revisions are really different, then they > should be supported in upstream gcc with different -mcpu values. > > Note that the removal of the two options should not break existing > full .config, since the hidden option BR2_arm1136jf_s becomes again a > visible option to select the CPU. But it would break a defconfig. As suggested on IRC, move the BR2_arm1136jf_s_r1 to Config.legacy, and have it select BR2_arm1136jf_s, so the user can re-use a defconfig. Regards, Yann E. MORIN. > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> > Cc: Benoît Thébaudeau <benoit.thebaudeau@advansee.com> > --- > arch/Config.in.arm | 15 +++------------ > 1 file changed, 3 insertions(+), 12 deletions(-) > > diff --git a/arch/Config.in.arm b/arch/Config.in.arm > index 201ff1d..6e3c4e6 100644 > --- a/arch/Config.in.arm > +++ b/arch/Config.in.arm > @@ -54,12 +54,8 @@ config BR2_arm926t > bool "arm926t" > select BR2_ARM_CPU_MAYBE_HAS_VFPV2 > select BR2_ARM_CPU_HAS_THUMB > -config BR2_arm1136jf_s_r0 > - bool "arm1136jf_s rev0" > - select BR2_ARM_CPU_HAS_VFPV2 > - select BR2_ARM_CPU_HAS_THUMB > -config BR2_arm1136jf_s_r1 > - bool "arm1136jf_s rev1" > +config BR2_arm1136jf_s > + bool "arm1136jf-s" > select BR2_ARM_CPU_HAS_VFPV2 > select BR2_ARM_CPU_HAS_THUMB > config BR2_arm1176jz_s > @@ -113,10 +109,6 @@ config BR2_iwmmxt > bool "iwmmxt" > endchoice > > -config BR2_arm1136jf_s > - bool > - default BR2_arm1136jf_s_r0 || BR2_arm1136jf_s_r1 > - > choice > prompt "Target ABI" > depends on BR2_arm || BR2_armeb > @@ -364,8 +356,7 @@ config BR2_GCC_TARGET_ARCH > default "armv4t" if BR2_arm920t > default "armv4t" if BR2_arm922t > default "armv5te" if BR2_arm926t > - default "armv6j" if BR2_arm1136jf_s_r0 > - default "armv6k" if BR2_arm1136jf_s_r1 > + default "armv6j" if BR2_arm1136jf_s > default "armv6zk" if BR2_arm1176jz_s > default "armv6zk" if BR2_arm1176jzf_s > default "armv7-a" if BR2_cortex_a5 > -- > 2.0.0 >
Dear Yann E. MORIN, On Mon, 15 Sep 2014 23:33:39 +0200, Yann E. MORIN wrote: > > Note that the removal of the two options should not break existing > > full .config, since the hidden option BR2_arm1136jf_s becomes again a > > visible option to select the CPU. > > But it would break a defconfig. Right. But I believe that when we change the default value of an option, we also break a defconfig, and we don't include that as part of the Config.in.legacy handling (but also because it's obviously more complicated). > As suggested on IRC, move the BR2_arm1136jf_s_r1 to Config.legacy, and > have it select BR2_arm1136jf_s, so the user can re-use a defconfig. Hum, why just BR2_arm1136jf_s_r1 and not BR2_arm1136jf_s_r0 ? Thanks for the review! Thomas
Dear Thomas Petazzoni, On Mon, Sep 15, 2014 at 11:39 PM, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote: > Dear Yann E. MORIN, > > On Mon, 15 Sep 2014 23:33:39 +0200, Yann E. MORIN wrote: > >> > Note that the removal of the two options should not break existing >> > full .config, since the hidden option BR2_arm1136jf_s becomes again a >> > visible option to select the CPU. >> >> But it would break a defconfig. > > Right. But I believe that when we change the default value of an > option, we also break a defconfig, and we don't include that as part of > the Config.in.legacy handling (but also because it's obviously more > complicated). > >> As suggested on IRC, move the BR2_arm1136jf_s_r1 to Config.legacy, and >> have it select BR2_arm1136jf_s, so the user can re-use a defconfig. > > Hum, why just BR2_arm1136jf_s_r1 and not BR2_arm1136jf_s_r0 ? This issue disappears if -march and -mtune are kept and -mcpu removed as I suggested. Regards, Benoît
Thomas, All, On 2014-09-15 23:39 +0200, Thomas Petazzoni spake thusly: > On Mon, 15 Sep 2014 23:33:39 +0200, Yann E. MORIN wrote: > > > Note that the removal of the two options should not break existing > > > full .config, since the hidden option BR2_arm1136jf_s becomes again a > > > visible option to select the CPU. > > > > But it would break a defconfig. > > Right. But I believe that when we change the default value of an > option, we also break a defconfig, and we don't include that as part of > the Config.in.legacy handling (but also because it's obviously more > complicated). > > > As suggested on IRC, move the BR2_arm1136jf_s_r1 to Config.legacy, and > > have it select BR2_arm1136jf_s, so the user can re-use a defconfig. > > Hum, why just BR2_arm1136jf_s_r1 and not BR2_arm1136jf_s_r0 ? Yes, of course. I re-did my mental flow, and not having _r0 in legacy does not fly. We need both in legacy, indeed. Regards, Yann E. MORIN.
Dear Benoît Thébaudeau, On Mon, 15 Sep 2014 23:44:52 +0200, Benoît Thébaudeau wrote: > >> As suggested on IRC, move the BR2_arm1136jf_s_r1 to Config.legacy, and > >> have it select BR2_arm1136jf_s, so the user can re-use a defconfig. > > > > Hum, why just BR2_arm1136jf_s_r1 and not BR2_arm1136jf_s_r0 ? > > This issue disappears if -march and -mtune are kept and -mcpu removed > as I suggested. Do you have an example C code that doesn't generate the same assembly code between BR2_arm1136jf_s_r0 and BR2_arm1136jf_s_r1 ? It would be useful for my testing. Thanks! Thomas
Benoît, Thomas, All, On 2014-09-15 23:44 +0200, Benoît Thébaudeau spake thusly: > On Mon, Sep 15, 2014 at 11:39 PM, Thomas Petazzoni > <thomas.petazzoni@free-electrons.com> wrote: > > Dear Yann E. MORIN, > > > > On Mon, 15 Sep 2014 23:33:39 +0200, Yann E. MORIN wrote: > > > >> > Note that the removal of the two options should not break existing > >> > full .config, since the hidden option BR2_arm1136jf_s becomes again a > >> > visible option to select the CPU. > >> > >> But it would break a defconfig. > > > > Right. But I believe that when we change the default value of an > > option, we also break a defconfig, and we don't include that as part of > > the Config.in.legacy handling (but also because it's obviously more > > complicated). > > > >> As suggested on IRC, move the BR2_arm1136jf_s_r1 to Config.legacy, and > >> have it select BR2_arm1136jf_s, so the user can re-use a defconfig. > > > > Hum, why just BR2_arm1136jf_s_r1 and not BR2_arm1136jf_s_r0 ? > > This issue disappears if -march and -mtune are kept and -mcpu removed > as I suggested. So, maybe we need something like this: https://gist.github.com/aeruder/341565b759822b352dd9 Regards, Yann E. MORIN.
Dear Thomas Petazzoni, On Mon, Sep 15, 2014 at 11:50 PM, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote: > Dear Benoît Thébaudeau, > > On Mon, 15 Sep 2014 23:44:52 +0200, Benoît Thébaudeau wrote: > >> >> As suggested on IRC, move the BR2_arm1136jf_s_r1 to Config.legacy, and >> >> have it select BR2_arm1136jf_s, so the user can re-use a defconfig. >> > >> > Hum, why just BR2_arm1136jf_s_r1 and not BR2_arm1136jf_s_r0 ? >> >> This issue disappears if -march and -mtune are kept and -mcpu removed >> as I suggested. > > Do you have an example C code that doesn't generate the same assembly > code between BR2_arm1136jf_s_r0 and BR2_arm1136jf_s_r1 ? It would be > useful for my testing. The instructions supported by armv6k but not by armv6j are: ldrex{h|b|d}, strex{h|b|d}, clrex, nop, sev, wfe, wfi and yield. This is especially useful for handwritten assembler implementations (in .S, or inline in .c) of some OS functions, typically atomic operations and context switches. Benoît
Dear Yann E. MORIN, On Tue, Sep 16, 2014 at 12:02 AM, Yann E. MORIN <yann.morin.1998@free.fr> wrote: > Benoît, Thomas, All, > > On 2014-09-15 23:44 +0200, Benoît Thébaudeau spake thusly: >> On Mon, Sep 15, 2014 at 11:39 PM, Thomas Petazzoni >> <thomas.petazzoni@free-electrons.com> wrote: >> > Dear Yann E. MORIN, >> > >> > On Mon, 15 Sep 2014 23:33:39 +0200, Yann E. MORIN wrote: >> > >> >> > Note that the removal of the two options should not break existing >> >> > full .config, since the hidden option BR2_arm1136jf_s becomes again a >> >> > visible option to select the CPU. >> >> >> >> But it would break a defconfig. >> > >> > Right. But I believe that when we change the default value of an >> > option, we also break a defconfig, and we don't include that as part of >> > the Config.in.legacy handling (but also because it's obviously more >> > complicated). >> > >> >> As suggested on IRC, move the BR2_arm1136jf_s_r1 to Config.legacy, and >> >> have it select BR2_arm1136jf_s, so the user can re-use a defconfig. >> > >> > Hum, why just BR2_arm1136jf_s_r1 and not BR2_arm1136jf_s_r0 ? >> >> This issue disappears if -march and -mtune are kept and -mcpu removed >> as I suggested. > > So, maybe we need something like this: > > https://gist.github.com/aeruder/341565b759822b352dd9 Exactly. Regards, Benoît
diff --git a/arch/Config.in.arm b/arch/Config.in.arm index 201ff1d..6e3c4e6 100644 --- a/arch/Config.in.arm +++ b/arch/Config.in.arm @@ -54,12 +54,8 @@ config BR2_arm926t bool "arm926t" select BR2_ARM_CPU_MAYBE_HAS_VFPV2 select BR2_ARM_CPU_HAS_THUMB -config BR2_arm1136jf_s_r0 - bool "arm1136jf_s rev0" - select BR2_ARM_CPU_HAS_VFPV2 - select BR2_ARM_CPU_HAS_THUMB -config BR2_arm1136jf_s_r1 - bool "arm1136jf_s rev1" +config BR2_arm1136jf_s + bool "arm1136jf-s" select BR2_ARM_CPU_HAS_VFPV2 select BR2_ARM_CPU_HAS_THUMB config BR2_arm1176jz_s @@ -113,10 +109,6 @@ config BR2_iwmmxt bool "iwmmxt" endchoice -config BR2_arm1136jf_s - bool - default BR2_arm1136jf_s_r0 || BR2_arm1136jf_s_r1 - choice prompt "Target ABI" depends on BR2_arm || BR2_armeb @@ -364,8 +356,7 @@ config BR2_GCC_TARGET_ARCH default "armv4t" if BR2_arm920t default "armv4t" if BR2_arm922t default "armv5te" if BR2_arm926t - default "armv6j" if BR2_arm1136jf_s_r0 - default "armv6k" if BR2_arm1136jf_s_r1 + default "armv6j" if BR2_arm1136jf_s default "armv6zk" if BR2_arm1176jz_s default "armv6zk" if BR2_arm1176jzf_s default "armv7-a" if BR2_cortex_a5
In commit 88cf3bb91792c9c04586e14f293d89a6e0c13e1d ("arch/Config.in.arm: Use armv6k for arm1136jf-s rev1"), Benoît Thébaudeau added separate options for the revision 0 and revision 1 of the ARM1136JF-S processor, so that different -march values could be used (armv6j for revision 0, armv6k for revision 1). However, this is preventing the removal of the BR2_GCC_TARGET_ARCH option, which we need to do to give only the CPU type to gcc, and let it decide the architecture variant that matches. This is because this story of revision 0 vs. revision 1 is the only case where -mcpu doesn't fully define the CPU. Moreover, a quick test with gcc shows that -march=armv6j -mcpu=arm1136jf-s is accepted, while -march=armv6k -mcpu=arm1136jf-s makes gcc complain: " warning: switch -mcpu=arm1136jf-s conflicts with -march=armv6k switch". In addition, gcc 5 will apparently no longer allow to pass all of --with-arch, --with-cpu and --with-tune, so we will anyway have to rely only on one of them. As a consequence, this commit basically reverts 88cf3bb91792c9c04586e14f293d89a6e0c13e1d and provides only one option for ARM1136JF-S. If the two revisions are really different, then they should be supported in upstream gcc with different -mcpu values. Note that the removal of the two options should not break existing full .config, since the hidden option BR2_arm1136jf_s becomes again a visible option to select the CPU. Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> Cc: Benoît Thébaudeau <benoit.thebaudeau@advansee.com> --- arch/Config.in.arm | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-)