Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(12)

Issue 2585043002: Improve saturated_cast performance (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
5 months, 1 week ago by jschuh
Modified:
5 months, 1 week ago
Reviewers:
Tom Sepez, Nico
CC:
chromium-reviews, vmpstr+watch_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Improve saturated_cast performance Plumb custom bounds through DstRangeRelationToSrcRange to remove duplicate comparisons. Refactor RangeConstraint from an enum to a class containing bools, which can be independently evaluated at compile-time. NOTRY=true Committed: https://crrev.com/8b34cfe3286eea1b7107cfa74eb19a5195e3dda9 Cr-Commit-Position: refs/heads/master@{#439508}

Patch Set 1 #

Patch Set 2 : nit #

Patch Set 3 : tweaks #

Total comments: 1

Patch Set 4 : fix comment nit #

Unified diffs Side-by-side diffs Delta from patch set Stats (+119 lines, -71 lines) Patch
M base/numerics/safe_conversions.h View 1 2 1 chunk +23 lines, -23 lines 0 comments Download
M base/numerics/safe_conversions_impl.h View 1 2 3 4 chunks +85 lines, -48 lines 0 comments Download
M base/numerics/safe_numerics_unittest.cc View 1 2 2 chunks +11 lines, -0 lines 0 comments Download
Commit queue not available (can’t edit this change).

Messages

Total messages: 34 (28 generated)
jschuh
ptal - This isn't changing any API visible behavior, but removes a number of redundant ...
5 months, 1 week ago (2016-12-18 19:33:26 UTC) #16
Tom Sepez
lgtm probably worth the additional byte, which realistically only costs anything on [u]int8_t's due to ...
5 months, 1 week ago (2016-12-19 17:49:01 UTC) #22
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2585043002/160001
5 months, 1 week ago (2016-12-19 18:28:05 UTC) #26
commit-bot: I haz the power
Committed patchset #4 (id:160001)
5 months, 1 week ago (2016-12-19 18:34:47 UTC) #29
commit-bot: I haz the power
Patchset 4 (id:??) landed as https://crrev.com/8b34cfe3286eea1b7107cfa74eb19a5195e3dda9 Cr-Commit-Position: refs/heads/master@{#439508}
5 months, 1 week ago (2016-12-19 18:38:24 UTC) #31
jschuh
5 months, 1 week ago (2016-12-19 21:12:03 UTC) #34
Message was sent while issue was closed.
A revert of this CL (patchset #4 id:160001) has been created in
https://codereview.chromium.org/2589973003/ by jschuh@chromium.org.

The reason for reverting is: thakis@ thinks it hit this bug:
https://llvm.org/bugs/show_bug.cgi?id=29122
Will re-land after it's resolved..
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 650457f06