Re: [RFC PATCH] kbuild: support llvm-ar
Great to hear! If you discover anything later, let me know. On Tue, Feb 19, 2019 at 10:11 PM Masahiro Yamada wrote: > > Hi Jordan, > > > On Thu, Feb 7, 2019 at 9:56 AM Jordan Rupprecht wrote: > > > > I have a patch up for review that fixes the second case of chopping > > off the directory when nesting thin archives: > > https://reviews.llvm.org/D57842. I'll commit it tomorrow if there are > > no more comments. > > > Sorry for late reply. > > Now I tested the latest llvm-ar, and it worked perfectly for kbuild. > Thanks for your work! > > > -- > Best Regards > Masahiro Yamada smime.p7s Description: S/MIME Cryptographic Signature
Re: [RFC PATCH] kbuild: support llvm-ar
Hi Jordan, On Thu, Feb 7, 2019 at 9:56 AM Jordan Rupprecht wrote: > > I have a patch up for review that fixes the second case of chopping > off the directory when nesting thin archives: > https://reviews.llvm.org/D57842. I'll commit it tomorrow if there are > no more comments. Sorry for late reply. Now I tested the latest llvm-ar, and it worked perfectly for kbuild. Thanks for your work! -- Best Regards Masahiro Yamada
Re: [RFC PATCH] kbuild: support llvm-ar
I have a patch up for review that fixes the second case of chopping
off the directory when nesting thin archives:
https://reviews.llvm.org/D57842. I'll commit it tomorrow if there are
no more comments.
I was looking at the first case of supporting the P modifier, and
found that, as implemented (with D57842 patched in), llvm-ar now seems
to work as if P is the default in some cases, e.g. with your initial
test case:
$ gcc -c -x c /dev/null -o foo.o
$ mkdir a
$ gcc -c -x c /dev/null -o a/foo.o
$ ar rcST a/built-in.a a/foo.o
$ rm -f built-in.a && ar rcST built-in.a a/built-in.a foo.o && cat built-in.a
!
// 8 `
foo.o/
/0 0 0 0 644 920 `
$ rm -f built-in.a && llvm-ar rcST built-in.a a/built-in.a foo.o &&
cat built-in.a
!
// 16`
a/foo.o/
foo.o/
/0 0 0 0 644 920 `
/9 0 0 0 644 920 `
It didn't work like that universally -- deleting archive members
doesn't work as expected -- so I'll have to do some more
investigation.
Anyway, once D57842 is committed (or if you want to patch it locally),
you *should* be able to reduce the llvm-ar special fix to just not
pass the P modifier, until we accept it.
On Thu, Jan 17, 2019 at 1:55 PM Nick Desaulniers
wrote:
>
> On Wed, Jan 16, 2019 at 10:34 PM Masahiro Yamada
> wrote:
> >
> > I want to avoid applying this patch because this patch is ugly, and
> > people are trying to fix llvm-ar. I tried the latest llvm-ar, and I
> > saw some improvement, but still cannot build the kernel with it.
> >
> > The main reason of this post is for the record, and suggest how llvm-ar
> > should work.
> >
> > As you may have noticed, there are two issues:
> >
> > [1] llvm-ar does not recognize the 'P' option
> > [2] llvm-ar does not flatten thin archives (not fixed yet)
> >
> > Let me explain each of them.
> >
> > [1] Why is 'P' option needed for GNU ar?
> >
> > It may not be so clear the reason of commit 9a6cfca4f413 ("kbuild:
> > thin archives use P option to ar").
> >
> > If two objects from different directories have the same file name,
> > GNU ar may drop the one when flattening thin archives.
> >
> > Here is a simple test case:
> >
> > $ gcc -c -x c /dev/null -o foo.o
> > $ mkdir a
> > $ gcc -c -x c /dev/null -o a/foo.o
> > $ ar rcST a/built-in.a a/foo.o
> > $ ar rcST built-in.a a/built-in.a foo.o
> > $ cat built-in.a
> > !
> > // 8 `
> > foo.o/
> >
> > /0 0 0 0 644 944 `
> >
> > We expect built-in.a contains both a/foo.o and foo.o, but the former
> > is lost. The 'P' option solves this.
> >
> > $ rm -f built-in.a
> > $ ar rcSTP built-in.a a/built-in.a foo.o
> > $ cat built-in.a
> > !
> > // 16`
> > a/foo.o/
> > foo.o/
> > /0 0 0 0 644 944 `
> > /9 0 0 0 644 944 `
> >
> > Interestingly, GNU ar works as expected without 'P' when the order of
> > a/built-in.a and foo.o are opposite:
> >
> > $ rm built-in.a
> > $ ar rcST built-in.a foo.o a/built-in.a
> > $ cat built-in.a
> > !
> > // 16`
> > foo.o/
> > a/foo.o/
> > /0 0 0 0 644 944 `
> > /7 0 0 0 644 944 `
> >
> > Anyway, even the latest GNU toolchain works like that, so
> > Kbuild does need the 'P' option.
> >
> > The real world example is sh.
> >
> > arch/sh/kernel/cpu/fpu.c
> > arch/sh/kernel/cpu/sh2a/fpu.c
> > arch/sh/kernel/cpu/sh4/fpu.c
> > arch/sh/kernel/cpu/sh5/fpu.c
> >
> > [2] flattening thin archives
> >
> > llvm-ar cannot flatten archives.
> >
> > This issue was filed in the following:
> > https://bugs.llvm.org/show_bug.cgi?id=37530
> >
> > Its status was marked as "RESOLVED FIXED", but actually not fixed
> > as of writing (Jan 17, 2019).
> >
> > The older version of llvm-ar worked like this:
> >
> > $ rm -f built-in.a a/built-in.a
> > $ llvm-ar rcST a/built-in.a a/foo.o
> > $ llvm-ar rcST built-in.a a/built-in.a
> > $ cat built-in.a !
> > // 14`
> > a/built-in.a/
> > /0 0 0 0 644 136 `
> >
> > Recently, two commits were pushed to the trunk.
> > [llvm-ar] Flatten thin archives.
> > [llvm-ar] Resubmit recursive thin archive test with fix for full path
> > names and better error messages
> >
> > As far as I tested, the latest llvm-ar works as follows:
> >
> > $ llvm-ar rcST a/built-in.a a/foo.o
> > $ llvm-ar rcST built-in.a a/built-in.a
> > $ cat built-in.a
> > !
> > // 8 `
> > foo.o/
> >
> >
Re: [RFC PATCH] kbuild: support llvm-ar
On Wed, Jan 16, 2019 at 10:34 PM Masahiro Yamada
wrote:
>
> I want to avoid applying this patch because this patch is ugly, and
> people are trying to fix llvm-ar. I tried the latest llvm-ar, and I
> saw some improvement, but still cannot build the kernel with it.
>
> The main reason of this post is for the record, and suggest how llvm-ar
> should work.
>
> As you may have noticed, there are two issues:
>
> [1] llvm-ar does not recognize the 'P' option
> [2] llvm-ar does not flatten thin archives (not fixed yet)
>
> Let me explain each of them.
>
> [1] Why is 'P' option needed for GNU ar?
>
> It may not be so clear the reason of commit 9a6cfca4f413 ("kbuild:
> thin archives use P option to ar").
>
> If two objects from different directories have the same file name,
> GNU ar may drop the one when flattening thin archives.
>
> Here is a simple test case:
>
> $ gcc -c -x c /dev/null -o foo.o
> $ mkdir a
> $ gcc -c -x c /dev/null -o a/foo.o
> $ ar rcST a/built-in.a a/foo.o
> $ ar rcST built-in.a a/built-in.a foo.o
> $ cat built-in.a
> !
> // 8 `
> foo.o/
>
> /0 0 0 0 644 944 `
>
> We expect built-in.a contains both a/foo.o and foo.o, but the former
> is lost. The 'P' option solves this.
>
> $ rm -f built-in.a
> $ ar rcSTP built-in.a a/built-in.a foo.o
> $ cat built-in.a
> !
> // 16`
> a/foo.o/
> foo.o/
> /0 0 0 0 644 944 `
> /9 0 0 0 644 944 `
>
> Interestingly, GNU ar works as expected without 'P' when the order of
> a/built-in.a and foo.o are opposite:
>
> $ rm built-in.a
> $ ar rcST built-in.a foo.o a/built-in.a
> $ cat built-in.a
> !
> // 16`
> foo.o/
> a/foo.o/
> /0 0 0 0 644 944 `
> /7 0 0 0 644 944 `
>
> Anyway, even the latest GNU toolchain works like that, so
> Kbuild does need the 'P' option.
>
> The real world example is sh.
>
> arch/sh/kernel/cpu/fpu.c
> arch/sh/kernel/cpu/sh2a/fpu.c
> arch/sh/kernel/cpu/sh4/fpu.c
> arch/sh/kernel/cpu/sh5/fpu.c
>
> [2] flattening thin archives
>
> llvm-ar cannot flatten archives.
>
> This issue was filed in the following:
> https://bugs.llvm.org/show_bug.cgi?id=37530
>
> Its status was marked as "RESOLVED FIXED", but actually not fixed
> as of writing (Jan 17, 2019).
>
> The older version of llvm-ar worked like this:
>
> $ rm -f built-in.a a/built-in.a
> $ llvm-ar rcST a/built-in.a a/foo.o
> $ llvm-ar rcST built-in.a a/built-in.a
> $ cat built-in.a !
> // 14`
> a/built-in.a/
> /0 0 0 0 644 136 `
>
> Recently, two commits were pushed to the trunk.
> [llvm-ar] Flatten thin archives.
> [llvm-ar] Resubmit recursive thin archive test with fix for full path names
> and better error messages
>
> As far as I tested, the latest llvm-ar works as follows:
>
> $ llvm-ar rcST a/built-in.a a/foo.o
> $ llvm-ar rcST built-in.a a/built-in.a
> $ cat built-in.a
> !
> // 8 `
> foo.o/
>
> /0 0 0 0 644 800 `
>
> OK, it flattens the thin archive, but the problem is it rips off
> the directory path.
>
> GNU ar works as follows:
>
> $ ar rcST a/built-in.a a/foo.o
> $ ar rcSTbuilt-in.a a/built-in.a
> $ cat built-in.a
> !
> // 10`
> a/foo.o/
>
> /0 0 0 0 644 800 `
>
> If you use the latest llvm-ar for building the kernel, you will see
> a mysterious 'llvm-ar: error: No such file or directory.' error.
> (I think the error message could be improved because it is unclear
> what is missing.)
>
> [Workaround in this patch]
>
> Currently, llvm-ar cannot flatten thin archives correctly. So, Kbuild
> uses '$(AR) t' to expand thin archives from subdirectories, then repack
> all objects into the current directory built-in.a.
>
> The 'P' for cmd_link_l_target is unneeded because Kbuild does not
> support subdir objects for lib-y in the first place.
>
> [How to use]
>
> Pass AR=llvm-ar in the configuration and build command:
>
> $ make AR=llvm-ar defconfig
> $ make AR=llvm-ar
>
> Or, simply in one line:
>
> $ make AR=llvm-ar defconfig all
Thank you Masahiro for the excellent feedback on what's missing with
llvm-ar. I will work with Jordan (cc'ed) to sort out these remaining
issues. I think we should do our best to fix these in llvm-ar to the
extent possible, before modifying Kbuild to support it.
>
> Signed-off-by: Masahiro Yamada
For the above reason;
Nacked-by: Nick Desaulniers
to prevent this from going in before we've had time to
[RFC PATCH] kbuild: support llvm-ar
I want to avoid applying this patch because this patch is ugly, and
people are trying to fix llvm-ar. I tried the latest llvm-ar, and I
saw some improvement, but still cannot build the kernel with it.
The main reason of this post is for the record, and suggest how llvm-ar
should work.
As you may have noticed, there are two issues:
[1] llvm-ar does not recognize the 'P' option
[2] llvm-ar does not flatten thin archives (not fixed yet)
Let me explain each of them.
[1] Why is 'P' option needed for GNU ar?
It may not be so clear the reason of commit 9a6cfca4f413 ("kbuild:
thin archives use P option to ar").
If two objects from different directories have the same file name,
GNU ar may drop the one when flattening thin archives.
Here is a simple test case:
$ gcc -c -x c /dev/null -o foo.o
$ mkdir a
$ gcc -c -x c /dev/null -o a/foo.o
$ ar rcST a/built-in.a a/foo.o
$ ar rcST built-in.a a/built-in.a foo.o
$ cat built-in.a
!
// 8 `
foo.o/
/0 0 0 0 644 944 `
We expect built-in.a contains both a/foo.o and foo.o, but the former
is lost. The 'P' option solves this.
$ rm -f built-in.a
$ ar rcSTP built-in.a a/built-in.a foo.o
$ cat built-in.a
!
// 16`
a/foo.o/
foo.o/
/0 0 0 0 644 944 `
/9 0 0 0 644 944 `
Interestingly, GNU ar works as expected without 'P' when the order of
a/built-in.a and foo.o are opposite:
$ rm built-in.a
$ ar rcST built-in.a foo.o a/built-in.a
$ cat built-in.a
!
// 16`
foo.o/
a/foo.o/
/0 0 0 0 644 944 `
/7 0 0 0 644 944 `
Anyway, even the latest GNU toolchain works like that, so
Kbuild does need the 'P' option.
The real world example is sh.
arch/sh/kernel/cpu/fpu.c
arch/sh/kernel/cpu/sh2a/fpu.c
arch/sh/kernel/cpu/sh4/fpu.c
arch/sh/kernel/cpu/sh5/fpu.c
[2] flattening thin archives
llvm-ar cannot flatten archives.
This issue was filed in the following:
https://bugs.llvm.org/show_bug.cgi?id=37530
Its status was marked as "RESOLVED FIXED", but actually not fixed
as of writing (Jan 17, 2019).
The older version of llvm-ar worked like this:
$ rm -f built-in.a a/built-in.a
$ llvm-ar rcST a/built-in.a a/foo.o
$ llvm-ar rcST built-in.a a/built-in.a
$ cat built-in.a !
// 14`
a/built-in.a/
/0 0 0 0 644 136 `
Recently, two commits were pushed to the trunk.
[llvm-ar] Flatten thin archives.
[llvm-ar] Resubmit recursive thin archive test with fix for full path names
and better error messages
As far as I tested, the latest llvm-ar works as follows:
$ llvm-ar rcST a/built-in.a a/foo.o
$ llvm-ar rcST built-in.a a/built-in.a
$ cat built-in.a
!
// 8 `
foo.o/
/0 0 0 0 644 800 `
OK, it flattens the thin archive, but the problem is it rips off
the directory path.
GNU ar works as follows:
$ ar rcST a/built-in.a a/foo.o
$ ar rcSTbuilt-in.a a/built-in.a
$ cat built-in.a
!
// 10`
a/foo.o/
/0 0 0 0 644 800 `
If you use the latest llvm-ar for building the kernel, you will see
a mysterious 'llvm-ar: error: No such file or directory.' error.
(I think the error message could be improved because it is unclear
what is missing.)
[Workaround in this patch]
Currently, llvm-ar cannot flatten thin archives correctly. So, Kbuild
uses '$(AR) t' to expand thin archives from subdirectories, then repack
all objects into the current directory built-in.a.
The 'P' for cmd_link_l_target is unneeded because Kbuild does not
support subdir objects for lib-y in the first place.
[How to use]
Pass AR=llvm-ar in the configuration and build command:
$ make AR=llvm-ar defconfig
$ make AR=llvm-ar
Or, simply in one line:
$ make AR=llvm-ar defconfig all
Signed-off-by: Masahiro Yamada
---
This patch can be cleanly applied to Linux 5.0-rc2 + the following:
https://patchwork.kernel.org/patch/10761625/
https://patchwork.kernel.org/patch/10767193/
https://patchwork.kernel.org/patch/10767195/
https://patchwork.kernel.org/patch/10767503/
https://patchwork.kernel.org/patch/10767509/
init/Kconfig | 3 +++
scripts/Makefile.build | 8 +++-
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/init/Kconfig b/init/Kconfig
index 513fa54..185274ac 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -23,6 +23,9 @@ config CLANG_VERSION
int
default $(shell,$(srctree)/scripts/clang-version.sh $(CC))
+config AR_IS_LLVM
+ def_bool
