Re: [Xen-devel] [PATCH] tools/xenalyze: Fix build with clang

[Ian Jackson]

George Dunlap writes ("Re: [PATCH] tools/xenalyze: Fix build with clang"):
> On 12/02/16 19:06, Andrew Cooper wrote:
> > Signed-off-by: Andrew Cooper 
> 
> All looks good, thanks.
> 
> Acked-by: George Dunlap 

Queued, thanks.

Ian.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] tools/xenalyze: Fix build with clang

[George Dunlap]

On 12/02/16 19:06, Andrew Cooper wrote:
> 1) EXIT_REASON_EXCEPTION_NMI is 0, and Clang complains:
> 
> xenalyze.c:513:33: error: initializer overrides prior initialization of this 
> subobject [-Werror,-Winitializer-overrides]
> [EXIT_REASON_EXCEPTION_NMI]="EXCEPTION_NMI",
> ^~~
> xenalyze.c:512:11: note: previous initialization is here
> [0] = "NONE",
>   ^~
> 
> 2) cr3_time_compare(), eip_compare(), ipi_send() and cr3_compare_start() are
>declared as nested functions, which is a GCCism not supported by Clang.
> 
>As they don't actually make use of the interesting feature offered by
>nested functions (i.e. dynamic scoping), move them to just being normal
>functions.
> 
> 3) clear_interval_summary(), update_cpi() and clear_interval_cpi() are all
>unused.  The former isn't reference anywhere, so is deleted, while the 
> other
>two are called from currently #if 0'd code.
> 
> Signed-off-by: Andrew Cooper 

All looks good, thanks.

Acked-by: George Dunlap 



> ---
> CC: George Dunlap 
> CC: Ian Campbell 
> CC: Ian Jackson 
> CC: Wei Liu 
> ---
>  tools/xentrace/xenalyze.c | 188 
> +++---
>  1 file changed, 92 insertions(+), 96 deletions(-)
> 
> diff --git a/tools/xentrace/xenalyze.c b/tools/xentrace/xenalyze.c
> index 6520790..1651302 100644
> --- a/tools/xentrace/xenalyze.c
> +++ b/tools/xentrace/xenalyze.c
> @@ -509,7 +509,6 @@ struct {
>  #define HVM_VMX_EXIT_REASON_MAX (EXIT_REASON_XSETBV+1)
>  
>  char * hvm_vmx_exit_reason_name[HVM_VMX_EXIT_REASON_MAX] = {
> -[0] = "NONE",
>  [EXIT_REASON_EXCEPTION_NMI]="EXCEPTION_NMI",
>  [EXIT_REASON_EXTERNAL_INTERRUPT]="EXTERNAL_INTERRUPT",
>  [EXIT_REASON_TRIPLE_FAULT]="TRIPLE_FAULT",
> @@ -2270,11 +2269,6 @@ static inline void update_summary(struct 
> event_cycle_summary *s, long long c) {
>  s->count++;
>  }
>  
> -static inline void clear_interval_summary(struct event_cycle_summary *s) {
> -s->interval.count = 0;
> -s->interval.cycles = 0;
> -}
> -
>  static inline void update_cycles(struct cycle_summary *s, long long c) {
>  /* We don't know ahead of time how many samples there are, and working
>   * with dynamic stuff is a pain, and unnecessary.  This algorithm will
> @@ -2322,6 +2316,7 @@ static inline void clear_interval_cycles(struct 
> interval_element *e) {
>  e->instructions = 0;
>  }
>  
> +#if 0
>  static inline void update_cpi(struct weighted_cpi_summary *s,
>unsigned long long i,
>unsigned long long c) {
> @@ -2367,6 +2362,7 @@ static inline void clear_interval_cpi(struct 
> weighted_cpi_summary *s) {
>  s->interval.count = 0;
>  s->interval.instructions = 0;
>  }
> +#endif
>  
>  static inline void print_cpu_affinity(struct cycle_summary *s, char *p) {
>  if(s->count) {
> @@ -2647,6 +2643,23 @@ void interval_cr3_value_check(struct cr3_value_struct 
> *cr3) {
>  }
>  }
>  
> +int cr3_time_compare(const void *_a, const void *_b) {
> +struct cr3_value_struct *a=*(typeof())_a;
> +struct cr3_value_struct *b=*(typeof())_b;
> +
> +if(a->total_time.interval.cycles < b->total_time.interval.cycles)
> +return 1;
> +else if(b->total_time.interval.cycles == a->total_time.interval.cycles) {
> +if(a->total_time.interval.count < b->total_time.interval.count)
> +return 1;
> +else if(a->total_time.interval.count == b->total_time.interval.count)
> +return 0;
> +else
> +return -1;
> +} else
> +return -1;
> +}
> +
>  void interval_cr3_schedule_ordered_output(void) {
>  struct cr3_value_struct *p;
>  int i;
> @@ -2654,23 +2667,6 @@ void interval_cr3_schedule_ordered_output(void) {
>  struct cr3_value_struct **qsort_array;
>  int N=0;
>  
> -int cr3_time_compare(const void *_a, const void *_b) {
> -struct cr3_value_struct *a=*(typeof())_a;
> -struct cr3_value_struct *b=*(typeof())_b;
> -
> -if(a->total_time.interval.cycles < b->total_time.interval.cycles)
> -return 1;
> -else if(b->total_time.interval.cycles == 
> a->total_time.interval.cycles) {
> -if(a->total_time.interval.count < b->total_time.interval.count)
> -return 1;
> -else if(a->total_time.interval.count == 
> b->total_time.interval.count)
> -return 0;
> -else
> -return -1;
> -} else
> -return -1;
> -}
> -
>  for(p=P.cr3.head; p; p=p->gnext)
>  N++;
>  
> @@ -2966,6 +2962,23 @@ void update_eip(struct eip_list_struct **head, 
> unsigned long long eip,
>  update_summary(>summary, cycles);
>  }
>  
> +int eip_compare(const void *_a, const void *_b) 

[Xen-devel] [PATCH] tools/xenalyze: Fix build with clang

[Andrew Cooper]

1) EXIT_REASON_EXCEPTION_NMI is 0, and Clang complains:

xenalyze.c:513:33: error: initializer overrides prior initialization of this 
subobject [-Werror,-Winitializer-overrides]
[EXIT_REASON_EXCEPTION_NMI]="EXCEPTION_NMI",
^~~
xenalyze.c:512:11: note: previous initialization is here
[0] = "NONE",
  ^~

2) cr3_time_compare(), eip_compare(), ipi_send() and cr3_compare_start() are
   declared as nested functions, which is a GCCism not supported by Clang.

   As they don't actually make use of the interesting feature offered by
   nested functions (i.e. dynamic scoping), move them to just being normal
   functions.

3) clear_interval_summary(), update_cpi() and clear_interval_cpi() are all
   unused.  The former isn't reference anywhere, so is deleted, while the other
   two are called from currently #if 0'd code.

Signed-off-by: Andrew Cooper 
---
CC: George Dunlap 
CC: Ian Campbell 
CC: Ian Jackson 
CC: Wei Liu 
---
 tools/xentrace/xenalyze.c | 188 +++---
 1 file changed, 92 insertions(+), 96 deletions(-)

diff --git a/tools/xentrace/xenalyze.c b/tools/xentrace/xenalyze.c
index 6520790..1651302 100644
--- a/tools/xentrace/xenalyze.c
+++ b/tools/xentrace/xenalyze.c
@@ -509,7 +509,6 @@ struct {
 #define HVM_VMX_EXIT_REASON_MAX (EXIT_REASON_XSETBV+1)
 
 char * hvm_vmx_exit_reason_name[HVM_VMX_EXIT_REASON_MAX] = {
-[0] = "NONE",
 [EXIT_REASON_EXCEPTION_NMI]="EXCEPTION_NMI",
 [EXIT_REASON_EXTERNAL_INTERRUPT]="EXTERNAL_INTERRUPT",
 [EXIT_REASON_TRIPLE_FAULT]="TRIPLE_FAULT",
@@ -2270,11 +2269,6 @@ static inline void update_summary(struct 
event_cycle_summary *s, long long c) {
 s->count++;
 }
 
-static inline void clear_interval_summary(struct event_cycle_summary *s) {
-s->interval.count = 0;
-s->interval.cycles = 0;
-}
-
 static inline void update_cycles(struct cycle_summary *s, long long c) {
 /* We don't know ahead of time how many samples there are, and working
  * with dynamic stuff is a pain, and unnecessary.  This algorithm will
@@ -2322,6 +2316,7 @@ static inline void clear_interval_cycles(struct 
interval_element *e) {
 e->instructions = 0;
 }
 
+#if 0
 static inline void update_cpi(struct weighted_cpi_summary *s,
   unsigned long long i,
   unsigned long long c) {
@@ -2367,6 +2362,7 @@ static inline void clear_interval_cpi(struct 
weighted_cpi_summary *s) {
 s->interval.count = 0;
 s->interval.instructions = 0;
 }
+#endif
 
 static inline void print_cpu_affinity(struct cycle_summary *s, char *p) {
 if(s->count) {
@@ -2647,6 +2643,23 @@ void interval_cr3_value_check(struct cr3_value_struct 
*cr3) {
 }
 }
 
+int cr3_time_compare(const void *_a, const void *_b) {
+struct cr3_value_struct *a=*(typeof())_a;
+struct cr3_value_struct *b=*(typeof())_b;
+
+if(a->total_time.interval.cycles < b->total_time.interval.cycles)
+return 1;
+else if(b->total_time.interval.cycles == a->total_time.interval.cycles) {
+if(a->total_time.interval.count < b->total_time.interval.count)
+return 1;
+else if(a->total_time.interval.count == b->total_time.interval.count)
+return 0;
+else
+return -1;
+} else
+return -1;
+}
+
 void interval_cr3_schedule_ordered_output(void) {
 struct cr3_value_struct *p;
 int i;
@@ -2654,23 +2667,6 @@ void interval_cr3_schedule_ordered_output(void) {
 struct cr3_value_struct **qsort_array;
 int N=0;
 
-int cr3_time_compare(const void *_a, const void *_b) {
-struct cr3_value_struct *a=*(typeof())_a;
-struct cr3_value_struct *b=*(typeof())_b;
-
-if(a->total_time.interval.cycles < b->total_time.interval.cycles)
-return 1;
-else if(b->total_time.interval.cycles == 
a->total_time.interval.cycles) {
-if(a->total_time.interval.count < b->total_time.interval.count)
-return 1;
-else if(a->total_time.interval.count == 
b->total_time.interval.count)
-return 0;
-else
-return -1;
-} else
-return -1;
-}
-
 for(p=P.cr3.head; p; p=p->gnext)
 N++;
 
@@ -2966,6 +2962,23 @@ void update_eip(struct eip_list_struct **head, unsigned 
long long eip,
 update_summary(>summary, cycles);
 }
 
+int eip_compare(const void *_a, const void *_b) {
+struct eip_list_struct *a=*(typeof())_a;
+struct eip_list_struct *b=*(typeof())_b;
+
+if(a->summary.cycles < b->summary.cycles)
+return 1;
+else if(b->summary.cycles == a->summary.cycles) {
+if(a->summary.count < b->summary.count)
+return 1;
+else if(a->summary.count == b->summary.count)
+return 0;
+else
+