Re: [PATCH v2 1/2] Makefile: Fix distcc compilation with x86 macros

[Masahiro Yamada]

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