Bug 136992 - FTL should store the four bytes of the cell header using a 32-bit store rather than four 8-bit stores
Summary: FTL should store the four bytes of the cell header using a 32-bit store rathe...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Filip Pizlo
URL:
Keywords:
Depends on:
Blocks: 136330
  Show dependency tree / graph
 
Reported: 2014-09-21 21:14 PDT by Filip Pizlo
Modified: 2014-09-22 08:35 PDT (History)
10 users (show)

See Also:

Attachments
the patch (4.70 KB, patch)
2014-09-21 21:19 PDT, Filip Pizlo 		no flags Details | Formatted Diff | Diff
the patch (4.08 KB, patch)
2014-09-21 21:20 PDT, Filip Pizlo 		no flags Details | Formatted Diff | Diff
the patch (4.84 KB, patch)
2014-09-21 23:17 PDT, Filip Pizlo 		sam: review+
Details | Formatted Diff | Diff
Show Obsolete (2) 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 Filip Pizlo 2014-09-21 21:14:30 PDT 
It's totally possible that LLVM was combining these stores.  But, I'm about to reorganize this code in a way that might make it harder for LLVM to infer this, so I'd rather make it explicit on the FTL side.
Comment 1 Filip Pizlo 2014-09-21 21:19:28 PDT 
Created attachment 238448 [details]
the patch
Comment 2 Filip Pizlo 2014-09-21 21:20:34 PDT 
Created attachment 238449 [details]
the patch

For real this time.
Comment 3 Filip Pizlo 2014-09-21 21:21:17 PDT 
Comment on attachment 238449 [details]
the patch

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

> Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp:4867
> -
> +    

Reverted locally.

> Source/JavaScriptCore/runtime/Structure.h:342
> -
> +    

Reverted locally.
Comment 4 Csaba Osztrogonác 2014-09-21 23:09:22 PDT 
(In reply to comment #2)
> Created an attachment (id=238449) [details]
> the patch
> 
> For real this time.

Please add changelog entry for this patch.
Comment 5 Filip Pizlo 2014-09-21 23:14:55 PDT 
(In reply to comment #4)
> (In reply to comment #2)
> > Created an attachment (id=238449) [details] [details]
> > the patch
> > 
> > For real this time.
> 
> Please add changelog entry for this patch.

Oops.
Comment 6 Filip Pizlo 2014-09-21 23:17:33 PDT 
Created attachment 238452 [details]
the patch

Including ChangeLog.
Comment 7 Filip Pizlo 2014-09-22 08:35:40 PDT 
Landed in http://trac.webkit.org/changeset/173837