Bug 58950 - Rationalize MacroAssembler branch methods
Summary: Rationalize MacroAssembler branch methods
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Gavin Barraclough
Depends on:
Reported: 2011-04-19 17:51 PDT by Gavin Barraclough
Modified: 2011-06-07 15:25 PDT (History)
6 users (show)

See Also:

The patch (89.26 KB, patch)
2011-04-19 17:53 PDT, Gavin Barraclough
oliver: review+
Details | Formatted Diff | Diff
fix issues (33.54 KB, patch)
2011-04-22 09:33 PDT, thouraya
no flags 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-04-19 17:51:11 PDT
The MacroAssembler currently exposes x86's weird behaviour that the setcc instruction only sets the low 8 bits of a register.  Stop that.

Having done so, to clarify remove the 'set32' prefix from test & compare instructions - these methods all now set a full 32/64 bit register (Ptr size).  The size in the function name should indicate the amount of data being compared.

Also split out the 'Condition' enum into 'RelationalCondition' and 'ResultCondition'.  The former is used in binary comparison, the latter compares the latter is a unary condition check on the result of an operation.
Comment 1 Gavin Barraclough 2011-04-19 17:53:51 PDT
Created attachment 90282 [details]
The patch
Comment 2 Oliver Hunt 2011-04-19 18:15:42 PDT
Comment on attachment 90282 [details]
The patch

Comment 3 Gavin Barraclough 2011-04-20 11:44:50 PDT
Fixed in r84399
Comment 4 WebKit Review Bot 2011-04-20 11:57:41 PDT
http://trac.webkit.org/changeset/84399 might have broken Qt Linux ARMv7 Release
Comment 5 thouraya 2011-04-22 09:15:35 PDT

The patch caused a build problem for SH4 platforms.

I created a patch to fix the issue.

Should I submit a patch here or create a new bug?

Comment 6 thouraya 2011-04-22 09:33:18 PDT
Created attachment 90713 [details]
fix issues


Attached a patch to fix the issue.

Comment 7 Gavin Barraclough 2011-04-24 12:51:58 PDT
Comment on attachment 90713 [details]
fix issues

Looks great!
Comment 8 thouraya 2011-04-25 07:18:58 PDT

I have a question: the patch can be landed even if the bug is not opened?

Comment 9 Adam Barth 2011-04-25 11:18:59 PDT
The bug needs to be open for the commit-queue to see the patch.
Comment 10 WebKit Commit Bot 2011-04-25 13:19:21 PDT
Comment on attachment 90713 [details]
fix issues

Clearing flags on attachment: 90713

Committed r84796: <http://trac.webkit.org/changeset/84796>