Bug 104103 – Implement add64 for ARM traditional assembler after r136601

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

[Csaba Osztrogonác]

r136601 broke the ARM traditional build:

/mnt/raptor2/slaves/qt5-linux-armv7-release/build/Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp: In member function 'void JSC::DFG::SpeculativeJIT::compile(JSC::DFG::Node&)':
/mnt/raptor2/slaves/qt5-linux-armv7-release/build/Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:5048:15: error: 'class JSC::DFG::JITCompiler' has no member named 'add64'

It seems ARM traditional assembler needs add64.

[Gabor Ballabas]

Created attachment 177706 [details]
Fix

[Zoltan Herczeg]

Comment on attachment 177706 [details]
Fix

good direction, but it is not finished.

View in context: https://bugs.webkit.org/attachment.cgi?id=177706&action=review

> Source/JavaScriptCore/assembler/MacroAssemblerARM.h:127
> +        move(TrustedImmPtr(address.m_ptr), ARMRegisters::S0);
> +        load32(Address(ARMRegisters::S0), ARMRegisters::S1);
> +        add32(imm, ARMRegisters::S1);
> +        store32(ARMRegisters::S1, ARMRegisters::S0);

This is actually the add32 version. You need to take care about the higher 32 bit with addc/subc. And you should use the adds directly (I know add32 now uses adds, but that may be go away. I mean I want it to go away :) )

[Gabor Ballabas]

Created attachment 177742 [details]
Patch

[Zoltan Herczeg]

Comment on attachment 177742 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=177742&action=review

> Source/JavaScriptCore/assembler/MacroAssemblerARM.h:905
> +        if (imm.m_value <= 0xff)
> +            m_assembler.adds(ARMRegisters::S0, ARMRegisters::S0, ARMAssembler::getOp2Byte(imm.m_value));
> +        else {
> +            m_assembler.adds(ARMRegisters::S0, ARMRegisters::S0, m_assembler.getImm(imm.m_value, ARMRegisters::S1));
> +            move(TrustedImmPtr(address.m_ptr), ARMRegisters::S1);
> +        }

This is not how we do at the moment. This should be a simple:

m_assembler.adds(dest, dest, m_assembler.getImm(imm.m_value, ARMRegisters::S0));

[Gabor Ballabas]

Created attachment 178778 [details]
Patch

[Vivek Galatage]

*** Bug 104085 has been marked as a duplicate of this bug. ***

[Zoltan Herczeg]

Comment on attachment 178778 [details]
Patch

r=me

[WebKit Review Bot]

Comment on attachment 178778 [details]
Patch

Clearing flags on attachment: 178778

Committed r137426: <http://trac.webkit.org/changeset/137426>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.