Bug 152035 - FTL B3 should have basic GetById support
Summary: FTL B3 should have basic GetById support
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Filip Pizlo
URL:
Keywords:
Depends on:
Blocks: 151808
  Show dependency treegraph
 
Reported: 2015-12-08 20:34 PST by Filip Pizlo
Modified: 2015-12-09 19:50 PST (History)
14 users (show)

See Also:


Attachments
work in progress (4.91 KB, patch)
2015-12-08 20:39 PST, Filip Pizlo
no flags Details | Formatted Diff | Diff
the patch (15.39 KB, patch)
2015-12-08 22:13 PST, Filip Pizlo
no flags Details | Formatted Diff | Diff
for real this time (14.81 KB, patch)
2015-12-08 22:20 PST, Filip Pizlo
sbarati: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Filip Pizlo 2015-12-08 20:34:37 PST
Patch forthcoming.
Comment 1 Filip Pizlo 2015-12-08 20:39:13 PST
Created attachment 266964 [details]
work in progress
Comment 2 Filip Pizlo 2015-12-08 22:13:17 PST
Created attachment 266967 [details]
the patch
Comment 3 WebKit Commit Bot 2015-12-08 22:14:33 PST
Attachment 266967 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp:6737:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp:6749:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp:6767:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Total errors found: 3 in 9 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Filip Pizlo 2015-12-08 22:20:28 PST
Created attachment 266969 [details]
for real this time
Comment 5 WebKit Commit Bot 2015-12-08 22:23:07 PST
Attachment 266969 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp:6737:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp:6749:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp:6767:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Total errors found: 3 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 6 Saam Barati 2015-12-08 22:44:03 PST
Comment on attachment 266969 [details]
for real this time

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

r=me

> Source/WTF/wtf/Box.h:59
> +    struct Data : ThreadSafeRefCounted<Data> {

shouldn't this be WTF_MAKE_FAST_ALLOCATED?
Comment 7 Filip Pizlo 2015-12-09 08:06:37 PST
(In reply to comment #6)
> Comment on attachment 266969 [details]
> for real this time
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=266969&action=review
> 
> r=me
> 
> > Source/WTF/wtf/Box.h:59
> > +    struct Data : ThreadSafeRefCounted<Data> {
> 
> shouldn't this be WTF_MAKE_FAST_ALLOCATED?

RefCounted and ThreadSafeRefCohnted both do WTF_MAKE_FAST_ALLOCATED and WTF_MAKE_NONCOPYABLE for you.
Comment 8 Saam Barati 2015-12-09 13:23:14 PST
(In reply to comment #7)
> (In reply to comment #6)
> > Comment on attachment 266969 [details]
> > for real this time
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=266969&action=review
> > 
> > r=me
> > 
> > > Source/WTF/wtf/Box.h:59
> > > +    struct Data : ThreadSafeRefCounted<Data> {
> > 
> > shouldn't this be WTF_MAKE_FAST_ALLOCATED?
> 
> RefCounted and ThreadSafeRefCohnted both do WTF_MAKE_FAST_ALLOCATED and
> WTF_MAKE_NONCOPYABLE for you.

Oh, cool. I didn't know that.
Comment 9 Filip Pizlo 2015-12-09 19:50:45 PST
Landed in http://trac.webkit.org/changeset/193890