Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(9)

Unified Diff: src/core/SkMath.cpp

Issue 1467513002: Revert of Fix UB in SkDivBits (Closed) Base URL: https://skia.googlesource.com/skia.git@master
Patch Set: Created 5 years, 1 month ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « BUILD.public ('k') | tools/dm_flags.json » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: src/core/SkMath.cpp
diff --git a/src/core/SkMath.cpp b/src/core/SkMath.cpp
index 9ca1569de60f300dbe0d9b9d0ba1b28be19aef68..af93d7ecb2cf850f00745c01783ca123621606e4 100644
--- a/src/core/SkMath.cpp
+++ b/src/core/SkMath.cpp
@@ -45,38 +45,21 @@
///////////////////////////////////////////////////////////////////////////////
+#define DIVBITS_ITER(n) \
+ case n: \
+ if ((numer = (numer << 1) - denom) >= 0) \
+ result |= 1 << (n - 1); else numer += denom
-#define DIVBITS_ITER(k) \
- case k: \
- if (numer*2 >= denom) { \
- numer = numer*2 - denom; \
- result |= 1 << (k-1); \
- } else { \
- numer *= 2; \
- }
-
-// NOTE: if you're thinking of editing this method, consider replacing it with
-// a simple shift and divide. This legacy method predated reliable hardware division.
-int32_t SkDivBits(int32_t n, int32_t d, int shift_bias) {
- SkASSERT(d != 0);
- if (n == 0) {
+int32_t SkDivBits(int32_t numer, int32_t denom, int shift_bias) {
+ SkASSERT(denom != 0);
+ if (numer == 0) {
return 0;
}
- // Make numer and denom positive, and sign hold the resulting sign
- // We'll be left-shifting numer, so it's important it's a uint32_t.
- // We put denom in a uint32_t just to keep things simple.
- int32_t sign = SkExtractSign(n ^ d);
-#if defined(SK_SUPPORT_LEGACY_DIVBITS_UB)
- // Blink layout tests are baselined to Clang optimizing through the UB.
- int32_t numer = SkAbs32(n);
- int32_t denom = SkAbs32(d);
-#else
- uint32_t numer = SkAbs32(n);
- uint32_t denom = SkAbs32(d);
-#endif
-
- // It's probably a bug to use n or d below here.
+ // make numer and denom positive, and sign hold the resulting sign
+ int32_t sign = SkExtractSign(numer ^ denom);
+ numer = SkAbs32(numer);
+ denom = SkAbs32(denom);
int nbits = SkCLZ(numer) - 1;
int dbits = SkCLZ(denom) - 1;
@@ -95,9 +78,10 @@
SkFixed result = 0;
// do the first one
- if (numer >= denom) {
- numer -= denom;
+ if ((numer -= denom) >= 0) {
result = 1;
+ } else {
+ numer += denom;
}
// Now fall into our switch statement if there are more bits to compute
« no previous file with comments | « BUILD.public ('k') | tools/dm_flags.json » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698