* Re: [PATCH RFC] riscv: Allow to build only with LLVM >= 17.0.0
2024-07-17 11:41 ` Alexandre Ghiti
@ 2024-07-17 12:06 ` Conor Dooley
2024-07-17 13:06 ` Conor Dooley
2024-07-17 12:43 ` Conor Dooley
2024-07-17 14:20 ` Alexandre Ghiti
2 siblings, 1 reply; 7+ messages in thread
From: Conor Dooley @ 2024-07-17 12:06 UTC (permalink / raw)
To: Alexandre Ghiti
Cc: Alexandre Ghiti, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Björn Töpel, Conor Dooley, Nathan Chancellor,
linux-kernel, linux-riscv, kernel test robot
[-- Attachment #1.1: Type: text/plain, Size: 3219 bytes --]
On Wed, Jul 17, 2024 at 01:41:23PM +0200, Alexandre Ghiti wrote:
> Hi Conor,
>
> On 17/07/2024 13:32, Conor Dooley wrote:
> > On Wed, Jul 17, 2024 at 01:17:16PM +0200, Alexandre Ghiti wrote:
> > > The following build failure happens when using LLVM < 17.0.0:
> > >
> > > kernel/sched/core.c:11873:7: error: cannot jump from this asm goto statement to one of its possible targets
> > >
> > > This is a known issue [1] so let's upgrade the minimal requirement for
> > > LLVM to the version 17.0.0, which is the first version to contain the
> > > fix.
> > I think doing this unilaterally is kinda insane, LLVM 17 isn't even a
> > year old. Debian testing doesn't have anything later than 16.
>
>
> Debian will very likely select the qspinlocks when available anyway, so
> they'll need llvm >= 17. And Debian won't ship a kernel >= 6.11 until some
> time right? So they'll probably update their infra to llvm >= 17 (and
> they'll probably do to take advantages of the new extensions).
What I mean is that you are going to prevent people building the kernel
with llvm on machines running anything but very recent rolling-release
distros. Your patch would stop most developers, including those who don't
care about your qspinlock stuff, even build testing with the version of
LLVM that their distro provides. I'm not talking about distros building
kernels in their build infrastructure.
>
>
> > Why does
> > it need to be done unilaterally rather than just when the qspinlock
> > stuff is built?
>
>
> We can do that indeed, it may happen again and we can keep requiring llvm 17
> on a per-config basis.
>
>
> > > Link: https://github.com/ClangBuiltLinux/linux/issues/1886#issuecomment-1645979992 [1]
> > > Reported-by: kernel test robot <lkp@intel.com>
> > > Closes: https://lore.kernel.org/oe-kbuild-all/202407041157.odTZAYZ6-lkp@intel.com/
> > > Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> > If Nathan wrote the patch, you need to set him as the author of the
> > patch :)
>
>
> I thought I did, how should I do that then?
>
>
> >
> > > Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
> > > ---
> > >
> > > This patch was done by Nathan, I'm just sending it as an RFC to get quicker
> > > feedbacks.
> > >
> > > I tested it successfully.
> > >
> > > Note that the build failure happens on the not-yet merged qspinlock
> > > patchset.
> > >
> > > scripts/min-tool-version.sh | 2 ++
> > > 1 file changed, 2 insertions(+)
> > >
> > > diff --git a/scripts/min-tool-version.sh b/scripts/min-tool-version.sh
> > > index 91c91201212c..e81eb7ed257d 100755
> > > --- a/scripts/min-tool-version.sh
> > > +++ b/scripts/min-tool-version.sh
> > > @@ -28,6 +28,8 @@ llvm)
> > > echo 15.0.0
> > > elif [ "$SRCARCH" = loongarch ]; then
> > > echo 18.0.0
> > > + elif [ "$SRCARCH" = riscv ]; then
> > > + echo 17.0.0
> > > else
> > > echo 13.0.1
> > > fi
> > > --
> > > 2.39.2
> > >
> > >
> > >
> > > _______________________________________________
> > > linux-riscv mailing list
> > > linux-riscv@lists.infradead.org
> > > http://lists.infradead.org/mailman/listinfo/linux-riscv
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
[-- Attachment #2: Type: text/plain, Size: 161 bytes --]
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH RFC] riscv: Allow to build only with LLVM >= 17.0.0
2024-07-17 12:06 ` Conor Dooley
@ 2024-07-17 13:06 ` Conor Dooley
0 siblings, 0 replies; 7+ messages in thread
From: Conor Dooley @ 2024-07-17 13:06 UTC (permalink / raw)
To: Alexandre Ghiti
Cc: Alexandre Ghiti, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Björn Töpel, Conor Dooley, Nathan Chancellor,
linux-kernel, linux-riscv, kernel test robot
[-- Attachment #1.1: Type: text/plain, Size: 1917 bytes --]
On Wed, Jul 17, 2024 at 01:06:39PM +0100, Conor Dooley wrote:
> On Wed, Jul 17, 2024 at 01:41:23PM +0200, Alexandre Ghiti wrote:
> > Hi Conor,
> >
> > On 17/07/2024 13:32, Conor Dooley wrote:
> > > On Wed, Jul 17, 2024 at 01:17:16PM +0200, Alexandre Ghiti wrote:
> > > > The following build failure happens when using LLVM < 17.0.0:
> > > >
> > > > kernel/sched/core.c:11873:7: error: cannot jump from this asm goto statement to one of its possible targets
> > > >
> > > > This is a known issue [1] so let's upgrade the minimal requirement for
> > > > LLVM to the version 17.0.0, which is the first version to contain the
> > > > fix.
> > > I think doing this unilaterally is kinda insane, LLVM 17 isn't even a
> > > year old. Debian testing doesn't have anything later than 16.
> >
> >
> > Debian will very likely select the qspinlocks when available anyway, so
> > they'll need llvm >= 17. And Debian won't ship a kernel >= 6.11 until some
> > time right? So they'll probably update their infra to llvm >= 17 (and
> > they'll probably do to take advantages of the new extensions).
>
> What I mean is that you are going to prevent people building the kernel
> with llvm on machines running anything but very recent rolling-release
> distros. Your patch would stop most developers, including those who don't
> care about your qspinlock stuff, even build testing with the version of
> LLVM that their distro provides. I'm not talking about distros building
> kernels in their build infrastructure.
>
> >
> >
> > > Why does
> > > it need to be done unilaterally rather than just when the qspinlock
> > > stuff is built?
> >
> >
> > We can do that indeed, it may happen again and we can keep requiring llvm 17
> > on a per-config basis.
Nathan pointed out to me that I misunderstood the build failure, and
that it happens whether or not the option is enabled. /sigh.
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
[-- Attachment #2: Type: text/plain, Size: 161 bytes --]
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH RFC] riscv: Allow to build only with LLVM >= 17.0.0
2024-07-17 11:41 ` Alexandre Ghiti
2024-07-17 12:06 ` Conor Dooley
@ 2024-07-17 12:43 ` Conor Dooley
2024-07-17 14:20 ` Alexandre Ghiti
2 siblings, 0 replies; 7+ messages in thread
From: Conor Dooley @ 2024-07-17 12:43 UTC (permalink / raw)
To: Alexandre Ghiti
Cc: Alexandre Ghiti, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Björn Töpel, Conor Dooley, Nathan Chancellor,
linux-kernel, linux-riscv, kernel test robot
[-- Attachment #1.1: Type: text/plain, Size: 2315 bytes --]
On Wed, Jul 17, 2024 at 01:41:23PM +0200, Alexandre Ghiti wrote:
> > > Link: https://github.com/ClangBuiltLinux/linux/issues/1886#issuecomment-1645979992 [1]
> > > Reported-by: kernel test robot <lkp@intel.com>
> > > Closes: https://lore.kernel.org/oe-kbuild-all/202407041157.odTZAYZ6-lkp@intel.com/
> > > Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> > If Nathan wrote the patch, you need to set him as the author of the
> > patch :)
>
>
> I thought I did, how should I do that then?
$ shazam 20240717-synapse-decade-a0d41bd7afce@spud
Grabbing thread from lore.kernel.org/all/20240717-synapse-decade-a0d41bd7afce@spud/t.mbox.gz
Checking for newer revisions
Grabbing search results from lore.kernel.org
Analyzing 4 messages in the thread
Looking for additional code-review trailers on lore.kernel.org
Checking attestation on all messages, may take a moment...
---
✓ [PATCH RFC] riscv: Allow to build only with LLVM >= 17.0.0
+ Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
✓ Signed: DKIM/rivosinc-com.20230601.gappssmtp.com (From: alexghiti@rivosinc.com)
---
Total patches: 1
---
Applying: riscv: Allow to build only with LLVM >= 17.0.0
$ git show
commit f64f8420ca518b5dde35224cfff7ccfd14e5b496 (HEAD)
Author: Alexandre Ghiti <alexghiti@rivosinc.com>
Date: Wed Jul 17 13:17:16 2024 +0200
riscv: Allow to build only with LLVM >= 17.0.0
The following build failure happens when using LLVM < 17.0.0:
kernel/sched/core.c:11873:7: error: cannot jump from this asm goto statement to one of its possible targets
This is a known issue [1] so let's upgrade the minimal requirement for
LLVM to the version 17.0.0, which is the first version to contain the
fix.
Link: https://github.com/ClangBuiltLinux/linux/issues/1886#issuecomment-1645979992 [1]
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202407041157.odTZAYZ6-lkp@intel.com/
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
When you commited it, you needed to set --author. Just adding his
signoff is not sufficient.
Cheers,
Conor.
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
[-- Attachment #2: Type: text/plain, Size: 161 bytes --]
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH RFC] riscv: Allow to build only with LLVM >= 17.0.0
2024-07-17 11:41 ` Alexandre Ghiti
2024-07-17 12:06 ` Conor Dooley
2024-07-17 12:43 ` Conor Dooley
@ 2024-07-17 14:20 ` Alexandre Ghiti
2 siblings, 0 replies; 7+ messages in thread
From: Alexandre Ghiti @ 2024-07-17 14:20 UTC (permalink / raw)
To: Conor Dooley, Alexandre Ghiti
Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Björn Töpel,
Conor Dooley, Nathan Chancellor, linux-kernel, linux-riscv,
kernel test robot
On 17/07/2024 13:41, Alexandre Ghiti wrote:
> Hi Conor,
>
> On 17/07/2024 13:32, Conor Dooley wrote:
>> On Wed, Jul 17, 2024 at 01:17:16PM +0200, Alexandre Ghiti wrote:
>>> The following build failure happens when using LLVM < 17.0.0:
>>>
>>> kernel/sched/core.c:11873:7: error: cannot jump from this asm goto
>>> statement to one of its possible targets
>>>
>>> This is a known issue [1] so let's upgrade the minimal requirement for
>>> LLVM to the version 17.0.0, which is the first version to contain the
>>> fix.
>> I think doing this unilaterally is kinda insane, LLVM 17 isn't even a
>> year old. Debian testing doesn't have anything later than 16.
>
>
> Debian will very likely select the qspinlocks when available anyway,
> so they'll need llvm >= 17. And Debian won't ship a kernel >= 6.11
> until some time right? So they'll probably update their infra to llvm
> >= 17 (and they'll probably do to take advantages of the new extensions).
>
>
>> Why does
>> it need to be done unilaterally rather than just when the qspinlock
>> stuff is built?
>
>
> We can do that indeed, it may happen again and we can keep requiring
> llvm 17 on a per-config basis.
>
>
>>> Link:
>>> https://github.com/ClangBuiltLinux/linux/issues/1886#issuecomment-1645979992
>>> [1]
>>> Reported-by: kernel test robot <lkp@intel.com>
>>> Closes:
>>> https://lore.kernel.org/oe-kbuild-all/202407041157.odTZAYZ6-lkp@intel.com/
>>> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
>> If Nathan wrote the patch, you need to set him as the author of the
>> patch :)
>
>
> I thought I did, how should I do that then?
>
>
>>
>>> Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
>>> ---
>>>
>>> This patch was done by Nathan, I'm just sending it as an RFC to get
>>> quicker
>>> feedbacks.
>>>
>>> I tested it successfully.
>>>
>>> Note that the build failure happens on the not-yet merged qspinlock
>>> patchset.
>>>
>>> scripts/min-tool-version.sh | 2 ++
>>> 1 file changed, 2 insertions(+)
>>>
>>> diff --git a/scripts/min-tool-version.sh b/scripts/min-tool-version.sh
>>> index 91c91201212c..e81eb7ed257d 100755
>>> --- a/scripts/min-tool-version.sh
>>> +++ b/scripts/min-tool-version.sh
>>> @@ -28,6 +28,8 @@ llvm)
>>> echo 15.0.0
>>> elif [ "$SRCARCH" = loongarch ]; then
>>> echo 18.0.0
>>> + elif [ "$SRCARCH" = riscv ]; then
>>> + echo 17.0.0
>>> else
>>> echo 13.0.1
>>> fi
>>> --
>>> 2.39.2
>>>
>>>
>>>
>>> _______________________________________________
>>> linux-riscv mailing list
>>> linux-riscv@lists.infradead.org
>>> http://lists.infradead.org/mailman/listinfo/linux-riscv
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv
So we discussed that during the patchwork meeting and this patch is not
wanted, the idea is rather to get rid of the build error.
I was given a few ideas so I'll try those and we'll see what the
resulting code looks like, hopefully not ugly otherwise I'll re-open the
discussion :)
Thanks Nathan for the patch,
Alex
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 7+ messages in thread