| Message ID | 1396558881-29631-2-git-send-email-paul@crapouillou.net |
|---|---|
| State | Rejected |
| Headers | show |
Paul, All, On 2014-04-03 23:01 +0200, Paul Cercueil spake thusly: > This option is actually already used in GCC's package. > > This allows to optimize the toolchain for a specific MIPS processor > while supporting more than one family of processors. Is that really needed? man gcc says: When this option is not used, GCC optimizes for the processor specified by -march. Since this patch would pass the same value to --with-arch and --with-tune, and since this is the default of gcc, is it really needed? Neither ACKing nor NAKing this patch. Can you explain a bit more why we would want that, given the above explanations? Regards, Yann E. MORIN. > Signed-Off-By: Paul Cercueil <paul@crapouillou.net> > --- > arch/Config.in.mips | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/arch/Config.in.mips b/arch/Config.in.mips > index 20951e0..e4160a2 100644 > --- a/arch/Config.in.mips > +++ b/arch/Config.in.mips > @@ -83,6 +83,16 @@ config BR2_GCC_TARGET_ARCH > default "mips64" if BR2_mips_64 > default "mips64r2" if BR2_mips_64r2 > > +config BR2_GCC_TARGET_TUNE > + default "mips1" if BR2_mips_1 > + default "mips2" if BR2_mips_2 > + default "mips3" if BR2_mips_3 > + default "mips4" if BR2_mips_4 > + default "mips32" if BR2_mips_32 > + default "mips32r2" if BR2_mips_32r2 > + default "mips64" if BR2_mips_64 > + default "mips64r2" if BR2_mips_64r2 > + > config BR2_MIPS_OABI32 > bool > default y if BR2_mips || BR2_mipsel > -- > 1.9.0 > > _______________________________________________ > buildroot mailing list > buildroot@busybox.net > http://lists.busybox.net/mailman/listinfo/buildroot
Hi Yann, This patch would pass the same value to --with-arch and --with-tune, unless you define a different value for BR2_GCC_TARGET_TUNE in your defconfig. We use that for the Ingenic jz4740 processor, which is a mips32 processor but running better the code tuned for mips32r2. On 03/04/2014 23:19, Yann E. MORIN wrote: > Paul, All, > > On 2014-04-03 23:01 +0200, Paul Cercueil spake thusly: >> This option is actually already used in GCC's package. >> >> This allows to optimize the toolchain for a specific MIPS processor >> while supporting more than one family of processors. > Is that really needed? man gcc says: > > When this option is not used, GCC optimizes for the processor > specified by -march. > > Since this patch would pass the same value to --with-arch and > --with-tune, and since this is the default of gcc, is it really > needed? > > Neither ACKing nor NAKing this patch. Can you explain a bit more why we > would want that, given the above explanations? > > Regards, > Yann E. MORIN. > >> Signed-Off-By: Paul Cercueil <paul@crapouillou.net> >> --- >> arch/Config.in.mips | 10 ++++++++++ >> 1 file changed, 10 insertions(+) >> >> diff --git a/arch/Config.in.mips b/arch/Config.in.mips >> index 20951e0..e4160a2 100644 >> --- a/arch/Config.in.mips >> +++ b/arch/Config.in.mips >> @@ -83,6 +83,16 @@ config BR2_GCC_TARGET_ARCH >> default "mips64" if BR2_mips_64 >> default "mips64r2" if BR2_mips_64r2 >> >> +config BR2_GCC_TARGET_TUNE >> + default "mips1" if BR2_mips_1 >> + default "mips2" if BR2_mips_2 >> + default "mips3" if BR2_mips_3 >> + default "mips4" if BR2_mips_4 >> + default "mips32" if BR2_mips_32 >> + default "mips32r2" if BR2_mips_32r2 >> + default "mips64" if BR2_mips_64 >> + default "mips64r2" if BR2_mips_64r2 >> + >> config BR2_MIPS_OABI32 >> bool >> default y if BR2_mips || BR2_mipsel >> -- >> 1.9.0 >> >> _______________________________________________ >> buildroot mailing list >> buildroot@busybox.net >> http://lists.busybox.net/mailman/listinfo/buildroot
Paul, All, On 2014-04-03 23:50 +0200, Paul Cercueil spake thusly: > This patch would pass the same value to --with-arch and --with-tune, unless > you define a different value for BR2_GCC_TARGET_TUNE in your defconfig. I see what you expect, but since BR2_GCC_TARGET_TUNE is a prompt-less option, setting it in a defconfig will be overriden when you use that defconfig. For example: $ cat defconfig BR2_MIPS=y BR2_GCC_TARGET_ARCH="FOO-YEM" $ make BR2_DEFCONFIG=$(pwd)/defconfig defconfig [...] $ grep FOO .config [empty, nada, zilch] So, even if you set it in your defconfig, BR2_GCC_TARGET_ARCH is lost. Ditto for BR2_GCC_TARGET_TUNE. > We > use that for the Ingenic jz4740 processor, which is a mips32 processor but > running better the code tuned for mips32r2. Oh, I see. But that's not gonna happen with this code, I'm afraid. Can you double-check that it is indeed working for you? Because if it does, we have a really big bug in Kconfig (Oh, no, not one more...) Regards, Yann E. MORIN. > On 03/04/2014 23:19, Yann E. MORIN wrote: > >Paul, All, > > > >On 2014-04-03 23:01 +0200, Paul Cercueil spake thusly: > >>This option is actually already used in GCC's package. > >> > >>This allows to optimize the toolchain for a specific MIPS processor > >>while supporting more than one family of processors. > >Is that really needed? man gcc says: > > > > When this option is not used, GCC optimizes for the processor > > specified by -march. > > > >Since this patch would pass the same value to --with-arch and > >--with-tune, and since this is the default of gcc, is it really > >needed? > > > >Neither ACKing nor NAKing this patch. Can you explain a bit more why we > >would want that, given the above explanations? > > > >Regards, > >Yann E. MORIN. > > > >>Signed-Off-By: Paul Cercueil <paul@crapouillou.net> > >>--- > >> arch/Config.in.mips | 10 ++++++++++ > >> 1 file changed, 10 insertions(+) > >> > >>diff --git a/arch/Config.in.mips b/arch/Config.in.mips > >>index 20951e0..e4160a2 100644 > >>--- a/arch/Config.in.mips > >>+++ b/arch/Config.in.mips > >>@@ -83,6 +83,16 @@ config BR2_GCC_TARGET_ARCH > >> default "mips64" if BR2_mips_64 > >> default "mips64r2" if BR2_mips_64r2 > >>+config BR2_GCC_TARGET_TUNE > >>+ default "mips1" if BR2_mips_1 > >>+ default "mips2" if BR2_mips_2 > >>+ default "mips3" if BR2_mips_3 > >>+ default "mips4" if BR2_mips_4 > >>+ default "mips32" if BR2_mips_32 > >>+ default "mips32r2" if BR2_mips_32r2 > >>+ default "mips64" if BR2_mips_64 > >>+ default "mips64r2" if BR2_mips_64r2 > >>+ > >> config BR2_MIPS_OABI32 > >> bool > >> default y if BR2_mips || BR2_mipsel > >>-- > >>1.9.0 > >> > >>_______________________________________________ > >>buildroot mailing list > >>buildroot@busybox.net > >>http://lists.busybox.net/mailman/listinfo/buildroot >
Hi, On 04/04/2014 00:12, Yann E. MORIN wrote: > Paul, All, > > On 2014-04-03 23:50 +0200, Paul Cercueil spake thusly: >> This patch would pass the same value to --with-arch and --with-tune, unless >> you define a different value for BR2_GCC_TARGET_TUNE in your defconfig. > I see what you expect, but since BR2_GCC_TARGET_TUNE is a prompt-less > option, setting it in a defconfig will be overriden when you use that > defconfig. > > For example: > > $ cat defconfig > BR2_MIPS=y > BR2_GCC_TARGET_ARCH="FOO-YEM" > $ make BR2_DEFCONFIG=$(pwd)/defconfig defconfig > [...] > $ grep FOO .config > [empty, nada, zilch] > > So, even if you set it in your defconfig, BR2_GCC_TARGET_ARCH is lost. > > Ditto for BR2_GCC_TARGET_TUNE. > >> We >> use that for the Ingenic jz4740 processor, which is a mips32 processor but >> running better the code tuned for mips32r2. > Oh, I see. But that's not gonna happen with this code, I'm afraid. > > Can you double-check that it is indeed working for you? Because if it > does, we have a really big bug in Kconfig (Oh, no, not one more...) > > Regards, > Yann E. MORIN. I confirm it works. Why is it a bug? Paul > >> On 03/04/2014 23:19, Yann E. MORIN wrote: >>> Paul, All, >>> >>> On 2014-04-03 23:01 +0200, Paul Cercueil spake thusly: >>>> This option is actually already used in GCC's package. >>>> >>>> This allows to optimize the toolchain for a specific MIPS processor >>>> while supporting more than one family of processors. >>> Is that really needed? man gcc says: >>> >>> When this option is not used, GCC optimizes for the processor >>> specified by -march. >>> >>> Since this patch would pass the same value to --with-arch and >>> --with-tune, and since this is the default of gcc, is it really >>> needed? >>> >>> Neither ACKing nor NAKing this patch. Can you explain a bit more why we >>> would want that, given the above explanations? >>> >>> Regards, >>> Yann E. MORIN. >>> >>>> Signed-Off-By: Paul Cercueil <paul@crapouillou.net> >>>> --- >>>> arch/Config.in.mips | 10 ++++++++++ >>>> 1 file changed, 10 insertions(+) >>>> >>>> diff --git a/arch/Config.in.mips b/arch/Config.in.mips >>>> index 20951e0..e4160a2 100644 >>>> --- a/arch/Config.in.mips >>>> +++ b/arch/Config.in.mips >>>> @@ -83,6 +83,16 @@ config BR2_GCC_TARGET_ARCH >>>> default "mips64" if BR2_mips_64 >>>> default "mips64r2" if BR2_mips_64r2 >>>> +config BR2_GCC_TARGET_TUNE >>>> + default "mips1" if BR2_mips_1 >>>> + default "mips2" if BR2_mips_2 >>>> + default "mips3" if BR2_mips_3 >>>> + default "mips4" if BR2_mips_4 >>>> + default "mips32" if BR2_mips_32 >>>> + default "mips32r2" if BR2_mips_32r2 >>>> + default "mips64" if BR2_mips_64 >>>> + default "mips64r2" if BR2_mips_64r2 >>>> + >>>> config BR2_MIPS_OABI32 >>>> bool >>>> default y if BR2_mips || BR2_mipsel >>>> -- >>>> 1.9.0 >>>> >>>> _______________________________________________ >>>> buildroot mailing list >>>> buildroot@busybox.net >>>> http://lists.busybox.net/mailman/listinfo/buildroot
Paul, All, On 2014-04-05 17:53 +0200, Paul Cercueil spake thusly: > On 04/04/2014 00:12, Yann E. MORIN wrote: [--SNIP--] > >Can you double-check that it is indeed working for you? Because if it > >does, we have a really big bug in Kconfig (Oh, no, not one more...) > > I confirm it works. I checked your patch: $ cat defconfig BR2_mips=y BR2_GCC_TARGET_TUNE="mips32r2" $ make BR2_DEFCONFIG=$(pwd)/defconfig defconfig [...] $ grep BR2_GCC_TARGET_TUNE .config BR2_GCC_TARGET_TUNE="mips32" So no, it does not work as you explained you were expecting it to work: --- This patch would pass the same value to --with-arch and --with-tune, unless you define a different value for BR2_GCC_TARGET_TUNE in your defconfig. We use that for the Ingenic jz4740 processor, which is a mips32 processor but running better the code tuned for mips32r2. --- From what I understand, you have a defconfig file where you manually wrote: BR2_GCC_TARGET_TUNE="mips32r2" But as shown above, that does not work. > Why is it a bug? Because, for a prompt-less value, kconfig is expected to discard any value and only use the 'default' values. And from the above, it is what is hapenning: a value from a defconfig is properly over-written by what kconfig computes for this prompt-less value. So I stand on my position: I NAK this patch, because it does not fix the explained reason for it. Regards, Yann E. MORIN.
On 04/03/2014 10:50 PM, Paul Cercueil wrote: > Hi Yann, > > This patch would pass the same value to --with-arch and --with-tune, > unless you define a different value for BR2_GCC_TARGET_TUNE in your > defconfig. We use that for the Ingenic jz4740 processor, which is a > mips32 processor but running better the code tuned for mips32r2. > Hi Paul, I don't understand this patch as well. If the jz4740 needs a mips32r2 userland, why not use Target Options-> Target Architecture Variant -> mips32r2? You want to use the same toolchain for non-mips32r2 targets as well? The TARGET_TUNE option has been removed for MIPS in f60dafe06833a17540608d1c8172d6535c513f1e "arch/mips: Set BR2_GCC_TARGET_ARCH for MIPS" and i see no good reason to bring it back. If you need to optimize for a different ISA, wouldn't it be possible to use custom CFLAGS in Toolchain->Target Optimizations?
diff --git a/arch/Config.in.mips b/arch/Config.in.mips index 20951e0..e4160a2 100644 --- a/arch/Config.in.mips +++ b/arch/Config.in.mips @@ -83,6 +83,16 @@ config BR2_GCC_TARGET_ARCH default "mips64" if BR2_mips_64 default "mips64r2" if BR2_mips_64r2 +config BR2_GCC_TARGET_TUNE + default "mips1" if BR2_mips_1 + default "mips2" if BR2_mips_2 + default "mips3" if BR2_mips_3 + default "mips4" if BR2_mips_4 + default "mips32" if BR2_mips_32 + default "mips32r2" if BR2_mips_32r2 + default "mips64" if BR2_mips_64 + default "mips64r2" if BR2_mips_64r2 + config BR2_MIPS_OABI32 bool default y if BR2_mips || BR2_mipsel
This option is actually already used in GCC's package. This allows to optimize the toolchain for a specific MIPS processor while supporting more than one family of processors. Signed-Off-By: Paul Cercueil <paul@crapouillou.net> --- arch/Config.in.mips | 10 ++++++++++ 1 file changed, 10 insertions(+)