On Tue, Oct 23, 2018 at 3:44 PM Nathan Chancellor <natechancel...@gmail.com> wrote: > > On Tue, Oct 23, 2018 at 03:08:53PM -0700, Nick Desaulniers wrote: > > On Tue, Oct 23, 2018 at 2:58 PM Nathan Chancellor > > <natechancel...@gmail.com> wrote: > > > > > > On Tue, Oct 23, 2018 at 01:01:22PM -0700, H. Peter Anvin wrote: > > > > On 10/23/18 11:40, Nick Desaulniers wrote: > > > > > On Mon, Oct 22, 2018 at 10:11 PM Nadav Amit <na...@vmware.com> wrote: > > > > >> > > > > >> at 5:37 PM, Nathan Chancellor <natechancel...@gmail.com> wrote: > > > > >> > > > > >> Commit 77b0bf55bc67 ("kbuild/Makefile: Prepare for using macros in > > > > >> inline assembly code to work around asm() related GCC inlining bugs") > > > > >> added this flag to KBUILD_CFLAGS, where it works perfectly fine with > > > > >> GCC. However, when building with Clang, all of the object files > > > > >> compile > > > > >> fine but the build hangs indefinitely at init/main.o, right before > > > > >> the > > > > >> linking stage. Don't include this flag when building with Clang. > > > > >> > > > > >> The kernel builds and boots to a shell in QEMU with both GCC and > > > > >> Clang > > > > >> with this patch applied. > > > > >> > > > > >> Link: > > > > >> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FClangBuiltLinux%2Flinux%2Fissues%2F213&data=02%7C01%7Cnamit%40vmware.com%7C871daebc2ca44947d28d08d638811fb5%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C1%7C0%7C636758524579997650&sdata=shuxW81QRrO3TSqbgf462wgZYdLeAKeQEdGRxmnUX30%3D&reserved=0 > > > > >> Signed-off-by: Nathan Chancellor <natechancel...@gmail.com> > > > > >> --- > > > > >> > > > > >> The reason this patch is labeled RFC is while I can verify that this > > > > >> fixes the issue, I'm not entirely sure why the '-Wa,-' works for GCC > > > > >> and not Clang. I looked into what the flag means and I couldn't > > > > >> really > > > > >> find anything so I just assume it's taking input from stdin? The > > > > >> issue > > > > >> could stem from how GCC forks gas versus how Clang does it. If this > > > > >> isn't of concern and the maintainers are happy with this patch as is, > > > > >> feel free to take it. > > > > >> > > > > > > > > Perhaps someone could actually, you know, time the build and see how > > > > much -pipe actually matters, if at all? > > > > > > > > -hpa > > > > > > > > > > Thank you for the suggestion! With the attached diff for removing > > > '-pipe' and 'make -j1' with defconfig (just to make sure any variance > > > would stand out), here are my results: > > > > > > -pipe (GCC): > > > > > > real 15m55.202s > > > user 14m17.748s > > > sys 1m47.496s > > > > > > No -pipe (GCC): > > > > > > real 16m4.430s > > > user 14m16.277s > > > sys 1m46.604s > > > > > > -pipe (Clang): > > > > > > real 21m26.016s > > > user 19m21.722s > > > sys 2m2.606s > > > > > > No -pipe (Clang): > > > > > > real 21m27.822s > > > user 19m22.092s > > > sys 2m4.151s > > > > Looks like Clang eats `-pipe`: > > https://github.com/llvm-mirror/clang/blob/391667a023f79287f9c40868f34f08c161555556/lib/Driver/Driver.cpp#L962 > > commit r110007 has the log: > > Driver: Start ripping out support for -pipe, which is worthless > > and complicates > > too many other things. > > > > In that case, we can either keep this change (I'll resend with the > explanation that Clang doesn't respect -pipe) or we can just rip out > -pipe for GCC too. Here are three separate results for GCC with my > normal jobs flag: > > -pipe (GCC): > > real 3m40.813s > real 3m44.449s > real 3m39.648s > > No -pipe (GCC): > > real 3m38.492s > real 3m38.335s > real 3m38.975s > > Practically no variance.
Thanks for these measurements. With these in mind I agree with HPA that `-pipe -Wa,-` doesn't buy us anything, and would be simpler to remove it for compatibility with Clang. > > Thanks, > Nathan > > > > > > > Certainly seems like -pipe doesn't make a ton of difference. If this is > > > a better fix, I am happy to draft up a proper commit message and send > > > it out for review. > > > > > > ================================================== > > > > > > diff --git a/arch/x86/Makefile b/arch/x86/Makefile > > > index 73f4831283ac..672c689c1faa 100644 > > > --- a/arch/x86/Makefile > > > +++ b/arch/x86/Makefile > > > @@ -213,8 +213,6 @@ ifdef CONFIG_X86_64 > > > KBUILD_LDFLAGS += $(call ld-option, -z max-page-size=0x200000) > > > endif > > > > > > -# Speed up the build > > > -KBUILD_CFLAGS += -pipe > > > # Workaround for a gcc prelease that unfortunately was shipped in a suse > > > release > > > KBUILD_CFLAGS += -Wno-sign-compare > > > # > > > @@ -239,7 +237,7 @@ archheaders: > > > archmacros: > > > $(Q)$(MAKE) $(build)=arch/x86/kernel arch/x86/kernel/macros.s > > > > > > -ASM_MACRO_FLAGS = -Wa,arch/x86/kernel/macros.s -Wa,- > > > +ASM_MACRO_FLAGS = -Wa,arch/x86/kernel/macros.s > > > export ASM_MACRO_FLAGS > > > KBUILD_CFLAGS += $(ASM_MACRO_FLAGS) > > > > > > > > > -- > > Thanks, > > ~Nick Desaulniers -- Thanks, ~Nick Desaulniers