Closed Bug 1486055 Opened Last year Closed Last year

Fix ANGLE compilation errors for MSVC Windows ARM64

Categories

(Core :: Graphics, defect, P3)

ARM64
Windows
defect

Tracking

()

RESOLVED FIXED
mozilla64
Tracking Status
firefox64 --- fixed

People

(Reporter: froydnj, Assigned: jgilbert)

References

(Blocks 1 open bug, )

Details

(Whiteboard: gfx-noted)

I see at least:

 0:23.41 c:\mozilla-central\gfx\angle\checkout\src\common/mathutil.h(158): error C3861: '__cpuid': identifier not found
 0:23.41 c:\mozilla-central\gfx\angle\checkout\src\common/mathutil.h(162): error C3861: '__cpuid': identifier not found
 0:23.41 c:\mozilla-central\gfx\angle\checkout\src\common/mathutil.h(935): error C3861: '__popcnt': identifier not found
 0:23.41 mozmake.EXE[4]: *** [c:/mozilla-central/config/rules.mk:1108: Float16ToFloat32.obj] Error 2

The first two are easily taken care of, just an !defined(_M_ARM64) in a preprocessor conditional.  The second requires some actual code, or possibly a bugfix from Microsoft to support __popcnt* on AArch64.
How feasible is it to patch these locally?  Or should we look into upstream and backporting?
Flags: needinfo?(jgilbert)
Priority: -- → P3
Whiteboard: gfx-noted
There are also lots of things like:

12:21.38 c:/mozilla-central/gfx/angle/checkout/src/image_util/loadimage.cpp(31): error C2065: '__m128i': undeclared identifier
12:21.38 c:/mozilla-central/gfx/angle/checkout/src/image_util/loadimage.cpp(31): error C2146: syntax error: missing ';' before identifier 'zeroWide'
12:21.38 c:/mozilla-central/gfx/angle/checkout/src/image_util/loadimage.cpp(31): error C2065: 'zeroWide': undeclared identifier
12:21.38 c:/mozilla-central/gfx/angle/checkout/src/image_util/loadimage.cpp(31): error C3861: '_mm_setzero_si128': identifier not found
12:21.38 c:/mozilla-central/gfx/angle/checkout/src/image_util/loadimage.cpp(52): error C2065: '__m128i': undeclared identifier
12:21.38 c:/mozilla-central/gfx/angle/checkout/src/image_util/loadimage.cpp(52): error C2146: syntax error: missing ';' before identifier 'sourceData'
12:21.38 c:/mozilla-central/gfx/angle/checkout/src/image_util/loadimage.cpp(52): error C2065: 'sourceData': undeclared identifier
12:21.38 c:/mozilla-central/gfx/angle/checkout/src/image_util/loadimage.cpp(53): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int
12:21.43 c:/mozilla-central/gfx/angle/checkout/src/image_util/loadimage.cpp(53): error C2146: syntax error: missing '>' before identifier '__m128i'
12:21.43 c:/mozilla-central/gfx/angle/checkout/src/image_util/loadimage.cpp(55): error C2065: 'sourceData': undeclared identifier
12:21.43 c:/mozilla-central/gfx/angle/checkout/src/image_util/loadimage.cpp(55): error C2065: 'zeroWide': undeclared identifier
12:21.43 c:/mozilla-central/gfx/angle/checkout/src/image_util/loadimage.cpp(55): error C3861: '_mm_unpacklo_epi8': identifier not found
12:21.43 c:/mozilla-central/gfx/angle/checkout/src/image_util/loadimage.cpp(57): error C2065: '__m128i': undeclared identifier
12:21.47 c:/mozilla-central/gfx/angle/checkout/src/image_util/loadimage.cpp(57): error C2146: syntax error: missing ';' before identifier 'lo'
12:21.47 c:/mozilla-central/gfx/angle/checkout/src/image_util/loadimage.cpp(57): error C2065: 'lo': undeclared identifier
12:21.47 c:/mozilla-central/gfx/angle/checkout/src/image_util/loadimage.cpp(57): error C2065: 'zeroWide': undeclared identifier
12:21.47 c:/mozilla-central/gfx/angle/checkout/src/image_util/loadimage.cpp(57): error C2065: 'sourceData': undeclared identifier
12:21.47 c:/mozilla-central/gfx/angle/checkout/src/image_util/loadimage.cpp(57): error C3861: '_mm_unpacklo_epi16': identifier not found

and a number of other errors in that vein.
For Angle, all patches are fixed in github.com/mozilla/angle and re-vendored into moz-central, while being upstreamed into angle/master.

Alternatively, just upstream into Angle and we'll pick up whatever's made it into Chrome Beta's Angle branch each Gecko cycle.
Flags: needinfo?(jgilbert)
(In reply to Jeff Gilbert [:jgilbert] from comment #4)
> For Angle, all patches are fixed in github.com/mozilla/angle and re-vendored
> into moz-central, while being upstreamed into angle/master.
> 
> Alternatively, just upstream into Angle and we'll pick up whatever's made it
> into Chrome Beta's Angle branch each Gecko cycle.

Do we have a policy for backporting patches?  If we don't backport patches, when is the next ANGLE update going to happen?
Flags: needinfo?(jgilbert)
(In reply to Nathan Froyd [:froydnj] from comment #5)
> (In reply to Jeff Gilbert [:jgilbert] from comment #4)
> > For Angle, all patches are fixed in github.com/mozilla/angle and re-vendored
> > into moz-central, while being upstreamed into angle/master.
> > 
> > Alternatively, just upstream into Angle and we'll pick up whatever's made it
> > into Chrome Beta's Angle branch each Gecko cycle.
> 
> Do we have a policy for backporting patches?  If we don't backport patches,
> when is the next ANGLE update going to happen?

I try to update ANGLE in the first week of each new cycle.
Flags: needinfo?(jgilbert)
(In reply to Jeff Gilbert [:jgilbert] from comment #6)
> (In reply to Nathan Froyd [:froydnj] from comment #5)
> > (In reply to Jeff Gilbert [:jgilbert] from comment #4)
> > > For Angle, all patches are fixed in github.com/mozilla/angle and re-vendored
> > > into moz-central, while being upstreamed into angle/master.
> > > 
> > > Alternatively, just upstream into Angle and we'll pick up whatever's made it
> > > into Chrome Beta's Angle branch each Gecko cycle.
> > 
> > Do we have a policy for backporting patches?  If we don't backport patches,
> > when is the next ANGLE update going to happen?
> 
> I try to update ANGLE in the first week of each new cycle.

Is there a bug for the ANGLE update for Firefox 64?
Flags: needinfo?(jgilbert)
(In reply to Nathan Froyd [:froydnj] from comment #7)
> (In reply to Jeff Gilbert [:jgilbert] from comment #6)
> > (In reply to Nathan Froyd [:froydnj] from comment #5)
> > > (In reply to Jeff Gilbert [:jgilbert] from comment #4)
> > > > For Angle, all patches are fixed in github.com/mozilla/angle and re-vendored
> > > > into moz-central, while being upstreamed into angle/master.
> > > > 
> > > > Alternatively, just upstream into Angle and we'll pick up whatever's made it
> > > > into Chrome Beta's Angle branch each Gecko cycle.
> > > 
> > > Do we have a policy for backporting patches?  If we don't backport patches,
> > > when is the next ANGLE update going to happen?
> > 
> > I try to update ANGLE in the first week of each new cycle.
> 
> Is there a bug for the ANGLE update for Firefox 64?

Bug 1489279
Flags: needinfo?(jgilbert)
Thanks for updating ANGLE.  Unfortunately, the ANGLE update does not fix everything:

 0:31.06 c:\mozilla-central\gfx\angle\checkout\src\common/mathutil.h(935): error C3861: '__popcnt': identifier not found
 0:31.08 mozmake.EXE[4]: *** [c:/mozilla-central/config/rules.mk:1124: Float16ToFloat32.obj] Error 2
 0:31.09 mozmake.EXE[4]: *** Waiting for unfinished jobs....
 0:31.16 PackedEnums.cpp
 0:31.16 c:\mozilla-central\gfx\angle\checkout\src\common/mathutil.h(935): error C3861: '__popcnt': identifier not found
 0:31.17 mozmake.EXE[4]: *** [c:/mozilla-central/config/rules.mk:1124: PackedEnums.obj] Error 2
 0:31.31 gfx/layers
 0:31.39 PackedGLEnums_autogen.cpp
 0:31.41 mozmake.EXE[3]: *** [c:/mozilla-central/config/recurse.mk:74: gfx/angle/targets/angle_common/target] Error 2
 0:31.43 mozmake.EXE[3]: *** Waiting for unfinished jobs....

What is the right way forward here?  Write a patch for our @mozilla fork, and then try to get it accepted upstream?
Flags: needinfo?(jgilbert)
What's the setup for this? Because it looks like it should just work.

https://msdn.microsoft.com/en-us/library/b0084kay.aspx:
> _WIN32 Defined as 1 when the compilation target is 32-bit ARM, 64-bit ARM, x86, or x64. Otherwise, undefined.
> _WIN64 Defined as 1 when the compilation target is 64-bit ARM or x64. Otherwise, undefined.

__popcnt seems to be defined in <intrin.h>, and `#ifdef _MSC_VER` is how we gate it elsewhere in the tree:
https://dxr.mozilla.org/mozilla-central/rev/2e3e89c9c68cd6b09d1853dc5426df3c04971c29/media/libyuv/libyuv/source/compare_win.cc#17

I suspect that our compiler (clang-cl?) is not defining _WIN32 on ARM, which while that sounds correct, is not what the docs say.

As such, I bet this is the issue:
> #if defined(_MSC_VER) && !defined(_M_ARM) && !defined(_M_ARM64)
> #include <intrin.h>
Assignee: nobody → jgilbert
Flags: needinfo?(jgilbert) → needinfo?(nfroyd)
Setting up an MSVC amd64_arm64 cross compiler env, I get:
> _MSC_VER
> _WIN32
> _M_ARM64
cl -E foo.h:
> #line 1 "foo.h"
> 1914
> 1
> 1

Which compiler are we using specifically?
FWIW, this is what I ran in a CMD prompt to get a cross-compiler:
> C:\Users\jgilbert>cd "C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\VC\Auxiliary\Build"
> C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\VC\Auxiliary\Build>vcvarsall.bat amd64_arm64
That is the correct compiler to be using.  We are using MSVC because clang-cl is currently non-functional on AArch64.

__popcnt{,64} is also defined as an intrinsic only on x86 and x86-64 in intrin.h (if I understand intrin.h, that is); I filed a bug with Microsoft.  I suppose we need to add an implementation of our own for the time being?
Flags: needinfo?(nfroyd)
Sounds like it.

FWIW, I took the docs to mean we would have __popcnt on ARM, but it's probably just out-of-date docs:
https://msdn.microsoft.com/en-us/library/bb385231.aspx
Summary: fix ANGLE compilation errors for aarch64 windows → Fix ANGLE compilation errors for MSVC Windows ARM64
Pushed by jgilbert@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8a7d0b7ce84e
Revendor ANGLE with popcnt on Windows+ARM fix.
https://hg.mozilla.org/mozilla-central/rev/8a7d0b7ce84e
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
You need to log in before you can comment on or make changes to this bug.