at 8:20 PM, Masahiro Yamada <[email protected]> wrote: > On Sat, Nov 17, 2018 at 6:02 AM Nadav Amit <[email protected]> wrote: >> From: Masahiro Yamada >> Sent: November 16, 2018 at 7:45:45 AM GMT >>> To: Nadav Amit <[email protected]> >>> Cc: Ingo Molnar <[email protected]>, Michal Marek <[email protected]>, >>> Thomas Gleixner <[email protected]>, Borislav Petkov <[email protected]>, H. >>> Peter Anvin <[email protected]>, X86 ML <[email protected]>, Linux Kbuild >>> mailing list <[email protected]>, Linux Kernel Mailing List >>> <[email protected]> >>> Subject: Re: [PATCH v2 1/2] Makefile: Fix distcc compilation with x86 macros >>> >>> >>> On Thu, Nov 15, 2018 at 1:01 PM Nadav Amit <[email protected]> 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 <[email protected]> >>>> Signed-off-by: Nadav Amit <[email protected]> >>> >>> >>> 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. > 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. 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. Regards, Nadav
