Contribution of PowerPC port (continuation of 422063005) - PPC opt 1

Contribution of PowerPC port (continuation of 422063005) - PPC opt 1

Contribution of PowerPC port (continuation of 422063005, 817143002,
866843003, and 901083004. This patch enables the OOL Constant Pool for Power.
Our measurements show that we get an overall boost of ~3% from this
optimization.

Subsequent patches will cover:
   - remaining optimizations for PPC (5)
   - remaining AIX changes not resolved by 4.8 compiler (4.8 is only recently available for AIX)
   - incremental updates required to ppc directories due to platform specific changes made
    in google repos while we complete the above steps.

With the update there are still some timeouts seen when run in simulated mode
which may be a result of the missing optimizations. Once we have all the optimizations in
we will review the simulation results and address/exclude tests as necessary
so that the simulated runs are clean.

	modified:   src/full-codegen.h
	modified:   src/globals.h

R=danno@chromium.org, svenpanne@chromium.org

BUG=

[michael_dawson, 2015-02-05 23:45:56]

First of optimizations, starting with the smallest change first

[Sven Panne, 2015-02-06 07:28:00]

https://codereview.chromium.org/882263003/diff/1/src/globals.h
File src/globals.h (right):

https://codereview.chromium.org/882263003/diff/1/src/globals.h#newcode77
src/globals.h:77: #define V8_OOL_CONSTANT_POOL 1
FYI (after some internal discussions): The OOL constant pool is currently broken
(violates the marking invariant), furthermore, we might remove it in the future.
So it would be good to avoid using it from the start. What's the reason you want
to enable it? Perhaps there's another way to achieve your goals.

[MTBrandyberry, 2015-02-06 22:53:07]

On 2015/02/06 07:28:00, Sven Panne wrote:
> https://codereview.chromium.org/882263003/diff/1/src/globals.h
> File src/globals.h (right):
> 
> https://codereview.chromium.org/882263003/diff/1/src/globals.h#newcode77
> src/globals.h:77: #define V8_OOL_CONSTANT_POOL 1
> FYI (after some internal discussions): The OOL constant pool is currently
broken
> (violates the marking invariant), furthermore, we might remove it in the
future.
> So it would be good to avoid using it from the start. What's the reason you
want
> to enable it? Perhaps there's another way to achieve your goals.

We gravitated toward OOL constant pools because they map fairly well
to the way traditionally compiled code on the Power platform handles
global data.  That is, via a TOC (table of contents) pointer placed in
a reserved register pointing to a table of data.

In the absence of a constant table, we are forced to construct address
constants using bitwise operations.  For 64-bit, in the general case,
this requires five instructions:

  lis    r8, <A>         // load bits 63-48
  ori    r8, r8, <B>     // load bits 47-32
  rldicr r8, r8, 32, 31  // shift into high word
  oris   r8, r8, <C>     // load bits 31-16
  ori    r8, r8, <D>     // load bits 15-0

This is compared to a single instruction load from the constant pool.

The additional path-length caused by this has a significant
performance penalty.  In conjunction with other optimizations to
reduce path length, the effect is even larger (e.g. due to instruction
grouping on Power8).

Inline constant pools aren't an immediately ideal solution due to the
fact that the Power architecture does not support pc-relative loads.

I can, however, imagine a hybrid solution where:
  (a) constants are emitted in the same buffer as the code (like
      inline pools but most likely after the code rather than
      interspersed)
in conjunction with:
  (b) a requirement that a reserved register hold the base of the
      table with logic to set it up in the prologue and save/restore to a
      dedicated slot in the frame (like OOL pools).

This may or may not be an improvement depending on the details of the
OOL issue you referenced.  It would still require some support and
maintenence in the common code for the general mechanism, but would
allow the constant pool array object to be done away with.

[Sven Panne, 2015-02-10 07:54:11]

svenpanne@chromium.org changed reviewers:
+ bmeurer@chromium.org, rmcilroy@chromium.org

[Sven Panne, 2015-02-10 07:54:58]

I've added 2 people who probably know more about our constant pool plans than I
do... :-)

[Benedikt Meurer, 2015-02-10 08:00:53]

> Inline constant pools aren't an immediately ideal solution due to the
> fact that the Power architecture does not support pc-relative loads.
> 
> I can, however, imagine a hybrid solution where:
>   (a) constants are emitted in the same buffer as the code (like
>       inline pools but most likely after the code rather than
>       interspersed)

Note that there was no final conclusion about inline/out-of-line constant pools
in V8. We had quite some trouble with the OOL constant pool, and it's currently
disabled because it (a) tanks performance and (b) breaks the marking invariant.
The main problem we face is code patching, and we want to use the OOL constant
pool to solve this problem. But everything else that is not subject to code
patching (i.e. double constants, references to heap objects, large integer
constants, etc.) could still live inside the code object (in as of ARM) or after
the code object (otherwise). For PPC it probably doesn't matter anyway, since
you need a dedicated register to point to the beginning of the constant pool, no
matter if its inline or out-of-line.

[rmcilroy, 2015-02-10 11:45:38]

> it's currently disabled because it (a) tanks performance

This is not quite true - it regressed Splay.Latency (~20%), but was performance
neutral on the other benchmarks on Arm
and the overall regression on Octane was pretty small (~1-2%).  It was enabled
for quite some time on Arm without anyone
complaining about performance ;).


(b) breaks the marking invariant.

This was the reason it was reverted on Arm.  Jaroslav wrote a test which exposes
this issue at: https://codereview.chromium.org/681633002/.  The main issue is
that when an IC transitions to a new state, it will update a constant pool entry
with a new pointer (which will be marked White by the GC - i.e., untraced). If
the code object was already marked Black then it will be remarked as marked Gray
(requiring another rescan), but the constant pool remains black even although it
now contains a white pointer - this is bad.

The fix is to mark the OOL constant pool as gray when the IC transition happens,
but this requires a bit of plumbing and I've not had a chance to make this fix. 
I've filed a bug with the discussion of this problem and some more details at:
http://code.google.com/p/v8/issues/detail?id=3881 - feel free to pick it up.

[michael_dawson, 2015-02-18 00:15:27]

Sorry for the late reply I was out at the Node summit last week.  Matt is
investigating the alternate solution outlined in his comment above.  Based
on what we learn from that we'll decide if we want to use that alternative
or fix the existing OOL implementation.  

If we decide to pursue the alternate implementation should we remove the
existing OOL implementation as part of that work ?  

We ask because there is some overlap in that some parts of 
the existing OOL implementation will be re-used in the alternate implementation
and we will need to do extra work to add the alternate while leaving the current
OOL implementation in place. 

In the mean time I'm going to put together another review to bring the PPC
directories
up to date with the changes for the last few weeks and then one for the next
optimization
we had in the list.

[Jakob Kummerow, 2015-02-18 10:08:04]

I would think that fixing the existing OOL constant pool implementation is a lot
less work than implementing a new one.

I'm not sure whether the existing implementation should be kept if you end up
deciding to do your own thing, but I would err on the side of caution and keep
it. Ross tells me that the performance benefit isn't great right now, but I'm
kind of hopeful that avoiding code patching altogether would be beneficial on
some platforms. For that, we need three pieces as far as I'm aware: (1) vector
ICs (work in progress), (2) OOLCP, (3) a non-patching code ageing mechanism.

[MTBrandyberry, 2015-02-19 22:03:19]

On 2015/02/18 10:08:04, Jakob wrote:
> I would think that fixing the existing OOL constant pool implementation is a
lot
> less work than implementing a new one.
> 
> I'm not sure whether the existing implementation should be kept if you end up
> deciding to do your own thing, but I would err on the side of caution and keep
> it. Ross tells me that the performance benefit isn't great right now, but I'm
> kind of hopeful that avoiding code patching altogether would be beneficial on
> some platforms. For that, we need three pieces as far as I'm aware: (1) vector
> ICs (work in progress), (2) OOLCP, (3) a non-patching code ageing mechanism.

For reference, here is the implementation of the alternate solution I proposed
in
comment #3.  This provides the same performance benefit on PPC as the current
OOL
solution, with none of the complexity and issues associated with the
ConstantPoolArray object.

Let us know whether this looks like a reasonable approach to you.

https://github.com/andrewlow/v8ppc/commit/9ef98ea5b70d2d5bfa712c5507ce6f2ac3d...

    Introduce alternate OOL constant pool mode.

    This provides an alternate mode for out-of-line constant pools.  The
    new mode supports the placement of a contiguous constant pool at the
    end of the code buffer rather than a separate heap object.  It
    leverages the traditional mode's common code infrastructure for
    maintaining a dedicated constant pool base register, but does not
    require the ConstantPoolArray heap object.

[Sven Panne, 2015-02-20 08:51:52]

tl;dr The kBootCodeSizeMultiplier change is OK, but the OOL constant pool is
broken and anything related to it should not be touched at all.

Longer version: We want to remove the OOL constant pool (probably in the next
quarter) and do something different, probably architecture-dependent. The
long-term vision is: Everything which is patched at runtime will live in the
type feedback vector, everything else which is constant (e.g. doubles, 64bit
constants, references etc.) will live in the code object and will only be
changed by the GC. More concretely: The ARM port will probably just stay like it
is with the interleaved constant pools, Intel ports will probably have some kind
of constant pool at the end of the code object (addressed in a IP-relative way
on x64, addressed via absolute addresses on ia32), etc.

What this means for the PPC port: Don't touch and/or use the OOL constant pool
and do whatever is needed to get things running for you locally to the PPC port.
Sorry if we didn't communicate this clearly enough, we needed to figure out
internally where we want to go first. :-}

[michael_dawson, 2015-02-20 21:52:13]

On 2015/02/20 08:51:52, Sven Panne wrote:
> tl;dr The kBootCodeSizeMultiplier change is OK, but the OOL constant pool is
> broken and anything related to it should not be touched at all.
> 
> Longer version: We want to remove the OOL constant pool (probably in the next
> quarter) and do something different, probably architecture-dependent. The
> long-term vision is: Everything which is patched at runtime will live in the
> type feedback vector, everything else which is constant (e.g. doubles, 64bit
> constants, references etc.) will live in the code object and will only be
> changed by the GC. More concretely: The ARM port will probably just stay like
it
> is with the interleaved constant pools, Intel ports will probably have some
kind
> of constant pool at the end of the code object (addressed in a IP-relative way
> on x64, addressed via absolute addresses on ia32), etc.
> 
> What this means for the PPC port: Don't touch and/or use the OOL constant pool
> and do whatever is needed to get things running for you locally to the PPC
port.
> Sorry if we didn't communicate this clearly enough, we needed to figure out
> internally where we want to go first. :-}

At the very least we are going to need changes in the common code to allow us to

maintain a dedicated constant pool register and slot in the stack frame. This is

because Power architecture does not support pc-relative loads.
We can limit this with a PPC specific guard name but we thought this could also 
be leveraged other platforms like MIPs.  

Was your concern with the way we guarded changes in the common code (using
existing
out of line flags)  for the base register and slot or that we require 
that support at all in the alternate approach.

[Sven Panne, 2015-02-23 08:00:03]

On 2015/02/20 21:52:13, michael_dawson wrote:
> At the very least we are going to need changes in the common code to allow us
to
> maintain a dedicated constant pool register and slot in the stack frame. This
is
> because Power architecture does not support pc-relative loads.
> We can limit this with a PPC specific guard name but we thought this could
also 
> be leveraged other platforms like MIPs.  

I don't think we need any changes in the common code at all for this: Somehow
you need to reload the constant pool register at various places (e.g. after
calls etc.), and there is no fundamental reason why this has to be done from the
stack frame. You can just reload the absolute address again, IIRC this needs 2
instructions on PPC, so you need some kind of relocation info for this
instruction pair. I don't know if you already have this kind of relocation, but
ARM (and probably MIPS, too) has it. It's not obvious at all that this is less
efficient than a memory load from the fram. Let's first get things running on
PPC with minimal changes to the rest of the system. Tuning can be done later
after extensive benchmarking.

> Was your concern with the way we guarded changes in the common code (using
existing
> out of line flags)  for the base register and slot or that we require 
> that support at all in the alternate approach.

Regarding the guards: Assume that no OOL-related #defines exist, they will
probably die. Introducing more usages will just make ripping them out later more
complicated. Regarding register/slot: See above.

[Sven Panne, 2015-02-23 08:00:50]

Just a remark: We can land the kBootCodeSizeMultiplier change alone, of course.

[michael_dawson, 2015-02-23 17:29:26]

On 2015/02/23 08:00:03, Sven Panne wrote:
> On 2015/02/20 21:52:13, michael_dawson wrote:
> > At the very least we are going to need changes in the common code to allow
us
> to
> > maintain a dedicated constant pool register and slot in the stack frame.
This
> is
> > because Power architecture does not support pc-relative loads.
> > We can limit this with a PPC specific guard name but we thought this could
> also 
> > be leveraged other platforms like MIPs.  
> 
> I don't think we need any changes in the common code at all for this: Somehow
> you need to reload the constant pool register at various places (e.g. after
> calls etc.), and there is no fundamental reason why this has to be done from
the
> stack frame. You can just reload the absolute address again, IIRC this needs 2
> instructions on PPC, so you need some kind of relocation info for this
> instruction pair. I don't know if you already have this kind of relocation,
but
> ARM (and probably MIPS, too) has it. It's not obvious at all that this is less
> efficient than a memory load from the fram. Let's first get things running on
> PPC with minimal changes to the rest of the system. Tuning can be done later
> after extensive benchmarking.

For this particular case the Octane benchmark shows that 
replacing the multi-instruction mov sequence (2 instructions
for 32-bit, but 5 instructions for 64-bit), where possible,
with a load from the constant pool yields about a 3% 
performance improvement 

From what we understand reloading the absolute address as 
suggested above, we think it would still require
changes to the common code. It would require an internal
reference relocation type which is not currently
supported by the serializer, and support to record the constant
pool's location in the Code object to be able to proplerly 
apply relocations.  We also don't believe that it would yield
the same performance improvement, particularly on 64 bit as
the extra path-length dominates the instruction groups.  
The addition of a 5 instruction mov after every call will also
significantly inflate our code size.

While you sort out how this will be handled across platforms we'd
like to be able to improve performance for PPC with the 
understanding that it will be reworked to fit into the 
common solution once its decided.

Would it be possible to set up a conference call to discuss, it
might be easier ? 

> 
> > Was your concern with the way we guarded changes in the common code (using
> existing
> > out of line flags)  for the base register and slot or that we require 
> > that support at all in the alternate approach.
> 
> Regarding the guards: Assume that no OOL-related #defines exist, they will
> probably die. Introducing more usages will just make ripping them out later
more
> complicated. Regarding register/slot: See above.

Fair enough.

[Sven Panne, 2015-02-24 08:46:36]

On 2015/02/23 17:29:26, michael_dawson wrote:
> For this particular case the Octane benchmark shows that 
> replacing the multi-instruction mov sequence (2 instructions
> for 32-bit, but 5 instructions for 64-bit), where possible,
> with a load from the constant pool yields about a 3% 
> performance improvement 

I think we should totally ignore any kind of optimizations for now, our top
priority is getting the PPC port integrated into the system with basically no
changes outside the PPC directory, have build bots, test coverage, etc. Even if
we paid 20%, I wouldn't care right now. Every hack and complication we introduce
now will only slow the whole process down.

> From what we understand reloading the absolute address as 
> suggested above, we think it would still require
> changes to the common code. It would require an internal
> reference relocation type which is not currently
> supported by the serializer, and support to record the constant
> pool's location in the Code object to be able to proplerly 
> apply relocations.  We also don't believe that it would yield
> the same performance improvement, particularly on 64 bit as
> the extra path-length dominates the instruction groups.  
> The addition of a 5 instruction mov after every call will also
> significantly inflate our code size.

As stated above, let's forget about performance tweaks for now and just get
things working in the easiest possible way. In our concrete case: Follow the
MIPS64 port and just load the constants directly in the code stream (see
MacroAsssembler:li in MIPS64). This way we don't need any kind of magic outside
the port.

Regarding the "#if V8_TARGET_ARCH_PPC" in serialize.cc: Would it be possible to
remove that by e.g. introducing a new RelocInfo::Mode which is just generated
for the PPC port, but otherwise architecture-independent? We do this for other
stuff already, see e.g. VENEER_POOL. As it is, this a bit of a hack and will
probably cause pain later.

> While you sort out how this will be handled across platforms we'd
> like to be able to improve performance for PPC with the 
> understanding that it will be reworked to fit into the 
> common solution once its decided.

Again, performance is not an issue at all at this point in time.

[michael_dawson, 2015-02-24 22:33:45]

>I think we should totally ignore any kind of optimizations for now, our top
> priority is getting the PPC port integrated into the system with basically no
> changes outside the PPC directory, have build bots, test coverage, 
 
In terms of turning on the build bots and getting test coverage would 
this be in simulated PPC mode ? I've been running in the simulated 
mode to validate the commits accepted so far and the tests pass 
other than a number of timeouts. (we also run in native mode in
our internal testing were results are btter)  If so, would the 
first step be to create a review that brings the ppc directories
up to date based on changes in the common code since our last update
along with disabling the tests which timeout ? I see that there are
excludes for timeouts in simulated mode on other platforms, although
the number I see in the PPC simulated runs is higher so this would
just be a first step.

For us, getting the performance opts in are important, otherwise we have to 
maintain the deltas in our repo, but I can see how getting the build bots 
and test coverage enabled is a good step.

[Sven Panne, 2015-02-25 07:41:25]

On 2015/02/24 22:33:45, michael_dawson wrote:
> In terms of turning on the build bots and getting test coverage would 
> this be in simulated PPC mode ?

As a first step, yes. I don't know what the plans are for bots with real PPC HW,
but even if/when we have such bots, the PPC simulator bots would still stay,
otherwise the simulator itself would definitely bit-rot.

> I've been running in the simulated 
> mode to validate the commits accepted so far and the tests pass 
> other than a number of timeouts. (we also run in native mode in
> our internal testing were results are btter)  If so, would the 
> first step be to create a review that brings the ppc directories
> up to date based on changes in the common code since our last update
> along with disabling the tests which timeout ? I see that there are
> excludes for timeouts in simulated mode on other platforms, although
> the number I see in the PPC simulated runs is higher so this would
> just be a first step.

That's perfectly fine. Having a relatively high number of tests marked as SLOW
and/or TIMEOUT for a new platform is the usual way to get a port working. Of
course this number should be reduced over time, but that's in the hands of
people caring about the platform port.

> For us, getting the performance opts in are important, otherwise we have to 
> maintain the deltas in our repo, but I can see how getting the build bots 
> and test coverage enabled is a good step.

So what needs to be done? The steps I see so far are:

   * Reduce this CL at hand to the kBootCodeSizeMultiplier change and land it
(that's harmless for the rest of the system and non-controversial).

   * Remove any use of V8_OOL_CONSTANT_POOL from your code and even from your
"internal deltas".

   * Remove the PPC platform-dependent hack in serialize.cc, perhaps by adding a
new RelocInfo::Mode.

   * Any other changes you still need to get things running and integrated. Are
there any?

[michael_dawson, 2015-02-25 21:04:38]

On 2015/02/25 07:41:25, Sven Panne wrote:
> On 2015/02/24 22:33:45, michael_dawson wrote:
> > In terms of turning on the build bots and getting test coverage would 
> > this be in simulated PPC mode ?
> 
> As a first step, yes. I don't know what the plans are for bots with real PPC
HW,
> but even if/when we have such bots, the PPC simulator bots would still stay,
> otherwise the simulator itself would definitely bit-rot.
> 
> > I've been running in the simulated 
> > mode to validate the commits accepted so far and the tests pass 
> > other than a number of timeouts. (we also run in native mode in
> > our internal testing were results are btter)  If so, would the 
> > first step be to create a review that brings the ppc directories
> > up to date based on changes in the common code since our last update
> > along with disabling the tests which timeout ? I see that there are
> > excludes for timeouts in simulated mode on other platforms, although
> > the number I see in the PPC simulated runs is higher so this would
> > just be a first step.
> 
> That's perfectly fine. Having a relatively high number of tests marked as SLOW
> and/or TIMEOUT for a new platform is the usual way to get a port working. Of
> course this number should be reduced over time, but that's in the hands of
> people caring about the platform port.
> 
> > For us, getting the performance opts in are important, otherwise we have to 
> > maintain the deltas in our repo, but I can see how getting the build bots 
> > and test coverage enabled is a good step.
> 
> So what needs to be done? The steps I see so far are:
> 
>    * Reduce this CL at hand to the kBootCodeSizeMultiplier change and land it
> (that's harmless for the rest of the system and non-controversial).
> 
>    * Remove any use of V8_OOL_CONSTANT_POOL from your code and even from your
> "internal deltas".
> 
>    * Remove the PPC platform-dependent hack in serialize.cc, perhaps by adding
a
> new RelocInfo::Mode.
> 
>    * Any other changes you still need to get things running and integrated.
Are
> there any?

What I've been looking at today is the tests which timeout.  I upped the timeout
to 120s and the number dropped from 38 to 13 so I'm seeing what 180s does.  I'm
assuming its better to give the tests a bit more time to complete so that we get
better coverage.  It takes a while to run the full suite in simulated mode to it
will take me a little while to find the right combination of larger timeout/test
exclusions.

[michael_dawson, 2015-02-27 17:55:43]

On 2015/02/25 21:04:38, michael_dawson wrote:
> On 2015/02/25 07:41:25, Sven Panne wrote:
> > On 2015/02/24 22:33:45, michael_dawson wrote:
> > > In terms of turning on the build bots and getting test coverage would 
> > > this be in simulated PPC mode ?
> > 
> > As a first step, yes. I don't know what the plans are for bots with real PPC
> HW,
> > but even if/when we have such bots, the PPC simulator bots would still stay,
> > otherwise the simulator itself would definitely bit-rot.
> > 
> > > I've been running in the simulated 
> > > mode to validate the commits accepted so far and the tests pass 
> > > other than a number of timeouts. (we also run in native mode in
> > > our internal testing were results are btter)  If so, would the 
> > > first step be to create a review that brings the ppc directories
> > > up to date based on changes in the common code since our last update
> > > along with disabling the tests which timeout ? I see that there are
> > > excludes for timeouts in simulated mode on other platforms, although
> > > the number I see in the PPC simulated runs is higher so this would
> > > just be a first step.
> > 
> > That's perfectly fine. Having a relatively high number of tests marked as
SLOW
> > and/or TIMEOUT for a new platform is the usual way to get a port working. Of
> > course this number should be reduced over time, but that's in the hands of
> > people caring about the platform port.
> > 
> > > For us, getting the performance opts in are important, otherwise we have
to 
> > > maintain the deltas in our repo, but I can see how getting the build bots 
> > > and test coverage enabled is a good step.
> > 
> > So what needs to be done? The steps I see so far are:
> > 
> >    * Reduce this CL at hand to the kBootCodeSizeMultiplier change and land
it
> > (that's harmless for the rest of the system and non-controversial).
> > 
> >    * Remove any use of V8_OOL_CONSTANT_POOL from your code and even from
your
> > "internal deltas".
> > 
> >    * Remove the PPC platform-dependent hack in serialize.cc, perhaps by
adding
> a
> > new RelocInfo::Mode.
> > 
> >    * Any other changes you still need to get things running and integrated.
> Are
> > there any?
> 
> What I've been looking at today is the tests which timeout.  I upped the
timeout
> to 120s and the number dropped from 38 to 13 so I'm seeing what 180s does. 
I'm
> assuming its better to give the tests a bit more time to complete so that we
get
> better coverage.  It takes a while to run the full suite in simulated mode to
it
> will take me a little while to find the right combination of larger
timeout/test
> exclusions.

Create https://codereview.chromium.org/965823002/ to get us more current and to 
remove used of V8_OOL_CONSTANT_POOL

[michael_dawson, 2015-03-07 01:57:56]

New review to cleanup serialize.cc and bring ppc dirs to be current
with changes over the last week. At time of submission it was current
with latest commits and ppc and ppc64 compiled/ran.

https://codereview.chromium.org/986553005/

[michael_dawson, 2015-03-16 22:07:23]

On 2015/03/07 01:57:56, michael_dawson wrote:
> New review to cleanup serialize.cc and bring ppc dirs to be current
> with changes over the last week. At time of submission it was current
> with latest commits and ppc and ppc64 compiled/ran.
> 
> https://codereview.chromium.org/986553005/

Ok, now that the PPC buildbots are up and running and
frequently passing, I think
the next step would be to get back to looking our
constant pool solution for PPC.  As a first step 
I could build a review which has our never version
which does not depend on any of the existing of the OOL
code but does require changes in the common code.

[Sven Panne, 2015-03-17 07:25:38]

On 2015/03/16 22:07:23, michael_dawson wrote:
> Ok, now that the PPC buildbots are up and running and
> frequently passing, I think
> the next step would be to get back to looking our
> constant pool solution for PPC.  As a first step 
> I could build a review which has our never version
> which does not depend on any of the existing of the OOL
> code but does require changes in the common code.

I would prefer if we fix things before we start optimizing: There are still a
few SKIPs in mjsunit.status for PPC, and these should be taken care of first.
Skipping regress/regress-1132 in simulator runs is OK, but all other SKIPs
should not be there.

[michael_dawson, 2015-03-17 22:33:35]

On 2015/03/17 07:25:38, Sven Panne wrote:
> On 2015/03/16 22:07:23, michael_dawson wrote:
> > Ok, now that the PPC buildbots are up and running and
> > frequently passing, I think
> > the next step would be to get back to looking our
> > constant pool solution for PPC.  As a first step 
> > I could build a review which has our never version
> > which does not depend on any of the existing of the OOL
> > code but does require changes in the common code.
> 
> I would prefer if we fix things before we start optimizing: There are still a
> few SKIPs in mjsunit.status for PPC, and these should be taken care of first.
> Skipping regress/regress-1132 in simulator runs is OK, but all other SKIPs
> should not be there.

I've looked through the excluded tests other than regress-1132

I think this one is likely still an issue in non-sim mode and I don't see any
activity on 
Issue 2857 for a long time, but I could remove from what's in the google repo
since we 
currently only run in simulated mode so far.
 
#issue 2857
'test-log/EquivalenceOfLoggingAndTraversal' : [SKIP],

These are still issues and are on our list of what to look at next
 mjsunit/mirror-object
 mjsunit/debug-references 

The rest I think I can either remove or change to SLOW so I'll do that.

[michael_dawson, 2015-03-25 19:32:22]

Ok with the latest changes we've removed the excludes from the test status files
so don't have any skips other than regress/regress-1132 which you indicated
would be ok.

I have a set of small changes to address compilation failures on AIX due to
recent changes but after that my plan would be to create the review which has
our optimization which does not depend on any of the existing of the OOL
code but does require changes in the common code.

[Sven Panne, 2015-03-26 07:20:33]

On 2015/03/25 19:32:22, michael_dawson wrote:
> Ok with the latest changes we've removed the excludes from the test status
files
> so don't have any skips other than regress/regress-1132 which you indicated
> would be ok.

Nice! And our waterfall columns for PPC now have a tendency to be green... :-)

> I have a set of small changes to address compilation failures on AIX due to
> recent changes but after that my plan would be to create the review which has
> our optimization which does not depend on any of the existing of the OOL
> code but does require changes in the common code.

Sounds OK, let's see what the changes to the common code are and how they fit to
our (vague) constant pool plans.

Regarding this CL at hand: Can we close this? Split this? ...?

[michael_dawson, 2015-03-26 15:37:54]

On 2015/03/26 07:20:33, Sven Panne wrote:
> On 2015/03/25 19:32:22, michael_dawson wrote:
> > Ok with the latest changes we've removed the excludes from the test status
> files
> > so don't have any skips other than regress/regress-1132 which you indicated
> > would be ok.
> 
> Nice! And our waterfall columns for PPC now have a tendency to be green... :-)
> 
> > I have a set of small changes to address compilation failures on AIX due to
> > recent changes but after that my plan would be to create the review which
has
> > our optimization which does not depend on any of the existing of the OOL
> > code but does require changes in the common code.
> 
> Sounds OK, let's see what the changes to the common code are and how they fit
to
> our (vague) constant pool plans.
> 
> Regarding this CL at hand: Can we close this? Split this? ...?

I've created https://codereview.chromium.org/1030353003/ with the changes to
enable the constant pool and have added all the reviewers from this review.  I
think
that is the right place to continue the discussion so I'll close this one.