MIPS: sligthly reduce deopt tables size.

MIPS: slightly reduce deopt tables size.

Port r22305 (00a3740)

BUG=
R=dusan.milosavljevic@rt-rk.com

Committed: https://code.google.com/p/v8/source/detail?r=22357

[kilvadyb, 2014-07-10 11:54:28]

On 2014/07/10 11:44:53, kilvadyb wrote:

PTAL:
In Deoptimizer::TableEntryGenerator::GeneratePrologue() we could reduce the size
of a table entry by one more instruction if we have a guarantee that count() is
less then 0xffff (== li macro instruction is one instruction long). Then we
could put li() into the delay slot of jr(t9). All the tests use small enough
entries.

[dusmil, 2014-07-10 15:26:24]

Yes, we can. Also add case where number of entries does not fit in 16-bit
offset, so the li()
is not in delay slot.

[kilvadyb, 2014-07-10 15:59:23]

On 2014/07/10 15:26:24, dusmil wrote:
> Yes, we can. Also add case where number of entries does not fit in 16-bit
> offset, so the li()
> is not in delay slot.

If we implement two cases (i.e. li can be two instructions) then we can not
reduce the size of an entry as that must be constant:
const int Deoptimizer::table_entry_size_ = 6 * Assembler::kInstrSize;
So the code will be slightly smaller/quicker but the size of entries would not
be reduced. But I make an update with the suggested modification.

[dusmil, 2014-07-10 16:23:06]

Right, I think it is not worth saving one instruction if we are going to
sacrifice
robustness. Current tests do not need large numbers of entries, but it does not
mean
some heavy benchmark will not.


This change LGTM.

[dusmil, 2014-07-10 16:35:43]

Ok, in the Arm/arm64 port it is used 16-bit constants for this.
Do it as you planned.

[kilvadyb, 2014-07-10 16:41:11]

On 2014/07/10 16:23:06, dusmil wrote:
> Right, I think it is not worth saving one instruction if we are going to
> sacrifice
> robustness. Current tests do not need large numbers of entries, but it does
not
> mean
> some heavy benchmark will not.
> 
> 
> This change LGTM.

I uploaded an optimized version. Also PTAL I think
const int Deoptimizer::table_entry_size_ = 7 * Assembler::kInstrSize;
was not correct as the code of an entry was 8 instructions long in worst case.
So I left it to be 7 after dropping the
addiu(sp, sp, -1 * kPointerSize);

[kilvadyb, 2014-07-10 16:43:37]

On 2014/07/10 16:35:43, dusmil wrote:
> Ok, in the Arm/arm64 port it is used 16-bit constants for this.
> Do it as you planned.

I missed that. So I will fix it according to the arm 16 bit constant. Thank you
for the reviews.

[kilvadyb, 2014-07-10 19:00:35]

On 2014/07/10 16:43:37, kilvadyb wrote:
> On 2014/07/10 16:35:43, dusmil wrote:
> > Ok, in the Arm/arm64 port it is used 16-bit constants for this.
> > Do it as you planned.
> 
> I missed that. So I will fix it according to the arm 16 bit constant. Thank
you
> for the reviews.

Dusan, PTAL. I could not find 16 bit constants in arm/arm64 code for table
entries. They use int for entry size and entry number/index values. But anyway I
created a new version which uses Branch(label) and ASSERTs that the index is
between the int16 boundaries.

[dusmil, 2014-07-11 10:44:19]

On 2014/07/10 19:00:35, kilvadyb wrote:
> On 2014/07/10 16:43:37, kilvadyb wrote:
> > On 2014/07/10 16:35:43, dusmil wrote:
> > > Ok, in the Arm/arm64 port it is used 16-bit constants for this.
> > > Do it as you planned.
> > 
> > I missed that. So I will fix it according to the arm 16 bit constant. Thank
> you
> > for the reviews.
> 
> Dusan, PTAL. I could not find 16 bit constants in arm/arm64 code for table
> entries. They use int for entry size and entry number/index values. But anyway
I
> created a new version which uses Branch(label) and ASSERTs that the index is
> between the int16 boundaries.

In the Arm64 port  "movz" instruction is used to load 16-bit immediate in
TableEntryGenerator::GeneratePrologue (deoptimizer-arm64.cc:352).
Also, there is an assert that maximum table entry size never exceeds int16.

Now we can reduce entry size to 2 instructions, Please update patch.

[dusmil, 2014-07-11 10:47:14]

https://codereview.chromium.org/387473003/diff/40001/src/mips/deoptimizer-mip...
File src/mips/deoptimizer-mips.cc (right):

https://codereview.chromium.org/387473003/diff/40001/src/mips/deoptimizer-mip...
src/mips/deoptimizer-mips.cc:323: const int Deoptimizer::table_entry_size_ = 4 *
Assembler::kInstrSize;
Real size now is 2 instructions.

https://codereview.chromium.org/387473003/diff/40001/src/mips/deoptimizer-mip...
src/mips/deoptimizer-mips.cc:339: // Pad the rest of the code.
We can remove padding when size is changed.

[kilvadyb, 2014-07-11 12:06:05]

https://codereview.chromium.org/387473003/diff/40001/src/mips/deoptimizer-mip...
File src/mips/deoptimizer-mips.cc (right):

https://codereview.chromium.org/387473003/diff/40001/src/mips/deoptimizer-mip...
src/mips/deoptimizer-mips.cc:323: const int Deoptimizer::table_entry_size_ = 4 *
Assembler::kInstrSize;
On 2014/07/11 10:47:14, dusmil wrote:
> Real size now is 2 instructions.

Done.

https://codereview.chromium.org/387473003/diff/40001/src/mips/deoptimizer-mip...
src/mips/deoptimizer-mips.cc:339: // Pad the rest of the code.
On 2014/07/11 10:47:14, dusmil wrote:
> We can remove padding when size is changed.

Done.

[dusmil, 2014-07-11 12:16:24]

LGTM

[kilvadyb, 2014-07-11 13:49:40]

Committed patchset #4 manually as r22357 (presubmit successful).