Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20181114204309.18645-1-namit@vmware.com> <20181114204309.18645-2-namit@vmware.com>
 <CAK7LNAREF_Pcejq1LJ=3K7h0M4o4oSRLfbeJc73O+8HV420Ziw@mail.gmail.com>
 <1C316B94-8F27-439C-B049-9947DC329780@vmware.com> <CAK7LNATFVHnPXh-uEA+T9QNw3OnSvkLLKMuJ_tF1v8bEveStSg@mail.gmail.com>
 <1F17E69E-06E8-4A5A-A434-1DCD6C6F0259@vmware.com>
In-Reply-To: <1F17E69E-06E8-4A5A-A434-1DCD6C6F0259@vmware.com>
From:   Masahiro Yamada <yamada.masahiro@socionext.com>
Date:   Wed, 21 Nov 2018 11:47:19 +0900
Message-ID: <CAK7LNATCmMTTEZUBhSVawVM_+r6KsuYnBCdxX_p3GqbKBH3DEw@mail.gmail.com>
Subject: Re: [PATCH v2 1/2] Makefile: Fix distcc compilation with x86 macros
To:     Nadav Amit <namit@vmware.com>
Cc:     Ingo Molnar <mingo@redhat.com>,
        Michal Marek <michal.lkml@markovi.net>,
        Thomas Gleixner <tglx@linutronix.de>,
        Borislav Petkov <bp@alien8.de>,
        "H. Peter Anvin" <hpa@zytor.com>, X86 ML <x86@kernel.org>,
        Linux Kbuild mailing list <linux-kbuild@vger.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Tue, Nov 20, 2018 at 2:21 AM Nadav Amit <namit@vmware.com> wrote:
>
> at 8:20 PM, Masahiro Yamada <yamada.masahiro@socionext.com> wrote:
>
> > On Sat, Nov 17, 2018 at 6:02 AM Nadav Amit <namit@vmware.com> wrote:
> >> From: Masahiro Yamada
> >> Sent: November 16, 2018 at 7:45:45 AM GMT
> >>> To: Nadav Amit <namit@vmware.com>
> >>> Cc: Ingo Molnar <mingo@redhat.com>, Michal Marek <michal.lkml@markovi=
.net>, Thomas Gleixner <tglx@linutronix.de>, Borislav Petkov <bp@alien8.de>=
, H. Peter Anvin <hpa@zytor.com>, X86 ML <x86@kernel.org>, Linux Kbuild mai=
ling list <linux-kbuild@vger.kernel.org>, Linux Kernel Mailing List <linux-=
kernel@vger.kernel.org>
> >>> 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@vmware.com> wrote:
> >>>> Introducing the use of asm macros in c-code broke distcc, since it o=
nly
> >>>> sends the preprocessed source file. The solution is to break the
> >>>> compilation into two separate phases of compilation and assembly, an=
d
> >>>> between the two concatenate the assembly macros and the compiled (ye=
t
> >>>> 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 i=
s
> >>>> configured using "CFLAGS=3D-DENABLE_REMOTE_ASSEMBLE".
> >>>>
> >>>> Reported-by: Logan Gunthorpe <logang@deltatee.com>
> >>>> Signed-off-by: Nadav Amit <namit@vmware.com>
> >>>
> >>>
> >>> 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 an=
d Asm
> >> and crazy C macros, for example for PV function call or the alternativ=
e
> >> 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 t=
o
> >> put it in, as it will indeed be (eventually) solved by the compiler, a=
nd
> >> 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 patch=
es
> is in unifying the assembly that is used in .c and .S files. Due to the l=
ack
> of m4-like preprocessor in the Linux make-system, the solution is a bit
> =E2=80=9Chacky=E2=80=9D (in other words, I don=E2=80=99t 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, whi=
ch
> 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 wil=
l
> be more painful.

Then, I'd be happier with the revert now.


--
Best Regards
Masahiro Yamada