Re: Linux 6.3-rc3

Re: Linux 6.3-rc3

[Nathan Chancellor]

On Mon, Mar 20, 2023 at 11:26:17AM -0700, Linus Torvalds wrote:
> On Mon, Mar 20, 2023 at 11:05 AM Nathan Chancellor <nathan@kernel.org> wrote:
> >
> > On the clang front, I am still seeing the following warning turned error
> > for arm64 allmodconfig at least:
> >
> >   drivers/gpu/host1x/dev.c:520:6: error: variable 'syncpt_irq' is uninitialized when used here [-Werror,-Wuninitialized]
> >           if (syncpt_irq < 0)
> >               ^~~~~~~~~~
> 
> Hmm. I do my arm64 allmodconfig builds with gcc, and I'm surprised
> that gcc doesn't warn about this.

Perhaps these would make doing allmodconfig builds with clang more
frequently less painful for you?

https://lore.kernel.org/llvm/20230319235619.GA18547@dev-arch.thelio-3990X/

> That syncpt_irq thing isn't written to anywhere, so that's pretty egregious.
> 
> We use -Wno-maybe-uninitialized because gcc gets it so wrong, but
> that's different from the "-Wuninitialized" thing (without the
> "maybe").
> 
> I've seen gcc mess this up when there is one single assignment,
> because then the SSA format makes it *so* easy to just use that
> assignment out-of-order (or unconditionally), but this case looks
> unusually clear-cut.
> 
> So the fact that gcc doesn't warn about it is outright odd.
> 
> > If that does not come to you through other means before -rc4, could you
> > just apply it directly so that I can stop applying it to our CI? :)
> 
> Bah. I took it now, there's no excuse for that thing.

Thanks!

> Do we have any gcc people around that could explain why gcc failed so
> miserably at this trivial case?

Cc'ing linux-toolchains. The start of the thread is here:

https://lore.kernel.org/CAHk-=wgSqpdkeJBb92M37JNTdRQJRnRUApraHKE8uGHTqQuu2Q@mail.gmail.com/

The problematic function before the fix is here:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/host1x/dev.c?id=3d3699bde4b043eea17993e4e76804a8128f0fdb#n487

I will see if I have some cycles to try and reduce something out for the
GCC folks.

Cheers,
Nathan

Re: Linux 6.3-rc3

[Nathan Chancellor]

On Mon, Mar 20, 2023 at 11:53:37AM -0700, Nathan Chancellor wrote:
> On Mon, Mar 20, 2023 at 11:26:17AM -0700, Linus Torvalds wrote:
> > On Mon, Mar 20, 2023 at 11:05 AM Nathan Chancellor <nathan@kernel.org> wrote:
> > >
> > > On the clang front, I am still seeing the following warning turned error
> > > for arm64 allmodconfig at least:
> > >
> > >   drivers/gpu/host1x/dev.c:520:6: error: variable 'syncpt_irq' is uninitialized when used here [-Werror,-Wuninitialized]
> > >           if (syncpt_irq < 0)
> > >               ^~~~~~~~~~
> > 
> > Hmm. I do my arm64 allmodconfig builds with gcc, and I'm surprised
> > that gcc doesn't warn about this.
> 
> Perhaps these would make doing allmodconfig builds with clang more
> frequently less painful for you?
> 
> https://lore.kernel.org/llvm/20230319235619.GA18547@dev-arch.thelio-3990X/
> 
> > That syncpt_irq thing isn't written to anywhere, so that's pretty egregious.
> > 
> > We use -Wno-maybe-uninitialized because gcc gets it so wrong, but
> > that's different from the "-Wuninitialized" thing (without the
> > "maybe").
> > 
> > I've seen gcc mess this up when there is one single assignment,
> > because then the SSA format makes it *so* easy to just use that
> > assignment out-of-order (or unconditionally), but this case looks
> > unusually clear-cut.
> > 
> > So the fact that gcc doesn't warn about it is outright odd.
> > 
> > > If that does not come to you through other means before -rc4, could you
> > > just apply it directly so that I can stop applying it to our CI? :)
> > 
> > Bah. I took it now, there's no excuse for that thing.
> 
> Thanks!
> 
> > Do we have any gcc people around that could explain why gcc failed so
> > miserably at this trivial case?
> 
> Cc'ing linux-toolchains. The start of the thread is here:
> 
> https://lore.kernel.org/CAHk-=wgSqpdkeJBb92M37JNTdRQJRnRUApraHKE8uGHTqQuu2Q@mail.gmail.com/
> 
> The problematic function before the fix is here:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/host1x/dev.c?id=3d3699bde4b043eea17993e4e76804a8128f0fdb#n487
> 
> I will see if I have some cycles to try and reduce something out for the
> GCC folks.

While setting up the reduction, I noticed that there is an instance of
-Wmaybe-uninitialized at this site. Seems odd that it is not sure, I will
reduce on that.

  ../drivers/gpu/host1x/dev.c: In function 'host1x_probe':
  ../drivers/gpu/host1x/dev.c:520:12: error: 'syncpt_irq' may be used uninitialized [-Werror=maybe-uninitialized]
    520 |         if (syncpt_irq < 0)
        |            ^
  ../drivers/gpu/host1x/dev.c:490:13: note: 'syncpt_irq' was declared here
    490 |         int syncpt_irq;
        |             ^~~~~~~~~~
  cc1: all warnings being treated as errors

Cheers,
Nathan

Re: Linux 6.3-rc3

[Kalle Valo]

Nathan Chancellor <nathan@kernel.org> writes:

> On Mon, Mar 20, 2023 at 11:26:17AM -0700, Linus Torvalds wrote:
>> On Mon, Mar 20, 2023 at 11:05 AM Nathan Chancellor <nathan@kernel.org> wrote:
>> >
>> > On the clang front, I am still seeing the following warning turned error
>> > for arm64 allmodconfig at least:
>> >
>> >   drivers/gpu/host1x/dev.c:520:6: error: variable 'syncpt_irq' is uninitialized when used here [-Werror,-Wuninitialized]
>> >           if (syncpt_irq < 0)
>> >               ^~~~~~~~~~
>> 
>> Hmm. I do my arm64 allmodconfig builds with gcc, and I'm surprised
>> that gcc doesn't warn about this.
>
> Perhaps these would make doing allmodconfig builds with clang more
> frequently less painful for you?
>
> https://lore.kernel.org/llvm/20230319235619.GA18547@dev-arch.thelio-3990X/

Thank you, at least for me this is really helpful. I tried now clang for
the first time but seeing a strange problem.

I prefer to define the compiler in GNUmakefile so it's easy to change
compilers and I don't need to remember the exact command line. So I have
this in the top level GNUmakefile (all the rest commented out):

LLVM=/opt/clang/llvm-16.0.0/bin/

If I run 'make oldconfig' it seems to use clang but after I run just
'make' it seems to switch back to the host GCC compiler and ask for GCC
specific config questions again. Workaround for this seems to be adding
'export LLVM' to GNUmakefile, after that also 'make' uses clang as
expected.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: Linux 6.3-rc3

[Nathan Chancellor]

On Wed, Mar 22, 2023 at 02:44:47PM +0200, Kalle Valo wrote:
> Nathan Chancellor <nathan@kernel.org> writes:
> 
> > On Mon, Mar 20, 2023 at 11:26:17AM -0700, Linus Torvalds wrote:
> >> On Mon, Mar 20, 2023 at 11:05 AM Nathan Chancellor <nathan@kernel.org> wrote:
> >> >
> >> > On the clang front, I am still seeing the following warning turned error
> >> > for arm64 allmodconfig at least:
> >> >
> >> >   drivers/gpu/host1x/dev.c:520:6: error: variable 'syncpt_irq' is uninitialized when used here [-Werror,-Wuninitialized]
> >> >           if (syncpt_irq < 0)
> >> >               ^~~~~~~~~~
> >> 
> >> Hmm. I do my arm64 allmodconfig builds with gcc, and I'm surprised
> >> that gcc doesn't warn about this.
> >
> > Perhaps these would make doing allmodconfig builds with clang more
> > frequently less painful for you?
> >
> > https://lore.kernel.org/llvm/20230319235619.GA18547@dev-arch.thelio-3990X/
> 
> Thank you, at least for me this is really helpful.

Really glad to hear! I hope this helps make testing and verifying
changes with clang and LLVM easier for developers and maintainers.

> I tried now clang for the first time but seeing a strange problem.
> 
> I prefer to define the compiler in GNUmakefile so it's easy to change
> compilers and I don't need to remember the exact command line. So I have
> this in the top level GNUmakefile (all the rest commented out):
> 
> LLVM=/opt/clang/llvm-16.0.0/bin/
> 
> If I run 'make oldconfig' it seems to use clang but after I run just
> 'make' it seems to switch back to the host GCC compiler and ask for GCC
> specific config questions again. Workaround for this seems to be adding
> 'export LLVM' to GNUmakefile, after that also 'make' uses clang as
> expected.

Interesting... I just tested with a basic GNUmakefile and everything
seems to work fine without an export. At the same time, the export
should not hurt anything, so as long as it works, that is what matters.

    $ gcc --version
    fish: Unknown command: gcc


    $ fd -t x . $CBL_TC_LLVM_STORE/16.0.0/bin -x basename
    clang-16
    llvm-nm
    llvm-objdump
    llvm-objcopy
    llvm-symbolizer
    llvm-strings
    llvm-readobj
    llvm-dwarfdump
    lld
    llvm-ar


    $ cat GNUmakefile
    LLVM := $(CBL_TC_LLVM_STORE)/16.0.0/bin/

    include Makefile


    $ make -sj(nproc) defconfig


    $ head -13 .config
    #
    # Automatically generated file; DO NOT EDIT.
    # Linux/x86 6.3.0-rc3 Kernel Configuration
    #
    CONFIG_CC_VERSION_TEXT="ClangBuiltLinux clang version 16.0.0"
    CONFIG_GCC_VERSION=0
    CONFIG_CC_IS_CLANG=y
    CONFIG_CLANG_VERSION=160000
    CONFIG_AS_IS_LLVM=y
    CONFIG_AS_VERSION=160000
    CONFIG_LD_VERSION=0
    CONFIG_LD_IS_LLD=y
    CONFIG_LLD_VERSION=160000


    $ make -sj(nproc) init/main.o


    $ $CBL_TC_LLVM_STORE/16.0.0/bin/llvm-readelf -p .comment init/main.o
    String dump of section '.comment':
    [     1] ClangBuiltLinux clang version 16.0.0


I added an informational print and I always saw the correct value:

diff --git a/Makefile b/Makefile
index a2c310df2145..070394c4cb8c 100644
--- a/Makefile
+++ b/Makefile
@@ -431,6 +431,7 @@ HOST_LFS_LDFLAGS := $(shell getconf LFS_LDFLAGS 2>/dev/null)
 HOST_LFS_LIBS := $(shell getconf LFS_LIBS 2>/dev/null)
 
 ifneq ($(LLVM),)
+$(info LLVM: $(LLVM))
 ifneq ($(filter %/,$(LLVM)),)
 LLVM_PREFIX := $(LLVM)
 else ifneq ($(filter -%,$(LLVM)),)

If you have any further issues, please do not hesitate to reach out!

Cheers,
Nathan

Re: Linux 6.3-rc3

[Nathan Chancellor]

On Wed, Mar 22, 2023 at 09:36:37AM -0700, Nathan Chancellor wrote:
> On Wed, Mar 22, 2023 at 02:44:47PM +0200, Kalle Valo wrote:
> > Nathan Chancellor <nathan@kernel.org> writes:
> > 
> > > On Mon, Mar 20, 2023 at 11:26:17AM -0700, Linus Torvalds wrote:
> > >> On Mon, Mar 20, 2023 at 11:05 AM Nathan Chancellor <nathan@kernel.org> wrote:
> > >> >
> > >> > On the clang front, I am still seeing the following warning turned error
> > >> > for arm64 allmodconfig at least:
> > >> >
> > >> >   drivers/gpu/host1x/dev.c:520:6: error: variable 'syncpt_irq' is uninitialized when used here [-Werror,-Wuninitialized]
> > >> >           if (syncpt_irq < 0)
> > >> >               ^~~~~~~~~~
> > >> 
> > >> Hmm. I do my arm64 allmodconfig builds with gcc, and I'm surprised
> > >> that gcc doesn't warn about this.
> > >
> > > Perhaps these would make doing allmodconfig builds with clang more
> > > frequently less painful for you?
> > >
> > > https://lore.kernel.org/llvm/20230319235619.GA18547@dev-arch.thelio-3990X/
> > 
> > Thank you, at least for me this is really helpful.
> 
> Really glad to hear! I hope this helps make testing and verifying
> changes with clang and LLVM easier for developers and maintainers.
> 
> > I tried now clang for the first time but seeing a strange problem.
> > 
> > I prefer to define the compiler in GNUmakefile so it's easy to change
> > compilers and I don't need to remember the exact command line. So I have
> > this in the top level GNUmakefile (all the rest commented out):
> > 
> > LLVM=/opt/clang/llvm-16.0.0/bin/
> > 
> > If I run 'make oldconfig' it seems to use clang but after I run just
> > 'make' it seems to switch back to the host GCC compiler and ask for GCC
> > specific config questions again. Workaround for this seems to be adding
> > 'export LLVM' to GNUmakefile, after that also 'make' uses clang as
> > expected.
> 
> Interesting... I just tested with a basic GNUmakefile and everything
> seems to work fine without an export. At the same time, the export
> should not hurt anything, so as long as it works, that is what matters.

Ah, the export is needed so that mixed-build works properly (see lines
324 to 361 in Makefile), as 'make' will be called to process each target
individually; without the export, LLVM is not set for the subsequent
'make' calls, so gcc is called. I just saw the same behavior as you did
while testing with

  $ make -j(nproc) clean defconfig all

without the export (GCC was used instead of LLVM).

>     $ gcc --version
>     fish: Unknown command: gcc
> 
> 
>     $ fd -t x . $CBL_TC_LLVM_STORE/16.0.0/bin -x basename
>     clang-16
>     llvm-nm
>     llvm-objdump
>     llvm-objcopy
>     llvm-symbolizer
>     llvm-strings
>     llvm-readobj
>     llvm-dwarfdump
>     lld
>     llvm-ar
> 
> 
>     $ cat GNUmakefile
>     LLVM := $(CBL_TC_LLVM_STORE)/16.0.0/bin/
> 
>     include Makefile
> 
> 
>     $ make -sj(nproc) defconfig
> 
> 
>     $ head -13 .config
>     #
>     # Automatically generated file; DO NOT EDIT.
>     # Linux/x86 6.3.0-rc3 Kernel Configuration
>     #
>     CONFIG_CC_VERSION_TEXT="ClangBuiltLinux clang version 16.0.0"
>     CONFIG_GCC_VERSION=0
>     CONFIG_CC_IS_CLANG=y
>     CONFIG_CLANG_VERSION=160000
>     CONFIG_AS_IS_LLVM=y
>     CONFIG_AS_VERSION=160000
>     CONFIG_LD_VERSION=0
>     CONFIG_LD_IS_LLD=y
>     CONFIG_LLD_VERSION=160000
> 
> 
>     $ make -sj(nproc) init/main.o
> 
> 
>     $ $CBL_TC_LLVM_STORE/16.0.0/bin/llvm-readelf -p .comment init/main.o
>     String dump of section '.comment':
>     [     1] ClangBuiltLinux clang version 16.0.0
> 
> 
> I added an informational print and I always saw the correct value:
> 
> diff --git a/Makefile b/Makefile
> index a2c310df2145..070394c4cb8c 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -431,6 +431,7 @@ HOST_LFS_LDFLAGS := $(shell getconf LFS_LDFLAGS 2>/dev/null)
>  HOST_LFS_LIBS := $(shell getconf LFS_LIBS 2>/dev/null)
>  
>  ifneq ($(LLVM),)
> +$(info LLVM: $(LLVM))
>  ifneq ($(filter %/,$(LLVM)),)
>  LLVM_PREFIX := $(LLVM)
>  else ifneq ($(filter -%,$(LLVM)),)
> 
> If you have any further issues, please do not hesitate to reach out!
> 
> Cheers,
> Nathan
> 

Re: Linux 6.3-rc3

[Kalle Valo]

Nathan Chancellor <nathan@kernel.org> writes:

> On Wed, Mar 22, 2023 at 02:44:47PM +0200, Kalle Valo wrote:
>> Nathan Chancellor <nathan@kernel.org> writes:
>> 
>> > Perhaps these would make doing allmodconfig builds with clang more
>> > frequently less painful for you?
>> >
>> > https://lore.kernel.org/llvm/20230319235619.GA18547@dev-arch.thelio-3990X/
>> 
>> Thank you, at least for me this is really helpful.
>
> Really glad to hear! I hope this helps make testing and verifying
> changes with clang and LLVM easier for developers and maintainers.

It really does. And I hope you are able to update these packages in
future as well so that it would be easy to get the latest clang.

>> I tried now clang for the first time but seeing a strange problem.
>> 
>> I prefer to define the compiler in GNUmakefile so it's easy to change
>> compilers and I don't need to remember the exact command line. So I have
>> this in the top level GNUmakefile (all the rest commented out):
>> 
>> LLVM=/opt/clang/llvm-16.0.0/bin/
>> 
>> If I run 'make oldconfig' it seems to use clang but after I run just
>> 'make' it seems to switch back to the host GCC compiler and ask for GCC
>> specific config questions again. Workaround for this seems to be adding
>> 'export LLVM' to GNUmakefile, after that also 'make' uses clang as
>> expected.
>
> Interesting... I just tested with a basic GNUmakefile and everything
> seems to work fine without an export. At the same time, the export
> should not hurt anything, so as long as it works, that is what matters.

Sure, once I figured out the quirks I can workaround them. I was just
hoping that other users would not have to go through the same hassle as
I did :)

> If you have any further issues, please do not hesitate to reach out!

This is nitpicking but it would be nice if the tarball contents wouldn't
conflict with each other. Now both llvm-16.0.0-aarch64.tar.gz and
llvm-16.0.0-x86_64.tar extract to the same directory llvm-16.0.0 with
same binary names. It would be much better if they would extract to
llvm-16.0.0-aarch64 and llvm-16.0.0-x86_64, respectively.

For example, Arnd's crosstool packages don't conflict with each other:

https://mirrors.edge.kernel.org/pub/tools/crosstool/

And maybe request a similar llvm directory under pub/tools to make it
more official? :)

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: Linux 6.3-rc3

[Nathan Chancellor]

On Fri, Mar 24, 2023 at 12:54:01PM +0200, Kalle Valo wrote:
> Nathan Chancellor <nathan@kernel.org> writes:
> 
> > On Wed, Mar 22, 2023 at 02:44:47PM +0200, Kalle Valo wrote:
> >> Nathan Chancellor <nathan@kernel.org> writes:
> >> 
> >> > Perhaps these would make doing allmodconfig builds with clang more
> >> > frequently less painful for you?
> >> >
> >> > https://lore.kernel.org/llvm/20230319235619.GA18547@dev-arch.thelio-3990X/
> >> 
> >> Thank you, at least for me this is really helpful.
> >
> > Really glad to hear! I hope this helps make testing and verifying
> > changes with clang and LLVM easier for developers and maintainers.
> 
> It really does. And I hope you are able to update these packages in
> future as well so that it would be easy to get the latest clang.

That is the current plan (I will push 16.0.1, 16.0.2, etc. as they are
released), I have a relatively automated process for this going forward.

> >> I tried now clang for the first time but seeing a strange problem.
> >> 
> >> I prefer to define the compiler in GNUmakefile so it's easy to change
> >> compilers and I don't need to remember the exact command line. So I have
> >> this in the top level GNUmakefile (all the rest commented out):
> >> 
> >> LLVM=/opt/clang/llvm-16.0.0/bin/
> >> 
> >> If I run 'make oldconfig' it seems to use clang but after I run just
> >> 'make' it seems to switch back to the host GCC compiler and ask for GCC
> >> specific config questions again. Workaround for this seems to be adding
> >> 'export LLVM' to GNUmakefile, after that also 'make' uses clang as
> >> expected.
> >
> > Interesting... I just tested with a basic GNUmakefile and everything
> > seems to work fine without an export. At the same time, the export
> > should not hurt anything, so as long as it works, that is what matters.
> 
> Sure, once I figured out the quirks I can workaround them. I was just
> hoping that other users would not have to go through the same hassle as
> I did :)
> 
> > If you have any further issues, please do not hesitate to reach out!
> 
> This is nitpicking but it would be nice if the tarball contents wouldn't
> conflict with each other. Now both llvm-16.0.0-aarch64.tar.gz and
> llvm-16.0.0-x86_64.tar extract to the same directory llvm-16.0.0 with
> same binary names. It would be much better if they would extract to
> llvm-16.0.0-aarch64 and llvm-16.0.0-x86_64, respectively.
> 
> For example, Arnd's crosstool packages don't conflict with each other:
> 
> https://mirrors.edge.kernel.org/pub/tools/crosstool/

I could certainly do that but what is the use case for extracting both?
You cannot run the aarch64 version on an x86_64 host and vice versa, so
why bother extracting them? I had figured the architecture would be
irrelevant once installed on the host, so I opted only to include it in
the tarball name. Perhaps I should make it clearer that these are the
host architectures, not the target architectures (because clang is
multi-targeted, unlike GCC)?

> And maybe request a similar llvm directory under pub/tools to make it
> more official? :)

Yes, I was talking that over with Nick recently, as having it under a
group on kernel.org would make taking over maintainership easier should
something happen to me :)

Thanks for all the feedback so far, it is much appreciated!

Cheers,
Nathan

Re: Linux 6.3-rc3

[Kalle Valo]

Nathan Chancellor <nathan@kernel.org> writes:

>> This is nitpicking but it would be nice if the tarball contents wouldn't
>> conflict with each other. Now both llvm-16.0.0-aarch64.tar.gz and
>> llvm-16.0.0-x86_64.tar extract to the same directory llvm-16.0.0 with
>> same binary names. It would be much better if they would extract to
>> llvm-16.0.0-aarch64 and llvm-16.0.0-x86_64, respectively.
>> 
>> For example, Arnd's crosstool packages don't conflict with each other:
>> 
>> https://mirrors.edge.kernel.org/pub/tools/crosstool/
>
> I could certainly do that but what is the use case for extracting both?
> You cannot run the aarch64 version on an x86_64 host and vice versa, so
> why bother extracting them?

Ah, I didn't realise that. I assumed llvm-16.0.0-aarch64.tar.gz was a
cross compiler. I'm sure you documented that in the page but hey who
reads the documentation ;)

> I had figured the architecture would be irrelevant once installed on
> the host, so I opted only to include it in the tarball name. Perhaps I
> should make it clearer that these are the host architectures, not the
> target architectures (because clang is multi-targeted, unlike GCC)?

Makes sense now. But I still think it's good style that a tarball named
llvm-16.0.0-aarch64.tar.gz extracts to llvm-16.0.0-aarch64.

>> And maybe request a similar llvm directory under pub/tools to make it
>> more official? :)
>
> Yes, I was talking that over with Nick recently, as having it under a
> group on kernel.org would make taking over maintainership easier should
> something happen to me :)

Yeah, sharing the load is always good.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: Linux 6.3-rc3

[Nathan Chancellor]

On Fri, Mar 24, 2023 at 05:23:12PM +0200, Kalle Valo wrote:
> Nathan Chancellor <nathan@kernel.org> writes:
> 
> >> This is nitpicking but it would be nice if the tarball contents wouldn't
> >> conflict with each other. Now both llvm-16.0.0-aarch64.tar.gz and
> >> llvm-16.0.0-x86_64.tar extract to the same directory llvm-16.0.0 with
> >> same binary names. It would be much better if they would extract to
> >> llvm-16.0.0-aarch64 and llvm-16.0.0-x86_64, respectively.
> >> 
> >> For example, Arnd's crosstool packages don't conflict with each other:
> >> 
> >> https://mirrors.edge.kernel.org/pub/tools/crosstool/
> >
> > I could certainly do that but what is the use case for extracting both?
> > You cannot run the aarch64 version on an x86_64 host and vice versa, so
> > why bother extracting them?
> 
> Ah, I didn't realise that. I assumed llvm-16.0.0-aarch64.tar.gz was a
> cross compiler. I'm sure you documented that in the page but hey who
> reads the documentation ;)

:)

I have adjusted the README to hopefully make that clearer.

> > I had figured the architecture would be irrelevant once installed on
> > the host, so I opted only to include it in the tarball name. Perhaps I
> > should make it clearer that these are the host architectures, not the
> > target architectures (because clang is multi-targeted, unlike GCC)?
> 
> Makes sense now. But I still think it's good style that a tarball named
> llvm-16.0.0-aarch64.tar.gz extracts to llvm-16.0.0-aarch64.

Indeed, I have adjusted it for future builds:

https://github.com/nathanchance/env/commit/314837e6706889138121a32140d2acdc7895d390

> >> And maybe request a similar llvm directory under pub/tools to make it
> >> more official? :)

We now have https://kernel.org/pub/tools/llvm/, which is about as
official as we can get I suppose :)

https://kernel.org/pub/linux/kernel/people/nathan/llvm/ now points
people there.

Cheers,
Nathan

Re: Linux 6.3-rc3

[Kalle Valo]

Nathan Chancellor <nathan@kernel.org> writes:

>> >> And maybe request a similar llvm directory under pub/tools to make it
>> >> more official? :)
>
> We now have https://kernel.org/pub/tools/llvm/, which is about as
> official as we can get I suppose :)

Nice, thanks. Bookmarked and I'll advertise this to wireless devs
whenever we have clang warnings.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: Linux 6.3-rc3

[Sedat Dilek]

On Wed, Mar 22, 2023 at 1:49 PM Kalle Valo <kvalo@kernel.org> wrote:
>
> Nathan Chancellor <nathan@kernel.org> writes:
>
> > On Mon, Mar 20, 2023 at 11:26:17AM -0700, Linus Torvalds wrote:
> >> On Mon, Mar 20, 2023 at 11:05 AM Nathan Chancellor <nathan@kernel.org> wrote:
> >> >
> >> > On the clang front, I am still seeing the following warning turned error
> >> > for arm64 allmodconfig at least:
> >> >
> >> >   drivers/gpu/host1x/dev.c:520:6: error: variable 'syncpt_irq' is uninitialized when used here [-Werror,-Wuninitialized]
> >> >           if (syncpt_irq < 0)
> >> >               ^~~~~~~~~~
> >>
> >> Hmm. I do my arm64 allmodconfig builds with gcc, and I'm surprised
> >> that gcc doesn't warn about this.
> >
> > Perhaps these would make doing allmodconfig builds with clang more
> > frequently less painful for you?
> >
> > https://lore.kernel.org/llvm/20230319235619.GA18547@dev-arch.thelio-3990X/
>
> Thank you, at least for me this is really helpful. I tried now clang for
> the first time but seeing a strange problem.
>
> I prefer to define the compiler in GNUmakefile so it's easy to change
> compilers and I don't need to remember the exact command line. So I have
> this in the top level GNUmakefile (all the rest commented out):
>
> LLVM=/opt/clang/llvm-16.0.0/bin/
>

Welcome to the LLVM/Clang world!

First try - First Cry...

In my build-environment I add (export) /path/to/llvm/bin to $PATH and
pass single CC LD AR etc. (what is substituted by LLVM=1):

make CC=clang LD=ld.lld AR=llvm-ar NM=llvm-nm STRIP=llvm-strip \
 OBJCOPY=llvm-objcopy OBJDUMP=llvm-objdump READELF=llvm-readelf \
 HOSTCC=clang HOSTCXX=clang++ HOSTAR=llvm-ar HOSTLD=ld.lld

Equivalent to:

make LLVM=1

I cannot comment on `make LLVM=/path/to/llvm/` and/or combinations
with `LLVM=1` as I have never used it

> If I run 'make oldconfig' it seems to use clang but after I run just
> 'make' it seems to switch back to the host GCC compiler and ask for GCC
> specific config questions again. Workaround for this seems to be adding
> 'export LLVM' to GNUmakefile, after that also 'make' uses clang as
> expected.
>

You have to pass `make LLVM=1` in any case... to `oldconfig` or when
adding any MAKEFLAGS like -j${number-of-available-cpus}.

Hope that helps.

Best regards,
-Sedat-

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/kbuild/llvm.rst
[2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/kbuild/llvm.rst#n52

Re: Linux 6.3-rc3

[Linus Torvalds]

On Wed, Mar 22, 2023 at 9:40 AM Sedat Dilek <sedat.dilek@gmail.com> wrote:
>
> You have to pass `make LLVM=1` in any case... to `oldconfig` or when
> adding any MAKEFLAGS like -j${number-of-available-cpus}.

I actually think we should look (again) at just making the compiler
choice (and the prefix) be a Kconfig option.

That would simplify *so* many use cases.

It used to be that gcc was "THE compiler" and anything else was just
an odd toy special case, but that's clearly not true any more.

So it would be lovely to make the kernel choice a Kconfig choice - so
you'd set it only at config time, and then after that a kernel build
wouldn't need special flags any more, and you'd never need to play
games with GNUmakefile or anything like that.

Yes, you'd still use environment variables (or make arguments) for
that initial Kconfig, but that's no different from the other
environment variables we already have, like KCONFIG_SEED that kconfig
uses internally, but also things like "$(ARCH)" that we already use
*inside* the Kconfig files themselves.

I really dislike how you have to set ARCH and CROSS_COMPILE etc
externally, and can't just have them *in* the config file.

So when you do cross-compiles, right now you have to do something like

    make ARCH=i386 allmodconfig

to build the .config file, but then you have to *repeat* that
ARCH=i386 when you actually build things:

    make ARCH=i386

because the ARCH choice ends up being in the .config file, but the
makefiles themselves always take it from the environment.

There are good historical reasons for our behavior (and probably a
number of extant practical reasons too), but it's a bit annoying, and
it would be lovely if we could start moving away from this model.

            Linus

Re: Linux 6.3-rc3

[Nick Desaulniers]

+ Masahiro and linux-kbuild for the proposal

On Wed, Mar 22, 2023 at 9:56 AM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Wed, Mar 22, 2023 at 9:40 AM Sedat Dilek <sedat.dilek@gmail.com> wrote:
> >
> > You have to pass `make LLVM=1` in any case... to `oldconfig` or when
> > adding any MAKEFLAGS like -j${number-of-available-cpus}.
>
> I actually think we should look (again) at just making the compiler
> choice (and the prefix) be a Kconfig option.
>
> That would simplify *so* many use cases.
>
> It used to be that gcc was "THE compiler" and anything else was just
> an odd toy special case, but that's clearly not true any more.

<3

>
> So it would be lovely to make the kernel choice a Kconfig choice - so
> you'd set it only at config time, and then after that a kernel build
> wouldn't need special flags any more, and you'd never need to play
> games with GNUmakefile or anything like that.
>
> Yes, you'd still use environment variables (or make arguments) for
> that initial Kconfig, but that's no different from the other
> environment variables we already have, like KCONFIG_SEED that kconfig
> uses internally, but also things like "$(ARCH)" that we already use
> *inside* the Kconfig files themselves.
>
> I really dislike how you have to set ARCH and CROSS_COMPILE etc
> externally, and can't just have them *in* the config file.

Not needing CROSS_COMPILE for LLVM=1 has been great. ;)

(Still need it for ARCH=s390 until LLD gets s390 support though)

>
> So when you do cross-compiles, right now you have to do something like
>
>     make ARCH=i386 allmodconfig
>
> to build the .config file, but then you have to *repeat* that
> ARCH=i386 when you actually build things:
>
>     make ARCH=i386
>
> because the ARCH choice ends up being in the .config file, but the
> makefiles themselves always take it from the environment.
>
> There are good historical reasons for our behavior (and probably a
> number of extant practical reasons too), but it's a bit annoying, and
> it would be lovely if we could start moving away from this model.
>
>             Linus
>


-- 
Thanks,
~Nick Desaulniers

Re: Linux 6.3-rc3

[Masahiro Yamada]

Hello Linus,


Thanks for giving me some more homeworks.


On Thu, Mar 23, 2023 at 1:56 AM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Wed, Mar 22, 2023 at 9:40 AM Sedat Dilek <sedat.dilek@gmail.com> wrote:
> >
> > You have to pass `make LLVM=1` in any case... to `oldconfig` or when
> > adding any MAKEFLAGS like -j${number-of-available-cpus}.
>
> I actually think we should look (again) at just making the compiler
> choice (and the prefix) be a Kconfig option.
>
> That would simplify *so* many use cases.
>
> It used to be that gcc was "THE compiler" and anything else was just
> an odd toy special case, but that's clearly not true any more.
>
> So it would be lovely to make the kernel choice a Kconfig choice - so
> you'd set it only at config time, and then after that a kernel build
> wouldn't need special flags any more, and you'd never need to play
> games with GNUmakefile or anything like that.


Presumably, this is the right direction.

To achieve it, Kconfig needs to have some mechanism to evaluate
shell commands dynamically.

If a user switches the toolchain set between GCC and LLVM
while running the Kconfig, $(cc-option) in Kconfig files must
be re-calculated.

Currently, Kconfig cannot do it. All macros are static - they are
expanded in the parse stage, and become constant strings.

Ulf Magnusson and I discussed the dynamic approach a few years back,
but I adopted the static way since it is much simpler.
We need to reconsider the dynamic approach to do this correctly.
I do not think it is too difficult technically.
We just need to come up with a decent syntax.



> Yes, you'd still use environment variables (or make arguments) for
> that initial Kconfig, but that's no different from the other
> environment variables we already have, like KCONFIG_SEED that kconfig
> uses internally, but also things like "$(ARCH)" that we already use
> *inside* the Kconfig files themselves.
>
> I really dislike how you have to set ARCH and CROSS_COMPILE etc
> externally, and can't just have them *in* the config file.
>
> So when you do cross-compiles, right now you have to do something like
>
>     make ARCH=i386 allmodconfig
>
> to build the .config file, but then you have to *repeat* that
> ARCH=i386 when you actually build things:
>
>     make ARCH=i386
>
> because the ARCH choice ends up being in the .config file, but the
> makefiles themselves always take it from the environment.
>
> There are good historical reasons for our behavior (and probably a
> number of extant practical reasons too), but it's a bit annoying, and
> it would be lovely if we could start moving away from this model.
>
>             Linus


Moving ARCH into the .config file needs careful thoughts, I think.

Not all targets include the .config file.
For example, "make clean", "make help", etc.

It is unclear which targets require explicit ARCH= option.

One solution is to move "archhelp", "CLEAN_FILES" etc.
from arch/*/Makefile to the top Makefile.
We will lose per-arch splitting in several places, though.


U-Boot adopts this model - 'ARCH' is determined in the Kconfig time,
so users do not need to give ARCH= option from the command line.

https://github.com/u-boot/u-boot/blob/v2023.01/arch/Kconfig#L44

You may get a quick idea of what it will look like.



I will take a look at this direction (the compiler choice in Kconfig first),
but it will not happen soonish due to the limited time for upstream work.


--
Best Regards

Masahiro Yamada

Re: Linux 6.3-rc3

[Jani Nikula]

On Sat, 25 Mar 2023, Masahiro Yamada <masahiroy@kernel.org> wrote:
> Hello Linus,
>
>
> Thanks for giving me some more homeworks.
>
>
> On Thu, Mar 23, 2023 at 1:56 AM Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
>>
>> On Wed, Mar 22, 2023 at 9:40 AM Sedat Dilek <sedat.dilek@gmail.com> wrote:
>> >
>> > You have to pass `make LLVM=1` in any case... to `oldconfig` or when
>> > adding any MAKEFLAGS like -j${number-of-available-cpus}.
>>
>> I actually think we should look (again) at just making the compiler
>> choice (and the prefix) be a Kconfig option.
>>
>> That would simplify *so* many use cases.
>>
>> It used to be that gcc was "THE compiler" and anything else was just
>> an odd toy special case, but that's clearly not true any more.
>>
>> So it would be lovely to make the kernel choice a Kconfig choice - so
>> you'd set it only at config time, and then after that a kernel build
>> wouldn't need special flags any more, and you'd never need to play
>> games with GNUmakefile or anything like that.
>
>
> Presumably, this is the right direction.
>
> To achieve it, Kconfig needs to have some mechanism to evaluate
> shell commands dynamically.
>
> If a user switches the toolchain set between GCC and LLVM
> while running the Kconfig, $(cc-option) in Kconfig files must
> be re-calculated.
>
> Currently, Kconfig cannot do it. All macros are static - they are
> expanded in the parse stage, and become constant strings.
>
> Ulf Magnusson and I discussed the dynamic approach a few years back,
> but I adopted the static way since it is much simpler.
> We need to reconsider the dynamic approach to do this correctly.
> I do not think it is too difficult technically.
> We just need to come up with a decent syntax.

I acknowledge being clueless about mostly everything that requires. But
in the mean time, how about just adding something like:

-include .env

near the beginning of the top Makefile?

You could shove the tools or ARCH or output dir etc. there, so you don't
have to remember to add them on the command line every time.

BR,
Jani.



>
>
>
>> Yes, you'd still use environment variables (or make arguments) for
>> that initial Kconfig, but that's no different from the other
>> environment variables we already have, like KCONFIG_SEED that kconfig
>> uses internally, but also things like "$(ARCH)" that we already use
>> *inside* the Kconfig files themselves.
>>
>> I really dislike how you have to set ARCH and CROSS_COMPILE etc
>> externally, and can't just have them *in* the config file.
>>
>> So when you do cross-compiles, right now you have to do something like
>>
>>     make ARCH=i386 allmodconfig
>>
>> to build the .config file, but then you have to *repeat* that
>> ARCH=i386 when you actually build things:
>>
>>     make ARCH=i386
>>
>> because the ARCH choice ends up being in the .config file, but the
>> makefiles themselves always take it from the environment.
>>
>> There are good historical reasons for our behavior (and probably a
>> number of extant practical reasons too), but it's a bit annoying, and
>> it would be lovely if we could start moving away from this model.
>>
>>             Linus
>
>
> Moving ARCH into the .config file needs careful thoughts, I think.
>
> Not all targets include the .config file.
> For example, "make clean", "make help", etc.
>
> It is unclear which targets require explicit ARCH= option.
>
> One solution is to move "archhelp", "CLEAN_FILES" etc.
> from arch/*/Makefile to the top Makefile.
> We will lose per-arch splitting in several places, though.
>
>
> U-Boot adopts this model - 'ARCH' is determined in the Kconfig time,
> so users do not need to give ARCH= option from the command line.
>
> https://github.com/u-boot/u-boot/blob/v2023.01/arch/Kconfig#L44
>
> You may get a quick idea of what it will look like.
>
>
>
> I will take a look at this direction (the compiler choice in Kconfig first),
> but it will not happen soonish due to the limited time for upstream work.
>
>
> --
> Best Regards
>
> Masahiro Yamada

-- 
Jani Nikula, Intel Open Source Graphics Center

Re: Linux 6.3-rc3

[Kalle Valo]

Jani Nikula <jani.nikula@linux.intel.com> writes:

> On Sat, 25 Mar 2023, Masahiro Yamada <masahiroy@kernel.org> wrote:
>
>> On Thu, Mar 23, 2023 at 1:56 AM Linus Torvalds
>> <torvalds@linux-foundation.org> wrote:
>>>
>>> On Wed, Mar 22, 2023 at 9:40 AM Sedat Dilek <sedat.dilek@gmail.com> wrote:
>>> >
>>> > You have to pass `make LLVM=1` in any case... to `oldconfig` or when
>>> > adding any MAKEFLAGS like -j${number-of-available-cpus}.
>>>
>>> I actually think we should look (again) at just making the compiler
>>> choice (and the prefix) be a Kconfig option.
>>>
>>> That would simplify *so* many use cases.
>>>
>>> It used to be that gcc was "THE compiler" and anything else was just
>>> an odd toy special case, but that's clearly not true any more.
>>>
>>> So it would be lovely to make the kernel choice a Kconfig choice - so
>>> you'd set it only at config time, and then after that a kernel build
>>> wouldn't need special flags any more, and you'd never need to play
>>> games with GNUmakefile or anything like that.
>>
>>
>> Presumably, this is the right direction.
>>
>> To achieve it, Kconfig needs to have some mechanism to evaluate
>> shell commands dynamically.
>>
>> If a user switches the toolchain set between GCC and LLVM
>> while running the Kconfig, $(cc-option) in Kconfig files must
>> be re-calculated.
>>
>> Currently, Kconfig cannot do it. All macros are static - they are
>> expanded in the parse stage, and become constant strings.
>>
>> Ulf Magnusson and I discussed the dynamic approach a few years back,
>> but I adopted the static way since it is much simpler.
>> We need to reconsider the dynamic approach to do this correctly.
>> I do not think it is too difficult technically.
>> We just need to come up with a decent syntax.
>
> I acknowledge being clueless about mostly everything that requires. But
> in the mean time, how about just adding something like:
>
> -include .env
>
> near the beginning of the top Makefile?
>
> You could shove the tools or ARCH or output dir etc. there, so you don't
> have to remember to add them on the command line every time.

Yes, please! Something like this, but officially supported, would be
just perfect for a lazy person like me.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: Linux 6.3-rc3

[Nathan Chancellor]

On Mon, Mar 20, 2023 at 01:30:17PM -0700, Linus Torvalds wrote:
> On Mon, Mar 20, 2023 at 1:05 PM Guenter Roeck <linux@roeck-us.net> wrote:
> >
> > I have noticed that gcc doesn't always warn about uninitialized variables
> > in most architectures.
> 
> Yeah, I'm getting the feeling that when the gcc people were trying to
> make -Wmaybe-uninitialized work better (when moving it into "-Wall"),
> they ended up moving a lot of "clearly uninitialized" cases into it.
> 
> So then because we disable the "maybe" case (with
> -Wno-maybe-uninitialized) because it had too many random false
> positives, we end up not seeing the obvious cases either.

Right, this seems like a subtle difference in semantics between
-Wuninitialized between clang and GCC. My naive attempt to reduce the
problem with cvise spits out:

$ cat dev.i
void *host1x_probe___trans_tmp_1;
void host1x_unregister();
int host1x_probe_syncpt_irqhost1x_probe() {
  int err;
  if (host1x_probe___trans_tmp_1)
    return 2;
  if (err)
    host1x_unregister();
  return err;
}

$ gcc -O2 -Wall -c -o /dev/null dev.i
dev.i: In function ‘host1x_probe_syncpt_irqhost1x_probe’:
dev.i:7:6: warning: ‘err’ may be used uninitialized [-Wmaybe-uninitialized]
    7 |   if (err)
      |      ^
dev.i:4:7: note: ‘err’ was declared here
    4 |   int err;
      |       ^~~

$ clang -Wall -fsyntax-only dev.i
dev.i:7:7: warning: variable 'err' is uninitialized when used here [-Wuninitialized]
  if (err)
      ^~~
dev.i:4:10: note: initialize the variable 'err' to silence this warning
  int err;
         ^
          = 0
1 warning generated.

If I remove the first branch, both compilers show -Wuninitialized.

$ cat dev.i
void *host1x_probe___trans_tmp_1;
void host1x_unregister();
int host1x_probe_syncpt_irqhost1x_probe() {
  int err;
  if (err)
    host1x_unregister();
  return err;
}

$ gcc -O2 -Wall -c -o /dev/null dev.i
dev.i: In function ‘host1x_probe_syncpt_irqhost1x_probe’:
dev.i:5:6: warning: ‘err’ is used uninitialized [-Wuninitialized]
    5 |   if (err)
      |      ^
dev.i:4:7: note: ‘err’ was declared here
    4 |   int err;
      |       ^~~

$ clang -Wall -fsyntax-only dev.i
dev.i:5:7: warning: variable 'err' is uninitialized when used here [-Wuninitialized]
  if (err)
      ^~~
dev.i:4:10: note: initialize the variable 'err' to silence this warning
  int err;
         ^
          = 0
1 warning generated.

It seems like clang takes into account that the branch has no effect on
how uninitialized err is, although it does acknowledge there may be
control flow where err is not used uninitialized because it is not used
at all by stating "when used here". I guess GCC does not make this
distinction and places it under -Wmaybe-uninitialized. I could be
totally wrong though :)

Cheers,
Nathan

Re: Linux 6.3-rc3

[Segher Boessenkool]

On Mon, Mar 20, 2023 at 03:06:31PM -0700, Nathan Chancellor wrote:
> It seems like clang takes into account that the branch has no effect on
> how uninitialized err is, although it does acknowledge there may be
> control flow where err is not used uninitialized because it is not used
> at all by stating "when used here". I guess GCC does not make this
> distinction and places it under -Wmaybe-uninitialized. I could be
> totally wrong though :)

In one place we have the comment

  /* Re-do the plain uninitialized variable check, as optimization may have
     straightened control flow.  Do this first so that we don't accidentally
     get a "may be" warning when we'd have seen an "is" warning later.  */

It seems we miss a similar case here?

In any case, please open a PR if you want this fixed.  Thanks!


Segher

Re: Linux 6.3-rc3

[Linus Torvalds]

On Mon, Mar 20, 2023 at 3:06 PM Nathan Chancellor <nathan@kernel.org> wrote:
>
> Right, this seems like a subtle difference in semantics between
> -Wuninitialized between clang and GCC.

I guess it's a bit ambiguous whether it's

 "X may be USED uninitialized"

or whether it is

 "X may BE uninitialized"

and then depending on how you see that ambiguity, the control flow matters.

In this case, there is absolutely no question that the variable is
uninitialized (since there is no write to it at all).

So it is very clearly and unambiguously uninitialized. And I do think
that as a result, "-Wuninitialized" should warn.

But at the same time, whether it is *used* or not depends on that
conditional, so I can see how it could be confusing and not be so
clear an unambiguous.

On the whole, I do wish that the logic would be "after dead code
removal, if some pseudo has no initializer, it should always warn,
regardless of any remaining dynamic conditoinals".

That "after dead code removal" might matter, because I could see where
config things (#ifdef's etc) would just remove the initialization of
some variable, and if the use is behind some static "if (0)", then
warning about it is all kinds of silly.

                     Linus