Closed Bug 1334777 Opened 3 years ago Closed 3 years ago

mozglue/build/SSE.cpp:170:15: error: 'bool mozilla::sse_private::has_avx()' defined but not used [-Werror=unused-function]

Categories

(Core :: mozglue, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla54
Tracking Status
firefox52 --- fixed
firefox-esr52 --- fixed
firefox53 --- fixed
firefox54 --- fixed

People

(Reporter: jbeich, Assigned: jbeich)

References

Details

(Keywords: regression)

Attachments

(1 file)

$ c++ -v
FreeBSD clang version 4.0.0 (branches/release_40 292732) (based on LLVM 4.0.0)
Target: x86_64-unknown-freebsd12.0
Thread model: posix
InstalledDir: /usr/bin
Found CUDA installation: /usr/local/cuda, version 7.5

$ c++ -v -march=native -xc -c -</dev/null -o /dev/null |& tr ' ' '\n' | fgrep avx
-avx512ifma
-avx512dq
-avx512er
-avx512f
-avx512bw
-avx512vl
-avx512cd
+avx
+avx2
-avx512vbmi
-avx512pf

$ echo ac_add_options --enable-warnings-as-errors >>.mozconfig
$ echo export CFLAGS=\'-O3 -march=native\' CXXFLAGS=\'-O3 -march=native'\ >>.mozconfig
$ ./mach build
[...]
mozglue/build/SSE.cpp:170:15: error: unused function 'has_avx' [-Werror,-Wunused-function]
  static bool has_avx()
              ^
1 error generated.
mozglue/build/SSE.cpp:45:17: error: unused function 'xgetbv' [-Werror,-Wunused-function]
static uint64_t xgetbv(uint32_t xcr) {
                ^
1 error generated.
Comment on attachment 8831422 [details]
Bug 1334777 - Hide unused has_avx() if CXXFLAGS have -mavx.

https://reviewboard.mozilla.org/r/108004/#review110488

It's funny that has_cpuid_bits doesn't have the same problem, but that's because there is no combination where SSE4A is enabled along all the others.

::: mozglue/build/SSE.cpp:174
(Diff revision 1)
>  
>  #if !defined(MOZILLA_PRESUME_SSE4_2)
>    bool sse4_2_enabled = has_cpuid_bits(1u, ecx, (1u<<20));
>  #endif
>  
> +#if !defined(MOZILLA_PRESUME_AVX)

While this works for -mavx2, it doesn't work for -mavx.
Attachment #8831422 - Flags: review?(mh+mozilla)
Comment on attachment 8831422 [details]
Bug 1334777 - Hide unused has_avx() if CXXFLAGS have -mavx.

https://reviewboard.mozilla.org/r/108004/#review111436
Attachment #8831422 - Flags: review?(mh+mozilla) → review+
Keywords: checkin-needed
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4f2f1e1c6362
Hide unused has_avx() if CXXFLAGS have -mavx. r=glandium
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/4f2f1e1c6362
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Please request Aurora/Beta approval on this when you get a chance.
Assignee: nobody → jbeich
Flags: needinfo?(jbeich)
Comment on attachment 8831422 [details]
Bug 1334777 - Hide unused has_avx() if CXXFLAGS have -mavx.

Let's remove one more compiler warning for ESR52 that gets in the way when debugging downstream builds.

Approval Request Comment
[Feature/Bug causing the regression]: bug 1270591 regression
[User impact if declined]: adding -march=sandybridge or higher to CXXFLAGS breaks build for MOZ_AUTOMATION or --enable-warnings-as-errors
[Is this code covered by automated tests?]: No, automation doesn't pass -march or -mavx
[Has the fix been verified in Nightly?]: Yes, by myself.
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No
[Why is the change risky/not risky?]: Can only break build.
[String changes made/needed]: None
Flags: needinfo?(jbeich)
Attachment #8831422 - Flags: approval-mozilla-beta?
Attachment #8831422 - Flags: approval-mozilla-aurora?
Comment on attachment 8831422 [details]
Bug 1334777 - Hide unused has_avx() if CXXFLAGS have -mavx.

Fix a build issue. Aurora53+.
Attachment #8831422 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment on attachment 8831422 [details]
Bug 1334777 - Hide unused has_avx() if CXXFLAGS have -mavx.

avoid clang warning in some configurations, beta52+

I guess a couple more ifdefs in that file won't hurt...
Attachment #8831422 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.