On Fri, Oct 24, 2025 at 4:33 AM Andrew Cooper <[email protected]> wrote: > > The subject should have a "xen: " prefix, as this applies to the > hypervisor and not other > > On 24/10/2025 1:16 am, Saman Dehghan wrote: > > This change enables compatibility for measuring code coverage > > with Clang versions 14 through 20 by supporting their > > Stale 14? It looks to be 11 now.
Sorry for the mistake. I meant "with Clang versions 11 through 20 by supporting their...". > > > respective raw profile formats. > > > > 1- Added support for LLVM raw profile versions 5, 6, 7, 8, 9, and 10. > > 2- Initialized llvm_profile_header for all versions based on llvm source > > code in > > compiler-rt/include/profile/InstrProfData.inc for each version. > > 3- We tested this patch for all Clang versions from 11 through 20 on x86 > > platform. > > 4- Fixed linking warnings related to coverage code in x86. > > > > Signed-off-by: Saman Dehghan <[email protected]> > > --- > > When sending multiple revisions, it's customary to put a short list here > if what you've changed from the previous revision. Changes since version 2: 1- Additionally support raw profile version 5, 6, 7 in clang 11, 12, 13. 2- Fix coverage related linking warnings in x86. 3- Revert unnecessary type changes, casting, etc. > > Also, you didn't accumulate your release ack from v2. > > Release-Acked-By: Oleksii Kurochko <[email protected]> Sorry we missed this. Thanks for reminding us. > > > xen/arch/x86/xen.lds.S | 6 ++++ > > xen/common/coverage/llvm.c | 73 ++++++++++++++++++++++++++++++++++---- > > xen/include/xen/xen.lds.h | 18 ++++++++++ > > 3 files changed, 91 insertions(+), 6 deletions(-) > > > > diff --git a/xen/common/coverage/llvm.c b/xen/common/coverage/llvm.c > > index 517b2aa8c2..e3272a546f 100644 > > --- a/xen/common/coverage/llvm.c > > +++ b/xen/common/coverage/llvm.c > > @@ -107,11 +145,34 @@ static int cf_check dump( > > struct llvm_profile_header header = { > > .magic = LLVM_PROFILE_MAGIC, > > .version = LLVM_PROFILE_VERSION, > > - .data_size = (END_DATA - START_DATA) / sizeof(struct > > llvm_profile_data), > > - .counters_size = (END_COUNTERS - START_COUNTERS) / > > sizeof(uint64_t), > > +#if __clang_major__ >= 13 > > + .binary_ids_size = 0, > > +#endif > > + .num_data = ((END_DATA + sizeof(struct llvm_profile_data) - 1) > > + - START_DATA) / sizeof(struct llvm_profile_data), > > There's a helper for this expression. > > DIV_ROUND_UP(END_DATA - START_DATA, sizeof(llvm_profile_data)) > > > + .padding_bytes_before_counters = 0, > > + .num_counters = ((END_COUNTERS + sizeof(uint64_t) - 1) > > + - START_COUNTERS) / sizeof(uint64_t), > > DIV_ROUND_UP(END_COUNTERS - START_COUNTERS, sizeof(uint64_t)) > > > > + .padding_bytes_after_counters = 0, > > +#if __clang_major__ >= 18 > > + .num_bitmap_bytes = 0, > > + .padding_bytes_after_bitmap_bytes = 0, > > +#endif > > .names_size = END_NAMES - START_NAMES, > > +#if __clang_major__ >= 14 > > + .counters_delta = START_COUNTERS - START_DATA, > > +#else > > .counters_delta = (uintptr_t)START_COUNTERS, > > +#endif > > + > > +#if __clang_major__ >= 18 > > + .bitmap_delta = 0, > > +#endif > > .names_delta = (uintptr_t)START_NAMES, > > +#if __clang_major__ >= 19 && __clang_major__ <= 20 > > + .num_vtables = 0, > > + .vnames_size = 0, > > +#endif > > Because this is a structure initialiser, everything set explicitly to 0 > can be omitted. This removes all #ifdef-ary except the .counters_delta > I believe, as well as the .padding_byte_* fields. > Is it undefined behaviour to leave struct members uninitialized for local variables? > The resulting diff is far smaller. > > > .value_kind_last = LLVM_PROFILE_NUM_KINDS - 1, > > }; > > unsigned int off = 0; > > diff --git a/xen/include/xen/xen.lds.h b/xen/include/xen/xen.lds.h > > index b126dfe887..42550a85a2 100644 > > --- a/xen/include/xen/xen.lds.h > > +++ b/xen/include/xen/xen.lds.h > > @@ -81,6 +81,24 @@ > > .stab.index 0 : { *(.stab.index) } \ > > .stab.indexstr 0 : { *(.stab.indexstr) } > > > > +#if defined(CONFIG_COVERAGE) && defined(CONFIG_CC_IS_CLANG) > > + > > +#define LLVM_COV_RW_DATA \ > > + DECL_SECTION(__llvm_prf_cnts) { *(__llvm_prf_cnts) } \ > > + DECL_SECTION(__llvm_prf_data) { *(__llvm_prf_data) } \ > > + DECL_SECTION(__llvm_prf_bits) { *(__llvm_prf_bits) } > > + > > +#define LLVM_COV_RO_DATA \ > > + DECL_SECTION(__llvm_prf_names) { *(__llvm_prf_names) } > > + > > +#define LLVM_COV_DEBUG \ > > + DECL_DEBUG(__llvm_covfun, 8) \ > > + DECL_DEBUG(__llvm_covmap, 8) > > +#else > > +#define LLVM_COV_RW_DATA > > +#define LLVM_COV_RO_DATA > > +#define LLVM_COV_DEBUG > > +#endif > > Newline here. > > But, there's no problem stating sections which are unused. I think the > outer #if/#else can be dropped. > > Otherwise, Acked-by: Andrew Cooper <[email protected]> > > I can fix these all up on commit, seeing as it's release acked for 4.21 Thank you for offering to fix them up. Let me know how I can help or if I need to send another version. Thanks, Saman > > ~Andrew
