Re: [PATCH 2/7] Makefile, x86, LLVM: disable unsupported optimization flags
Hi. 2017-04-06 4:11 GMT+09:00 Michael Davidson : > It "works" for the cases that I currently care about but I have to say > that I am uneasy about adding -Werror to the cc-option test in this > way. > > Suppose that one of the *other* flags that is implicitly passed to the > compiler by cc-option - eg something that was explicitly specified in > $(KBUILD_CFLAGS) - triggers a warning. In that case all calls to > cc-option will silently fail because of the -Werror and valid options > will not be detected correctly. Theoretically, options explicitly specified in KBUILD_CFLAGS should be always valid. Options that may not be supported in some cases should be wrapped with $(call cc-option ). > If everyone is OK with that because "it shouldn't normally ever > happen" then that is fine, but if does result in a subtle change from > existing behavior (and a trap that I almost immediately fell into > after applying a similar patch). There is a rare case where a particular combination fails (such as the conflict between -pg and -ffunction-sections as reported in https://patchwork.kernel.org/patch/9624573/). In a such case, we may end up with swapping the order, but this should not happen quite often. > On Wed, Apr 5, 2017 at 12:01 PM, Matthias Kaehlcke wrote: >> Hi Masahiro, >> >> El Thu, Apr 06, 2017 at 03:08:26AM +0900 Masahiro Yamada ha dit: >> >>> 2017-03-17 9:15 GMT+09:00 Michael Davidson : >>> > Unfortunately, while clang generates a warning about these flags >>> > being unsupported it still exits with a status of 0 so we have >>> > to explicitly disable them instead of just using a cc-option check. >>> > >>> > Signed-off-by: Michael Davidson >>> >>> >>> Instead, does the following work for you? >>> https://patchwork.kernel.org/patch/9657285/ >> >> Thanks for the pointer, I was about to give this change (or rather its >> ancestor) a rework myself :) >> >>> You need to use >>> $(call cc-option, ...) >>> for -falign-jumps=1 and -falign-loops=1 >> >> I can confirm that this works. >> >> Thanks >> >> Matthias > -- > To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Best Regards Masahiro Yamada
Re: [PATCH 2/7] Makefile, x86, LLVM: disable unsupported optimization flags
It "works" for the cases that I currently care about but I have to say that I am uneasy about adding -Werror to the cc-option test in this way. Suppose that one of the *other* flags that is implicitly passed to the compiler by cc-option - eg something that was explicitly specified in $(KBUILD_CFLAGS) - triggers a warning. In that case all calls to cc-option will silently fail because of the -Werror and valid options will not be detected correctly. If everyone is OK with that because "it shouldn't normally ever happen" then that is fine, but if does result in a subtle change from existing behavior (and a trap that I almost immediately fell into after applying a similar patch). On Wed, Apr 5, 2017 at 12:01 PM, Matthias Kaehlcke wrote: > Hi Masahiro, > > El Thu, Apr 06, 2017 at 03:08:26AM +0900 Masahiro Yamada ha dit: > >> 2017-03-17 9:15 GMT+09:00 Michael Davidson : >> > Unfortunately, while clang generates a warning about these flags >> > being unsupported it still exits with a status of 0 so we have >> > to explicitly disable them instead of just using a cc-option check. >> > >> > Signed-off-by: Michael Davidson >> >> >> Instead, does the following work for you? >> https://patchwork.kernel.org/patch/9657285/ > > Thanks for the pointer, I was about to give this change (or rather its > ancestor) a rework myself :) > >> You need to use >> $(call cc-option, ...) >> for -falign-jumps=1 and -falign-loops=1 > > I can confirm that this works. > > Thanks > > Matthias
Re: [PATCH 2/7] Makefile, x86, LLVM: disable unsupported optimization flags
Hi Masahiro, El Thu, Apr 06, 2017 at 03:08:26AM +0900 Masahiro Yamada ha dit: > 2017-03-17 9:15 GMT+09:00 Michael Davidson : > > Unfortunately, while clang generates a warning about these flags > > being unsupported it still exits with a status of 0 so we have > > to explicitly disable them instead of just using a cc-option check. > > > > Signed-off-by: Michael Davidson > > > Instead, does the following work for you? > https://patchwork.kernel.org/patch/9657285/ Thanks for the pointer, I was about to give this change (or rather its ancestor) a rework myself :) > You need to use > $(call cc-option, ...) > for -falign-jumps=1 and -falign-loops=1 I can confirm that this works. Thanks Matthias
Re: [PATCH 2/7] Makefile, x86, LLVM: disable unsupported optimization flags
Hi Michael, 2017-03-17 9:15 GMT+09:00 Michael Davidson : > Unfortunately, while clang generates a warning about these flags > being unsupported it still exits with a status of 0 so we have > to explicitly disable them instead of just using a cc-option check. > > Signed-off-by: Michael Davidson Instead, does the following work for you? https://patchwork.kernel.org/patch/9657285/ You need to use $(call cc-option, ...) for -falign-jumps=1 and -falign-loops=1 -- Best Regards Masahiro Yamada
Re: [PATCH 2/7] Makefile, x86, LLVM: disable unsupported optimization flags
On 03/17/17 14:32, H. Peter Anvin wrote: > > NAK. Fix your compiler, or use a wrapper script or something. It is > absolutely *not* acceptable to disable this since future versions of > clang *should* support that. > > That being said, it might make sense to look for a key pattern like > "(un|not )supported" on stderr the try-run macro. Is there really no > -Wno- or -Werror= option to turn off this craziness? > Well, guess what... I found it myself. -W{no-,error=}ignored-optimization-argument Either variant will make this sane. -hpa
Re: [PATCH 2/7] Makefile, x86, LLVM: disable unsupported optimization flags
On 03/16/17 17:15, Michael Davidson wrote: > Unfortunately, while clang generates a warning about these flags > being unsupported it still exits with a status of 0 so we have > to explicitly disable them instead of just using a cc-option check. > > Signed-off-by: Michael Davidson > --- > Makefile | 2 ++ > arch/x86/Makefile | 2 ++ > 2 files changed, 4 insertions(+) > > diff --git a/Makefile b/Makefile > index b21fd0ca2946..5e97e5fc1eea 100644 > --- a/Makefile > +++ b/Makefile > @@ -629,7 +629,9 @@ ARCH_AFLAGS := > ARCH_CFLAGS := > include arch/$(SRCARCH)/Makefile > > +ifneq ($(cc-name),clang) > KBUILD_CFLAGS+= $(call cc-option,-fno-delete-null-pointer-checks,) > +endif > KBUILD_CFLAGS+= $(call cc-disable-warning,frame-address,) > > ifdef CONFIG_LD_DEAD_CODE_DATA_ELIMINATION > diff --git a/arch/x86/Makefile b/arch/x86/Makefile > index 2d449337a360..894a8d18bf97 100644 > --- a/arch/x86/Makefile > +++ b/arch/x86/Makefile > @@ -87,11 +87,13 @@ else > KBUILD_AFLAGS += -m64 > KBUILD_CFLAGS += -m64 > > +ifneq ($(cc-name),clang) > # Align jump targets to 1 byte, not the default 16 bytes: > KBUILD_CFLAGS += -falign-jumps=1 > > # Pack loops tightly as well: > KBUILD_CFLAGS += -falign-loops=1 > +endif > > # Don't autogenerate traditional x87 instructions > KBUILD_CFLAGS += $(call cc-option,-mno-80387) > NAK. Fix your compiler, or use a wrapper script or something. It is absolutely *not* acceptable to disable this since future versions of clang *should* support that. That being said, it might make sense to look for a key pattern like "(un|not )supported" on stderr the try-run macro. Is there really no -Wno- or -Werror= option to turn off this craziness? -hpa