X87: Move Hydrogen and Lithium to src/crankshaft/

X87: Move Hydrogen and Lithium to src/crankshaft/

port 81ee94b6507cf3ee9d44d18b8ccb1f1cc9045be1 (r31410).

   contributed by zhengxing.li@intel.com

   original commit message:

   additional comment:
   The original r31410 patch needs some additional changes for x87
    1. The frames-x87.h is under src/x87 instead of src/crankshaft/x87

R=weiliang.lin@intel.com
BUG=

Committed: https://crrev.com/efbbf0fdcf6b0ae7a5937ef95fcb08eb18d80c6d
Cr-Commit-Position: refs/heads/master@{#31466}

[chunyang.dai, 2015-10-22 03:04:23]

PTAL.

[chunyang.dai, 2015-10-22 03:07:38]

Description was changed from

==========
X87: Move Hydrogen and Lithium to src/crankshaft/

port 81ee94b6507cf3ee9d44d18b8ccb1f1cc9045be1 (r31410).

   contributed by zhengxing.li@intel.com

   original commit message:

   additional comment:
   The original r31410 patch needs some additional changes for x87
    1. The frames-x87.h is under src/x87 instread of src/crankshaft/x87
    2. The header files should be in alphabetical order

R=weiliang.lin@intel.com
BUG=
==========

to

==========
X87: Move Hydrogen and Lithium to src/crankshaft/

port 81ee94b6507cf3ee9d44d18b8ccb1f1cc9045be1 (r31410).

   contributed by zhengxing.li@intel.com

   original commit message:

   additional comment:
   The original r31410 patch needs some additional changes for x87
    1. The frames-x87.h is under src/x87 instead of src/crankshaft/x87
    2. The header files should be in alphabetical order

R=weiliang.lin@intel.com
BUG=
==========

[Weiliang, 2015-10-22 03:16:26]

The CQ bit was checked by weiliang.lin@intel.com

[Weiliang, 2015-10-22 03:16:26]

lgtm

[commit-bot: I haz the power, 2015-10-22 03:16:35]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1417193002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1417193002/1

[commit-bot: I haz the power, 2015-10-22 03:19:13]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-10-22 03:19:13]

Try jobs failed on following builders:
  v8_presubmit on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_presubmit/builds/6912)

[Jakob Kummerow, 2015-10-22 09:03:30]

jkummerow@chromium.org changed reviewers:
+ jkummerow@chromium.org

[Jakob Kummerow, 2015-10-22 09:03:30]

Sorry for the frames-x87.h breakage; I tried to test the refactoring, but at the
time, the x87 port didn't compile even without my change.

OWNERS files for src/crankshaft/ are coming up in
https://codereview.chromium.org/1415863003/.

Rubberstamp LGTM for now.

https://codereview.chromium.org/1417193002/diff/1/src/crankshaft/x87/lithium-...
File src/crankshaft/x87/lithium-gap-resolver-x87.cc (right):

https://codereview.chromium.org/1417193002/diff/1/src/crankshaft/x87/lithium-...
src/crankshaft/x87/lithium-gap-resolver-x87.cc:7: #include
"src/crankshaft/x87/lithium-codegen-x87.h"
The previous ordering was intentional and conforms to the style guide:

https://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Names_and_Or...

[Weiliang, 2015-10-22 09:05:34]

On 2015/10/22 09:03:30, Jakob wrote:
> Sorry for the frames-x87.h breakage; I tried to test the refactoring, but at
the
> time, the x87 port didn't compile even without my change.
> 
> OWNERS files for src/crankshaft/ are coming up in
> https://codereview.chromium.org/1415863003/.
> 
> Rubberstamp LGTM for now.
> 
>
https://codereview.chromium.org/1417193002/diff/1/src/crankshaft/x87/lithium-...
> File src/crankshaft/x87/lithium-gap-resolver-x87.cc (right):
> 
>
https://codereview.chromium.org/1417193002/diff/1/src/crankshaft/x87/lithium-...
> src/crankshaft/x87/lithium-gap-resolver-x87.cc:7: #include
> "src/crankshaft/x87/lithium-codegen-x87.h"
> The previous ordering was intentional and conforms to the style guide:
> 
>
https://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Names_and_Or...

It does not matter. :) Actually we are waiting your CL laned.(1415863003)

Thanks
-Weiliang

[Weiliang, 2015-10-22 09:15:42]

https://codereview.chromium.org/1417193002/diff/1/src/crankshaft/x87/lithium-...
File src/crankshaft/x87/lithium-gap-resolver-x87.cc (right):

https://codereview.chromium.org/1417193002/diff/1/src/crankshaft/x87/lithium-...
src/crankshaft/x87/lithium-gap-resolver-x87.cc:7: #include
"src/crankshaft/x87/lithium-codegen-x87.h"
On 2015/10/22 09:03:30, Jakob wrote:
> The previous ordering was intentional and conforms to the style guide:
> 
>
https://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Names_and_Or...

Yes, thanks for pointing it out. @cy, please revert this change.

[chunyang.dai, 2015-10-22 09:45:56]

hello. Jakob.
If we revert this change of the include file order, the presubmit check 
will report "not in alphabetical order" error. 

How to handle it?

Thanks.

[Jakob Kummerow, 2015-10-22 09:56:24]

On 2015/10/22 09:45:56, chunyang.dai wrote:
> hello. Jakob.
> If we revert this change of the include file order, the presubmit check 
> will report "not in alphabetical order" error. 

Really? That is surprising. Pretty much every single .cc file in the codebase
follows the style guide and includes its own .h first, and the presubmit check
is perfectly happy with that. Is your cpplint.py up to date?

[zhengxing.li, 2015-10-22 10:20:38]

On 2015/10/22 09:56:24, Jakob wrote:
> On 2015/10/22 09:45:56, chunyang.dai wrote:
> > hello. Jakob.
> > If we revert this change of the include file order, the presubmit check 
> > will report "not in alphabetical order" error. 
> 
> Really? That is surprising. Pretty much every single .cc file in the codebase
> follows the style guide and includes its own .h first, and the presubmit check
> is perfectly happy with that. Is your cpplint.py up to date?

Hi, Jakob:

Yes, the cpplint.py is up to date. 

The issue is:
If We keep the blank line between the lithium-gap-resolver-x87.h and
lithium-codegen-x87.h, Your change in lithium-gap-resolver-x87.cc can pass the
presubmit.py check.

Otherwise, If we remove the blank line, the presubmit.py will report "not in
alphabetical order". Strange, right?

For our change, there's no such issue.

Thanks!
--zhengxing

[Jakob Kummerow, 2015-10-22 10:23:36]

Yes, the blank line should be kept. Just look at the style guide, or at any
other .cc file in V8 for examples.

[chunyang.dai, 2015-10-22 11:04:50]

Hi, Jakob.
thanks for your guidance.
I reverted the change.

thanks.

[chunyang.dai, 2015-10-22 11:05:35]

The CQ bit was checked by chunyang.dai@intel.com

[chunyang.dai, 2015-10-22 11:05:35]

The patchset sent to the CQ was uploaded after l-g-t-m from
weiliang.lin@intel.com, jkummerow@chromium.org
Link to the patchset: https://codereview.chromium.org/1417193002/#ps20001
(title: "Fix alphabetical order error.")

[commit-bot: I haz the power, 2015-10-22 11:05:43]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1417193002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1417193002/20001

[chunyang.dai, 2015-10-22 11:07:09]

Description was changed from

==========
X87: Move Hydrogen and Lithium to src/crankshaft/

port 81ee94b6507cf3ee9d44d18b8ccb1f1cc9045be1 (r31410).

   contributed by zhengxing.li@intel.com

   original commit message:

   additional comment:
   The original r31410 patch needs some additional changes for x87
    1. The frames-x87.h is under src/x87 instead of src/crankshaft/x87
    2. The header files should be in alphabetical order

R=weiliang.lin@intel.com
BUG=
==========

to

==========
X87: Move Hydrogen and Lithium to src/crankshaft/

port 81ee94b6507cf3ee9d44d18b8ccb1f1cc9045be1 (r31410).

   contributed by zhengxing.li@intel.com

   original commit message:

   additional comment:
   The original r31410 patch needs some additional changes for x87
    1. The frames-x87.h is under src/x87 instead of src/crankshaft/x87

R=weiliang.lin@intel.com
BUG=
==========

[commit-bot: I haz the power, 2015-10-22 11:28:31]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-10-22 11:28:46]

Patchset 2 (id:??) landed as
https://crrev.com/efbbf0fdcf6b0ae7a5937ef95fcb08eb18d80c6d
Cr-Commit-Position: refs/heads/master@{#31466}