Re: [Minios-devel] [UNIKRAFT PATCHv4 16/43] uk/arch: Implement ukarch_find_lsbit for Arm64

[Sharan Santhanam <sharan.santhanam@xxxxxxxxx>]

Hello Wei Chen,

On 07/16/2018 05:06 AM, Wei Chen wrote:
> > -----Original Message-----
> > From: Sharan Santhanam <sharan.santhanam@xxxxxxxxx>
> > Sent: 2018年7月13日 20:45
> > To: Julien Grall <Julien.Grall@xxxxxxx>; Wei Chen <Wei.Chen@xxxxxxx>; minios-
> > devel@xxxxxxxxxxxxx
> > Subject: Re: [Minios-devel] [UNIKRAFT PATCHv4 16/43] uk/arch: Implement
> > ukarch_find_lsbit for Arm64
> >
> > Hello,
> >
> >
> > On 07/13/2018 11:44 AM, Julien Grall wrote:
> > > Hi,
> > >
> > > On 13/07/18 10:20, Wei Chen wrote:
> > > > > > +#ifndef __UKARCH_ATOMIC_H__
> > > > > > +#error Do not include this header directly
> > > > > > +#endif
> > > > > > +
> > > > > For finding the lsbit shouldn't we use the builtin function
> > > > > __builtin_ffs, __builtin_ffsl?
> > > AFAICT __builtin_ffs is GNU-ism. Do we really want to tie ourself to it?
> > Yes, this is a good point I had overlooked previously. On further
> > looking into it, this builtin function is supported on gcc[1] and
> > clang[2]. We may need to consider what other compiler tool chains do we
> > need to be compatible with.
> >
> > > Also, do you know if that is supported correctly on all GCC versions we
> > > targets?
> > The __builitin_ffs were introduced as a part of the gcc version3.3
> > but the earliest documentation where it is been described is in 3.4 [1].
> >
> >
> > > > >
> > > > When I was implementing the ukarch_find_lsbit, I found both Arm32 and
> > > > X86_64 were not using the builtin-functions. X86_64 is using "bsfq",
> > > > Arm32 is using the same instructions as Arm64.
> > I agree in terms of consistency, we may use the current implementation.
> >
> > I wanted to clarify if we made an explicit decision to provide our own
> > implementation for the ffs.
> Later, in other patch series, maybe we can have some method to detect the
> __builitin_ffs supported status. If it's supported by current compiler we
> can use it, otherwise we will use current implementation.
>
> How do you think about it?
I agree.

> > > To be honest, I would keep the implement as it is.
> > >
> > > Cheers,
> > [1]gcc:
> > https://gcc.gnu.org/onlinedocs/gcc-3.4.6/gcc/Other-Builtins.html#Other-
> > Builtins
> >
> > [2] Clang:
> > https://github.com/llvm-
> > mirror/clang/blob/release_26/include/clang/Basic/Builtins.def
> >
> >
> > Thanks & Regards
> > Sharan
> 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.
Thanks & Regards
Sharan

_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel