Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2585043002:
Improve saturated_cast performance (Closed)
DescriptionImprove 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 #
Messages
Total messages: 34 (28 generated)
Patchset #4 (id:60001) has been deleted
Patchset #3 (id:40001) has been deleted
Patchset #2 (id:20001) has been deleted
Patchset #1 (id:1) has been deleted
The CQ bit was checked by jschuh@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by jschuh@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
The CQ bit was checked by jschuh@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
jschuh@chromium.org changed reviewers: + tsepez@chromium.org
ptal - This isn't changing any API visible behavior, but removes a number of redundant run-time branches and allows compile-time constants to propagate where they couldn't before (because I was previously squashing two bools into an enum).
Patchset #3 (id:120001) has been deleted
The CQ bit was checked by jschuh@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm probably worth the additional byte, which realistically only costs anything on [u]int8_t's due to alignment padding. https://codereview.chromium.org/2585043002/diff/140001/base/numerics/safe_con... File base/numerics/safe_conversions_impl.h (right): https://codereview.chromium.org/2585043002/diff/140001/base/numerics/safe_con... base/numerics/safe_conversions_impl.h:171: RANGE_OVERFLOW = 0x1, // Value would underflow. nit: the comment still don't line up with the names ...
Description was changed from ========== 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. ========== to ========== 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 ==========
The CQ bit was checked by jschuh@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from tsepez@chromium.org Link to the patchset: https://codereview.chromium.org/2585043002/#ps160001 (title: "fix comment nit")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 160001, "attempt_start_ts": 1482172052230630,
"parent_rev": "3cc5fab2face03fd7f0442ca4ea0a63be925a6fa", "commit_rev":
"fe61470aa6a11e61b6471648fd2663e2e59d1bb0"}
Message was sent while issue was closed.
Description was changed from ========== 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 ========== to ========== 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 Review-Url: https://codereview.chromium.org/2585043002 ==========
Message was sent while issue was closed.
Committed patchset #4 (id:160001)
Message was sent while issue was closed.
Description was changed from ========== 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 Review-Url: https://codereview.chromium.org/2585043002 ========== to ========== 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} ==========
Message was sent while issue was closed.
Patchset 4 (id:??) landed as https://crrev.com/8b34cfe3286eea1b7107cfa74eb19a5195e3dda9 Cr-Commit-Position: refs/heads/master@{#439508}
Message was sent while issue was closed.
Description was changed from ========== 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} ========== to ========== 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} ==========
Message was sent while issue was closed.
jschuh@chromium.org changed reviewers: + thakis@chromium.org
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.. |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
