Bug 71982 - Renovate ARMv7 assembler/macro-assembler
Summary: Renovate ARMv7 assembler/macro-assembler
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
: 71730 (view as bug list)
Depends on: 137871
Blocks:
  Show dependency treegraph
 
Reported: 2011-11-09 17:50 PST by Gavin Barraclough
Modified: 2014-10-20 02:17 PDT (History)
3 users (show)

See Also:


Attachments
Fix (36.98 KB, patch)
2011-11-09 17:52 PST, Gavin Barraclough
ggaren: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Gavin Barraclough 2011-11-09 17:50:43 PST
ARMv7Assembler:
* add support for strb (byte stores)
* rename the VMOV_CtoS opcodes (there are currently backwards!)
* add support for adc (add with carry)
* add support for vsqrt, vabs
* add support for vmov (between FPRs, and to/from GPR pairs).
* remove '_F64' postfixes from instructions (these aren't helpful, functions can already be distinguished by their signatures).
* rename vcvt_F64_S32  to vcvt_signedToFloatingPoint, the prior postfix was unhelpful in failing to distinguish the types (S32 indicates a single precision register, but the type could be float, int32, or uint32).
* rename vcvtr_S32_F64 to vcvt_floatingPointToSigned, as for previous, also vcvtr was the incorrect name for the operation (the emitted instruction truncates).

MacroAssemblerARMv7:
* add 3-operand versions of and32, lshift32, or32, rshift32, urshift32, sub32, xor32, 
* add store8, and store32 imm to base-index.
* fix load32WithCompactAddressOffsetPatch to work for all gprs (the fix is a little kludgy but functional; to do better we'll have to also fix the repatching code).
* Update supportsFloating* flags (all features now supported).
* add moveDouble, storeDouble to absolute address, addDouble to absolute address
* add 3-operand double operations.
* implement sqrtDouble/absDouble
* add branchTruncateDoubleToInt32, implement truncateDoubleToInt32
* move should do nothing if src == dest
* branchTest8-on-memory can be implemented in terms of branchTest32-on-register (branchTest8-on-register has been removed).
* add 3-operand branchAdd32, branchSub32, also branchAdd32 absolute address.
Comment 1 Gavin Barraclough 2011-11-09 17:52:27 PST
Created attachment 114413 [details]
Fix
Comment 2 WebKit Review Bot 2011-11-09 17:55:39 PST
Attachment 114413 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1

Last 3072 characters of output:
e/JavaScriptCore/ChangeLog:20:  Need whitespace between colon and description  [changelog/filechangedescriptionwhitespace] [5]
Source/JavaScriptCore/ChangeLog:21:  Need whitespace between colon and description  [changelog/filechangedescriptionwhitespace] [5]
Source/JavaScriptCore/ChangeLog:22:  Need whitespace between colon and description  [changelog/filechangedescriptionwhitespace] [5]
Source/JavaScriptCore/ChangeLog:23:  Need whitespace between colon and description  [changelog/filechangedescriptionwhitespace] [5]
Source/JavaScriptCore/ChangeLog:24:  Need whitespace between colon and description  [changelog/filechangedescriptionwhitespace] [5]
Source/JavaScriptCore/ChangeLog:25:  Need whitespace between colon and description  [changelog/filechangedescriptionwhitespace] [5]
Source/JavaScriptCore/ChangeLog:26:  Need whitespace between colon and description  [changelog/filechangedescriptionwhitespace] [5]
Source/JavaScriptCore/ChangeLog:27:  Need whitespace between colon and description  [changelog/filechangedescriptionwhitespace] [5]
Source/JavaScriptCore/ChangeLog:28:  Need whitespace between colon and description  [changelog/filechangedescriptionwhitespace] [5]
Source/JavaScriptCore/ChangeLog:29:  Need whitespace between colon and description  [changelog/filechangedescriptionwhitespace] [5]
Source/JavaScriptCore/assembler/ARMv7Assembler.h:1420:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/JavaScriptCore/assembler/ARMv7Assembler.h:1593:  vcvt_signedToFloatingPoint is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/assembler/ARMv7Assembler.h:1599:  vcvt_floatingPointToSigned is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/assembler/MacroAssemblerARMv7.h:245:  shift_amount is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/assembler/MacroAssemblerARMv7.h:260:  shift_amount is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/assembler/MacroAssemblerARMv7.h:317:  shift_amount is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/assembler/MacroAssemblerARMv7.h:332:  shift_amount is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/assembler/MacroAssemblerARMv7.h:342:  shift_amount is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/assembler/MacroAssemblerARMv7.h:357:  shift_amount is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/assembler/MacroAssemblerARMv7.h:835:  Should have only a single space after a punctuation in a comment.  [whitespace/comments] [5]
Total errors found: 29 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Geoffrey Garen 2011-11-09 18:14:29 PST
Comment on attachment 114413 [details]
Fix

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

r=me

> Source/JavaScriptCore/assembler/MacroAssemblerARMv7.h:332
> +    void rshift32(RegisterID shift_amount, RegisterID dest)

shift_amount => shiftAmount is The WebKit Style (TM).
Comment 4 Gavin Barraclough 2011-11-09 18:33:17 PST
Fixed in r99798
Comment 5 Gavin Barraclough 2011-11-09 22:32:18 PST
*** Bug 71730 has been marked as a duplicate of this bug. ***
Comment 6 Csaba Osztrogonác 2014-10-20 02:16:27 PDT
Re-opened since this is blocked by bug 137871
Comment 7 Csaba Osztrogonác 2014-10-20 02:17:31 PDT
close again, because it was reopened acidentally/automatically by webkit-patch