xen: Drop logic for old clang versions.

[XEN PATCH] xen: Drop logic for old clang versions.

Posted by nicola.vetrini@gmail.com 2 months, 3 weeks ago

From: Nicola Vetrini <nicola.vetrini@bugseng.com>

The enforced toolchain baseline for clang is version 11,
therefore this logic is effectively dead code.

No functional change.

Signed-off-by: Nicola Vetrini <nicola.vetrini@gmail.com>
---
Mentioned in https://gitlab.com/xen-project/xen/-/issues/201
---
 xen/common/coverage/llvm.c   | 4 ----
 xen/include/xen/self-tests.h | 9 +--------
 2 files changed, 1 insertion(+), 12 deletions(-)

diff --git a/xen/common/coverage/llvm.c b/xen/common/coverage/llvm.c
index 50d7a3c5d301..517b2aa8c202 100644
--- a/xen/common/coverage/llvm.c
+++ b/xen/common/coverage/llvm.c
@@ -44,12 +44,8 @@
     ((uint64_t)'f' << 16) | ((uint64_t)'R' << 8)  | ((uint64_t)129)
 #endif
 
-#if __clang_major__ >= 4 || (__clang_major__ == 3 && __clang_minor__ >= 9)
 #define LLVM_PROFILE_VERSION    4
 #define LLVM_PROFILE_NUM_KINDS  2
-#else
-#error "clang version not supported with coverage"
-#endif
 
 struct llvm_profile_data {
     uint64_t name_ref;
diff --git a/xen/include/xen/self-tests.h b/xen/include/xen/self-tests.h
index bd8a4867aa40..c57cceb3b962 100644
--- a/xen/include/xen/self-tests.h
+++ b/xen/include/xen/self-tests.h
@@ -10,16 +10,12 @@
 #include <xen/lib.h>
 
 /*
- * Check that fn(val) can be calcuated by the compiler, and that it gives the
+ * Check that fn(val) can be calculated by the compiler, and that it gives the
  * expected answer.
  *
- * Clang < 8 can't fold constants through static inlines, causing this to
- * fail.  Simply skip it for incredibly old compilers.
- *
  * N.B. fn is intentionally not bracketed to allow us to test function-like
  * macros too.
  */
-#if !defined(CONFIG_CC_IS_CLANG) || CONFIG_CLANG_VERSION >= 80000
 #define COMPILE_CHECK(fn, val, res)                                     \
     do {                                                                \
         typeof(fn(val)) real = fn(val);                                 \
@@ -29,9 +25,6 @@
         else if ( real != (res) )                                       \
             BUILD_ERROR("Compile time check '" STR(fn(val) == res) "' failed"); \
     } while ( 0 )
-#else
-#define COMPILE_CHECK(fn, val, res)
-#endif
 
 /*
  * Check that Xen's runtime logic for fn(val) gives the expected answer.  This
-- 
2.43.0

Re: [XEN PATCH] xen: Drop logic for old clang versions.

Posted by Andrew Cooper 2 months, 2 weeks ago

On 10/08/2025 2:03 pm, nicola.vetrini@gmail.com wrote:
> From: Nicola Vetrini <nicola.vetrini@bugseng.com>
>
> The enforced toolchain baseline for clang is version 11,
> therefore this logic is effectively dead code.
>
> No functional change.
>
> Signed-off-by: Nicola Vetrini <nicola.vetrini@gmail.com>

Thanks for the patch, but your From and SoB lines don't match.  Which is
it supposed to be?

~Andrew

Re: [XEN PATCH] xen: Drop logic for old clang versions.

Posted by Nicola Vetrini 2 months, 2 weeks ago

On 2025-08-11 10:07, Andrew Cooper wrote:
> On 10/08/2025 2:03 pm, nicola.vetrini@gmail.com wrote:
>> From: Nicola Vetrini <nicola.vetrini@bugseng.com>
>> 
>> The enforced toolchain baseline for clang is version 11,
>> therefore this logic is effectively dead code.
>> 
>> No functional change.
>> 
>> Signed-off-by: Nicola Vetrini <nicola.vetrini@gmail.com>
> 
> Thanks for the patch, but your From and SoB lines don't match.  Which 
> is
> it supposed to be?
> 
> ~Andrew

The gmail one, git misconfiguration, sorry. Should I resend?

-- 
Nicola Vetrini, B.Sc.
Software Engineer
BUGSENG (https://bugseng.com)
LinkedIn: https://www.linkedin.com/in/nicola-vetrini-a42471253

Re: [XEN PATCH] xen: Drop logic for old clang versions.

Posted by Andrew Cooper 2 months, 2 weeks ago

On 11/08/2025 10:13 am, Nicola Vetrini wrote:
> On 2025-08-11 10:07, Andrew Cooper wrote:
>> On 10/08/2025 2:03 pm, nicola.vetrini@gmail.com wrote:
>>> From: Nicola Vetrini <nicola.vetrini@bugseng.com>
>>>
>>> The enforced toolchain baseline for clang is version 11,
>>> therefore this logic is effectively dead code.
>>>
>>> No functional change.
>>>
>>> Signed-off-by: Nicola Vetrini <nicola.vetrini@gmail.com>
>>
>> Thanks for the patch, but your From and SoB lines don't match.  Which is
>> it supposed to be?
>>
>> ~Andrew
>
> The gmail one, git misconfiguration, sorry. Should I resend?
>

No need.  I can fix up on commit.  Thanks.

~Andrew

Re: [XEN PATCH] xen: Drop logic for old clang versions.

Posted by Roger Pau Monné 2 months, 2 weeks ago

On Sun, Aug 10, 2025 at 03:03:53PM +0200, nicola.vetrini@gmail.com wrote:
> From: Nicola Vetrini <nicola.vetrini@bugseng.com>
> 
> The enforced toolchain baseline for clang is version 11,
> therefore this logic is effectively dead code.
> 
> No functional change.
> 
> Signed-off-by: Nicola Vetrini <nicola.vetrini@gmail.com>

Acked-by: Roger Pau Monné <roger.pau@citrix.com>

> ---
> Mentioned in https://gitlab.com/xen-project/xen/-/issues/201
> ---
>  xen/common/coverage/llvm.c   | 4 ----
>  xen/include/xen/self-tests.h | 9 +--------
>  2 files changed, 1 insertion(+), 12 deletions(-)
> 
> diff --git a/xen/common/coverage/llvm.c b/xen/common/coverage/llvm.c
> index 50d7a3c5d301..517b2aa8c202 100644
> --- a/xen/common/coverage/llvm.c
> +++ b/xen/common/coverage/llvm.c
> @@ -44,12 +44,8 @@
>      ((uint64_t)'f' << 16) | ((uint64_t)'R' << 8)  | ((uint64_t)129)
>  #endif
>  
> -#if __clang_major__ >= 4 || (__clang_major__ == 3 && __clang_minor__ >= 9)
>  #define LLVM_PROFILE_VERSION    4
>  #define LLVM_PROFILE_NUM_KINDS  2
> -#else
> -#error "clang version not supported with coverage"
> -#endif

Rant: most of the LLVM coverage stuff is already kind of dead code, as
the format of the data changes between versions and there's no way for
LLVM to generate the blob itself using a builtin function or
similar.  We haven't kept up with new formats, and now it's not
possible to parse the output coverage data when using newish LLVM
versions.

Linux converts it's LLVM coverage data to gcov format (which AFAIK is
way more stable), and exports it in gcov format.  We should consider
importing that from Linux.

Thanks, Roger.

Re: [XEN PATCH] xen: Drop logic for old clang versions.

Posted by Nicola Vetrini 2 months, 2 weeks ago

On 2025-08-11 09:47, Roger Pau Monné wrote:
> On Sun, Aug 10, 2025 at 03:03:53PM +0200, nicola.vetrini@gmail.com 
> wrote:
>> From: Nicola Vetrini <nicola.vetrini@bugseng.com>
>> 
>> The enforced toolchain baseline for clang is version 11,
>> therefore this logic is effectively dead code.
>> 
>> No functional change.
>> 
>> Signed-off-by: Nicola Vetrini <nicola.vetrini@gmail.com>
> 
> Acked-by: Roger Pau Monné <roger.pau@citrix.com>
> 
>> ---
>> Mentioned in https://gitlab.com/xen-project/xen/-/issues/201
>> ---
>>  xen/common/coverage/llvm.c   | 4 ----
>>  xen/include/xen/self-tests.h | 9 +--------
>>  2 files changed, 1 insertion(+), 12 deletions(-)
>> 
>> diff --git a/xen/common/coverage/llvm.c b/xen/common/coverage/llvm.c
>> index 50d7a3c5d301..517b2aa8c202 100644
>> --- a/xen/common/coverage/llvm.c
>> +++ b/xen/common/coverage/llvm.c
>> @@ -44,12 +44,8 @@
>>      ((uint64_t)'f' << 16) | ((uint64_t)'R' << 8)  | ((uint64_t)129)
>>  #endif
>> 
>> -#if __clang_major__ >= 4 || (__clang_major__ == 3 && __clang_minor__ 
>> >= 9)
>>  #define LLVM_PROFILE_VERSION    4
>>  #define LLVM_PROFILE_NUM_KINDS  2
>> -#else
>> -#error "clang version not supported with coverage"
>> -#endif
> 
> Rant: most of the LLVM coverage stuff is already kind of dead code, as
> the format of the data changes between versions and there's no way for
> LLVM to generate the blob itself using a builtin function or
> similar.  We haven't kept up with new formats, and now it's not
> possible to parse the output coverage data when using newish LLVM
> versions.
> 
> Linux converts it's LLVM coverage data to gcov format (which AFAIK is
> way more stable), and exports it in gcov format.  We should consider
> importing that from Linux.
> 
> Thanks, Roger.

Fair point. Generally I found coverage reports generated using LLVM 
tooling a bit more informative than gcov's, but I don't know how much 
work would it be to adapt Xen for newer versions of LLVM.

-- 
Nicola Vetrini, B.Sc.
Software Engineer
BUGSENG (https://bugseng.com)
LinkedIn: https://www.linkedin.com/in/nicola-vetrini-a42471253

Re: [XEN PATCH] xen: Drop logic for old clang versions.

Posted by Roger Pau Monné 2 months, 2 weeks ago

On Mon, Aug 11, 2025 at 11:37:46AM +0200, Nicola Vetrini wrote:
> On 2025-08-11 09:47, Roger Pau Monné wrote:
> > On Sun, Aug 10, 2025 at 03:03:53PM +0200, nicola.vetrini@gmail.com
> > wrote:
> > > From: Nicola Vetrini <nicola.vetrini@bugseng.com>
> > > 
> > > The enforced toolchain baseline for clang is version 11,
> > > therefore this logic is effectively dead code.
> > > 
> > > No functional change.
> > > 
> > > Signed-off-by: Nicola Vetrini <nicola.vetrini@gmail.com>
> > 
> > Acked-by: Roger Pau Monné <roger.pau@citrix.com>
> > 
> > > ---
> > > Mentioned in https://gitlab.com/xen-project/xen/-/issues/201
> > > ---
> > >  xen/common/coverage/llvm.c   | 4 ----
> > >  xen/include/xen/self-tests.h | 9 +--------
> > >  2 files changed, 1 insertion(+), 12 deletions(-)
> > > 
> > > diff --git a/xen/common/coverage/llvm.c b/xen/common/coverage/llvm.c
> > > index 50d7a3c5d301..517b2aa8c202 100644
> > > --- a/xen/common/coverage/llvm.c
> > > +++ b/xen/common/coverage/llvm.c
> > > @@ -44,12 +44,8 @@
> > >      ((uint64_t)'f' << 16) | ((uint64_t)'R' << 8)  | ((uint64_t)129)
> > >  #endif
> > > 
> > > -#if __clang_major__ >= 4 || (__clang_major__ == 3 &&
> > > __clang_minor__ >= 9)
> > >  #define LLVM_PROFILE_VERSION    4
> > >  #define LLVM_PROFILE_NUM_KINDS  2
> > > -#else
> > > -#error "clang version not supported with coverage"
> > > -#endif
> > 
> > Rant: most of the LLVM coverage stuff is already kind of dead code, as
> > the format of the data changes between versions and there's no way for
> > LLVM to generate the blob itself using a builtin function or
> > similar.  We haven't kept up with new formats, and now it's not
> > possible to parse the output coverage data when using newish LLVM
> > versions.
> > 
> > Linux converts it's LLVM coverage data to gcov format (which AFAIK is
> > way more stable), and exports it in gcov format.  We should consider
> > importing that from Linux.
> > 
> > Thanks, Roger.
> 
> Fair point. Generally I found coverage reports generated using LLVM tooling
> a bit more informative than gcov's, but I don't know how much work would it
> be to adapt Xen for newer versions of LLVM.

It's a pain because they keep changing the blob format between
versions, so we would basically need code in Xen to be able to
generate the right blob for each possible clang version.

It would be helpful if clang provided the helpers to generate the
coverage data in a version agnostic way, but so far I haven't found a
way to do it.  I've raised a question with upstream LLVM project, but
no replies:

https://github.com/llvm/llvm-project/issues/123034

Regards, Roger.

Re: [XEN PATCH] xen: Drop logic for old clang versions.

Posted by Nicola Vetrini 2 months, 2 weeks ago

On 2025-08-11 16:35, Roger Pau Monné wrote:
> On Mon, Aug 11, 2025 at 11:37:46AM +0200, Nicola Vetrini wrote:
>> On 2025-08-11 09:47, Roger Pau Monné wrote:
>> > On Sun, Aug 10, 2025 at 03:03:53PM +0200, nicola.vetrini@gmail.com
>> > wrote:
>> > > From: Nicola Vetrini <nicola.vetrini@bugseng.com>
>> > >
>> > > The enforced toolchain baseline for clang is version 11,
>> > > therefore this logic is effectively dead code.
>> > >
>> > > No functional change.
>> > >
>> > > Signed-off-by: Nicola Vetrini <nicola.vetrini@gmail.com>
>> >
>> > Acked-by: Roger Pau Monné <roger.pau@citrix.com>
>> >
>> > > ---
>> > > Mentioned in https://gitlab.com/xen-project/xen/-/issues/201
>> > > ---
>> > >  xen/common/coverage/llvm.c   | 4 ----
>> > >  xen/include/xen/self-tests.h | 9 +--------
>> > >  2 files changed, 1 insertion(+), 12 deletions(-)
>> > >
>> > > diff --git a/xen/common/coverage/llvm.c b/xen/common/coverage/llvm.c
>> > > index 50d7a3c5d301..517b2aa8c202 100644
>> > > --- a/xen/common/coverage/llvm.c
>> > > +++ b/xen/common/coverage/llvm.c
>> > > @@ -44,12 +44,8 @@
>> > >      ((uint64_t)'f' << 16) | ((uint64_t)'R' << 8)  | ((uint64_t)129)
>> > >  #endif
>> > >
>> > > -#if __clang_major__ >= 4 || (__clang_major__ == 3 &&
>> > > __clang_minor__ >= 9)
>> > >  #define LLVM_PROFILE_VERSION    4
>> > >  #define LLVM_PROFILE_NUM_KINDS  2
>> > > -#else
>> > > -#error "clang version not supported with coverage"
>> > > -#endif
>> >
>> > Rant: most of the LLVM coverage stuff is already kind of dead code, as
>> > the format of the data changes between versions and there's no way for
>> > LLVM to generate the blob itself using a builtin function or
>> > similar.  We haven't kept up with new formats, and now it's not
>> > possible to parse the output coverage data when using newish LLVM
>> > versions.
>> >
>> > Linux converts it's LLVM coverage data to gcov format (which AFAIK is
>> > way more stable), and exports it in gcov format.  We should consider
>> > importing that from Linux.
>> >
>> > Thanks, Roger.
>> 
>> Fair point. Generally I found coverage reports generated using LLVM 
>> tooling
>> a bit more informative than gcov's, but I don't know how much work 
>> would it
>> be to adapt Xen for newer versions of LLVM.
> 
> It's a pain because they keep changing the blob format between
> versions, so we would basically need code in Xen to be able to
> generate the right blob for each possible clang version.
> 
> It would be helpful if clang provided the helpers to generate the
> coverage data in a version agnostic way, but so far I haven't found a
> way to do it.  I've raised a question with upstream LLVM project, but
> no replies:
> 
> https://github.com/llvm/llvm-project/issues/123034
> 
> Regards, Roger.

Perhaps the "coverage" and "llvm-cov" tag might help get more traction; 
there is at least one developer working on the source-based code 
coverage area in the past months with various improvements.

-- 
Nicola Vetrini, B.Sc.
Software Engineer
BUGSENG (https://bugseng.com)
LinkedIn: https://www.linkedin.com/in/nicola-vetrini-a42471253