Re: Linux 6.3-rc3
Nathan Chancellor 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
On Fri, Mar 24, 2023 at 05:23:12PM +0200, Kalle Valo wrote: > Nathan Chancellor 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
Jani Nikula writes:
> On Sat, 25 Mar 2023, Masahiro Yamada wrote:
>
>> On Thu, Mar 23, 2023 at 1:56 AM Linus Torvalds
>> wrote:
>>>
>>> On Wed, Mar 22, 2023 at 9:40 AM Sedat Dilek 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
On Sat, 25 Mar 2023, Masahiro Yamada wrote:
> Hello Linus,
>
>
> Thanks for giving me some more homeworks.
>
>
> On Thu, Mar 23, 2023 at 1:56 AM Linus Torvalds
> wrote:
>>
>> On Wed, Mar 22, 2023 at 9:40 AM Sedat Dilek 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
Hello Linus,
Thanks for giving me some more homeworks.
On Thu, Mar 23, 2023 at 1:56 AM Linus Torvalds
wrote:
>
> On Wed, Mar 22, 2023 at 9:40 AM Sedat Dilek 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
Nathan Chancellor 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
On Fri, Mar 24, 2023 at 12:54:01PM +0200, Kalle Valo wrote: > Nathan Chancellor writes: > > > On Wed, Mar 22, 2023 at 02:44:47PM +0200, Kalle Valo wrote: > >> Nathan Chancellor 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
Nathan Chancellor writes: > On Wed, Mar 22, 2023 at 02:44:47PM +0200, Kalle Valo wrote: >> Nathan Chancellor 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
On Mon, 20 Mar 2023 at 19:05, Nathan Chancellor wrote: > > On Sun, Mar 19, 2023 at 01:50:21PM -0700, Linus Torvalds wrote: > > So rc3 is fairly big, but that's not hugely usual: it's when a lot of > > the fixes tick up as it takes a while before people find and start > > reporting issues. > > ... > > > Please test and report any issues you find, > > 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) > ^~ > drivers/gpu/host1x/dev.c:490:16: note: initialize the variable 'syncpt_irq' > to silence this warning > int syncpt_irq; > ^ > = 0 > 1 error generated. > > There is an obvious fix that has been available on the mailing list for > some time: > > https://lore.kernel.org/20230127221418.2522612-1-a...@kernel.org/ > > It appears there was some sort of process snafu, since the fix never got > applied to the drm tree before the main pull for 6.3 and I have not been > able to get anyone to apply it to a tree targeting -rc releases. > > https://lore.kernel.org/Y%2FeULFO4jbivQ679@dev-arch.thelio-3990X/ > https://lore.kernel.org/67f9fe7f-392a-9acd-1a4d-0a43da634...@nvidia.com/ > > 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? :) I'll include it in the next drm-fixes pull. -Daniel > > Cheers, > Nathan -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: Linux 6.3-rc3
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 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 > > >> 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=16 > CONFIG_AS_IS_LLVM=y > CONFIG_AS_VERSION=16 > CONFIG_LD_VERSION=0 > CONFIG_LD_IS_LLD=y > CONFIG_LLD_VERSION=16 > > > $ 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
+ Masahiro and linux-kbuild for the proposal
On Wed, Mar 22, 2023 at 9:56 AM Linus Torvalds
wrote:
>
> On Wed, Mar 22, 2023 at 9:40 AM Sedat Dilek 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
On Wed, Mar 22, 2023 at 9:40 AM Sedat Dilek 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
On Wed, Mar 22, 2023 at 1:49 PM Kalle Valo wrote:
>
> Nathan Chancellor 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
> >> 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
On Wed, Mar 22, 2023 at 02:44:47PM +0200, Kalle Valo wrote: > Nathan Chancellor 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 > >> 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=16 CONFIG_AS_IS_LLVM=y CONFIG_AS_VERSION=16 CONFIG_LD_VERSION=0 CONFIG_LD_IS_LLD=y CONFIG_LLD_VERSION=16 $ 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 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 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
On Mon, Mar 20, 2023 at 3:06 PM Nathan Chancellor 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
Re: Linux 6.3-rc3
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
On Mon, Mar 20, 2023 at 01:30:17PM -0700, Linus Torvalds wrote:
> On Mon, Mar 20, 2023 at 1:05 PM Guenter Roeck 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
On Mon, Mar 20, 2023 at 1:05 PM Guenter Roeck 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. Linus
Re: Linux 6.3-rc3
On Mon, Mar 20, 2023 at 11:26:17AM -0700, Linus Torvalds wrote: > On Mon, Mar 20, 2023 at 11:05 AM Nathan Chancellor 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. > > 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. > > Do we have any gcc people around that could explain why gcc failed so > miserably at this trivial case? > I have noticed that gcc doesn't always warn about uninitialized variables in most architectures. The conditional btrfs build failure (only seen on sparc and parisc) is similar: gcc is silent even if I on purpose create and use uninitialized variables. Since the gcc version I use is the same for all architectures, I thought it must have something to do with compile options (like maybe the option to always initialize stack variables, or with some gcc plugin), but I have been unable to track it down. Guenter
Re: Linux 6.3-rc3
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 > > 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-=wgsqpdkejbb92m37jntdrqjrnruaprahke8ughtqqu...@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
On Mon, Mar 20, 2023 at 11:56 AM Nathan Chancellor wrote: > > I did see a patch fly by to fix that: > > https://lore.kernel.org/20230316082035.567520-3-christian.koe...@amd.com/ > > It seems like the DRM_TEGRA half of it is broken though: > > https://lore.kernel.org/202303170635.a2rsq1wu-...@intel.com/ Hmm. x86-64 has 'vmap()' too. So I think that DRM_TEGRA breakage is likely just due to a missing header file include that then (by luck and mistake) gets included on arm. You need for 'vmap()'. There might be something else going on, I didn't look deeply at it. Linus
Re: Linux 6.3-rc3
On Mon, Mar 20, 2023 at 11:49:55AM -0700, Linus Torvalds wrote: > On Mon, Mar 20, 2023 at 11:26 AM Linus Torvalds > wrote: > > > > Hmm. I do my arm64 allmodconfig builds with gcc, and I'm surprised > > that gcc doesn't warn about this. > > Side note: I'm also wondering why that TEGRA_HOST1X config has that > ARM dependency in > > depends on ARCH_TEGRA || (ARM && COMPILE_TEST) > > because it seems to build just fine at least on x86-64 if I change it to be > just > > depends on ARCH_TEGRA || COMPILE_TEST > > ie there seems to be nothing ARM-specific in there. > > Limiting it to just the tegra platform by default makes 100% sense, > but that "only do compile-testing on ARM" seems a bit bogus. > > That limit goes back to forever (commit 6f44c2b5280f: "gpu: host1x: > Increase compile test coverage" back in Nov 2013), so maybe things > didn't use to work as well back in the dark ages? > > None of this explains why gcc didn't catch it, but at least allowing > the build on x86-64 would likely have made it easier for people to see > clang catching this. I did see a patch fly by to fix that: https://lore.kernel.org/20230316082035.567520-3-christian.koe...@amd.com/ It seems like the DRM_TEGRA half of it is broken though: https://lore.kernel.org/202303170635.a2rsq1wu-...@intel.com/ Cheers, Nathan
Re: Linux 6.3-rc3
On Mon, Mar 20, 2023 at 11:26:17AM -0700, Linus Torvalds wrote: > On Mon, Mar 20, 2023 at 11:05 AM Nathan Chancellor 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-=wgsqpdkejbb92m37jntdrqjrnruaprahke8ughtqqu...@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
On Mon, Mar 20, 2023 at 11:26 AM Linus Torvalds wrote: > > Hmm. I do my arm64 allmodconfig builds with gcc, and I'm surprised > that gcc doesn't warn about this. Side note: I'm also wondering why that TEGRA_HOST1X config has that ARM dependency in depends on ARCH_TEGRA || (ARM && COMPILE_TEST) because it seems to build just fine at least on x86-64 if I change it to be just depends on ARCH_TEGRA || COMPILE_TEST ie there seems to be nothing ARM-specific in there. Limiting it to just the tegra platform by default makes 100% sense, but that "only do compile-testing on ARM" seems a bit bogus. That limit goes back to forever (commit 6f44c2b5280f: "gpu: host1x: Increase compile test coverage" back in Nov 2013), so maybe things didn't use to work as well back in the dark ages? None of this explains why gcc didn't catch it, but at least allowing the build on x86-64 would likely have made it easier for people to see clang catching this. Linus
Re: Linux 6.3-rc3
On Mon, Mar 20, 2023 at 11:05 AM Nathan Chancellor 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. 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. Do we have any gcc people around that could explain why gcc failed so miserably at this trivial case? Linus
Re: Linux 6.3-rc3
On Sun, Mar 19, 2023 at 01:50:21PM -0700, Linus Torvalds wrote: > So rc3 is fairly big, but that's not hugely usual: it's when a lot of > the fixes tick up as it takes a while before people find and start > reporting issues. ... > Please test and report any issues you find, 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) ^~ drivers/gpu/host1x/dev.c:490:16: note: initialize the variable 'syncpt_irq' to silence this warning int syncpt_irq; ^ = 0 1 error generated. There is an obvious fix that has been available on the mailing list for some time: https://lore.kernel.org/20230127221418.2522612-1-a...@kernel.org/ It appears there was some sort of process snafu, since the fix never got applied to the drm tree before the main pull for 6.3 and I have not been able to get anyone to apply it to a tree targeting -rc releases. https://lore.kernel.org/Y%2FeULFO4jbivQ679@dev-arch.thelio-3990X/ https://lore.kernel.org/67f9fe7f-392a-9acd-1a4d-0a43da634...@nvidia.com/ 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? :) Cheers, Nathan
