Bug 138934 - Add some comments to describe the DFG UseKind representations.
Summary: Add some comments to describe the DFG UseKind representations.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Lam
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-11-20 14:09 PST by Mark Lam
Modified: 2014-11-20 15:40 PST (History)
4 users (show)

See Also:

Attachments
the patch. (1.85 KB, patch)
2014-11-20 14:12 PST, Mark Lam 		no flags Details | Formatted Diff | Diff
patch 2: addressed Geoff's feedback. (1.85 KB, patch)
2014-11-20 15:16 PST, Mark Lam 		fpizlo: review-
Details | Formatted Diff | Diff
patch 3: UntypedUse is at enum 0 again, with extra comments. (2.14 KB, patch)
2014-11-20 15:23 PST, Mark Lam 		no flags Details | Formatted Diff | Diff
patch 4: don't like the comment about UntypedUse in last patch. Updated. (2.14 KB, patch)
2014-11-20 15:31 PST, Mark Lam 		fpizlo: review+
Details | Formatted Diff | Diff
patch 5: moved al the JSValue representation UseKinds up nelow UntypedUse as Filip suggested. (2.11 KB, patch)
2014-11-20 15:36 PST, Mark Lam 		fpizlo: review+
Details | Formatted Diff | Diff
Show Obsolete (4) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Lam 2014-11-20 14:09:54 PST 
Also regrouped the UseKind enums by representation.
Comment 1 Mark Lam 2014-11-20 14:12:55 PST 
Created attachment 241980 [details]
the patch.
Comment 2 Geoffrey Garen 2014-11-20 15:13:43 PST 
Comment on attachment 241980 [details]
the patch.

View in context: https://bugs.webkit.org/attachment.cgi?id=241980&action=review

> Source/JavaScriptCore/dfg/DFGUseKind.h:38
>      UntypedUse,

Which case is UntypedUse? (Seems like it should be JSValue, right?)

> Source/JavaScriptCore/dfg/DFGUseKind.h:43
> +    //    in a FP register.

"an"
Comment 3 Mark Lam 2014-11-20 15:16:11 PST 
Created attachment 241989 [details]
patch 2: addressed Geoff's feedback.
Comment 4 Filip Pizlo 2014-11-20 15:17:53 PST 
Comment on attachment 241989 [details]
patch 2: addressed Geoff's feedback.

View in context: https://bugs.webkit.org/attachment.cgi?id=241989&action=review

> Source/JavaScriptCore/dfg/DFGUseKind.h:54
>      UntypedUse,

This could be dangerous; I believe that UntypedUse is expected to be zero.
Comment 5 Filip Pizlo 2014-11-20 15:18:17 PST 
Comment on attachment 241989 [details]
patch 2: addressed Geoff's feedback.

Please arrange to have UntypedUse be the first thing, as before.
Comment 6 Mark Lam 2014-11-20 15:23:34 PST 
Created attachment 241992 [details]
patch 3: UntypedUse is at enum 0 again, with extra comments.
Comment 7 Mark Lam 2014-11-20 15:31:45 PST 
Created attachment 241995 [details]
patch 4: don't like the comment about UntypedUse in last patch.  Updated.
Comment 8 Filip Pizlo 2014-11-20 15:33:57 PST 
Comment on attachment 241995 [details]
patch 4: don't like the comment about UntypedUse in last patch.  Updated.

You could also have moved all of the other JS representations up with UntypedUse.  That would make the most logical sense to me.
Comment 9 Mark Lam 2014-11-20 15:36:49 PST 
Created attachment 241996 [details]
patch 5: moved al the JSValue representation UseKinds up nelow UntypedUse as Filip suggested.
Comment 10 Mark Lam 2014-11-20 15:40:25 PST 
Thanks for the review.  Landed in r176425: <http://trac.webkit.org/r176425>.