Closed Bug 734335 Opened 8 years ago Closed 8 years ago

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

Categories

(Core :: Gecko Profiler, defect)

Other
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla13

People

(Reporter: glandium, Assigned: glandium)

References

Details

Attachments

(1 file)

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
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.
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.
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.
Attachment #604389 - Flags: review?(khuey)
Attachment #604389 - Flags: review?(bgirard)
Comment on attachment 604389 [details] [diff] [review]
Only build SPS on supported platforms

Looks good. Thanks for the clean up.
Attachment #604389 - Flags: review?(bgirard) → review+
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 ?
(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.
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 ?
https://hg.mozilla.org/mozilla-central/rev/5168ba8c86f0
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla13
Depends on: 743428
You need to log in before you can comment on or make changes to this bug.