Bug 208731 – Fix some issues in the ARM64 moveConditionallyAfterFloatingPointCompare() and moveDoubleConditionallyAfterFloatingPointCompare().

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Mark Lam]

Details to follow in the ChangeLog.

<rdar://problem/59222568>

[Mark Lam]

Created attachment 392762 [details]
proposed patch.

[Saam Barati]

Comment on attachment 392762 [details]
proposed patch.

View in context: https://bugs.webkit.org/attachment.cgi?id=392762&action=review

> Source/JavaScriptCore/assembler/MacroAssemblerARM64.h:2030
>          if (cond == DoubleNotEqual) {

can you file a bug that DoubleNotEqual really means DoubleNotEqualAndNotUnordered?

[Saam Barati]

Comment on attachment 392762 [details]
proposed patch.

there is more to be done here. We're calling it "cmov", but we're now implementing it as select. Is that ok?

[Saam Barati]

Comment on attachment 392762 [details]
proposed patch.

Since these moveConditionally are really select, maybe we should rename it?

[Mark Lam]

Thanks for the review.

Filed the following for the naming issues:
1. https://bugs.webkit.org/show_bug.cgi?id=208736 Add "AndOrdered" to the names of ordered DoubleConditions.
2. https://bugs.webkit.org/show_bug.cgi?id=208738 Consider renaming moveConditionally... MacroAssembler emitters.

[Mark Lam]

Landed in r258038: <http://trac.webkit.org/r258038>.

[David Kilzer (:ddkilzer)]

(In reply to Mark Lam from comment #6)
> Landed in r258038: <http://trac.webkit.org/r258038>.

This caused a build failure on Windows 10 build bots:

C:\cygwin\home\buildbot\worker\win10-debug\build\Source\JavaScriptCore\assembler\testmasm.cpp(532): error C2220: the following warning is treated as an error [C:\cygwin\home\buildbot\worker\win10-debug\build\WebKitBuild\Debug\Source\JavaScriptCore\shell\testmasmLib.vcxproj]
C:\cygwin\home\buildbot\worker\win10-debug\build\Source\JavaScriptCore\assembler\testmasm.cpp(532): warning C4715: '``anonymous namespace'::testCompareDouble'::`2'::<lambda_3>::operator()': not all control paths return a value [C:\cygwin\home\buildbot\worker\win10-debug\build\WebKitBuild\Debug\Source\JavaScriptCore\shell\testmasmLib.vcxproj]
C:\cygwin\home\buildbot\worker\win10-debug\build\Source\JavaScriptCore\assembler\testmasm.cpp(603): warning C4715: '``anonymous namespace'::testCompareDoubleSameArg'::`2'::<lambda_3>::operator()': not all control paths return a value [C:\cygwin\home\buildbot\worker\win10-debug\build\WebKitBuild\Debug\Source\JavaScriptCore\shell\testmasmLib.vcxproj]
C:\cygwin\home\buildbot\worker\win10-debug\build\Source\JavaScriptCore\assembler\testmasm.cpp(727): warning C4715: '``anonymous namespace'::testMoveConditionallyFloatingPoint<double,unsigned int>'::`2'::<lambda_1>::operator()': not all control paths return a value [C:\cygwin\home\buildbot\worker\win10-debug\build\WebKitBuild\Debug\Source\JavaScriptCore\shell\testmasmLib.vcxproj]
C:\cygwin\home\buildbot\worker\win10-debug\build\Source\JavaScriptCore\assembler\testmasm.cpp(727): warning C4715: '``anonymous namespace'::testMoveConditionallyFloatingPoint<float,unsigned int>'::`2'::<lambda_1>::operator()': not all control paths return a value [C:\cygwin\home\buildbot\worker\win10-debug\build\WebKitBuild\Debug\Source\JavaScriptCore\shell\testmasmLib.vcxproj]
C:\cygwin\home\buildbot\worker\win10-debug\build\Source\JavaScriptCore\assembler\testmasm.cpp(727): warning C4715: '``anonymous namespace'::testMoveConditionallyFloatingPoint<double,double>'::`2'::<lambda_1>::operator()': not all control paths return a value [C:\cygwin\home\buildbot\worker\win10-debug\build\WebKitBuild\Debug\Source\JavaScriptCore\shell\testmasmLib.vcxproj]
C:\cygwin\home\buildbot\worker\win10-debug\build\Source\JavaScriptCore\assembler\testmasm.cpp(1299): warning C4715: '``anonymous namespace'::testMoveConditionallyFloatingPointSameArg<double,unsigned int>'::`2'::<lambda_1>::operator()': not all control paths return a value [C:\cygwin\home\buildbot\worker\win10-debug\build\WebKitBuild\Debug\Source\JavaScriptCore\shell\testmasmLib.vcxproj]
C:\cygwin\home\buildbot\worker\win10-debug\build\Source\JavaScriptCore\assembler\testmasm.cpp(727): warning C4715: '``anonymous namespace'::testMoveConditionallyFloatingPoint<float,double>'::`2'::<lambda_1>::operator()': not all control paths return a value [C:\cygwin\home\buildbot\worker\win10-debug\build\WebKitBuild\Debug\Source\JavaScriptCore\shell\testmasmLib.vcxproj]
C:\cygwin\home\buildbot\worker\win10-debug\build\Source\JavaScriptCore\assembler\testmasm.cpp(1299): warning C4715: '``anonymous namespace'::testMoveConditionallyFloatingPointSameArg<float,unsigned int>'::`2'::<lambda_1>::operator()': not all control paths return a value [C:\cygwin\home\buildbot\worker\win10-debug\build\WebKitBuild\Debug\Source\JavaScriptCore\shell\testmasmLib.vcxproj]
C:\cygwin\home\buildbot\worker\win10-debug\build\Source\JavaScriptCore\assembler\testmasm.cpp(1299): warning C4715: '``anonymous namespace'::testMoveConditionallyFloatingPointSameArg<float,double>'::`2'::<lambda_1>::operator()': not all control paths return a value [C:\cygwin\home\buildbot\worker\win10-debug\build\WebKitBuild\Debug\Source\JavaScriptCore\shell\testmasmLib.vcxproj]
C:\cygwin\home\buildbot\worker\win10-debug\build\Source\JavaScriptCore\assembler\testmasm.cpp(1299): warning C4715: '``anonymous namespace'::testMoveConditionallyFloatingPointSameArg<double,double>'::`2'::<lambda_1>::operator()': not all control paths return a value [C:\cygwin\home\buildbot\worker\win10-debug\build\WebKitBuild\Debug\Source\JavaScriptCore\shell\testmasmLib.vcxproj]

<https://build.webkit.org/builders/Apple%20Win%2010%20Debug%20%28Build%29/builds/11014/steps/compile-webkit/logs/stdio>

Committed r258062: <http://trac.webkit.org/r258062>