Date: Mon, 15 Sep 2025 15:44:08 -0600 From: Warner Losh <imp@bsdimp.com> To: "Bjoern A. Zeeb" <bz@freebsd.org> Cc: Warner Losh <imp@freebsd.org>, =?UTF-8?B?SmVhbi1Tw6liYXN0aWVuIFDDqWRyb24=?= <dumbbell@freebsd.org>, src-committers <src-committers@freebsd.org>, "<dev-commits-src-all@freebsd.org>" <dev-commits-src-all@freebsd.org>, "<dev-commits-src-main@freebsd.org>" <dev-commits-src-main@freebsd.org> Subject: Re: git: 5e0a4859f28a - main - iwlwifi: Don't compile for gcc before 14 Message-ID: <CANCZdfo6Q9cGFQAMXREvSgNEtC-9HfyxCJcMcoNXO5F8ot973g@mail.gmail.com> In-Reply-To: <ps3s6s5n-ss7p-9754-21o0-21715q641394@serrofq.bet> References: <202509150304.58F34BWJ035102@gitrepo.freebsd.org> <p5327015-51o4-3r0r-1rq4-347r7q995677@SerrOFQ.bet> <CANCZdfpQYj6a0rYeHXNCQe9WQy=LQZj49-DMxKxDFeYFMu-DfQ@mail.gmail.com> <ps3s6s5n-ss7p-9754-21o0-21715q641394@serrofq.bet>
next in thread | previous in thread | raw e-mail | index | archive | help
[-- Attachment #1 --] On Mon, Sep 15, 2025 at 3:32 PM Bjoern A. Zeeb <bz@freebsd.org> wrote: > On Mon, 15 Sep 2025, Warner Losh wrote: > > > On Mon, Sep 15, 2025, 1:26 PM Bjoern A. Zeeb <bz@freebsd.org> wrote: > > > >> On Mon, 15 Sep 2025, Warner Losh wrote: > >> > >>> The branch main has been updated by imp: > >>> > >>> URL: > >> > https://cgit.FreeBSD.org/src/commit/?id=5e0a4859f28ad4869f7a73faf42debc355a370bf > >>> > >>> commit 5e0a4859f28ad4869f7a73faf42debc355a370bf > >>> Author: Warner Losh <imp@FreeBSD.org> > >>> AuthorDate: 2025-09-14 18:03:16 +0000 > >>> Commit: Warner Losh <imp@FreeBSD.org> > >>> CommitDate: 2025-09-15 03:03:45 +0000 > >>> > >>> iwlwifi: Don't compile for gcc before 14 > >>> > >>> gcc 13 and earlier don't have __builtin_bitcountg. The linux wifi > kpi > >>> uses this unconditionally. While in this one use, it might not be > >>> needed, I opted to not compile iwlwifi when building gcc12 or 13 > >> rather > >>> than risk breaking it for everbody else. > >>> > >>> With this change gcc12 builds the kernel. Maybe this will stop > jenkins > >>> email for every commit I make. > >>> > >>> Sponsored by: Netflix > >>> --- > >>> sys/modules/Makefile | 3 +++ > >>> 1 file changed, 3 insertions(+) > >>> > >>> diff --git a/sys/modules/Makefile b/sys/modules/Makefile > >>> index 5315d518afd8..f9fdbca78869 100644 > >>> --- a/sys/modules/Makefile > >>> +++ b/sys/modules/Makefile > >>> @@ -576,7 +576,10 @@ _mlx5ib= mlx5ib > >>> ${MACHINE_CPUARCH} == "i386" > >>> _ena= ena > >>> _gve= gve > >>> +# gcc13 and earlier lack __builtin_bitcountg used by linux emulation > >> > >> (a) I beleive there is no __builtin_bitcountg but you mean > >> __builtin_popcountg > >> both here and in the commit message. > >> > > > > Yes. > > > > (b) That was introduced in 7cbc4d875971860d941cc15d7f42e6cfeffbfe66 for > DRM > >> > >> (c) There is no direct use in any LinuxKPI based wireless driver: > >> % grep -r __builtin_popcountg sys/contrib/dev > >> % > >> > >> (4) iwlwifi only uses HWEIGHT32, which was changed by the aforementioned > >> commit > >> in LinuxKPI. > >> > > > > Yes. I noticed all that. Didn't see a trivial way to fix it right. > > > > (5) Please do it right and in the place where it is actually defined to > be > >> used, > >> in LinuxKPI, and not here as it can be easily fixed there with an > >> #ifdef or > >> otherwise as there were alternatives on the review if I remember > >> correctly. > >> > > > > Knock yourselves out. I was tired of the CI jobs whining and this was the > > easiest way to make that stop. I've got too many things on my plate to > > refine this more. If you want to support older gcc for this driver, > that's > > up to you. The ci jobs have been failing for weeks if not longer > > Okay, will do. > > But gcc12/13 had been whining for months on a lot of things constantly. > I really don't know how a day to do it right would have made a difference. > I thought this was right since I didn't see a good way to backport the macros to gcc12/13, but your point is well taken. But at the same time, I had a few minutes and this was the last thing in the way. Had there been a bunch more, I'd likely have gotten reviews for everything. Warner [-- Attachment #2 --] <div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote gmail_quote_container"><div dir="ltr" class="gmail_attr">On Mon, Sep 15, 2025 at 3:32 PM Bjoern A. Zeeb <<a href="mailto:bz@freebsd.org">bz@freebsd.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Mon, 15 Sep 2025, Warner Losh wrote:<br> <br> > On Mon, Sep 15, 2025, 1:26 PM Bjoern A. Zeeb <<a href="mailto:bz@freebsd.org" target="_blank">bz@freebsd.org</a>> wrote:<br> ><br> >> On Mon, 15 Sep 2025, Warner Losh wrote:<br> >><br> >>> The branch main has been updated by imp:<br> >>><br> >>> URL:<br> >> <a href="https://cgit.FreeBSD.org/src/commit/?id=5e0a4859f28ad4869f7a73faf42debc355a370bf" rel="noreferrer" target="_blank">https://cgit.FreeBSD.org/src/commit/?id=5e0a4859f28ad4869f7a73faf42debc355a370bf</a><br>; >>><br> >>> commit 5e0a4859f28ad4869f7a73faf42debc355a370bf<br> >>> Author: Warner Losh <imp@FreeBSD.org><br> >>> AuthorDate: 2025-09-14 18:03:16 +0000<br> >>> Commit: Warner Losh <imp@FreeBSD.org><br> >>> CommitDate: 2025-09-15 03:03:45 +0000<br> >>><br> >>> iwlwifi: Don't compile for gcc before 14<br> >>><br> >>> gcc 13 and earlier don't have __builtin_bitcountg. The linux wifi kpi<br> >>> uses this unconditionally. While in this one use, it might not be<br> >>> needed, I opted to not compile iwlwifi when building gcc12 or 13<br> >> rather<br> >>> than risk breaking it for everbody else.<br> >>><br> >>> With this change gcc12 builds the kernel. Maybe this will stop jenkins<br> >>> email for every commit I make.<br> >>><br> >>> Sponsored by: Netflix<br> >>> ---<br> >>> sys/modules/Makefile | 3 +++<br> >>> 1 file changed, 3 insertions(+)<br> >>><br> >>> diff --git a/sys/modules/Makefile b/sys/modules/Makefile<br> >>> index 5315d518afd8..f9fdbca78869 100644<br> >>> --- a/sys/modules/Makefile<br> >>> +++ b/sys/modules/Makefile<br> >>> @@ -576,7 +576,10 @@ _mlx5ib= mlx5ib<br> >>> ${MACHINE_CPUARCH} == "i386"<br> >>> _ena= ena<br> >>> _gve= gve<br> >>> +# gcc13 and earlier lack __builtin_bitcountg used by linux emulation<br> >><br> >> (a) I beleive there is no __builtin_bitcountg but you mean<br> >> __builtin_popcountg<br> >> both here and in the commit message.<br> >><br> ><br> > Yes.<br> ><br> > (b) That was introduced in 7cbc4d875971860d941cc15d7f42e6cfeffbfe66 for DRM<br> >><br> >> (c) There is no direct use in any LinuxKPI based wireless driver:<br> >> % grep -r __builtin_popcountg sys/contrib/dev<br> >> %<br> >><br> >> (4) iwlwifi only uses HWEIGHT32, which was changed by the aforementioned<br> >> commit<br> >> in LinuxKPI.<br> >><br> ><br> > Yes. I noticed all that. Didn't see a trivial way to fix it right.<br> ><br> > (5) Please do it right and in the place where it is actually defined to be<br> >> used,<br> >> in LinuxKPI, and not here as it can be easily fixed there with an<br> >> #ifdef or<br> >> otherwise as there were alternatives on the review if I remember<br> >> correctly.<br> >><br> ><br> > Knock yourselves out. I was tired of the CI jobs whining and this was the<br> > easiest way to make that stop. I've got too many things on my plate to<br> > refine this more. If you want to support older gcc for this driver, that's<br> > up to you. The ci jobs have been failing for weeks if not longer<br> <br> Okay, will do.<br> <br> But gcc12/13 had been whining for months on a lot of things constantly.<br> I really don't know how a day to do it right would have made a difference.<br></blockquote><div><br></div><div>I thought this was right since I didn't see a good way to backport the macros to gcc12/13, but your point is well taken. But at the same time, I had a few minutes and this was the last thing in the way. Had there been a bunch more, I'd likely have gotten reviews for everything.</div><div><br></div><div>Warner</div></div></div>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CANCZdfo6Q9cGFQAMXREvSgNEtC-9HfyxCJcMcoNXO5F8ot973g>