Bug 71982 – Renovate ARMv7 assembler/macro-assembler

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Gavin Barraclough]

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.

[Gavin Barraclough]

Created attachment 114413 [details]
Fix

[WebKit Review Bot]

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.

[Geoffrey Garen]

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).

[Gavin Barraclough]

Fixed in r99798

[Gavin Barraclough]

*** Bug 71730 has been marked as a duplicate of this bug. ***

[Csaba Osztrogonác]

Re-opened since this is blocked by bug 137871

[Csaba Osztrogonác]

close again, because it was reopened acidentally/automatically by webkit-patch