Implement SSE denormal disabler for windows.
Created attachment 111796 [details] Patch
Attachment 111796 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebCore/ChangeLog:4: Line contains tab character. [whitespace/tab] [5] Source/WebCore/ChangeLog:5: Line contains tab character. [whitespace/tab] [5] Total errors found: 2 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 111797 [details] Patch
This implements the denormal disabler for Windows similar to what is done on Mac and Linux. Chris, although we talked about not checking, I decided that we should check if SSE2 is available before we try to run instructions that might not be available. I didn't want to cause crashes for people running chrome on windows on old machines without SSE2. The same check is done for Mac/Linux because I merged most of the code into one implementation.
Comment on attachment 111797 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=111797&action=review > Source/WebCore/platform/audio/DenormalDisabler.h:39 > #if defined(__GNUC__) && (defined(__i386__) || defined(__x86_64__)) Are you sure that inline assembly is supported in the 64-bit MSVC compiler? http://stackoverflow.com/questions/2980674/fesetround-with-msvc-x64 indicates it isn't. Could you use Microsoft-specific intrinsics instead of assembly? See http://msdn.microsoft.com/en-gb/library/e9b52ceh.aspx .
Created attachment 111838 [details] Patch
Comment on attachment 111797 [details] Patch _controlfp_s intrinsic is used now. No inline assembly is used because it is not available on x64. (http://msdn.microsoft.com/en-us/library/wbk4z78b.aspx). Ripped out the check for SSE2 since we're not doing that on Windows and weren't doing it on Linux/Mac previously.
Comment on attachment 111838 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=111838&action=review Looks good to me. One question, but r=me. > Source/WebCore/platform/audio/DenormalDisabler.h:55 > + _controlfp_s(&unused, _DN_FLUSH, _MCW_DN); Since the guard in flushDenormalFloatToZero includes (!_M_IX86_FP), would it make sense to elide this block and the _controlfp_s call in the destructor if (!_M_IX86_FP)?
Comment on attachment 111838 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=111838&action=review >> Source/WebCore/platform/audio/DenormalDisabler.h:55 >> + _controlfp_s(&unused, _DN_FLUSH, _MCW_DN); > > Since the guard in flushDenormalFloatToZero includes (!_M_IX86_FP), would it make sense to elide this block and the _controlfp_s call in the destructor if (!_M_IX86_FP)? I thought about that, but the OS(WINDOWS) and COMPILER(MSVC) makes it clear this is a windows thing, which is harder to tell if you just had !_M_IX86_FP. But at some expense in readability and duplication. No problem if you think changing it makes it better.
(In reply to comment #9) > (From update of attachment 111838 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=111838&action=review > > >> Source/WebCore/platform/audio/DenormalDisabler.h:55 > >> + _controlfp_s(&unused, _DN_FLUSH, _MCW_DN); > > > > Since the guard in flushDenormalFloatToZero includes (!_M_IX86_FP), would it make sense to elide this block and the _controlfp_s call in the destructor if (!_M_IX86_FP)? > > I thought about that, but the OS(WINDOWS) and COMPILER(MSVC) makes it clear this is a windows thing, which is harder to tell if you just had !_M_IX86_FP. But at some expense in readability and duplication. > > No problem if you think changing it makes it better. I see. What I was suggesting was to add the #if (!_M_IX86_FP) include guard inside the existing OS(WINDOWS) include guards. It's fine as is; the extra work on non-SSE2 platforms should be negligible.
Comment on attachment 111838 [details] Patch Clearing flags on attachment: 111838 Committed r98029: <http://trac.webkit.org/changeset/98029>
All reviewed patches have been landed. Closing bug.