configury: abort when builtin atomics cannot be built and configure'd…

[ggouaillardet]

… with --enable-builtin-atomics

Signed-off-by: Gilles Gouaillardet [email protected]

[ibm-ompi]

The IBM CI (PGI Compiler) build failed! Please review the log, linked below.

Gist: https://gist.github.com/745b7c7dad9eb0a931caa3c93d9781ec

[ggouaillardet]

@hjelmn can you please review this ?

since f33bbfd, configure does not abort any more if --enable-builtin-atomic is used but builtin atomics cannot be built.
my guess is this is not the intended behavior, but i'd like to double check it before i commit.

fwiw, you can simply reproduce this with
configure CFLAGS='-m32 -march=i386' CXXFLAGS='-m32 -march=i386'

[jsquyres]

@ggouaillardet @hjelmn Is this PR now moot (with all the recent discussion about the ASM)?

[bwbarrett]

@jsquyres I don't think so, just different. There's still builtin atomics vs. inline assembly to worry about.

@ggouaillardet, please don't add release branch labels to PRs destined for master (see https://github.com/open-mpi/ompi/wiki/SubmittingPullRequests)

[ggouaillardet]

@jsquyres
i agree with @bwbarrett, this PR is unrelated to the recent ASM changes and is hence not moot.

[jsquyres]

@bwbarrett @ggouaillardet Got it.

[hppritcha]

@hjelmn please review, I'd like to get this in the rc1

[rhc54]

I can confirm this works as intended

[jjhursey]

bot:ibm:pgi:retest

[rhc54]

@jjhursey Given that this PR introduces the "fail if sync builtin atomics fails", I don't think it can pass the PGI compiler test until we actually fix those atomics - yes?

[jjhursey]

@rhc54 That's what I want to check with this CI recheck (the previous test was over 2 weeks ago, so the log was cleared). It looks like it's passing fine so far. Just wanted to keep an eye on it. If PGI CI passes then we are good to continue with this PR.

[rhc54]

ok, then we have to figure out what is missing over in pmix as it fails this test, yet appears (to my scan) to have identical atomics now.

[hjelmn]

Why does this have a v2.1.2 milestone? We do not enable builtin atomics by default in v2.1.2.

[hjelmn]

Also not relevant for v3.0.0 anymore. builtins are not enabled by there by default either.

[rhc54]

Maybe we need to review the entire atomics thing at next week's meeting. I confess I'm getting confused and a little frustrated at the way the atomic support keeps jerking around. It doesn't seem that there is any coherent understanding right now as to what is actually being built, when it is to be built, and why.

[hjelmn]

v2.0.x, v2.1.x, and v3.0.x have no change from past behavior. That is we use inline asm unless --enable-builtin-atomics=yes is specified. On master we try builtins by default then try the inline asm. I do not want it failing if we have inline asm but no builtins unless the configure line says --enable-builtin-atomics=yes. It need to find a compiler to test this case to ensure this PR is correct.

[hjelmn]

I should also point out it is likely v3.1.x will use inline asm by default. The plan is to use C11 atomics for v4.0.x unless we hit a roadblock on requiring C11.

[hjelmn]

@ggouaillardet We need to figure out exactly what we want to be the behavior here. Looking at this I think it will be ok. Should have it figured out soon.

[bosilca]

If we only print a warning, the hope that users (or even developers) notice that a requested parameter could not be satisfied is unrealistic, as we all know nobody's reading the configure output unless it aborts.

👍 on this patch.