Bug 156260

Summary: MIPS: support Signed cond in branchTest32()
Product: WebKit Reporter: Guillaume Emont <guijemont>
Component: JavaScriptCoreAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: annulen, commit-queue, jbriance, keith_miller, mark.lam, msaboff, saam
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Other   
OS: Linux   
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Guillaume Emont
Reported 2016-04-05 16:37:11 PDT
This is needed since r197688 makes use of it.
Attachments
Patch (2.97 KB, patch)
2016-04-05 16:42 PDT, Guillaume Emont 		no flags
DetailsFormatted DiffDiff
Patch (2.97 KB, patch)
2016-04-05 16:45 PDT, Guillaume Emont 		no flags
DetailsFormatted DiffDiff
Patch (2.91 KB, patch)
2016-04-06 09:45 PDT, Guillaume Emont 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Guillaume Emont
Comment 1 2016-04-05 16:42:36 PDT
Created attachment 275714 [details] Patch A proposed patch addressing the issue
WebKit Commit Bot
Comment 2 2016-04-05 16:43:42 PDT
Attachment 275714 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/assembler/MacroAssemblerMIPS.h:1521: Extra space before ) [whitespace/parens] [2] Total errors found: 1 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.
Guillaume Emont
Comment 3 2016-04-05 16:45:28 PDT
Created attachment 275716 [details] Patch A proposed patch addressing the issue
Julien Brianceau
Comment 4 2016-04-06 00:49:15 PDT
Comment on attachment 275716 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=275716&action=review > Source/JavaScriptCore/assembler/MacroAssemblerMIPS.h:1505 > + if ((cond != Signed) && m_fixedWidth) > + m_assembler.nop(); To me, these 2 lines are not needed. > Source/JavaScriptCore/assembler/MacroAssemblerMIPS.h:1515 > + ASSERT_NOT_REACHED(); Very good idea to add this, but please use "RELEASE_ASSERT_NOT_REACHED" instead. > Source/JavaScriptCore/assembler/MacroAssemblerMIPS.h:1532 > + ASSERT_NOT_REACHED(); Ditto.
Guillaume Emont
Comment 5 2016-04-06 09:45:35 PDT
Created attachment 275777 [details] Patch New version addressing Julien's remarks. Thanks for the quick review!
Julien Brianceau
Comment 6 2016-04-06 09:49:17 PDT
Looks good to me :)
Mark Lam
Comment 7 2016-04-06 09:53:49 PDT
Comment on attachment 275777 [details] Patch r=me
WebKit Commit Bot
Comment 8 2016-04-08 18:20:28 PDT
Comment on attachment 275777 [details] Patch Clearing flags on attachment: 275777 Committed r199261: <http://trac.webkit.org/changeset/199261>
WebKit Commit Bot
Comment 9 2016-04-08 18:20:32 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.