Bump compiler versions, as GCC is expected to implement it
in version 15 and current clang-18 still has no support for
counted_by(). Also fix clang URL - the project has moved
to github.
Signed-off-by: Sergey Senozhatsky <[email protected]>
---
include/linux/compiler_attributes.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/include/linux/compiler_attributes.h b/include/linux/compiler_attributes.h
index 28566624f008..1829c24c3357 100644
--- a/include/linux/compiler_attributes.h
+++ b/include/linux/compiler_attributes.h
@@ -95,11 +95,11 @@
#endif
/*
- * Optional: only supported since gcc >= 14
- * Optional: only supported since clang >= 18
+ * Optional: only supported since gcc >= 15
+ * Optional: only supported since clang >= 19
*
* gcc: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108896
- * clang: https://reviews.llvm.org/D148381
+ * clang: https://github.com/llvm/llvm-project/pull/76348
*/
#if __has_attribute(__counted_by__)
# define __counted_by(member) __attribute__((__counted_by__(member)))
--
2.43.0.472.g3155946c3a-goog
On 1/9/24 07:36, Sergey Senozhatsky wrote:
> Bump compiler versions, as GCC is expected to implement it
> in version 15 and current clang-18 still has no support for
> counted_by(). Also fix clang URL - the project has moved
> to github.
>
> Signed-off-by: Sergey Senozhatsky <[email protected]>
Reviewed-by: Gustavo A. R. Silva <[email protected]>
Thanks!
--
Gustavo
> ---
> include/linux/compiler_attributes.h | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/compiler_attributes.h b/include/linux/compiler_attributes.h
> index 28566624f008..1829c24c3357 100644
> --- a/include/linux/compiler_attributes.h
> +++ b/include/linux/compiler_attributes.h
> @@ -95,11 +95,11 @@
> #endif
>
> /*
> - * Optional: only supported since gcc >= 14
> - * Optional: only supported since clang >= 18
> + * Optional: only supported since gcc >= 15
> + * Optional: only supported since clang >= 19
> *
> * gcc: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108896
> - * clang: https://reviews.llvm.org/D148381
> + * clang: https://github.com/llvm/llvm-project/pull/76348
> */
> #if __has_attribute(__counted_by__)
> # define __counted_by(member) __attribute__((__counted_by__(member)))
On Tue, Jan 09, 2024 at 10:36:24PM +0900, Sergey Senozhatsky wrote:
> Bump compiler versions, as GCC is expected to implement it
> in version 15 and current clang-18 still has no support for
> counted_by(). Also fix clang URL - the project has moved
> to github.
>
> Signed-off-by: Sergey Senozhatsky <[email protected]>
> ---
> include/linux/compiler_attributes.h | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/compiler_attributes.h b/include/linux/compiler_attributes.h
> index 28566624f008..1829c24c3357 100644
> --- a/include/linux/compiler_attributes.h
> +++ b/include/linux/compiler_attributes.h
> @@ -95,11 +95,11 @@
> #endif
>
> /*
> - * Optional: only supported since gcc >= 14
> - * Optional: only supported since clang >= 18
> + * Optional: only supported since gcc >= 15
> + * Optional: only supported since clang >= 19
It is still possible in theory for this feature to make clang-18, as the
release/18.x branch is not scheduled to be cut until the fourth Tuesday
in January, which is two weeks from now. I don't have a good feeling for
how close that pull request is to being mergeable though, so this is
fine for now. I assume this won't go to Linus immediately so we would
have time to change it if necessary.
The link change is obviously good.
Reviewed-by: Nathan Chancellor <[email protected]>
I should probably go through and try to update all the other
review.llvm.org links to GitHub as well...
> *
> * gcc: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108896
> - * clang: https://reviews.llvm.org/D148381
> + * clang: https://github.com/llvm/llvm-project/pull/76348
> */
> #if __has_attribute(__counted_by__)
> # define __counted_by(member) __attribute__((__counted_by__(member)))
> --
> 2.43.0.472.g3155946c3a-goog
>
On Tue, Jan 9, 2024 at 4:32 PM Nathan Chancellor <[email protected]> wrote:
>
> It is still possible in theory for this feature to make clang-18, as the
> release/18.x branch is not scheduled to be cut until the fourth Tuesday
> in January, which is two weeks from now. I don't have a good feeling for
> how close that pull request is to being mergeable though, so this is
> fine for now. I assume this won't go to Linus immediately so we would
> have time to change it if necessary.
Yeah, I was wondering about the deadline too. If LLVM's `-rc1` is the
latest time possible to merge it, we can wait the couple weeks (which
are conveniently the merge window) and I apply it afterwards with the
result :)
Cheers,
Miguel
On Tue, Jan 09, 2024 at 08:42:24PM +0100, Miguel Ojeda wrote:
> On Tue, Jan 9, 2024 at 4:32 PM Nathan Chancellor <[email protected]> wrote:
> >
> > It is still possible in theory for this feature to make clang-18, as the
> > release/18.x branch is not scheduled to be cut until the fourth Tuesday
> > in January, which is two weeks from now. I don't have a good feeling for
> > how close that pull request is to being mergeable though, so this is
> > fine for now. I assume this won't go to Linus immediately so we would
> > have time to change it if necessary.
>
> Yeah, I was wondering about the deadline too. If LLVM's `-rc1` is the
> latest time possible to merge it, we can wait the couple weeks (which
> are conveniently the merge window) and I apply it afterwards with the
> result :)
If I understand the doucmentation at [1] correctly, the first round of
testing starts with -rc1 and ends with -rc2, so if the feature is not
merged by -rc2, it won't make that release cycle. I think counted_by
might be a hard sell even after -rc1 because the feature is not exactly
small but it is also not expansive (it is relatively self contained
from what I can tell). So I think your plan is reasonable.
Another alternative would be to split this patch in to three distinct
patches, not sure if that would be overkill for this though.
1. Update the clang review link from reviews.llvm.org to github.com
2. Update the GCC version from 14 to 15.
3. Update the Clang version from 18 to 19.
The first two patches could be picked up immediately and the third one
could be sat on to see how the review and acceptance process works out
over the next couple of weeks. Up to you/Sergey. Thanks for taking a
look!
[1]: https://llvm.org/docs/HowToReleaseLLVM.html#release-process-summary
Cheers,
Nathan
On Tue, Jan 09, 2024 at 12:56:52PM -0700, Nathan Chancellor wrote:
> On Tue, Jan 09, 2024 at 08:42:24PM +0100, Miguel Ojeda wrote:
> > On Tue, Jan 9, 2024 at 4:32 PM Nathan Chancellor <[email protected]> wrote:
> > >
> > > It is still possible in theory for this feature to make clang-18, as the
> > > release/18.x branch is not scheduled to be cut until the fourth Tuesday
> > > in January, which is two weeks from now. I don't have a good feeling for
> > > how close that pull request is to being mergeable though, so this is
> > > fine for now. I assume this won't go to Linus immediately so we would
> > > have time to change it if necessary.
> >
> > Yeah, I was wondering about the deadline too. If LLVM's `-rc1` is the
> > latest time possible to merge it, we can wait the couple weeks (which
> > are conveniently the merge window) and I apply it afterwards with the
> > result :)
>
> If I understand the doucmentation at [1] correctly, the first round of
> testing starts with -rc1 and ends with -rc2, so if the feature is not
> merged by -rc2, it won't make that release cycle. I think counted_by
> might be a hard sell even after -rc1 because the feature is not exactly
> small but it is also not expansive (it is relatively self contained
> from what I can tell). So I think your plan is reasonable.
>
> Another alternative would be to split this patch in to three distinct
> patches, not sure if that would be overkill for this though.
>
> 1. Update the clang review link from reviews.llvm.org to github.com
> 2. Update the GCC version from 14 to 15.
> 3. Update the Clang version from 18 to 19.
>
> The first two patches could be picked up immediately and the third one
> could be sat on to see how the review and acceptance process works out
> over the next couple of weeks. Up to you/Sergey. Thanks for taking a
> look!
Yeah, I think either the above split or just wait until the Clang 18
cut, since we've got a while before the next kernel merge window.
--
Kees Cook
On (24/01/09 13:12), Kees Cook wrote:
> > If I understand the doucmentation at [1] correctly, the first round of
> > testing starts with -rc1 and ends with -rc2, so if the feature is not
> > merged by -rc2, it won't make that release cycle. I think counted_by
> > might be a hard sell even after -rc1 because the feature is not exactly
> > small but it is also not expansive (it is relatively self contained
> > from what I can tell). So I think your plan is reasonable.
> >
> > Another alternative would be to split this patch in to three distinct
> > patches, not sure if that would be overkill for this though.
> >
> > 1. Update the clang review link from reviews.llvm.org to github.com
> > 2. Update the GCC version from 14 to 15.
> > 3. Update the Clang version from 18 to 19.
> >
> > The first two patches could be picked up immediately and the third one
> > could be sat on to see how the review and acceptance process works out
> > over the next couple of weeks. Up to you/Sergey. Thanks for taking a
> > look!
>
> Yeah, I think either the above split or just wait until the Clang 18
> cut, since we've got a while before the next kernel merge window.
Thanks everyone for the comments!
I'd probably prefer to split the patch and take the obvious and
trivial fixups now, and once clang -rc2 is tagged we can return
to clang min version requirement (if we won't forget :))
On (24/01/09 08:32), Nathan Chancellor wrote:
>
> I should probably go through and try to update all the other
> review.llvm.org links to GitHub as well...
It might be something wrong only with the counted_by() link
(https://reviews.llvm.org/D148381) as other reviews.llvm.org
links seem to be working fine.
I randomly tested https://reviews.llvm.org/D75044 and
https://reviews.llvm.org/D123515 and they don't 404, unlike
counted_by() link.