EXT and MFHC1 instructions from MIPSR2 implemented and used where it is possible. Also, done some code size optimizations that were discovered in meantime.
Created attachment 337882 [details] Patch
Attachment 337882 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/assembler/MIPSAssembler.h:241: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/JavaScriptCore/assembler/MIPSAssembler.h:242: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/JavaScriptCore/assembler/MIPSAssembler.h:243: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] Total errors found: 3 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
Can anyone take a look at this change?
Comment on attachment 337882 [details] Patch I don't have much MIPS expertise but LGTM. Someone who knows the MIPS instruction set should really review. Also, would be better if you add some tests in testmasm.cpp that exercises your macro assembler functions.
Yusuke Suzuki, are you OK with the change?
Ping.
Comment on attachment 337882 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=337882&action=review The logic of the code looks good, but there are so much duplicate code. Can we clean up them? > Source/JavaScriptCore/assembler/MacroAssemblerMIPS.h:2933 > + if (!address.scale) > + m_assembler.addu(addrTempRegister, address.index, address.base); > + else { > + m_assembler.sll(addrTempRegister, address.index, address.scale); > + m_assembler.addu(addrTempRegister, addrTempRegister, address.base); > + } > + if (address.offset < -32768 || address.offset > 32767) { > + m_assembler.lui(immTempRegister, (address.offset + 0x8000) >> 16); > + m_assembler.addu(addrTempRegister, addrTempRegister, immTempRegister); > + } This code pattern is so repeated. Can we extract this pattern to remove duplicate code? > Source/JavaScriptCore/assembler/MacroAssemblerMIPS.h:2934 > + m_assembler.swc1(src, addrTempRegister, address.offset); Fix this indent.
Created attachment 339717 [details] Patch Clean up duplicate code.
Attachment 339717 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/assembler/MIPSAssembler.h:241: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/JavaScriptCore/assembler/MIPSAssembler.h:242: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/JavaScriptCore/assembler/MIPSAssembler.h:243: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] Total errors found: 3 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 339717 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=339717&action=review > Source/JavaScriptCore/assembler/MacroAssemblerMIPS.h:323 > + void loadAddress(BaseIndex address, bool loadTrail = true) Use `enum class` instead of `bool`. In this case, not using default parameter is better. And `loadTrail` is a bit confusing. Maybe, `enum class LoadAddressMode { DontAddOffset, AddOffset };` would be more descriptive.
Created attachment 339958 [details] Patch 'Enum class' instead 'bool', removed default parameter.
Attachment 339958 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/assembler/MIPSAssembler.h:241: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/JavaScriptCore/assembler/MIPSAssembler.h:242: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/JavaScriptCore/assembler/MIPSAssembler.h:243: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] Total errors found: 3 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 339958 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=339958&action=review > Source/JavaScriptCore/assembler/MacroAssemblerMIPS.h:117 > + ScaleAndAddOffset, I think this enum name is a bit misleading since `offset` is only added if `offset` is large / small enough.
(In reply to Yusuke Suzuki from comment #13) > Comment on attachment 339958 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=339958&action=review > > > Source/JavaScriptCore/assembler/MacroAssemblerMIPS.h:117 > > + ScaleAndAddOffset, > > I think this enum name is a bit misleading since `offset` is only added if > `offset` is large / small enough. Maybe the other enum name would be fine. Like, `ScaleAndAddOffsetIfOffsetIsOutOfBounds`.
Created attachment 339960 [details] Patch Fixed. :)
Attachment 339960 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/assembler/MIPSAssembler.h:241: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/JavaScriptCore/assembler/MIPSAssembler.h:242: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] ERROR: Source/JavaScriptCore/assembler/MIPSAssembler.h:243: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] Total errors found: 3 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 339958 [details] Patch Attachment 339958 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/7623156 New failing tests: http/tests/security/XFrameOptions/x-frame-options-deny-multiple-clients.html
Created attachment 339962 [details] Archive of layout-test-results from ews107 for mac-sierra-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews107 Port: mac-sierra-wk2 Platform: Mac OS X 10.12.6
Comment on attachment 339960 [details] Patch Clearing flags on attachment: 339960 Committed r231554: <https://trac.webkit.org/changeset/231554>
Seems landed.
<rdar://problem/40651435>
Comment on attachment 339958 [details] Patch Clearing r? flag as a variant of the patch landed in r231554.