From 010ce2bc74a82bb2fc310b145799468637d5fcb5 Mon Sep 17 00:00:00 2001 From: Mike Reed Date: Wed, 9 May 2018 13:53:59 -0400 Subject: rewrite while to for loop to avoid last increment Can we efficiently avoid the last "increment" step for these sorts of loops? We are always calculating one more value than we actually need (we do this everywhere we forward-difference something). Bug: oss-fuzz:8146 Cq-Include-Trybots: skia.primary:Test-Debian9-Clang-GCE-CPU-AVX2-x86_64-Release-All-SKNX_NO_SIMD Change-Id: I8b2838cc51370d53b8854191fce9ff87e5dc669d Reviewed-on: https://skia-review.googlesource.com/127042 Commit-Queue: Mike Reed Reviewed-by: Cary Clark --- src/opts/SkBitmapProcState_opts_SSE2.cpp | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) (limited to 'src/opts') diff --git a/src/opts/SkBitmapProcState_opts_SSE2.cpp b/src/opts/SkBitmapProcState_opts_SSE2.cpp index 9ddf269ec9..efe8f4dd2e 100644 --- a/src/opts/SkBitmapProcState_opts_SSE2.cpp +++ b/src/opts/SkBitmapProcState_opts_SSE2.cpp @@ -361,10 +361,31 @@ void ClampX_ClampY_filter_scale_SSE2(const SkBitmapProcState& s, uint32_t xy[], } // while count >= 4 } // if count >= 4 + /* while (count-- > 0) { *xy++ = ClampX_ClampY_pack_filter(fx, maxX, one); fx += dx; } + We'd like to write this as above, but that form allows fx to get 1-iteration too big/small + when count is 0, and this can trigger a UBSAN error, even though we won't in fact use that + last (undefined) value for fx. + + Here is an alternative that should always be efficient, but seems much harder to read: + + if (count > 0) { + for (;;) { + *xy++ = ClampX_ClampY_pack_filter(fx, maxX, one); + if (--count == 0) break; + fx += dx; + } + } + + For now, we'll try this variant: more compact than the if/for version, and we hope the + compiler will get rid of the integer multiply. + */ + for (int i = 0; i < count; ++i) { + *xy++ = ClampX_ClampY_pack_filter(fx + i*dx, maxX, one); + } } } -- cgit v1.2.3