SkCpu w/o static initializer

I think I cracked it.

Though, this may not technically be legal C++...
I've only got one definition of SkCpu::gCachedFeatures,
but two different declarations: non-const in SkCpu.cpp, const elsewhere.

Is this...
   - legal C++?
   - not C++ but probably works as I think?
   - not C++ and will probably blow up?
   - who knows, let's see?

I have tested that the features are cached properly, read properly, and that the generated code treats SkCpu::gCachedFeatures as a global constant outside SkCpu.cpp.  So it all observably works optimally.

Expanding testing to more bots.
TBR=reed@google.com

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1905683003

Review URL: https://codereview.chromium.org/1905683003

3 files changed, 21 insertions, 47 deletions

diff --git a/src/core/SkCpu.cpp b/src/core/SkCpu.cpp
index 4030fce2e1..b296f0c02a 100644
--- a/src/core/SkCpu.cpp
+++ b/src/core/SkCpu.cpp
@@ -5,8 +5,9 @@
  * found in the LICENSE file.
  */
 
+#define SkCpu_IMPL
 #include "SkCpu.h"
-#include "SkOncePtr.h"
+#include "SkOnce.h"
 
 #if defined(SK_CPU_X86)
     #if defined(SK_BUILD_FOR_WIN32)
@@ -78,13 +79,17 @@
 
 #endif
 
-#if defined(__GNUC__) || defined(__clang__)
-    SK_DECLARE_STATIC_ONCE_PTR(uint32_t, gCachedCpuFeatures);
-    uint32_t SkCpu::RuntimeCpuFeatures() {
-        return *gCachedCpuFeatures.get([]{ return new uint32_t{read_cpu_features()}; });
-    }
+#if defined(_MSC_VER)
+    const uint32_t SkCpu::gCachedFeatures = read_cpu_features();
+
+    void SkCpu::CacheRuntimeFeatures() {}
 
 #else
-    const uint32_t SkCpu::gCachedCpuFeatures = read_cpu_features();
+    uint32_t SkCpu::gCachedFeatures = 0;
+
+    void SkCpu::CacheRuntimeFeatures() {
+        static SkOnce once;
+        once([] { gCachedFeatures = read_cpu_features(); });
+    }
 
 #endif
diff --git a/src/core/SkCpu.h b/src/core/SkCpu.h
index 2a41d37b16..b212da482c 100644
--- a/src/core/SkCpu.h
+++ b/src/core/SkCpu.h
@@ -29,54 +29,21 @@ struct SkCpu {
         VFP_FP16 = 1 << 2,
     };
 
+    static void CacheRuntimeFeatures();
     static bool Supports(uint32_t);
-
 private:
-    // Consider a loop like this that expands 16-bit floats out to 32-bit, does math, and repacks:
-    //    for (int i = 0; i < N; i++) {
-    //        if (SkCpu::Supports(SkCpu::F16C)) {
-    //            f32s = SkCpu::F16C_cvtph_ps(f16s);
-    //        } else {
-    //            f32s = some_slower_f16_to_f32_routine(f16s);
-    //        }
-    //
-    //        ... do some math with f32s ...
-    //
-    //        if (SkCpu::Supports(SkCpu::F16C)) {
-    //            f16s = SkCpu::F16C_cvtps_ph(f32s);
-    //        } else {
-    //            f16s = some_slower_f32_to_f16_routine(f32s);
-    //        }
-    //    }
-    //
-    // We would like SkCpu::Supports() to participate in common sub-expression elimination,
-    // so that it's called exactly 1 time, rather than N or 2N times.  This is especially
-    // important when the if-else blocks you see above are really inline functions.
-    //
-    // The key to this is to make sure to implement RuntimeCpuFeatures() with the same
-    // capacity for common sub-expression elimination.
-    //
-    // __attribute__((const)) works perfectly when available.
-    //
-    // When it's not (MSVC), we fall back to a static initializer.
-    // (Static intializers would work fine everywhere, but Chrome really dislikes them.)
-
-#if defined(__GNUC__) || defined(__clang__)  // i.e. GCC, Clang, or clang-cl
-    __attribute__((const))
-    static uint32_t RuntimeCpuFeatures();
+#if defined(_MSC_VER) || !defined(SkCpu_IMPL)
+    static const uint32_t gCachedFeatures;
 #else
-    static const uint32_t gCachedCpuFeatures;
-    static uint32_t RuntimeCpuFeatures() {
-        return gCachedCpuFeatures;
-    }
+    static       uint32_t gCachedFeatures;
 #endif
 };
 
 inline bool SkCpu::Supports(uint32_t mask) {
-    uint32_t features = RuntimeCpuFeatures();
+    uint32_t features = gCachedFeatures;
 
-    // If we mask in compile-time known lower limits, the compiler can completely
-    // drop many calls to RuntimeCpuFeatures().
+    // If we mask in compile-time known lower limits, the compiler can
+    // often compile away this entire function.
 #if SK_CPU_X86
     #if SK_CPU_SSE_LEVEL >= SK_CPU_SSE_LEVEL_SSE1
     features |= SSE1;
diff --git a/src/core/SkGraphics.cpp b/src/core/SkGraphics.cpp
index a2456d808d..81d3b3974f 100644
--- a/src/core/SkGraphics.cpp
+++ b/src/core/SkGraphics.cpp
@@ -10,6 +10,7 @@
 
 #include "SkBlitter.h"
 #include "SkCanvas.h"
+#include "SkCpu.h"
 #include "SkGeometry.h"
 #include "SkGlyphCache.h"
 #include "SkImageFilter.h"
@@ -46,6 +47,7 @@ void SkGraphics::GetVersion(int32_t* major, int32_t* minor, int32_t* patch) {
 
 void SkGraphics::Init() {
     // SkGraphics::Init() must be thread-safe and idempotent.
+    SkCpu::CacheRuntimeFeatures();
     SkOpts::Init();
 
 #ifdef SK_DEVELOPER