Hi Erik,
Thank you for the reviews. We merged the changes with the newer bleeding
edge
(svn 4311) to
1. take care of the function calls generated in macro-assembler-arm.cc;
2. use the existing ARMv7 feature;
3. re-evalute the benefit.
Most of the comments and concerns are addressed. The regression test
(except the
serialization tests) passes. We see about 2.6% improvement on SunSpider and
v8-suite. In the current changes, I think we don't need CPU::FlushICache in
set_target_address_at.
1. set_target_address_at is called by set_target_object and
set_target_address.
2. set_target_object is called in objects.cc followed with
"CPU::FlushICache(instruction_start(), instruction_size())".
3. calls to set_target_address
3.1 the function call to set_target_address in objects.cc is also followed
by a
FlushICache.
3.2 the function call to set_target_address in ic-inh.h is to update the
target
of a function call. In macro-assembler-arm.cc, we remain the use of "ldr +
blx";
no movw/movt is generated for this case. Thus, set_target_address will
update
the const pool, not the ICache.
3.3 set_target_address is called in debug.cc, mark-compact.cc, and
liveedit.cc.
Currently, we're not quite clear about what those function calls do and how
to
triger those function calls. If you have any insights on this, please let us
know.
-Kun
http://codereview.chromium.org/1128009/diff/1/5
File src/arm/assembler-arm-inl.h (right):
http://codereview.chromium.org/1128009/diff/1/5#newcode89
src/arm/assembler-arm-inl.h:89: Instr instr = Memory::int32_at(pc);
On 2010/03/22 11:50:09, Erik Corry wrote:
Done.
http://codereview.chromium.org/1128009/diff/1/5#newcode91
src/arm/assembler-arm-inl.h:91: Instr instr_movt = Memory::int32_at(pc +
4);
On 2010/03/22 11:50:09, Erik Corry wrote:
Done.
http://codereview.chromium.org/1128009/diff/1/5#newcode92
src/arm/assembler-arm-inl.h:92: if ((instr_movt & 0xff00000) ==
0x3400000) {
On 2010/03/22 11:50:09, Erik Corry wrote:
Done.
http://codereview.chromium.org/1128009/diff/1/5#newcode103
src/arm/assembler-arm-inl.h:103: if ((instr & 0x0f7f0000) == 0x051f0000)
{
On 2010/03/22 11:50:09, Erik Corry wrote:
Done.
http://codereview.chromium.org/1128009/diff/1/5#newcode268
src/arm/assembler-arm-inl.h:268: if ((instr & 0x0f7f0000) == 0x051f0000)
On 2010/03/22 11:50:09, Erik Corry wrote:
Done.
http://codereview.chromium.org/1128009/diff/1/5#newcode286
src/arm/assembler-arm-inl.h:286: // movw movt
On 2010/03/22 11:50:09, Erik Corry wrote:
Done.
http://codereview.chromium.org/1128009/diff/1/5#newcode287
src/arm/assembler-arm-inl.h:287: uint32_t *new_pc = (unsigned int*)pc;
On 2010/03/22 11:50:09, Erik Corry wrote:
Done.
http://codereview.chromium.org/1128009/diff/1/5#newcode291
src/arm/assembler-arm-inl.h:291: if ((*(new_pc+1) & 0xff00000) ==
0x3400000) {
On 2010/03/22 11:50:09, Erik Corry wrote:
Done.
http://codereview.chromium.org/1128009/diff/1/6
File src/arm/assembler-arm.cc (right):
http://codereview.chromium.org/1128009/diff/1/6#newcode69
src/arm/assembler-arm.cc:69:
On 2010/03/22 11:50:09, Erik Corry wrote:
Done.
http://codereview.chromium.org/1128009/diff/1/6#newcode635
src/arm/assembler-arm.cc:635: Condition cond =
static_cast<Condition>(instr & CondMask);
On 2010/03/22 11:50:09, Erik Corry wrote:
Done.
http://codereview.chromium.org/1128009/diff/1/6#newcode636
src/arm/assembler-arm.cc:636: if ((x.rmode_ !=
RelocInfo::EMBEDDED_OBJECT) &&
On 2010/03/22 11:50:09, Erik Corry wrote:
EMBEDDED_OBJECT causes failures. The reason is unknown, so it's disabled
now.
http://codereview.chromium.org/1128009/diff/1/6#newcode642
src/arm/assembler-arm.cc:642: if (x.imm32_ > 65535)
On 2010/03/22 11:50:09, Erik Corry wrote:
Done.
http://codereview.chromium.org/1128009/diff/1/6#newcode966
src/arm/assembler-arm.cc:966: emit(cond | (0x34)*B20 | ((src.imm32_ >>
16 >> 12) & 0xf)*B16
On 2010/03/22 11:50:09, Erik Corry wrote:
Done.
http://codereview.chromium.org/1128009/diff/1/6#newcode967
src/arm/assembler-arm.cc:967: | dst.code()*B12 | (((src.imm32_ >> 16)&
0xfff)));
On 2010/03/22 11:50:09, Erik Corry wrote:
Done.
http://codereview.chromium.org/1128009/diff/1/6#newcode1714
src/arm/assembler-arm.cc:1714: bool withconstpool) {
On 2010/03/22 11:50:09, Erik Corry wrote:
Done.
http://codereview.chromium.org/1128009/diff/1/6#newcode1822
src/arm/assembler-arm.cc:1822: if (((instr & (7*B25 | P | U | B | W |
15*B16 | Off12Mask)) !=
On 2010/03/22 11:50:09, Erik Corry wrote:
Done.
http://codereview.chromium.org/1128009/diff/1/9
File src/arm/disasm-arm.cc (right):
http://codereview.chromium.org/1128009/diff/1/9#newcode664
src/arm/disasm-arm.cc:664:
//------------------------------------------------------------
On 2010/03/22 11:50:09, Erik Corry wrote:
Done.
http://codereview.chromium.org/1128009/diff/1/9#newcode703
src/arm/disasm-arm.cc:703: Format(instr, "movw'cond 'mw"); // movw
instruction
On 2010/03/22 11:50:09, Erik Corry wrote:
Done.
http://codereview.chromium.org/1128009/diff/1/9#newcode729
src/arm/disasm-arm.cc:729: Format(instr, "movt'cond 'mt"); // movt
instruction
On 2010/03/22 11:50:09, Erik Corry wrote:
Done.
http://codereview.chromium.org/1128009/diff/1/8
File src/arm/simulator-arm.cc (right):
http://codereview.chromium.org/1128009/diff/1/8#newcode1531
src/arm/simulator-arm.cc:1531: // Format(instr, "movw'cond 'rd, 'rn,
'imm");
On 2010/03/22 11:50:09, Erik Corry wrote:
Done.
http://codereview.chromium.org/1128009/diff/1/8#newcode1532
src/arm/simulator-arm.cc:1532: alu_out = (instr->Immed4Field() << 12) |
instr->Offset12Field();
On 2010/03/22 11:50:09, Erik Corry wrote:
Done.
http://codereview.chromium.org/1128009/diff/1/8#newcode1574
src/arm/simulator-arm.cc:1574: // Format(instr, "movt'cond 'rd, 'rn,
'imm");
On 2010/03/22 11:50:09, Erik Corry wrote:
Done.
http://codereview.chromium.org/1128009/diff/1/8#newcode2039
src/arm/simulator-arm.cc:2039: int offset_8 = 0x000000FF &
instr->Bits(7, 0);
On 2010/03/22 11:50:09, Erik Corry wrote:
Done.
http://codereview.chromium.org/1128009/diff/1/8#newcode2094
src/arm/simulator-arm.cc:2094: /*if((unsigned int) instr == 0x579241ac)
On 2010/03/22 11:50:09, Erik Corry wrote:
Done.
http://codereview.chromium.org/1128009/diff/1/3
File src/objects.cc (right):
http://codereview.chromium.org/1128009/diff/1/3#newcode5012
src/objects.cc:5012: Instr instr = Memory::int32_at(it.rinfo()->pc());
On 2010/03/22 11:50:09, Erik Corry wrote:
Done.
http://codereview.chromium.org/1128009/diff/1/3#newcode5021
src/objects.cc:5021: if ((instr_movt & 0xff00000) == 0x3400000) {
On 2010/03/22 11:50:09, Erik Corry wrote:
Done.
http://codereview.chromium.org/1128009/diff/1/3#newcode5022
src/objects.cc:5022: unsigned int offset_movt = ((instr_movt & 0xf0000)
4) |
On 2010/03/22 11:50:09, Erik Corry wrote:
Done.
http://codereview.chromium.org/1128009/diff/1/3#newcode5027
src/objects.cc:5027: Code** p = reinterpret_cast<Code **>(p1);
On 2010/03/22 11:50:09, Erik Corry wrote:
Done.
http://codereview.chromium.org/1128009
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
To unsubscribe, reply using "remove me" as the subject.
