734335 - Build failure because of missing NoBarrier_Store on non-arm non-x86

Status: RESOLVED FIXED

(Core :: Gecko Profiler, defect)

Description

[Mike Hommey [:glandium]]

tools/profiler/sps/platform.h: In member function 'void Sampler::SetActive(bool)':
tools/profiler/sps/platform.h:271:63: error: 'NoBarrier_Store' was not declared in this scope

Comment 1

[Mike Hommey [:glandium]]

There are IMHO two fundamental problems:
- The code is being built on platforms that don't support it.
- The code is using custom definitions of types coming from chromium, when they could be using headers from ipc/chromium/src (like atomicops.h)

I'll address the former.

Comment 2

[Mike Hommey [:glandium]]

I don't know if the intent for the --enable-sps was to always be enabled, but that's how it currently is. Is SPS expected to be enabled on all builds? If yes, then it should be a --disable-sps.

Comment 3

[Mike Hommey [:glandium]]

Attachment: Only build SPS on supported platforms

This still enables sps by default, assuming this is what was meant, but disable it by default on unsupported platforms. This also makes --disable-sps actually do something, which wasn't the case before.

Comment 4

[Benoit Girard (:BenWa)]

Comment on attachment 604389 [details] [diff] [review]
Only build SPS on supported platforms

Looks good. Thanks for the clean up.

Comment 5

[Mike Hommey [:glandium]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5168ba8c86f0

Comment 6

[Landry Breuil (:gaston)]

I'm only reading the diff and didnt actually check it on OpenBSD (buildbot will as soon as inbound is merged to central), but isn't the logic reversed ?

From my understanding, by default MOZ_ENABLE_PROFILER_SPS=1 is set/defined, and unset on supported platforms... and --disable-sps unsets it. Shouldn't it be the other way round ?

Comment 7

[Mike Hommey [:glandium]]

(In reply to Landry Breuil from comment #6)
> by default MOZ_ENABLE_PROFILER_SPS=1 is set/defined,
> and unset on supported platforms... and --disable-sps unsets it. Shouldn't
> it be the other way round ?

It defaults to 1 on supported platforms and nothing on unsupported platforms. Then --disable-sps allows to unset it on supported platforms.

Comment 8

[Landry Breuil (:gaston)]

Doh, i see where my confusion comes from... misread the case.

Wouldnt it be simpler/clearer to do :

MOZ_ENABLE_PROFILER_SPS=
case "${OS_TARGET}" in
   Android)
   case "${CPU_ARCH}" in
       x86 | arm) MOZ_ENABLE_PROFILER_SPS=1 ;;
       *) ;;
   esac
   Linux)
   case "${CPU_ARCH}" in
       x86 | x86_64) MOZ_ENABLE_PROFILER_SPS=1 ;;
       *) ;;
   esac
   WINNT|Darwin) MOZ_ENABLE_PROFILER_SPS=1 ;;
   *) ;;
esac   


Or it's a rule of thumb to have somewhat reversed logic in configure.in ?

Comment 9

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/5168ba8c86f0