Bug 794091 introduces two pure JavaScript patches that call into js-ctypes. These patches work nicely but break Win 64 PGO builds with the following error: e:\builds\moz2_slave\try-w64\build\js\src\ctypes\ctypes.cpp(1687) : fatal error C1001: An internal error has occurred in the compiler. Line 1687 points to function |jsvalToSize|. This error is blocking landing of off main thread Session Store writes, which is itself one of the high priority Snappy goals. Full TryServer log: https://tbpl.mozilla.org/php/getParsedLog.php?id=16935855&tree=Try#error2

I will try some options... Maybe, we should use mozjs.dll for Win64, too. Or, turn off PGO for ctypes only.

Also, PGOCVT "internal errors" issue is filed as https://connect.microsoft.com/VisualStudio/feedback/details/766253 for VS2012. PG0001 doesn't depend on bug 794091. (Before bug 794091, this occurs...)

It seems that |#pragma optimize("", off)| around the breakage site works around the issue: https://tbpl.mozilla.org/?tree=Try&rev=e04004655e71 I will clean up the patch and submit.

(In reply to David Rajchenbach Teller [:Yoric] from comment #3) > It seems that |#pragma optimize("", off)| around the breakage site works > around the issue: > https://tbpl.mozilla.org/?tree=Try&rev=e04004655e71 > > I will clean up the patch and submit. Can we use optimize("g", off) instead of? "g" means -GL, and "" means turn off all optimization. We should keep stack protection (/GS option) if possible

(In reply to Makoto Kato from comment #4) > Can we use optimize("g", off) instead of? "g" means -GL, and "" means turn > off all optimization. We should keep stack protection (/GS option) if > possible Checking out: https://tbpl.mozilla.org/?tree=Try&rev=ff8986e49b4a

So far, no success with "g". However, no success anymore with "" since I attempted to clean up my code. I will keep expanding |optimize("", off)| until it works. Makoko Kato, could you also test your approach in the meantime?

What might work is this: CTypes.$(OBJ_SUFFIX): PROFILE_GEN_CFLAGS= in js/src/Makefile.in (with the proper ifeq for win64)

successful with -GL-. Some cpp files uses this option for Win32 to turn on PGO. https://tbpl.mozilla.org/?tree=Try&rev=c33c6862c06c

Comment on attachment 680879 [details] [diff] [review] fix ># HG changeset patch ># Parent 5516d85af0928b6b7cdd250c1abe5879565db2f6 >Bug 810661 - Don't PGO for Ctypes.cpp > >diff --git a/js/src/Makefile.in b/js/src/Makefile.in >--- a/js/src/Makefile.in >+++ b/js/src/Makefile.in >@@ -716,16 +716,20 @@ CFLAGS += -fp:precise > > ifeq ($(CPU_ARCH),x86) > # Workaround compiler bug on PGO (Bug 721284) > MonoIC.$(OBJ_SUFFIX): CXXFLAGS += -GL- > Compiler.$(OBJ_SUFFIX): CXXFLAGS += -GL- > # Ditto (Bug 772303) > RegExp.$(OBJ_SUFFIX): CXXFLAGS += -GL- > endif >+# Ditto (Bug 810661) >+ifeq ($(CPU_ARCH),x86_64) >+CTypes.$(OBJ_SUFFIX): CXXFLAGS += -GL- >+endif > endif # _MSC_VER > > ifeq ($(OS_ARCH),FreeBSD) > EXTRA_LIBS += -pthread > endif > ifeq ($(OS_ARCH),Linux) > EXTRA_LIBS += -ldl > endif

Comment on attachment 680879 [details] [diff] [review] fix Review of attachment 680879 [details] [diff] [review]: ----------------------------------------------------------------- Fun.

Attaching an alternative fix suggested by glandium. TryRun: https://tbpl.mozilla.org/?tree=Try&rev=6fb38a606e45 I am not versed enough in VC++ to determine which variant of the patch is best.

(In reply to David Rajchenbach Teller [:Yoric] from comment #13) > Created attachment 680927 [details] [diff] [review] > Alternative fix > > Attaching an alternative fix suggested by glandium. > TryRun: https://tbpl.mozilla.org/?tree=Try&rev=6fb38a606e45 > > I am not versed enough in VC++ to determine which variant of the patch is > best. Yoric, I already landed workaround in m-i. glandium suggestion is same as changeset 8c5ad849146f.

(In reply to Makoto Kato from comment #14) > Yoric, I already landed workaround in m-i. glandium suggestion is same as > changeset 8c5ad849146f. Ah, great, thanks.