Re: [Minios-devel] [UNIKRAFT PATCH v2 6/6] arch/arm64: Implement bitops for arm64

["Justin He (Arm Technology China)" <Justin.He@xxxxxxx>]

Hi Julien

> -----Original Message-----
> From: Julien Grall <julien.grall@xxxxxxx>
> Sent: 2019年9月27日 18:01
> To: Justin He (Arm Technology China) <Justin.He@xxxxxxx>; minios-
> devel@xxxxxxxxxxxxxxxxxxxx; Simon Kuenzer <simon.kuenzer@xxxxxxxxx>
> Cc: Sharan.Santhanam@xxxxxxxxx; Felipe Huici <felipe.huici@xxxxxxxxx>;
> Kaly Xin (Arm Technology China) <Kaly.Xin@xxxxxxx>;
> Santiago.Pagani@xxxxxxxxx
> Subject: Re: [UNIKRAFT PATCH v2 6/6] arch/arm64: Implement bitops for
> arm64
>
>
>
> On 27/09/2019 10:56, Justin He (Arm Technology China) wrote:
> > Hi Julien
>
> Hi,
>
> >
> >> -----Original Message-----
> >> From: Julien Grall <julien.grall@xxxxxxx>
> >> Sent: 2019年9月27日 17:42
> >> To: Justin He (Arm Technology China) <Justin.He@xxxxxxx>; minios-
> >> devel@xxxxxxxxxxxxxxxxxxxx; Simon Kuenzer <simon.kuenzer@xxxxxxxxx>
> >> Cc: Sharan.Santhanam@xxxxxxxxx; Felipe Huici <felipe.huici@xxxxxxxxx>;
> >> Kaly Xin (Arm Technology China) <Kaly.Xin@xxxxxxx>;
> >> Santiago.Pagani@xxxxxxxxx
> >> Subject: Re: [UNIKRAFT PATCH v2 6/6] arch/arm64: Implement bitops
> for
> >> arm64
> >>
> >> Hi Jia,
> >>
> >> On 27/09/2019 07:53, Jia He wrote:
> >>> Without this patch, compiler will report warning as follows:
> >>>     In file included from plat/common/arm/time.c:41:0:
> >>> include/uk/bitops.h: In function 'uk_get_count_order':
> >>> include/uk/bitops.h:89:10: warning: implicit declaration of function
> >>> 'ukarch_fls'; did you mean 'ukarch_ffsl'? [-Wimplicit-function-
> declaration]
> >>>     order = ukarch_fls(count);
> >>>             ^~~~~~~~~~
> >>>             ukarch_ffsl
> >>> include/uk/bitops.h: In function 'uk_find_last_bit':
> >>> include/uk/bitops.h:154:18: warning: implicit declaration of function
> >>> 'ukarch_flsl'; did you mean 'ukarch_ffsl'? [-Wimplicit-function-
> declaration]
> >>>       return (bit + ukarch_flsl(mask));
> >>>                     ^~~~~~~~~~~
> >>>                     ukarch_ffsl
> >>
> >> This explains why you add ukarch_fls and ukarch_flsl. But this does not
> >> explain
> >> why you reimplemented ukarch_ffsl with a builtin.
> >>
> > Okay, I will post my test result.
> > I compared the two implementation (gcc builtin and ukarch_ffsl original
> one )
>
> TBH, using builtin is better as you can let the GCC folks to maintain the
> function and therefore one less for Unikraft community to take care of :).
>
> There are a few cases (such as atomics) where this may not be the best
> solution
> to use builtin as you may want to further optimize it at runtime. But that's
> a
> different topic ;).
>
> Anyway, I would suggest to move the rework of ukarch_ffsl in a separate
> patch.

Thanks, ok with that

>
> >
> > for (x=0; x<0xfffffff; x++)
> >
> > original:
> > real    0m1.723s
> > user    0m1.723s
> > sys     0m0.000s
> >
> > gcc builtin:
> > real    0m1.550s
> > user    0m1.546s
> > sys     0m0.004s
>
> If you plan to add the test result in the commit message, then please
> mention
> the compiler version used and also the platform for testing.
>

Sure, thanks

> Cheers,
>
> --
> Julien Grall
IMPORTANT NOTICE: The contents of this email and any attachments are 
confidential and may also be privileged. If you are not the intended recipient, 
please notify the sender immediately and do not disclose the contents to any 
other person, use it for any purpose, or store or copy the information in any 
medium. Thank you.
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel