[Buildroot] [PATCH 03/21] toolchain: add BR2_TOOLCHAIN_HAS_SYNC_x hidden booleans
Yann E. MORIN
yann.morin.1998 at free.fr
Wed Jan 27 22:47:54 UTC 2016
Thomas, All,
On 2016-01-27 22:46 +0100, Thomas Petazzoni spake thusly:
> On Mon, 25 Jan 2016 19:27:37 +0100, Yann E. MORIN wrote:
> > > +# - On ARMv7, there is no problem, it can be directly implemented in
> > > +# userspace.
> > > +#
> > > +# - On < ARMv7, it requires help from the kernel. Unfortunately, the
> > > +# libgcc code implementing 8 bytes __sync with the help from the
> > > +# kernel calls __write() when a failure occurs, which is a function
> > > +# internal to glibc, not available in uClibc and musl. This means
> > > +# that the 8 bytes __sync operations are not available on < ARMv7
> > > +# with uClibc and musl. This problem was fixed as part of gcc
> > > +# PR68059, which was backported to the gcc 5 branch, but isn't yet
> > > +# part of any gcc 5.x release.
> > > +#
> > > +config BR2_TOOLCHAIN_ARM_HAS_SYNC_8
> > > + bool
> > > + default y
> > > + depends on BR2_arm || BR2_armeb
> > > + depends on BR2_TOOLCHAIN_GCC_AT_LEAST_4_7
> > > + depends on BR2_TOOLCHAIN_USES_GLIBC || BR2_ARM_CPU_ARMV7A
> >
> > I had a bit of difficulty parsing that one. It was not obvious from the
> > commit log that gcc >= 4.7 was required, while the comment above makes
> > it really explicit. Here's how I understood the commit log:
> >
> > - for armv7, with gcc-4.7+, it's OK
> > - for armv6-, it needs glibc and there is no gcc condition.
> >
> > But the comment here states it differently, and matches the code.
> >
> > Can you just confirm that I misunderstand the commit log?
>
> Well, the commit log has:
>
> __sync __atomic gcc
>
> ARM Y Y Y X Y Y Y Y 4.8, 4.7
> ARM Y Y Y - 4.5
>
> So, as you can see:
>
> - With a gcc 4.5 toolchain, there is no __sync for 8-byte types at all.
>
> - Starting with gcc 4.7, there *might* be a __sync for 8-byte types,
> but only if you're using glibc *or* if you're using ARMv7.
>
> Hence the conditions:
>
> depends on BR2_TOOLCHAIN_GCC_AT_LEAST_4_7
> depends on BR2_TOOLCHAIN_USES_GLIBC || BR2_ARM_CPU_ARMV7A
>
> So I believe the code, the comment and the commit log are matching
> here. Am I missing something ?
What I meant, is that I had some problem matching this part of the commit
log:
> * X indicates a very special case for 8 bytes __sync built-ins on
> ARM. On ARMv7, there is no problem, starting from gcc 4.7, the
> __sync built-in for 8 bytes integers is implemented, fully in
> userspace. For cores < ARMv7, doing a 8 bytes atomic operation
> requires help from the kernel. Unfortunately, the libgcc code
> implementing this uses the __write() function to display an error,
> and this function is internal to glibc. Therefore, if you're using
> glibc everything is fine, but if you're using uClibc or musl, you
> cannot link an application that uses 8 bytes __sync
> operations. This has been fixed as part of gcc PR68095, merged in
> the gcc 5 branch but not yet part of any gcc release.
... with the actual dependencies you wrote.
But reading the blurb above again, it looks OK now. It's just a little
bit complex enough to need a second^Wthird^Wfourth read to fully grasp.
;-)
> > Well, I'd prefer we have a simpler conditional block, maybe something
> > like:
> >
> > config BR2_TOOLCHAIN_i386_HAS_SYNC_8
> > bool
> > default y
> > depends on BR2_i386
> > depends on !BR2_x86_i386
> > depends on !BR2_x86_i486
> > depends on !BR2_x86_c3
> > depends on !BR2_x86_winchip_c6
> > depends on !BR2_x86_winchip2
> >
> > config BR2_TOOLCHAIN_HAS_SYNC_8
> > bool
> > default y if BR2_ARCH_IS_64
> > default y if BR2_TOOLCHAIN_i386_HAS_SYNC_8
> > default y if BR2_TOOLCHAIN_ARM_HAS_SYNC_8
>
> Adopted.
>
> > Also, I'm not sure about the 'default y if BR2_ARCH_IS_64' and I think
> > we should just list those architectures (4 of them: AArch64, mips64,
> > sparc64 and x86_64, with that last one already covered with i386).
>
> I beg to disagree here. I don't see why a 64 bits architecture would
> not provide the 8-byte intrinsics. I would like to suggest to keep it
> as is for now, and revisit when/if we get a 64 bits architecture that
> doesn't provide those 8-byte intrinsics.
I was not implying 64-bit archictectures would not provide them. I
suggested we use the actual architectures just to get a similar
condition as we have for the other _SYNC_X options, for which we only
use the architecture names (or an intermediate option derived from the
architecture).
But indeed, a 64-bit arch that does not provide _SYNC_8 woudl be really
weird (and broken).
So, OK.
Regards,
Yann E. MORIN.
--
.-----------------.--------------------.------------------.--------------------.
| Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ |
| +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. |
'------------------------------^-------^------------------^--------------------'
More information about the buildroot
mailing list