Roll back -ffast-math

Summary:
@public

`-ffast-math` does not have measurable performance benefits.

By using `NaN` for *undefined* values again, we can squeeze `YGFloatOptional` into 32 bits.
This will also enable us to store `YGValue` (or a variant of it) in 32 bits.

Reviewed By: SidharthGuglani

Differential Revision: D13119110

fbshipit-source-id: 4e6964240bf74ebc22d8783107b89d536a1a0842
This commit is contained in:
David Aurelio
2018-12-06 07:35:07 -08:00
committed by Facebook Github Bot
parent b9972cee6e
commit 0962c5220c
7 changed files with 21 additions and 77 deletions

View File

@@ -1,4 +1,4 @@
/** /**
* Copyright (c) Facebook, Inc. and its affiliates. * Copyright (c) Facebook, Inc. and its affiliates.
* *
* This source code is licensed under the MIT license found in the * This source code is licensed under the MIT license found in the
@@ -9,28 +9,11 @@ namespace Facebook.Yoga
{ {
public static class YogaConstants public static class YogaConstants
{ {
/** public const float Undefined = float.NaN;
* Large positive number signifies that the property(float) is undefined. Earlier we used to have
* YGundefined as NAN, but the downside of this is that we can't use -ffast-math compiler flag as
* it assumes all floating-point calculation involve and result into finite numbers. For more
* information regarding -ffast-math compiler flag in clang, have a look at
* https://clang.llvm.org/docs/UsersManual.html#cmdoption-ffast-math
*/
public const float Undefined = 10E20F;
public static bool IsUndefined(float value) public static bool IsUndefined(float value)
{ {
// Value of a float in the case of it being not defined is 10.1E20. Earlier it used to be NAN, return float.IsNaN(value);
// the benefit of which
// was that if NAN is involved in any mathematical expression the result was NAN. But since we
// want to have `-ffast-math`
// flag being used by compiler which assumes that the floating point values are not NAN and Inf,
// we represent YGUndefined as 10.1E20.
// But now if YGUndefined is involved in any mathematical operations this value(10.1E20) would
// change.
// So the following check makes sure that if the value is outside a range (-10E8, 10E8) then it
// is undefined.
return value >= 10E8F || value <= -10E8;
} }
public static bool IsUndefined(YogaValue value) public static bool IsUndefined(YogaValue value)

View File

@@ -1,35 +1,17 @@
/* /**
* Copyright (c) Facebook, Inc. and its affiliates. * Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the LICENSE
* file in the root directory of this source tree.
* *
* This source code is licensed under the MIT license found in the LICENSE
* file in the root directory of this source tree.
*/ */
package com.facebook.yoga; package com.facebook.yoga;
public class YogaConstants { public class YogaConstants {
/** public static final float UNDEFINED = Float.NaN;
* Large positive number signifies that the property(float) is undefined. Earlier we used to have
* YGundefined as NAN, but the downside of this is that we can't use -ffast-math compiler flag as
* it assumes all floating-point calculation involve and result into finite numbers. For more
* information regarding -ffast-math compiler flag in clang, have a look at
* https://clang.llvm.org/docs/UsersManual.html#cmdoption-ffast-math
*/
public static final float UNDEFINED = (float) (10E20);
public static boolean isUndefined(float value) { public static boolean isUndefined(float value) {
// Value of a float in the case of it being not defined is 10.1E20. Earlier it used to be NAN, return Float.compare(value, UNDEFINED) == 0;
// the benefit of which
// was that if NAN is involved in any mathematical expression the result was NAN. But since we
// want to have `-ffast-math`
// flag being used by compiler which assumes that the floating point values are not NAN and Inf,
// we represent YGUndefined as 10.1E20.
// But now if YGUndefined is involved in any mathematical operations this value(10.1E20) would
// change.
// So the following check makes sure that if the value is outside a range (-10E8, 10E8) then it
// is undefined.
return (Float.compare(value, (float) 10E8) >= 0 || Float.compare(value, (float) -10E8) <= 0);
} }
public static boolean isUndefined(YogaValue value) { public static boolean isUndefined(YogaValue value) {

View File

@@ -58,7 +58,6 @@ BASE_COMPILER_FLAGS = [
"-Wall", "-Wall",
"-Werror", "-Werror",
"-O3", "-O3",
"-ffast-math",
] ]
LIBRARY_COMPILER_FLAGS = BASE_COMPILER_FLAGS + [ LIBRARY_COMPILER_FLAGS = BASE_COMPILER_FLAGS + [

View File

@@ -57,22 +57,12 @@ bool YGValueEqual(const YGValue a, const YGValue b);
// difference between two floats is less than 0.0001f or both are undefined. // difference between two floats is less than 0.0001f or both are undefined.
bool YGFloatsEqual(const float a, const float b); bool YGFloatsEqual(const float a, const float b);
// We need custom max function, since we want that, if one argument is
// YGUndefined then the max funtion should return the other argument as the max
// value. We wouldn't have needed a custom max function if YGUndefined was NAN
// as fmax has the same behaviour, but with NAN we cannot use `-ffast-math`
// compiler flag.
float YGFloatMax(const float a, const float b); float YGFloatMax(const float a, const float b);
YGFloatOptional YGFloatOptionalMax( YGFloatOptional YGFloatOptionalMax(
const YGFloatOptional& op1, const YGFloatOptional& op1,
const YGFloatOptional& op2); const YGFloatOptional& op2);
// We need custom min function, since we want that, if one argument is
// YGUndefined then the min funtion should return the other argument as the min
// value. We wouldn't have needed a custom min function if YGUndefined was NAN
// as fmin has the same behaviour, but with NAN we cannot use `-ffast-math`
// compiler flag.
float YGFloatMin(const float a, const float b); float YGFloatMin(const float a, const float b);
// This custom float comparision function compares the array of float with // This custom float comparision function compares the array of float with
@@ -106,7 +96,9 @@ inline bool YGFlexDirectionIsRow(const YGFlexDirection flexDirection) {
flexDirection == YGFlexDirectionRowReverse; flexDirection == YGFlexDirectionRowReverse;
} }
inline YGFloatOptional YGResolveValue(const YGValue value, const float ownerSize) { inline YGFloatOptional YGResolveValue(
const YGValue value,
const float ownerSize) {
switch (value.unit) { switch (value.unit) {
case YGUnitUndefined: case YGUnitUndefined:
case YGUnitAuto: case YGUnitAuto:

View File

@@ -27,15 +27,7 @@ namespace facebook {
namespace yoga { namespace yoga {
inline bool isUndefined(float value) { inline bool isUndefined(float value) {
// Value of a float in the case of it being not defined is 10.1E20. Earlier return std::isnan(value);
// it used to be NAN, the benefit of which was that if NAN is involved in any
// mathematical expression the result was NAN. But since we want to have
// `-ffast-math` flag being used by compiler which assumes that the floating
// point values are not NAN and Inf, we represent YGUndefined as 10.1E20. But
// now if YGUndefined is involved in any mathematical operations this
// value(10.1E20) would change. So the following check makes sure that if the
// value is outside a range (-10E8, 10E8) then it is undefined.
return value >= 10E8 || value <= -10E8;
} }
} // namespace yoga } // namespace yoga

View File

@@ -18,10 +18,7 @@
/* define fmaxf if < VC12 */ /* define fmaxf if < VC12 */
#if _MSC_VER < 1800 #if _MSC_VER < 1800
__forceinline const float fmaxf(const float a, const float b) { __forceinline const float fmaxf(const float a, const float b) {
if (!YGFloatIsUndefined(a) && !YGFloatIsUndefined(b)) { return (a > b) ? a : b;
return (a > b) ? a : b;
}
return YGFloatIsUndefined(a) ? b : a;
} }
#endif #endif
#endif #endif

View File

@@ -17,14 +17,13 @@
#include <stdbool.h> #include <stdbool.h>
#endif #endif
/** Large positive number signifies that the property(float) is undefined. // Not defined in MSVC++
*Earlier we used to have YGundefined as NAN, but the downside of this is that #ifndef NAN
*we can't use -ffast-math compiler flag as it assumes all floating-point static const unsigned long __nan[2] = {0xffffffff, 0x7fffffff};
*calculation involve and result into finite numbers. For more information #define NAN (*(const float*)__nan)
*regarding -ffast-math compiler flag in clang, have a look at #endif
*https://clang.llvm.org/docs/UsersManual.html#cmdoption-ffast-math
**/ #define YGUndefined NAN
#define YGUndefined 10E20F
#include "YGEnums.h" #include "YGEnums.h"
#include "YGMacros.h" #include "YGMacros.h"