Bug 114934

Summary: Memory barrier support should also ensure that we always do a compiler fence
Product: WebKit Reporter: Filip Pizlo <fpizlo>
Component: Web Template FrameworkAssignee: Filip Pizlo <fpizlo>
Status: RESOLVED FIXED    
Severity: Normal CC: barraclough, benjamin, cmarcelo, commit-queue, ggaren, jberlin, mark.lam, mhahnenberg, msaboff, oliver, sam
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
the patch
none
fix windows
buildbot: commit-queue-
the patch
none
the patch
none
the patch msaboff: review+

Filip Pizlo
Reported 2013-04-21 22:51:05 PDT
To prevent the compiler from doing its own reorderings
Attachments
the patch (4.56 KB, patch)
2013-04-21 22:56 PDT, Filip Pizlo
no flags
fix windows (4.85 KB, patch)
2013-04-21 23:08 PDT, Filip Pizlo
buildbot: commit-queue-
the patch (5.19 KB, patch)
2013-04-22 08:24 PDT, Filip Pizlo
no flags
the patch (5.17 KB, patch)
2013-04-22 08:47 PDT, Filip Pizlo
no flags
the patch (5.17 KB, patch)
2013-04-22 08:48 PDT, Filip Pizlo
msaboff: review+
Filip Pizlo
Comment 1 2013-04-21 22:56:22 PDT
Created attachment 198992 [details] the patch
WebKit Commit Bot
Comment 2 2013-04-21 22:58:42 PDT
Attachment 198992 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WTF/ChangeLog', u'Source/WTF/wtf/Atomics.h']" exit_code: 1 Source/WTF/wtf/Atomics.h:213: armV7_dmb is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] Source/WTF/wtf/Atomics.h:219: armV7_dmb_st is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] Source/WTF/wtf/Atomics.h:233: x86_mfence is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] Total errors found: 3 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.
Filip Pizlo
Comment 3 2013-04-21 23:00:04 PDT
(In reply to comment #2) > Attachment 198992 [details] did not pass style-queue: > > Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WTF/ChangeLog', u'Source/WTF/wtf/Atomics.h']" exit_code: 1 > Source/WTF/wtf/Atomics.h:213: armV7_dmb is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] > Source/WTF/wtf/Atomics.h:219: armV7_dmb_st is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] > Source/WTF/wtf/Atomics.h:233: x86_mfence is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] > Total errors found: 3 in 2 files > > > If any of these errors are false positives, please file a bug against check-webkit-style. This style is intentional. It would be weird to call "armV7_dmb" something like "dmb" instead, since that would probably be both confusing and would lead to too much namespace pollution. It would also be weird to call it "armV7DMB", since then it's less clear that we're referring to the well-known "dmb" instruction on ARM. Likewise for "armV7_dmb_st" and "x86_mfence".
Filip Pizlo
Comment 4 2013-04-21 23:08:39 PDT
Created attachment 198994 [details] fix windows _ReadWriteBarrier() is the Windows intrinsic for doing the equivalent of asm volatile("" ::: "memory").
WebKit Commit Bot
Comment 5 2013-04-21 23:11:51 PDT
Attachment 198994 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WTF/ChangeLog', u'Source/WTF/wtf/Atomics.h']" exit_code: 1 Source/WTF/wtf/Atomics.h:218: armV7_dmb is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] Source/WTF/wtf/Atomics.h:224: armV7_dmb_st is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] Source/WTF/wtf/Atomics.h:238: x86_mfence is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] Total errors found: 3 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.
Build Bot
Comment 6 2013-04-21 23:42:05 PDT
Filip Pizlo
Comment 7 2013-04-22 08:24:33 PDT
Created attachment 199034 [details] the patch Fix Windows again.
WebKit Commit Bot
Comment 8 2013-04-22 08:27:12 PDT
Attachment 199034 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WTF/ChangeLog', u'Source/WTF/wtf/Atomics.h']" exit_code: 1 Source/WTF/wtf/Atomics.h:218: armV7_dmb is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] Source/WTF/wtf/Atomics.h:224: armV7_dmb_st is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] Source/WTF/wtf/Atomics.h:238: x86_mfence is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] Total errors found: 3 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.
Darin Adler
Comment 9 2013-04-22 08:39:20 PDT
Comment on attachment 199034 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=199034&action=review > Source/WTF/ChangeLog:27 > + (WTF): WTF indeed. Please remove bogus lines like this or get someone to fix the script that generates them.
Filip Pizlo
Comment 10 2013-04-22 08:47:58 PDT
Created attachment 199035 [details] the patch Fix ChangeLog (removed the bogus WTF line).
Filip Pizlo
Comment 11 2013-04-22 08:48:50 PDT
Created attachment 199036 [details] the patch Fixed ChangeLog again - s/marged/marked
WebKit Commit Bot
Comment 12 2013-04-22 08:50:36 PDT
Attachment 199036 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WTF/ChangeLog', u'Source/WTF/wtf/Atomics.h']" exit_code: 1 Source/WTF/wtf/Atomics.h:218: armV7_dmb is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] Source/WTF/wtf/Atomics.h:224: armV7_dmb_st is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] Source/WTF/wtf/Atomics.h:238: x86_mfence is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] Total errors found: 3 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.
Filip Pizlo
Comment 13 2013-04-22 09:31:56 PDT
Jessie Berlin
Comment 14 2013-04-22 19:14:08 PDT
(In reply to comment #13) > Landed in http://trac.webkit.org/changeset/148888 This broke the Windows build: http://build.webkit.org/builders/Apple%20Win%20Debug%20%28Build%29/builds/65945/steps/compile-webkit/logs/stdio 7>Linking... 7> Creating library C:\cygwin\home\buildbot\slave\win-debug\build\WebKitBuild\Debug\lib\JavaScriptCore.lib and object C:\cygwin\home\buildbot\slave\win-debug\build\WebKitBuild\Debug\lib\JavaScriptCore.exp 7>WeakSet.obj : error LNK2001: unresolved external symbol __ReadWriteBarrier 7>WTF.lib(MetaAllocator.obj) : error LNK2001: unresolved external symbol __ReadWriteBarrier 7>GCThreadSharedData.obj : error LNK2001: unresolved external symbol __ReadWriteBarrier 7>MarkedAllocator.obj : error LNK2001: unresolved external symbol __ReadWriteBarrier 7>MarkedSpace.obj : error LNK2001: unresolved external symbol __ReadWriteBarrier 7>MarkStack.obj : error LNK2001: unresolved external symbol __ReadWriteBarrier 7>BlockAllocator.obj : error LNK2001: unresolved external symbol __ReadWriteBarrier 7>ConservativeRoots.obj : error LNK2001: unresolved external symbol __ReadWriteBarrier 7>CopiedSpace.obj : error LNK2019: unresolved external symbol __ReadWriteBarrier referenced in function "private: void __thiscall JSC::CopiedBlock::zeroFillWilderness(void)" (?zeroFillWilderness@CopiedBlock@JSC@@AAEXXZ) 7>CopyVisitor.obj : error LNK2001: unresolved external symbol __ReadWriteBarrier 7>JSLock.obj : error LNK2001: unresolved external symbol __ReadWriteBarrier 7>JSObject.obj : error LNK2001: unresolved external symbol __ReadWriteBarrier 7>ProfilerDatabase.obj : error LNK2001: unresolved external symbol __ReadWriteBarrier 7>SourceProvider.obj : error LNK2001: unresolved external symbol __ReadWriteBarrier 7>C:\cygwin\home\buildbot\slave\win-debug\build\WebKitBuild\Debug\bin\JavaScriptCore.dll : fatal error LNK1120: 1 unresolved externals 7>Project : warning PRJ0018 : The following environment variables were not found: 7>$(ENABLE_UNDO_MANAGER) 7>Build log was saved at "file://C:\cygwin\home\buildbot\slave\win-debug\build\WebKitBuild\Debug\obj\JavaScriptCore\BuildLog.htm" 7>JavaScriptCore - 15 error(s), 0 warning(s)
Note You need to log in before you can comment on or make changes to this bug.