Bug 124149 – Build fail on CPU(ARM_TRADITIONAL) after r159039

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

[Gabor Rapcsanyi]

Build fail on CPU(ARM_TRADITIONAL) after r159039

[Gabor Rapcsanyi]

Created attachment 216575 [details]
proposed fix

[Geoffrey Garen]

Comment on attachment 216575 [details]
proposed fix

It looks like this change will cut back the ARMv7Assembler from max register r15 to max register r13, since ARMv7Assembler::lastRegister() returns r13.

Is that the right thing to do? Your ChangeLog doesn't explain why you did it, or why it's right. 

Regardless of whether it's right, it's bad for a build fix patch on one platform to include a behavior change on another platform. They should be two separate patches.

[Gabor Rapcsanyi]

That's the ARMAssembler not the ARMv7Assembler and it has no effect on it. I just put these functions to the right place form MacroAssemblerARM to ARMAssembler as the other Assemblers work. I don't see how this patch effects on the ARMv7Assembler.


(In reply to comment #2)
> (From update of attachment 216575 [details])
> It looks like this change will cut back the ARMv7Assembler from max register r15 to max register r13, since ARMv7Assembler::lastRegister() returns r13.
> 
> Is that the right thing to do? Your ChangeLog doesn't explain why you did it, or why it's right. 
> 
> Regardless of whether it's right, it's bad for a build fix patch on one platform to include a behavior change on another platform. They should be two separate patches.

[Csaba Osztrogonác]

(In reply to comment #3)
> That's the ARMAssembler not the ARMv7Assembler and it has no effect on it. I just put these functions to the right place form MacroAssemblerARM to ARMAssembler as the other Assemblers work. I don't see how this patch effects on the ARMv7Assembler.

CPU(ARM) in FPRInfo.h is true on ARM_TRADITIONAL and ARM_THUMB2 too.

[Geoffrey Garen]

> That's the ARMAssembler not the ARMv7Assembler and it has no effect on it.

I see. I thought, incorrectly, that MacroAssemblerARM was used by all ARM platforms.

[Geoffrey Garen]

Comment on attachment 216575 [details]
proposed fix

r=me

[WebKit Commit Bot]

Comment on attachment 216575 [details]
proposed fix

Rejecting attachment 216575 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-01', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 216575, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
t.org/repository/webkit/trunk ...
Authentication realm: <http://svn.webkit.org:80> Mac OS Forge
Password for '[e-mail]': 
Authentication realm: <http://svn.webkit.org:80> Mac OS Forge
Username: Use of uninitialized value $username in chomp at /usr/share/git-core/perl/Git/SVN/Prompt.pm line 114.
error: git-svn died of signal 11

Failed to run "['git', 'svn', 'dcommit', '--rmdir']" exit_code: 139 cwd: /Volumes/Data/EWS/WebKit
Updating OpenSource
Current branch master is up to date.

Full output: http://webkit-queues.appspot.com/results/22599400

[Csaba Osztrogonác]

Comment on attachment 216575 [details]
proposed fix

Let's try CQ again, the SVN server is sick today.

[WebKit Commit Bot]

Comment on attachment 216575 [details]
proposed fix

Clearing flags on attachment: 216575

Committed r159055: <http://trac.webkit.org/changeset/159055>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.