Bug 136992

Summary: FTL should store the four bytes of the cell header using a 32-bit store rather than four 8-bit stores
Product: WebKit Reporter: Filip Pizlo <fpizlo>
Component: JavaScriptCoreAssignee: Filip Pizlo <fpizlo>
Status: RESOLVED FIXED    
Severity: Normal CC: barraclough, ggaren, mark.lam, mhahnenb, mmirman, msaboff, nrotem, oliver, ossy, sam
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 136330    
Attachments:
Description Flags
the patch
none
the patch
none
the patch sam: review+

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