On Tue, Nov 20, 2018 at 2:21 AM Nadav Amit <namit@xxxxxxxxxx> wrote: > > at 8:20 PM, Masahiro Yamada <yamada.masahiro@xxxxxxxxxxxxx> wrote: > > > On Sat, Nov 17, 2018 at 6:02 AM Nadav Amit <namit@xxxxxxxxxx> wrote: > >> From: Masahiro Yamada > >> Sent: November 16, 2018 at 7:45:45 AM GMT > >>> To: Nadav Amit <namit@xxxxxxxxxx> > >>> Cc: Ingo Molnar <mingo@xxxxxxxxxx>, Michal Marek <michal.lkml@xxxxxxxxxxx>, Thomas Gleixner <tglx@xxxxxxxxxxxxx>, Borislav Petkov <bp@xxxxxxxxx>, H. Peter Anvin <hpa@xxxxxxxxx>, X86 ML <x86@xxxxxxxxxx>, Linux Kbuild mailing list <linux-kbuild@xxxxxxxxxxxxxxx>, Linux Kernel Mailing List <linux-kernel@xxxxxxxxxxxxxxx> > >>> Subject: Re: [PATCH v2 1/2] Makefile: Fix distcc compilation with x86 macros > >>> > >>> > >>> On Thu, Nov 15, 2018 at 1:01 PM Nadav Amit <namit@xxxxxxxxxx> wrote: > >>>> Introducing the use of asm macros in c-code broke distcc, since it only > >>>> sends the preprocessed source file. The solution is to break the > >>>> compilation into two separate phases of compilation and assembly, and > >>>> between the two concatenate the assembly macros and the compiled (yet > >>>> not assembled) source file. Since this is less efficient, this > >>>> compilation mode is only used when distcc or icecc are used. > >>>> > >>>> Note that the assembly stage should also be distributed, if distcc is > >>>> configured using "CFLAGS=-DENABLE_REMOTE_ASSEMBLE". > >>>> > >>>> Reported-by: Logan Gunthorpe <logang@xxxxxxxxxxxx> > >>>> Signed-off-by: Nadav Amit <namit@xxxxxxxxxx> > >>> > >>> > >>> Wow, this is so ugly. > >> > >> Indeed, it is not pretty from the Makefile system point of view. > >> > >> But it does have merits when it comes to the actual use (by developers). If > >> you look on x86, there are a lot of duplicated implementation for C and Asm > >> and crazy C macros, for example for PV function call or the alternative > >> mechanism. The code is also less readable in its current form. > >> > >>> I realized how much I hated this by now. > >>> > >>> My question is, how long do we need to carry this? > >> > >> If it is purely about performance, I am not sure it would make sense to > >> put it in, as it will indeed be (eventually) solved by the compiler, and > >> the penalty is not too great. > > > > > > It is unfortunate to mess up the code > > by having two different ways to achieve the same goal. > > Indeed, but I fail to see how this statement fits with the next sentence. I am not tracking the progress on GCC side. I just did not sure if the 'asm inline' work was on the right track. > > When GCC with asm inline support is shipped, > > would you revert 77b0bf55 ? > > This patch and the following ones were not written to be reverted, i.e., > reverting them later may not be easy since they remove redundant C inline > assembly chunks. > > Since gcc will solve the inline assembly issues, the value of these patches > is in unifying the assembly that is used in .c and .S files. Due to the lack > of m4-like preprocessor in the Linux make-system, the solution is a bit > “hacky” (in other words, I don’t see a reasonable alternative solution). > > So there is a downside in the form of performance degradation of distcc, and > hacky (not too hacky?) Makefile changes. On the upside, except addressing > the gcc statement cost computation (inline) issue, the patch removes > redundant code, and improves assembly code readability. It is true that the assembly part itself is readable but the readability as a whole is worse IMHO. Each macro implementation was split into "asm volatile" (in C) + ".macro" (in ASM) parts. > In addition, it > provides the option to make assembly manipulations after compilation, which > HPA and others (me) look into. > > Anyhow, if after all, the downside is considered greater than the upside, > it is best to remove the patches sooner than later, as a revert later will > be more painful. Then, I'd be happier with the revert now. -- Best Regards Masahiro Yamada
