Bug 156260 – MIPS: support Signed cond in branchTest32()

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

[Guillaume Emont]

This is needed since r197688 makes use of it.

[Guillaume Emont]

Created attachment 275714 [details]
Patch

A proposed patch addressing the issue

[WebKit Commit Bot]

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]

Created attachment 275716 [details]
Patch

A proposed patch addressing the issue

[Julien Brianceau]

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]

Created attachment 275777 [details]
Patch

New version addressing Julien's remarks. Thanks for the quick review!

[Julien Brianceau]

Looks good to me :)

[Mark Lam]

Comment on attachment 275777 [details]
Patch

r=me

[WebKit Commit Bot]

Comment on attachment 275777 [details]
Patch

Clearing flags on attachment: 275777

Committed r199261: <http://trac.webkit.org/changeset/199261>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.