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

RESOLVED FIXED in mozilla13

Status

()

Core
Gecko Profiler
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: glandium, Assigned: glandium)

Tracking

Trunk
mozilla13
Other
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

6 years ago
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
(Assignee)

Comment 1

6 years ago
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.
(Assignee)

Comment 2

6 years ago
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.
(Assignee)

Comment 3

6 years ago
Created attachment 604389 [details] [diff] [review]
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.
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+
Attachment #604389 - Flags: review?(khuey) → review+
(Assignee)

Comment 5

6 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/5168ba8c86f0
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 ?
(Assignee)

Comment 7

6 years ago
(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
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla13
Blocks: 699918
Depends on: 743428
Blocks: 735871
Blocks: 744026
You need to log in before you can comment on or make changes to this bug.