Skip site navigation (1)Skip section navigation (2)
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
--000000000000f6dd5e063edde708
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Mon, Sep 15, 2025 at 3:32=E2=80=AFPM Bjoern A. Zeeb <bz@freebsd.org> wro=
te:

> On Mon, 15 Sep 2025, Warner Losh wrote:
>
> > On Mon, Sep 15, 2025, 1:26=E2=80=AFPM Bjoern A. Zeeb <bz@freebsd.org> w=
rote:
> >
> >> On Mon, 15 Sep 2025, Warner Losh wrote:
> >>
> >>> The branch main has been updated by imp:
> >>>
> >>> URL:
> >>
> https://cgit.FreeBSD.org/src/commit/?id=3D5e0a4859f28ad4869f7a73faf42debc=
355a370bf
> >>>
> >>> 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=3D mlx5ib
> >>>     ${MACHINE_CPUARCH} =3D=3D "i386"
> >>> _ena=3D         ena
> >>> _gve=3D         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 aforemention=
ed
> >> 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 t=
he
> > 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

--000000000000f6dd5e063edde708
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div dir=3D"ltr"><br></div><br><div class=3D"gmail_quote g=
mail_quote_container"><div dir=3D"ltr" class=3D"gmail_attr">On Mon, Sep 15,=
 2025 at 3:32=E2=80=AFPM Bjoern A. Zeeb &lt;<a href=3D"mailto:bz@freebsd.or=
g">bz@freebsd.org</a>&gt; wrote:<br></div><blockquote class=3D"gmail_quote"=
 style=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);p=
adding-left:1ex">On Mon, 15 Sep 2025, Warner Losh wrote:<br>
<br>
&gt; On Mon, Sep 15, 2025, 1:26=E2=80=AFPM Bjoern A. Zeeb &lt;<a href=3D"ma=
ilto:bz@freebsd.org" target=3D"_blank">bz@freebsd.org</a>&gt; wrote:<br>
&gt;<br>
&gt;&gt; On Mon, 15 Sep 2025, Warner Losh wrote:<br>
&gt;&gt;<br>
&gt;&gt;&gt; The branch main has been updated by imp:<br>
&gt;&gt;&gt;<br>
&gt;&gt;&gt; URL:<br>
&gt;&gt; <a href=3D"https://cgit.FreeBSD.org/src/commit/?id=3D5e0a4859f28ad=
4869f7a73faf42debc355a370bf" rel=3D"noreferrer" target=3D"_blank">https://c=
git.FreeBSD.org/src/commit/?id=3D5e0a4859f28ad4869f7a73faf42debc355a370bf</=
a><br>
&gt;&gt;&gt;<br>
&gt;&gt;&gt; commit 5e0a4859f28ad4869f7a73faf42debc355a370bf<br>
&gt;&gt;&gt; Author:=C2=A0 =C2=A0 =C2=A0Warner Losh &lt;imp@FreeBSD.org&gt;=
<br>
&gt;&gt;&gt; AuthorDate: 2025-09-14 18:03:16 +0000<br>
&gt;&gt;&gt; Commit:=C2=A0 =C2=A0 =C2=A0Warner Losh &lt;imp@FreeBSD.org&gt;=
<br>
&gt;&gt;&gt; CommitDate: 2025-09-15 03:03:45 +0000<br>
&gt;&gt;&gt;<br>
&gt;&gt;&gt;=C2=A0 =C2=A0 iwlwifi: Don&#39;t compile for gcc before 14<br>
&gt;&gt;&gt;<br>
&gt;&gt;&gt;=C2=A0 =C2=A0 gcc 13 and earlier don&#39;t have __builtin_bitco=
untg. The linux wifi kpi<br>
&gt;&gt;&gt;=C2=A0 =C2=A0 uses this unconditionally. While in this one use,=
 it might not be<br>
&gt;&gt;&gt;=C2=A0 =C2=A0 needed, I opted to not compile iwlwifi when build=
ing gcc12 or 13<br>
&gt;&gt; rather<br>
&gt;&gt;&gt;=C2=A0 =C2=A0 than risk breaking it for everbody else.<br>
&gt;&gt;&gt;<br>
&gt;&gt;&gt;=C2=A0 =C2=A0 With this change gcc12 builds the kernel. Maybe t=
his will stop jenkins<br>
&gt;&gt;&gt;=C2=A0 =C2=A0 email for every commit I make.<br>
&gt;&gt;&gt;<br>
&gt;&gt;&gt;=C2=A0 =C2=A0 Sponsored by:=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0Netflix<br>
&gt;&gt;&gt; ---<br>
&gt;&gt;&gt; sys/modules/Makefile | 3 +++<br>
&gt;&gt;&gt; 1 file changed, 3 insertions(+)<br>
&gt;&gt;&gt;<br>
&gt;&gt;&gt; diff --git a/sys/modules/Makefile b/sys/modules/Makefile<br>
&gt;&gt;&gt; index 5315d518afd8..f9fdbca78869 100644<br>
&gt;&gt;&gt; --- a/sys/modules/Makefile<br>
&gt;&gt;&gt; +++ b/sys/modules/Makefile<br>
&gt;&gt;&gt; @@ -576,7 +576,10 @@ _mlx5ib=3D mlx5ib<br>
&gt;&gt;&gt;=C2=A0 =C2=A0 =C2=A0${MACHINE_CPUARCH} =3D=3D &quot;i386&quot;<=
br>
&gt;&gt;&gt; _ena=3D=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0ena<br>
&gt;&gt;&gt; _gve=3D=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0gve<br>
&gt;&gt;&gt; +# gcc13 and earlier lack __builtin_bitcountg used by linux em=
ulation<br>
&gt;&gt;<br>
&gt;&gt; (a) I beleive there is no __builtin_bitcountg but you mean<br>
&gt;&gt; __builtin_popcountg<br>
&gt;&gt; both here and in the commit message.<br>
&gt;&gt;<br>
&gt;<br>
&gt; Yes.<br>
&gt;<br>
&gt; (b) That was introduced in 7cbc4d875971860d941cc15d7f42e6cfeffbfe66 fo=
r DRM<br>
&gt;&gt;<br>
&gt;&gt; (c) There is no direct use in any LinuxKPI based wireless driver:<=
br>
&gt;&gt; % grep -r __builtin_popcountg sys/contrib/dev<br>
&gt;&gt; %<br>
&gt;&gt;<br>
&gt;&gt; (4) iwlwifi only uses HWEIGHT32, which was changed by the aforemen=
tioned<br>
&gt;&gt; commit<br>
&gt;&gt;=C2=A0 =C2=A0 =C2=A0 in LinuxKPI.<br>
&gt;&gt;<br>
&gt;<br>
&gt; Yes. I noticed all that. Didn&#39;t see a trivial way to fix it right.=
<br>
&gt;<br>
&gt; (5) Please do it right and in the place where it is actually defined t=
o be<br>
&gt;&gt; used,<br>
&gt;&gt;=C2=A0 =C2=A0 =C2=A0 in LinuxKPI, and not here as it can be easily =
fixed there with an<br>
&gt;&gt; #ifdef or<br>
&gt;&gt;=C2=A0 =C2=A0 =C2=A0 otherwise as there were alternatives on the re=
view if I remember<br>
&gt;&gt; correctly.<br>
&gt;&gt;<br>
&gt;<br>
&gt; Knock yourselves out. I was tired of the CI jobs whining and this was =
the<br>
&gt; easiest way to make that stop. I&#39;ve got too many things on my plat=
e to<br>
&gt; refine this more. If you want to support older gcc for this driver, th=
at&#39;s<br>
&gt; 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&#39;t know how a day to do it right would have made a differen=
ce.<br></blockquote><div><br></div><div>I thought this was right since I di=
dn&#39;t see a good way to backport the macros to gcc12/13, but your point =
is well taken.=C2=A0 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&#39;d likely hav=
e gotten reviews for everything.</div><div><br></div><div>Warner</div></div=
></div>

--000000000000f6dd5e063edde708--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CANCZdfo6Q9cGFQAMXREvSgNEtC-9HfyxCJcMcoNXO5F8ot973g>