From f6bb619a1629dc5a3935b5ae384be25eb038acd0 Mon Sep 17 00:00:00 2001 From: John Stiles Date: Thu, 20 Jan 2022 19:50:13 -0500 Subject: [PATCH] Fix underflow/overflow issues in skvm::approx_pow2. The skvm implementation of pow2 uses a clever bit-twiddling trick to generate a floating-point value that can be cast to integer, then bit- punned to float, to generate its result. However, the bit trick fails for large inputs, and the bit-punning step generates a nonsense result. This is now fixed by using a well-positioned clamp. Change-Id: I55143a98324f5f518d0875149a0b6ce6d734ded0 Bug: skia:12847 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/497283 Auto-Submit: John Stiles Reviewed-by: Brian Osman Commit-Queue: Brian Osman --- src/core/SkVM.cpp | 6 +++++- tests/SkVMTest.cpp | 8 +++++--- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/core/SkVM.cpp b/src/core/SkVM.cpp index d6e4ae411c..518ae049da 100644 --- a/src/core/SkVM.cpp +++ b/src/core/SkVM.cpp @@ -847,12 +847,16 @@ namespace skvm { } F32 Builder::approx_pow2(F32 x) { + constexpr float kInfinityBits = 0x7f800000; + F32 f = fract(x); F32 approx = add(x, 121.274057500f); approx = sub(approx, mul( 1.490129070f, f)); approx = add(approx, div(27.728023300f, sub(4.84252568f, f))); + approx = mul(1.0f * (1<<23), approx); + approx = clamp(approx, 0, kInfinityBits); // guard against underflow/overflow - return pun_to_F32(round(mul(1.0f * (1<<23), approx))); + return pun_to_F32(round(approx)); } F32 Builder::approx_powf(F32 x, F32 y) { diff --git a/tests/SkVMTest.cpp b/tests/SkVMTest.cpp index ef82610ccc..eb88ea05fb 100644 --- a/tests/SkVMTest.cpp +++ b/tests/SkVMTest.cpp @@ -2134,7 +2134,9 @@ DEF_TEST(SkVM_approx_math, r) { auto compare = [r](int N, const float values[], const float expected[]) { for (int i = 0; i < N; ++i) { - REPORTER_ASSERT(r, SkScalarNearlyEqual(values[i], expected[i], 0.001f)); + REPORTER_ASSERT(r, (values[i] == expected[i]) || + SkScalarNearlyEqual(values[i], expected[i], 0.001f), + "evaluated to %g, but expected %g", values[i], expected[i]); } }; @@ -2151,12 +2153,12 @@ DEF_TEST(SkVM_approx_math, r) { // pow2 { - float values[] = {-2, -1, 0, 1, 2, 3}; + float values[] = {-80, -5, -2, -1, 0, 1, 2, 3, 5, 160}; constexpr int N = SK_ARRAY_COUNT(values); eval(N, values, [](skvm::Builder* b, skvm::F32 v) { return b->approx_pow2(v); }); - const float expected[] = {0.25f, 0.5f, 1, 2, 4, 8}; + const float expected[] = {0, 0.03125f, 0.25f, 0.5f, 1, 2, 4, 8, 32, INFINITY}; compare(N, values, expected); }