| Summary: | ENABLED(JIT) && !ENABLED(DFG_JIT) breaks the build | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Allison Lortie (desrt) <desrt> | ||||||||
| Component: | JavaScriptCore | Assignee: | Nobody <webkit-unassigned> | ||||||||
| Status: | RESOLVED FIXED | ||||||||||
| Severity: | Normal | CC: | benjamin, berto, cmarcelo, commit-queue, dbatyai.u-szeged, fpizlo, ggaren, lantw44, mark.lam, mhahnenberg, msaboff, oliver, ossy, zan | ||||||||
| Priority: | P2 | ||||||||||
| Version: | 528+ (Nightly build) | ||||||||||
| Hardware: | Unspecified | ||||||||||
| OS: | Unspecified | ||||||||||
| Attachments: |
|
||||||||||
(In reply to comment #0) > The fix to this bug is probably to change some cases of ENABLE(JIT) > to ENABLE(DFG_JIT) Yes, probably, it seems that this was introduced when the jsCStack was merged, adding Michael Saboff to Cc. > but ultimately it seems like it might also be a good idea to enable > DFG_JIT on FreeBSD. I agree, can you test if it works or it doesn't build yet? :) Thanks! It still doesn't build because of the 'too long argument list' to the linker problem. I can verify that adding OS(FREEBSD) to the list does get it past the build problem described here, though. Okay. I managed to fix a couple of issues to get a complete build. I can verify that forcing DFG_JIT to be enabled on FreeBSD results in a working browser. It scores 100/100 on Acid3, at least. Is there any other 'more scientific' way that I can test that? Created attachment 223847 [details]
Enable DFG_JIT on FreeBSD
(In reply to comment #4) > Enable DFG_JIT on FreeBSD It's probably fine to enable this on FreeBSD, but there's still the original problem of why it no longer builds without it. (In reply to comment #5) > (In reply to comment #4) > > Enable DFG_JIT on FreeBSD > > It's probably fine to enable this on FreeBSD, but there's still the > original problem of why it no longer builds without it. The reasons are probably the same as those outlined here: https://bugs.webkit.org/show_bug.cgi?id=126502#c4 (In reply to comment #6) > > It's probably fine to enable this on FreeBSD, but there's still > > the original problem of why it no longer builds without it. > The reasons are probably the same as those outlined here: > https://bugs.webkit.org/show_bug.cgi?id=126502#c4 I was thinking the same, but then we should probably remove the #ifdefs altogether. With DFG_JIT on FreeBSD, stress tests pass 8212/8212. The mozilla tests have 45 failures out of 1127, but apparently this is normal (and indeed, many of the failures have links to already-open bugs). Created attachment 224057 [details]
Build fix for enabled JIT with disabled DFG_JIT
(In reply to comment #9) > Created an attachment (id=224057) [details] > Build fix for enabled JIT with disabled DFG_JIT Thanks for the patch, although I haven't had the chance to test it yet. Even if we fix the build we can also enable DFG_JIT on FreeBSD, I just filed bug 128898 for that. Comment on attachment 224057 [details] Build fix for enabled JIT with disabled DFG_JIT CallLinkStatus.cpp, GetByIdStatus.cpp and PutByIdStatus.cpp were already fixed by https://trac.webkit.org/changeset/164164 Comment on attachment 224057 [details] Build fix for enabled JIT with disabled DFG_JIT View in context: https://bugs.webkit.org/attachment.cgi?id=224057&action=review r-, because it can't be applied now. > Source/JavaScriptCore/jit/Repatch.cpp:825 > +#if ENABLE(DFG_JIT) > MacroAssembler::Jump definitelyNotMarked = DFG::SpeculativeJIT::genericWriteBarrier(jit, owner, scratch1, scratch2); > - MacroAssembler::Call call = storeToWriteBarrierBuffer(jit, owner, scratch1, scratch2, allocator); > definitelyNotMarked.link(&jit); > +#endif // ENABLE(DFG_JIT) > + > + MacroAssembler::Call call = storeToWriteBarrierBuffer(jit, owner, scratch1, scratch2, allocator); Are you sure if this is the proper fix? I don't understand this code, but these lines looks to me as they can't be separated. I tried the proposed fix for Repatch.cpp, it builds but I got some test failures:
stress/tricky-array-bounds-checks.js.always-trigger-copy-phase: Exception: Error: bad result for trick: ,,,false
stress/tricky-array-bounds-checks.js.default: Exception: Error: bad result for trick: ,,,false
stress/tricky-array-bounds-checks.js.no-llint: Exception: Error: bad result for trick: ,,,false
stress/tricky-array-bounds-checks.js.no-cjit-validate-phases: Exception: Error: bad result for trick: ,,,false
stress/tricky-array-bounds-checks.js.default: ERROR: Unexpected exit code: 3
stress/tricky-array-bounds-checks.js.always-trigger-copy-phase: ERROR: Unexpected exit code: 3
stress/tricky-array-bounds-checks.js.no-llint: ERROR: Unexpected exit code: 3
stress/tricky-array-bounds-checks.js.dfg-eager: Exception: Error: bad result for trick: ,,,false
stress/tricky-array-bounds-checks.js.dfg-eager-no-cjit-validate: Exception: Error: bad result for trick: ,,,false
stress/tricky-array-bounds-checks.js.no-cjit-validate-phases: ERROR: Unexpected exit code: 3
stress/tricky-array-bounds-checks.js.dfg-eager: ERROR: Unexpected exit code: 3
stress/tricky-array-bounds-checks.js.dfg-eager-no-cjit-validate: ERROR: Unexpected exit code: 3
8352/8352 (failed 6)
** The following JSC stress test failures have been introduced:
stress/tricky-array-bounds-checks.js.default
stress/tricky-array-bounds-checks.js.no-llint
stress/tricky-array-bounds-checks.js.always-trigger-copy-phase
stress/tricky-array-bounds-checks.js.no-cjit-validate-phases
stress/tricky-array-bounds-checks.js.dfg-eager
stress/tricky-array-bounds-checks.js.dfg-eager-no-cjit-validate
Results for JSC stress tests:
6 failures found.
I'm not sure if they were caused by this change or other bug.
I checked the failures, they aren't related to this fix, they fail on trunk with runtime dissabled DFG too. (In reply to comment #14) > I checked the failures, they aren't related to this fix, > they fail on trunk with runtime dissabled DFG too. New bug report for this failire: https://bugs.webkit.org/show_bug.cgi?id=128910 (In reply to comment #14) > I checked the failures, they aren't related to this fix, > they fail on trunk with runtime dissabled DFG too. You still have a review- flag on this bug. (In reply to comment #16) > (In reply to comment #14) > > I checked the failures, they aren't related to this fix, > > they fail on trunk with runtime dissabled DFG too. > > You still have a review- flag on this bug. I r- -ed it, because it is obsolete now, see https://bugs.webkit.org/show_bug.cgi?id=128549#c11 for details. Maybe the fix in Source/JavaScriptCore/jit/Repatch.cpp is good. It worked for me without new regressions. But I'm not sure if it is the proper fix. Comment about it from a JSC expert would be welcome. Created attachment 225780 [details]
Proposed fix v2
Comment on attachment 225780 [details] Proposed fix v2 Thanks for looking into this. However, I think this issue has been solved in a more appropriate refactoring in https://bugs.webkit.org/show_bug.cgi?id=129760. Please re-test after that lands (or try the patch in that bug) to see if there are still other outstanding issue for the !ENABLE(DFG_JIT) build. (In reply to comment #19) > (From update of attachment 225780 [details]) > Thanks for looking into this. However, I think this issue has been solved in a more appropriate refactoring in https://bugs.webkit.org/show_bug.cgi?id=129760. Please re-test after that lands (or try the patch in that bug) to see if there are still other outstanding issue for the !ENABLE(DFG_JIT) build. Thanks for the heads up. I tried the patch and managed to build with disabled DFG JIT without any problems. ENABLE(JIT) && !ENABLE(DFG_JIT) build works fine on ToT. |
Building on FreeBSD results in JIT being enabled, but not DFG_JIT. This is a result of this check: #if !defined(ENABLE_DFG_JIT) && ENABLE(JIT) && !COMPILER(MSVC) /* Enable the DFG JIT on X86 and X86_64. Only tested on Mac and GNU/Linux. */ #if (CPU(X86) || CPU(X86_64)) && (OS(DARWIN) || OS(LINUX)) #define ENABLE_DFG_JIT 1 #endif As a result, some files in JavascriptScore fail to compile in a fairly simple (presumably as a result of nobody testing this combination). Source/JavaScriptCore/bytecode/CallLinkStatus.cpp:125:24: error: no member named 'hasExitSite' in 'JSC::CodeBlock' if (profiledBlock->hasExitSite(locker, DFG::FrequentExitSite(bytecodeIndex, BadCache)) ~~~~~~~~~~~~~ ^ Source/JavaScriptCore/bytecode/CallLinkStatus.cpp:125:49: error: no member named 'FrequentExitSite' in namespace 'JSC::DFG' if (profiledBlock->hasExitSite(locker, DFG::FrequentExitSite(bytecodeIndex, BadCache)) ~~~~~^ Source/JavaScriptCore/bytecode/CallLinkStatus.cpp:126:27: error: no member named 'hasExitSite' in 'JSC::CodeBlock' || profiledBlock->hasExitSite(locker, DFG::FrequentExitSite(bytecodeIndex, BadCacheWatchpoint)) ~~~~~~~~~~~~~ ^ Source/JavaScriptCore/bytecode/CallLinkStatus.cpp:126:52:Source/JavaScriptCore/bytecode/GetByIdStatus.cpp:40:27: error: no member named 'hasExitSite' in 'JSC::CodeBlock' return profiledBlock->hasExitSite(locker, DFG::FrequentExitSite(bytecodeIndex, BadCache, jitType)) ~~~~~~~~~~~~~ ^ error: no member named 'FrequentExitSite' in namespace 'JSC::DFG' || profiledBlock->hasExitSite(locker, DFG::FrequentExitSite(bytecodeIndex, BadCacheWatchpoint)) ~~~~~^ Source/JavaScriptCore/bytecode/CallLinkStatus.cpp:127:27: error: no member named 'hasExitSite' in 'JSC::CodeBlock' || profiledBlock->hasExitSite(locker, DFG::FrequentExitSite(bytecodeIndex, BadExecutable))) ~~~~~~~~~~~~~ ^ Source/JavaScriptCore/bytecode/GetByIdStatus.cpp:40:52: error: no member named 'FrequentExitSite' in namespace 'JSC::DFG' return profiledBlock->hasExitSite(locker, DFG::FrequentExitSite(bytecodeIndex, BadCache, jitType)) ~~~~~^ Source/JavaScriptCore/bytecode/GetByIdStatus.cpp:41:27: error: no member named 'hasExitSite' in 'JSC::CodeBlock' || profiledBlock->hasExitSite(locker, DFG::FrequentExitSite(bytecodeIndex, BadCacheWatchpoint, jitType)) ~~~~~~~~~~~~~ ^ Source/JavaScriptCore/bytecode/CallLinkStatus.cpp:127:52: error: no member named 'FrequentExitSite' in namespace 'JSC::DFG' || profiledBlock->hasExitSite(locker, DFG::FrequentExitSite(bytecodeIndex, BadExecutable))) ~~~~~^ Source/JavaScriptCore/bytecode/CallLinkStatus.cpp:142:24: error: no member named 'hasExitSite' in 'JSC::CodeBlock' if (profiledBlock->hasExitSite(locker, DFG::FrequentExitSite(bytecodeIndex, BadFunction))) ~~~~~~~~~~~~~ ^ Source/JavaScriptCore/bytecode/GetByIdStatus.cpp:41:52: error: no member named 'FrequentExitSite' in namespace 'JSC::DFG' || profiledBlock->hasExitSite(locker, DFG::FrequentExitSite(bytecodeIndex, BadCacheWatchpoint, jitType)) ~~~~~^ Source/JavaScriptCore/bytecode/GetByIdStatus.cpp:42:27: error: no member named 'hasExitSite' in 'JSC::CodeBlock' || profiledBlock->hasExitSite(locker, DFG::FrequentExitSite(bytecodeIndex, BadWeakConstantCache, jitType)) ~~~~~~~~~~~~~ ^ Source/JavaScriptCore/bytecode/CallLinkStatus.cpp:142:49: error: no member named 'FrequentExitSite' in namespace 'JSC::DFG' if (profiledBlock->hasExitSite(locker, DFG::FrequentExitSite(bytecodeIndex, BadFunction))) ~~~~~^ Source/JavaScriptCore/bytecode/GetByIdStatus.cpp:42:52: error: no member named 'FrequentExitSite' in namespace 'JSC::DFG' || profiledBlock->hasExitSite(locker, DFG::FrequentExitSite(bytecodeIndex, BadWeakConstantCache, jitType)) ~~~~~^ Source/JavaScriptCore/bytecode/GetByIdStatus.cpp:43:27: error: no member named 'hasExitSite' in 'JSC::CodeBlock' || profiledBlock->hasExitSite(locker, DFG::FrequentExitSite(bytecodeIndex, BadWeakConstantCacheWatchpoin... ~~~~~~~~~~~~~ ^ CXX Source/JavaScriptCore/bytecode/libjavascriptcoregtk_3_0_la-LazyOperandValueProfile.lo Source/JavaScriptCore/bytecode/GetByIdStatus.cpp:43:52: error: no member named 'FrequentExitSite' in namespace 'JSC::DFG' || profiledBlock->hasExitSite(locker, DFG::FrequentExitSite(bytecodeIndex, BadWeakConstantCacheWatchpoin... ~~~~~^ CXX Source/JavaScriptCore/bytecode/libjavascriptcoregtk_3_0_la-MethodOfGettingAValueProfile.lo 8 errors generated. The first of these errors, at CallLinkStatus.cpp:125:24, is caused by a use of DFG::FrequentExitSite guarded by only ENABLE(JIT) whereas its site of definition is guarded by ENABLE(DFG_JIT): #ifndef DFGExitProfile_h #define DFGExitProfile_h #if ENABLE(DFG_JIT) ... namespace JSC { namespace DFG { class FrequentExitSite { ... A earlier use of DFG::FrequentExitSite in the same file (line 94) is guarded by ENABLE(DFG_JIT) and there are no problems there. The fix to this bug is probably to change some cases of ENABLE(JIT) to ENABLE(DFG_JIT) but ultimately it seems like it might also be a good idea to enable DFG_JIT on FreeBSD.