| Message ID | alpine.DEB.2.21.2307180025120.62448@angie.orcam.me.uk (mailing list archive) |
|---|---|
| Headers | show |
| Series | MIPS: Fix build issues from the introduction of `need-compiler' | expand |
Hi, Maciej, Even if patch-2 resolves the problem, I don't think patch-3 is necessary because the original patch makes code simpler and more compact. Huacai On Tue, Jul 18, 2023 at 10:42 PM Maciej W. Rozycki <macro@orcam.me.uk> wrote: > > Hi, > > With the addition of the `need-compiler' variable the `Makefile.compiler' > fragment is not included with no-build targets such as `modules_install', > which in turn means $(call cc-option,), etc. are no-ops with these targets > and any attempt to evaluate these function calls causes all kinds of weird > behaviour to happen. > > The solution is to avoid making these calls in the first place, as they > are surely irrelevant where the compiler is not going to be otherwise > invoked. This small patch series fixes two places known-affected in the > MIPS Makefile fragment and also included a follow-up revert of an earlier > misguided attempt. See individual change descriptions for details. > > Verified with `decstation_64_defconfig' and `fuloong2e_defconfig' using > `modules_install'. Please apply. > > Maciej
On Tue, 18 Jul 2023, Huacai Chen wrote: > Even if patch-2 resolves the problem, I don't think patch-3 is > necessary because the original patch makes code simpler and more > compact. Please don't top-post on a public mailing list. If you're referring to this part: +ifdef CONFIG_CPU_LOONGSON64 +cflags-$(CONFIG_CPU_LOONGSON64) += -Wa,--trap +cflags-$(CONFIG_CC_IS_GCC) += -march=loongson3a +cflags-$(CONFIG_CC_IS_CLANG) += -march=mips64r2 +endif then it can be done with a separate clean-up. Otherwise it'll have been lost in the noise. Firstly: cflags-$(CONFIG_CPU_LOONGSON64) += -Wa,--trap doesn't have to be wrapped in `ifdef CONFIG_CPU_LOONGSON64'. Secondly: cflags-$(CONFIG_CC_IS_GCC) += -march=loongson3a cflags-$(CONFIG_CC_IS_CLANG) += -march=mips64r2 document compiler peculiarities. Does Clang support, or intend to, `-march=loongson3a'? If so, what version can we expect this stuff in? GCC has had it since 4.6 or Y2010, which is pretty long ago. Last but not least there are formatting anomalies there, which may have to be dealt with in a separate change. Maciej
Hi, Maciej, On Wed, Jul 19, 2023 at 11:39 PM Maciej W. Rozycki <macro@orcam.me.uk> wrote: > > On Tue, 18 Jul 2023, Huacai Chen wrote: > > > Even if patch-2 resolves the problem, I don't think patch-3 is > > necessary because the original patch makes code simpler and more > > compact. > > Please don't top-post on a public mailing list. > > If you're referring to this part: > > +ifdef CONFIG_CPU_LOONGSON64 > +cflags-$(CONFIG_CPU_LOONGSON64) += -Wa,--trap > +cflags-$(CONFIG_CC_IS_GCC) += -march=loongson3a > +cflags-$(CONFIG_CC_IS_CLANG) += -march=mips64r2 > +endif > > then it can be done with a separate clean-up. Otherwise it'll have been > lost in the noise. > > Firstly: > > cflags-$(CONFIG_CPU_LOONGSON64) += -Wa,--trap > > doesn't have to be wrapped in `ifdef CONFIG_CPU_LOONGSON64'. > > Secondly: > > cflags-$(CONFIG_CC_IS_GCC) += -march=loongson3a > cflags-$(CONFIG_CC_IS_CLANG) += -march=mips64r2 > > document compiler peculiarities. Does Clang support, or intend to, > `-march=loongson3a'? If so, what version can we expect this stuff in? > GCC has had it since 4.6 or Y2010, which is pretty long ago. GCC support loongson3a/mips64r2, Clang only support mips64r2. If we use $(call cc-option,-march=loongson3a,-march=mips64r2) both GCC and Clang can work and we don't need to care about the compiler. Huacai > > Last but not least there are formatting anomalies there, which may have > to be dealt with in a separate change. > > Maciej
Hi Huacai, > > Secondly: > > > > cflags-$(CONFIG_CC_IS_GCC) += -march=loongson3a > > cflags-$(CONFIG_CC_IS_CLANG) += -march=mips64r2 > > > > document compiler peculiarities. Does Clang support, or intend to, > > `-march=loongson3a'? If so, what version can we expect this stuff in? > > GCC has had it since 4.6 or Y2010, which is pretty long ago. > GCC support loongson3a/mips64r2, Clang only support mips64r2. If we use > $(call cc-option,-march=loongson3a,-march=mips64r2) > both GCC and Clang can work and we don't need to care about the compiler. This may well be a change we desire, but it has to be made and reviewed on its own rather than being buried within a set of unrelated changes. Then the rationale has to be given in the change description and a comment put in code explaining that it's not the usual case of old/new compiler, so that we can catch it later and remove should Clang developers decide to include `-march=loongson3a' support and our version requirements catch up. Maciej
On Tue, Jul 18, 2023 at 03:37:13PM +0100, Maciej W. Rozycki wrote: > Hi, > > With the addition of the `need-compiler' variable the `Makefile.compiler' > fragment is not included with no-build targets such as `modules_install', > which in turn means $(call cc-option,), etc. are no-ops with these targets > and any attempt to evaluate these function calls causes all kinds of weird > behaviour to happen. > > The solution is to avoid making these calls in the first place, as they > are surely irrelevant where the compiler is not going to be otherwise > invoked. This small patch series fixes two places known-affected in the > MIPS Makefile fragment and also included a follow-up revert of an earlier > misguided attempt. See individual change descriptions for details. > > Verified with `decstation_64_defconfig' and `fuloong2e_defconfig' using > `modules_install'. Please apply. series applied to mips-next. Thomas.