From 5d9683ee859dacb20219cc62fc7db82f8fdb1571 Mon Sep 17 00:00:00 2001 From: Benedikt Meurer Date: Mon, 11 Dec 2017 12:47:08 +0100 Subject: [PATCH] [turbofan] Update documentation on BinaryOperationFeedback. Explain why we still have kNumber in addition to kNumberOrOddball, although the original motivation, which was Crankshaft, is gone now. Bug: v8:7109 Change-Id: I33016fbfa96bb0db57473b6d0c720fa1389d11f1 Reviewed-on: https://chromium-review.googlesource.com/817439 Reviewed-by: Georg Neis Commit-Queue: Benedikt Meurer Cr-Commit-Position: refs/heads/master@{#49991} --- src/globals.h | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/globals.h b/src/globals.h index ed7f6c0a72..62b203e4e3 100644 --- a/src/globals.h +++ b/src/globals.h @@ -1254,14 +1254,17 @@ inline uint32_t ObjectHash(Address address) { // Type feedback is encoded in such a way that, we can combine the feedback // at different points by performing an 'OR' operation. Type feedback moves // to a more generic type when we combine feedback. -// kSignedSmall -> kSignedSmallInputs -> kNumber -> kNumberOrOddball -> kAny -// kString -> kAny -// kBigInt -> kAny -// TODO(mythria): Remove kNumber type when crankshaft can handle Oddballs -// similar to Numbers. We don't need kNumber feedback for Turbofan. Extra -// information about Number might reduce few instructions but causes more -// deopts. We collect Number only because crankshaft does not handle all -// cases of oddballs. +// +// kSignedSmall -> kSignedSmallInputs -> kNumber -> kNumberOrOddball -> kAny +// kString -> kAny +// kBigInt -> kAny +// +// Technically we wouldn't need the separation between the kNumber and the +// kNumberOrOddball values here, since for binary operations, we always +// truncate oddballs to numbers. In practice though it causes TurboFan to +// generate quite a lot of unused code though if we always handle numbers +// and oddballs everywhere, although in 99% of the use sites they are only +// used with numbers. class BinaryOperationFeedback { public: enum {