Bug 164733 - Add storeFence support for ARMv7
Summary: Add storeFence support for ARMv7
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Csaba Osztrogonác
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-11-14 14:12 PST by Filip Pizlo
Modified: 2017-01-04 11:58 PST (History)
7 users (show)

See Also:


Attachments
Patch (6.79 KB, patch)
2016-11-15 03:36 PST, Csaba Osztrogonác
saam: review+
Details | Formatted Diff | Diff
Patch for landing (6.21 KB, patch)
2016-12-06 06:33 PST, Csaba Osztrogonác
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Filip Pizlo 2016-11-14 14:12:21 PST
Patch forthcoming
Comment 1 Csaba Osztrogonác 2016-11-15 03:36:56 PST
Created attachment 294831 [details]
Patch
Comment 2 WebKit Commit Bot 2016-11-15 03:39:28 PST
Attachment 294831 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/assembler/ARMAssembler.h:218:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
Total errors found: 1 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Csaba Osztrogonác 2016-11-20 23:29:21 PST
Ping?
Comment 4 Csaba Osztrogonác 2016-11-23 00:01:20 PST
(In reply to comment #3)
> Ping?

Ping2
Comment 5 Csaba Osztrogonác 2016-11-28 03:51:40 PST
(In reply to comment #3)
> Ping?

Ping3
Comment 6 Saam Barati 2016-11-28 13:39:57 PST
Comment on attachment 294831 [details]
Patch

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

> Source/JavaScriptCore/assembler/MacroAssemblerARM.h:-1482
> -    

please revert

> Source/JavaScriptCore/assembler/MacroAssemblerARMv7.h:-1341
> -    

Please revert changes in this file.
Comment 7 Csaba Osztrogonác 2016-12-01 05:32:22 PST
(In reply to comment #6)
> Comment on attachment 294831 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=294831&action=review
> 
> > Source/JavaScriptCore/assembler/MacroAssemblerARM.h:-1482
> > -    
> 
> please revert
> 
> > Source/JavaScriptCore/assembler/MacroAssemblerARMv7.h:-1341
> > -    
> 
> Please revert changes in this file.

You meant only extra whitespace removal changes, don't you?
Comment 8 Mark Lam 2016-12-01 07:23:29 PST
Comment on attachment 294831 [details]
Patch

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

>>> Source/JavaScriptCore/assembler/MacroAssemblerARM.h:-1482
>>> -    
>> 
>> please revert
> 
> You meant only extra whitespace removal changes, don't you?

Yes, that is what Saam meant.  Normally, if whitespace changes are in the vicinity of the changed code, it'd be ok.  But in this case, it seems unrelated to the patch.  To keep the patch minimal and aligned to its intent, it is best to leave the whitespace changes to another patch.  I think you can land the white space removal in a separate unreviewed patch if you like (just like we do for minor comment typo fix ups and build fixes).
Comment 9 Csaba Osztrogonác 2016-12-06 06:33:40 PST
Created attachment 296286 [details]
Patch for landing
Comment 10 WebKit Commit Bot 2016-12-06 06:35:36 PST
Attachment 296286 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/assembler/ARMAssembler.h:218:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
Total errors found: 1 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 11 WebKit Commit Bot 2016-12-06 07:34:48 PST
Comment on attachment 296286 [details]
Patch for landing

Clearing flags on attachment: 296286

Committed r209392: <http://trac.webkit.org/changeset/209392>