[Mesa-dev] [PATCH] configure.ac: rework -latomic check

[Thomas Petazzoni]

Hello,

On Mon, 7 May 2018 14:07:05 -0700, Matt Turner wrote:
> On Mon, May 7, 2018 at 4:34 AM, Thomas Petazzoni
> <thomas.petazzoni at bootlin.com> wrote:
> > The configure.ac logic added in commit
> > 2ef7f23820a67e958c2252bd81eb0458903ebf33 ("configure: check if
> > -latomic is needed for __atomic_*") makes the assumption that if a
> > 64-bit atomic intrinsic test program fails to link without -latomic,
> > it is because we must use -latomic.  
> 
> I've confused. What is the alternative? Does this patch make Mesa
> work when it didn't before?

Yes, it makes Mesa build when it didn't before. I didn't runtime test
it though.

> Does the "whether __sync_add_and_fetch_8 is supported" test then
> detect that on your PPC system that we don't have 64-bit atomics and
> choose to enable -DMISSING_64BIT_ATOMICS?

Yes, -DMISSING_64BIT_ATOMICS ends up being defined.

> If that's the case, them my understanding is that we incorrectly
> assume -latomic is needed when in fact there's nothing we can do to
> make 64-bit __atomic built-ins work on that platform.

With that specific old gcc version. With newer gcc versions, libatomic
is provided, and 64-bit atomics are therefore available.

> So we're currently deciding that __atomic built-ins are usable since
> we're only compiling-testing a 32-bit operation, and then wrongly
> assuming that -latomic can make 64-bit operations work. Is that right?

The current autoconf code is doing this:

 - Let's *compile* (not link) a 32-bit atomic test program. If it
   works, we for now assume GCC_ATOMIC_BUILTINS_SUPPORTED=1

 - If GCC_ATOMIC_BUILTINS_SUPPORTED=1 thanks to the previous test, we
   *link* a 64-bit atomic test program, with no special linker flags.
   If it works, we're all set. If it doesn't, we assume that by adding
   -latomic to LIBS, everything will be OK.

It is this last part that is wrong: it *assumes* that adding -latomic
will always guarantee that 64-bit atomics will be available, but that
is wrong. It forget to actually verify that, and my patch simply
changes that.

Hopefully this clarifies what is happening here. Don't hesitate to ask
for further details if needed.

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com