Closed Bug 1298470 Opened 3 years ago Closed 3 years ago

define the correct SSSE3_FLAGS when compiling with clang-cl

Categories

(Firefox Build System :: General, defect)

All
Windows
defect
Not set

Tracking

(firefox51 fixed)

RESOLVED FIXED
mozilla51
Tracking Status
firefox51 --- fixed

People

(Reporter: froydnj, Assigned: froydnj)

References

Details

Attachments

(1 file)

clang-cl is more picky than MSVC when you use intrinsics: it only lets you use particular intrinsics if you have specifically enabled the relevant instruction sets on the command line (for the most part).  So when compiling things that expect to be able to use SSSE3, MSVC is happy to let -arch:SSE2 be enough, while clang-cl wants -mssse3.
This isn't the right thing to do.  This not only enables the SSSE3 intrinsics, but it *also* tells the clang code generator that it should use the SSSE3 instruction set where possible, which will make the resulting binary assume the availability of the said instruction set.

There's some existing places where in the moz.build file we pass -mssse3 to clang-cl only for those files that need it.  See <https://dxr.mozilla.org/mozilla-central/source/gfx/skia/generate_mozbuild.py#105> for an example.  We should use the same pattern for other code which requires SSSE3 instructions.
Flags: needinfo?(nfroyd)
(In reply to :Ehsan Akhgari from comment #2)
> This isn't the right thing to do.  This not only enables the SSSE3
> intrinsics, but it *also* tells the clang code generator that it should use
> the SSSE3 instruction set where possible, which will make the resulting
> binary assume the availability of the said instruction set.
> 
> There's some existing places where in the moz.build file we pass -mssse3 to
> clang-cl only for those files that need it.  See
> <https://dxr.mozilla.org/mozilla-central/source/gfx/skia/generate_mozbuild.
> py#105> for an example.  We should use the same pattern for other code which
> requires SSSE3 instructions.

This is basically what SSSE3_FLAGS is for.  We're not enabling the use of SSSE3 globally.
Flags: needinfo?(nfroyd)
You're right, nevermind.  I misread the patch.  Sorry!
Comment on attachment 8785421 [details]
Bug 1298470 - use the correct SSSE3_FLAGS for clang-cl;

https://reviewboard.mozilla.org/r/74622/#review73074
Attachment #8785421 - Flags: review?(mh+mozilla) → review+
Pushed by nfroyd@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/987bda408af3
use the correct SSSE3_FLAGS for clang-cl; r=glandium
https://hg.mozilla.org/mozilla-central/rev/987bda408af3
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.