search

weltyc / ntest

NTest othello program
27 stars 12 forks source link

Ntest - build with both VS 2013 and gcc 4.7 #4

Closed vladpetric closed 10 years ago

vladpetric commented 10 years ago

Hi Chris,

This is the change - pretty massive but it's really mostly just minor stuff spread across the codebase to allow the code to build cleanly on both platforms. There are ifdefs on WIN32 here and there, but with one exception they're minor. In the case of GameX.cpp, I implemented the synchronization via boost. That implementation is ifdef-ed out for Windows though (even though it'd probably work). Generally, installing boost on linux is fairly straightforward, as distro repositories have it.

Happy to clarify any issues.

Vlad

weltyc commented 10 years ago

This looks great!

Here are my first questions. Some of them are probably very simple because I'm not familiar with newer C++ compilers.

Should we make the resources directory mimic the directory structure that the object file will expect? i.e. move resources/solver*.txt to resources/resources/solver.txt, so all the builder needs to do is copy the object file to a copy of resources/

Pos2.cpp - PassBB why remove inline? Is compiler now allowed to inline without inline declaration?

BitBoard.h - #pragma once is handled by GCC >= 3.4, and other code requires GCC >=4, so why the include guard?

Book.h - uses more RAM if not using fields? This is problem because book-building machines are near their RAM limit.

utils.cpp GetBaseDir - is there no equivalent of GetModuleFilename on Linux? This will require users to cd to the directory before running.

OsObjects.cpp - Why do we need COsMove::In(istringstream&) when we already have COsMove::In(istream&)?

pattern/types.h - does changing TConfig from u4 to uint_16 hurt performance? It did last time I tested, but that was on an old 32-bit machine.

n64/utils.cpp - don't wrap scanf in assert, else won't be able to read if NDEBUG.

odk/types.h - should u4 be defined as uint32_t?

Do you have a windows exe that you could email me? I would like to test the object code but ideally avoid setting up Visual Studio.

Thanks

Chris

On Sun, Jan 12, 2014 at 9:42 PM, vladpetric notifications@github.comwrote:

Hi Chris,

This is the change - pretty massive but it's really mostly just minor stuff spread across the codebase to allow the code to build cleanly on both platforms. There are ifdefs on WIN32 here and there, but with one exception they're minor. In the case of GameX.cpp, I implemented the synchronization via boost. That implementation is ifdef-ed out for Windows though (even though it'd probably work). Generally, installing boost on linux is fairly straightforward, as distro repositories have it.

Happy to clarify any issues.

Vlad

You can merge this Pull Request by running

git pull https://github.com/vladpetric/ntest master

Or view, comment on, or merge it at:

https://github.com/weltyc/ntest/pull/4 Commit Summary

  • Massive change to allow ntest to be built with both VS 2013 and gcc 4.7
  • Remove crap file

File Changes

  • A BuildREADME.txthttps://github.com/weltyc/ntest/pull/4/files#diff-0(128)
  • A src/CMakeLists.txthttps://github.com/weltyc/ntest/pull/4/files#diff-1(34)
  • M src/Evaluator.cpphttps://github.com/weltyc/ntest/pull/4/files#diff-2(8)
  • M src/Evaluator.hhttps://github.com/weltyc/ntest/pull/4/files#diff-3(1)
  • M src/GameX.cpphttps://github.com/weltyc/ntest/pull/4/files#diff-4(80)
  • M src/MPCCalc.cpphttps://github.com/weltyc/ntest/pull/4/files#diff-5(2)
  • M src/PlayerComputer.cpphttps://github.com/weltyc/ntest/pull/4/files#diff-6(2)
  • M src/Pos2.cpphttps://github.com/weltyc/ntest/pull/4/files#diff-7(12)
  • M src/Pos2.h https://github.com/weltyc/ntest/pull/4/files#diff-8(18)
  • M src/Search.cpphttps://github.com/weltyc/ntest/pull/4/files#diff-9(11)
  • M src/SmartBook.cpphttps://github.com/weltyc/ntest/pull/4/files#diff-10(2)
  • M src/SpeedTest.cpphttps://github.com/weltyc/ntest/pull/4/files#diff-11(4)
  • M src/core/BitBoard.cpphttps://github.com/weltyc/ntest/pull/4/files#diff-12(2)
  • M src/core/BitBoard.hhttps://github.com/weltyc/ntest/pull/4/files#diff-13(13)
  • M src/core/BitBoardTest.cpphttps://github.com/weltyc/ntest/pull/4/files#diff-14(18)
  • M src/core/Book.cpphttps://github.com/weltyc/ntest/pull/4/files#diff-15(60)
  • M src/core/Book.hhttps://github.com/weltyc/ntest/pull/4/files#diff-16(12)
  • M src/core/BookTest.cpphttps://github.com/weltyc/ntest/pull/4/files#diff-17(2)
  • A src/core/CMakeLists.txthttps://github.com/weltyc/ntest/pull/4/files#diff-18(1)
  • M src/core/Cache.cpphttps://github.com/weltyc/ntest/pull/4/files#diff-19(17)
  • M src/core/CalcParams.cpphttps://github.com/weltyc/ntest/pull/4/files#diff-20(4)
  • M src/core/CalcParams.hhttps://github.com/weltyc/ntest/pull/4/files#diff-21(4)
  • M src/core/Debug.hhttps://github.com/weltyc/ntest/pull/4/files#diff-22(3)
  • M src/core/HeightInfo.cpphttps://github.com/weltyc/ntest/pull/4/files#diff-23(1)
  • M src/core/MPCStats.cpphttps://github.com/weltyc/ntest/pull/4/files#diff-24(18)
  • M src/core/MVK.cpphttps://github.com/weltyc/ntest/pull/4/files#diff-25(4)
  • M src/core/Moves.cpphttps://github.com/weltyc/ntest/pull/4/files#diff-26(16)
  • M src/core/QPosition.cpphttps://github.com/weltyc/ntest/pull/4/files#diff-27(2)
  • M src/core/QPosition.hhttps://github.com/weltyc/ntest/pull/4/files#diff-28(5)
  • M src/core/QPositionTest.cpphttps://github.com/weltyc/ntest/pull/4/files#diff-29(2)
  • M src/core/Store.cpphttps://github.com/weltyc/ntest/pull/4/files#diff-30(27)
  • M src/core/Store.hhttps://github.com/weltyc/ntest/pull/4/files#diff-31(11)
  • A src/game/CMakeLists.txthttps://github.com/weltyc/ntest/pull/4/files#diff-32(1)
  • M src/game/Game.cpphttps://github.com/weltyc/ntest/pull/4/files#diff-33(4)
  • M src/game/Player.hhttps://github.com/weltyc/ntest/pull/4/files#diff-34(3)
  • M src/game/PlayerHuman.cpphttps://github.com/weltyc/ntest/pull/4/files#diff-35(15)
  • M src/main.cpphttps://github.com/weltyc/ntest/pull/4/files#diff-36(21)
  • A src/n64/CMakeLists.txthttps://github.com/weltyc/ntest/pull/4/files#diff-37(1)
  • M src/n64/hash.cpphttps://github.com/weltyc/ntest/pull/4/files#diff-38(9)
  • M src/n64/hash.hhttps://github.com/weltyc/ntest/pull/4/files#diff-39(3)
  • M src/n64/hashTest.cpphttps://github.com/weltyc/ntest/pull/4/files#diff-40(4)
  • M src/n64/magic.cpphttps://github.com/weltyc/ntest/pull/4/files#diff-41(6)
  • M src/n64/n64.cpphttps://github.com/weltyc/ntest/pull/4/files#diff-42(1)
  • M src/n64/qssert.hhttps://github.com/weltyc/ntest/pull/4/files#diff-43(8)
  • M src/n64/solve.cpphttps://github.com/weltyc/ntest/pull/4/files#diff-44(20)
  • M src/n64/solveTest.cpphttps://github.com/weltyc/ntest/pull/4/files#diff-45(12)
  • M src/n64/stdafx.hhttps://github.com/weltyc/ntest/pull/4/files#diff-46(8)
  • M src/n64/test.cpphttps://github.com/weltyc/ntest/pull/4/files#diff-47(4)
  • M src/n64/types.hhttps://github.com/weltyc/ntest/pull/4/files#diff-48(24)
  • M src/n64/utils.cpphttps://github.com/weltyc/ntest/pull/4/files#diff-49(32)
  • M src/n64/utils.hhttps://github.com/weltyc/ntest/pull/4/files#diff-50(27)
  • A src/odk/CMakeLists.txthttps://github.com/weltyc/ntest/pull/4/files#diff-51(1)
  • M src/odk/GGSMessage.hhttps://github.com/weltyc/ntest/pull/4/files#diff-52(4)
  • M src/odk/OsMessage.cpphttps://github.com/weltyc/ntest/pull/4/files#diff-53(3)
  • M src/odk/OsObjects.cpphttps://github.com/weltyc/ntest/pull/4/files#diff-54(23)
  • M src/odk/OsObjects.hhttps://github.com/weltyc/ntest/pull/4/files#diff-55(3)
  • M src/odk/ggsstream.cpphttps://github.com/weltyc/ntest/pull/4/files#diff-56(7)
  • M src/odk/sockbuf.cpphttps://github.com/weltyc/ntest/pull/4/files#diff-57(33)
  • M src/odk/sockbuf.hhttps://github.com/weltyc/ntest/pull/4/files#diff-58(5)
  • M src/odk/types.hhttps://github.com/weltyc/ntest/pull/4/files#diff-59(7)
  • M src/options.cpphttps://github.com/weltyc/ntest/pull/4/files#diff-60(5)
  • A src/pattern/CMakeLists.txthttps://github.com/weltyc/ntest/pull/4/files#diff-61(1)
  • M src/pattern/FastFlip.cpphttps://github.com/weltyc/ntest/pull/4/files#diff-62(2)
  • M src/pattern/Pattern.cpphttps://github.com/weltyc/ntest/pull/4/files#diff-63(2)
  • M src/pattern/Pattern.hhttps://github.com/weltyc/ntest/pull/4/files#diff-64(2)
  • M src/pattern/Patterns.cpphttps://github.com/weltyc/ntest/pull/4/files#diff-65(4)
  • M src/pattern/RowFlip.hhttps://github.com/weltyc/ntest/pull/4/files#diff-66(6)
  • M src/pattern/flipFunc64.cpphttps://github.com/weltyc/ntest/pull/4/files#diff-67(5)
  • M src/pattern/patternJ.hhttps://github.com/weltyc/ntest/pull/4/files#diff-68(2)
  • M src/pattern/types.hhttps://github.com/weltyc/ntest/pull/4/files#diff-69(3)

Patch Links:

— Reply to this email directly or view it on GitHubhttps://github.com/weltyc/ntest/pull/4 .

vladpetric commented 10 years ago

All these are very valid comments :). I will address every single one of them. And certainly, I can send you a binary too.

More of a technical question - do you know how to update an existing pull request?

(btw for some reason I can respond inline ... ugh google e-mail app for iOs)

a) moving the resources Yeah that's probably a good idea, I can handle it.

b) Pos2.cpp PassBB inline - the problem with having the inline in the cpp file is that the compiler. I can move the function

c) pragma - yeah, ifdef windows should be enough.

d) bit fields - I had no idea, will revert. Usually it takes a lot more cpu to access bitfields.

e) GetBaseDir - there is, but I thought that that function is never really used so I didn't feel like porting it :). No worries, I'll do it.

f) OsObjects.cpp - for some reason it won't cast ... Maybe I can template the function so that there's a single body.

g) u4 to uint_16 - actually, it might slow things down on Windows! I'll give it a shot. On linux/gcc it does not ...

h) n64/utils.cpp - good catch, thanks!

i) odk/types.h - that's probably better yes.

Should we make the resources directory mimic the directory structure that the object file will expect? i.e. move resources/solver*.txt to resources/resources/solver.txt, so all the builder needs to do is copy the object file to a copy of resources/

Pos2.cpp - PassBB why remove inline? Is compiler now allowed to inline without inline declaration?

BitBoard.h - #pragma once is handled by GCC >= 3.4, and other code requires GCC >=4, so why the include guard?

Book.h - uses more RAM if not using fields? This is problem because book-building machines are near their RAM limit.

utils.cpp GetBaseDir - is there no equivalent of GetModuleFilename on Linux? This will require users to cd to the directory before running.

OsObjects.cpp - Why do we need COsMove::In(istringstream&) when we already have COsMove::In(istream&)?

pattern/types.h - does changing TConfig from u4 to uint_16 hurt performance? It did last time I tested, but that was on an old 32-bit machine.

n64/utils.cpp - don't wrap scanf in assert, else won't be able to read if NDEBUG.

odk/types.h - should u4 be defined as uint32_t?

Do you have a windows exe that you could email me? I would like to test the object code but ideally avoid setting up Visual Studio.

Thanks

Chris

On Sun, Jan 12, 2014 at 9:42 PM, vladpetric <notifications@github.com<javascript:_e({}, 'cvml', 'notifications@github.com');>>wrote:

Hi Chris,

This is the change - pretty massive but it's really mostly just minor stuff spread across the codebase to allow the code to build cleanly on both platforms. There are ifdefs on WIN32 here and there, but with one exception they're minor. In the case of GameX.cpp, I implemented the synchronization via boost. That implementation is ifdef-ed out for Windows though (even though it'd probably work). Generally, installing boost on linux is fairly straightforward, as distro repositories have it.

Happy to clarify any issues.

Vlad

You can merge this Pull Request by running

git pull https://github.com/vladpetric/ntest master

Or view, comment on, or merge it at:

https://github.com/weltyc/ntest/pull/4 Commit Summary

  • Massive change to allow ntest to be built with both VS 2013 and gcc 4.7
  • Remove crap file

File Changes

Patch Links:

— Reply to this email directly or view it on GitHub< https://github.com/weltyc/ntest/pull/4> .

— Reply to this email directly or view it on GitHubhttps://github.com/weltyc/ntest/pull/4#issuecomment-32194653 .

weltyc commented 10 years ago

Thanks! Unfortunately I'm brand new to GitHub. Is it easier to just send a new pull request?

On Mon, Jan 13, 2014 at 12:21 PM, vladpetric notifications@github.comwrote:

All these are very valid comments :). I will address every single one of them. And certainly, I can send you a binary too.

More of a technical question - do you know how to update an existing pull request?

(btw for some reason I can respond inline ... ugh google e-mail app for iOs)

a) moving the resources Yeah that's probably a good idea, I can handle it.

b) Pos2.cpp PassBB inline - the problem with having the inline in the cpp file is that the compiler. I can move the function

c) pragma - yeah, ifdef windows should be enough.

d) bit fields - I had no idea, will revert. Usually it takes a lot more cpu to access bitfields.

e) GetBaseDir - there is, but I thought that that function is never really used so I didn't feel like porting it :). No worries, I'll do it.

f) OsObjects.cpp - for some reason it won't cast ... Maybe I can template the function so that there's a single body.

g) u4 to uint_16 - actually, it might slow things down on Windows! I'll give it a shot. On linux/gcc it does not ...

h) n64/utils.cpp - good catch, thanks!

i) odk/types.h - that's probably better yes.

Should we make the resources directory mimic the directory structure that the object file will expect? i.e. move resources/solver*.txt to resources/resources/solver.txt, so all the builder needs to do is copy the object file to a copy of resources/

Pos2.cpp - PassBB why remove inline? Is compiler now allowed to inline without inline declaration?

BitBoard.h - #pragma once is handled by GCC >= 3.4, and other code requires GCC >=4, so why the include guard?

Book.h - uses more RAM if not using fields? This is problem because book-building machines are near their RAM limit.

utils.cpp GetBaseDir - is there no equivalent of GetModuleFilename on Linux? This will require users to cd to the directory before running.

OsObjects.cpp - Why do we need COsMove::In(istringstream&) when we already have COsMove::In(istream&)?

pattern/types.h - does changing TConfig from u4 to uint_16 hurt performance? It did last time I tested, but that was on an old 32-bit machine.

n64/utils.cpp - don't wrap scanf in assert, else won't be able to read if NDEBUG.

odk/types.h - should u4 be defined as uint32_t?

Do you have a windows exe that you could email me? I would like to test the object code but ideally avoid setting up Visual Studio.

Thanks

Chris

On Sun, Jan 12, 2014 at 9:42 PM, vladpetric <notifications@github.com<javascript:_e({}, 'cvml', 'notifications@github.com');>>wrote:

Hi Chris,

This is the change - pretty massive but it's really mostly just minor stuff spread across the codebase to allow the code to build cleanly on both platforms. There are ifdefs on WIN32 here and there, but with one exception they're minor. In the case of GameX.cpp, I implemented the synchronization via boost. That implementation is ifdef-ed out for Windows though (even though it'd probably work). Generally, installing boost on linux is fairly straightforward, as distro repositories have it.

Happy to clarify any issues.

Vlad

You can merge this Pull Request by running

git pull https://github.com/vladpetric/ntest master

Or view, comment on, or merge it at:

https://github.com/weltyc/ntest/pull/4 Commit Summary

  • Massive change to allow ntest to be built with both VS 2013 and gcc 4.7
  • Remove crap file

File Changes

Patch Links:

— Reply to this email directly or view it on GitHub< https://github.com/weltyc/ntest/pull/4> .

— Reply to this email directly or view it on GitHub< https://github.com/weltyc/ntest/pull/4#issuecomment-32194653> .

— Reply to this email directly or view it on GitHubhttps://github.com/weltyc/ntest/pull/4#issuecomment-32195624 .

vladpetric commented 10 years ago

Yeah, I'll figure out something this evening :).

BTw, for b) the problem is that an inline function in a cpp file may not end up in the object file at all. So the GNU linker generates an error. An inline function in a header file will be seen by all c(pp) files that need it. Best to move the small function to the header file.

I oftentimes write text non-linearly.

Vlad

On Monday, January 13, 2014, weltyc wrote:

Thanks! Unfortunately I'm brand new to GitHub. Is it easier to just send a new pull request?

On Mon, Jan 13, 2014 at 12:21 PM, vladpetric <notifications@github.com<javascript:_e({}, 'cvml', 'notifications@github.com');>>wrote:

All these are very valid comments :). I will address every single one of them. And certainly, I can send you a binary too.

More of a technical question - do you know how to update an existing pull request?

(btw for some reason I can respond inline ... ugh google e-mail app for iOs)

a) moving the resources Yeah that's probably a good idea, I can handle it.

b) Pos2.cpp PassBB inline - the problem with having the inline in the cpp file is that the compiler. I can move the function

c) pragma - yeah, ifdef windows should be enough.

d) bit fields - I had no idea, will revert. Usually it takes a lot more cpu to access bitfields.

e) GetBaseDir - there is, but I thought that that function is never really used so I didn't feel like porting it :). No worries, I'll do it.

f) OsObjects.cpp - for some reason it won't cast ... Maybe I can template the function so that there's a single body.

g) u4 to uint_16 - actually, it might slow things down on Windows! I'll give it a shot. On linux/gcc it does not ...

h) n64/utils.cpp - good catch, thanks!

i) odk/types.h - that's probably better yes.

Should we make the resources directory mimic the directory structure that the object file will expect? i.e. move resources/solver*.txt to resources/resources/solver.txt, so all the builder needs to do is copy the object file to a copy of resources/

Pos2.cpp - PassBB why remove inline? Is compiler now allowed to inline without inline declaration?

BitBoard.h - #pragma once is handled by GCC >= 3.4, and other code requires GCC >=4, so why the include guard?

Book.h - uses more RAM if not using fields? This is problem because book-building machines are near their RAM limit.

utils.cpp GetBaseDir - is there no equivalent of GetModuleFilename on Linux? This will require users to cd to the directory before running.

OsObjects.cpp - Why do we need COsMove::In(istringstream&) when we already have COsMove::In(istream&)?

pattern/types.h - does changing TConfig from u4 to uint_16 hurt performance? It did last time I tested, but that was on an old 32-bit machine.

n64/utils.cpp - don't wrap scanf in assert, else won't be able to read if NDEBUG.

odk/types.h - should u4 be defined as uint32_t?

Do you have a windows exe that you could email me? I would like to test the object code but ideally avoid setting up Visual Studio.

Thanks

Chris

On Sun, Jan 12, 2014 at 9:42 PM, vladpetric <notifications@github.com<javascript:_e({}, 'cvml', 'notifications@github.com');><javascript:_e({},

'cvml', 'notifications@github.com <javascript:_e({}, 'cvml', 'notifications@github.com');>');>>wrote:

Hi Chris,

This is the change - pretty massive but it's really mostly just minor stuff spread across the codebase to allow the code to build cleanly on both platforms. There are ifdefs on WIN32 here and there, but with one exception they're minor. In the case of GameX.cpp, I implemented the synchronization via boost. That implementation is ifdef-ed out for Windows though (even though it'd probably work). Generally, installing boost on linux is fairly straightforward, as distro repositories have it.

Happy to clarify any issues.

Vlad

You can merge this Pull Request by running

git pull https://github.com/vladpetric/ntest master

Or view, comment on, or merge it at:

https://github.com/weltyc/ntest/pull/4 Commit Summary

  • Massive change to allow ntest to be built with both VS 2013 and gcc 4.7
  • Remove crap file

File Changes

— Reply to this email directly or view it on GitHub< https://github.com/weltyc/ntest/pull/4#issuecomment-32195624> .

— Reply to this email directly or view it on GitHubhttps://github.com/weltyc/ntest/pull/4#issuecomment-32197215 .

vladpetric commented 10 years ago

So I did everything except for two things (TTBOMK): a) resource re-shuffling b) GetModuleFilename (see comment).

I'll send you the .exe as well.

Pos2.cpp - PassBB why remove inline? Is compiler now allowed to inline without inline declaration?

BitBoard.h - #pragma once is handled by GCC >= 3.4, and other code requires GCC >=4, so why the include guard?

Book.h - uses more RAM if not using fields? This is problem because book-building machines are near their RAM limit.

utils.cpp GetBaseDir - is there no equivalent of GetModuleFilename on Linux? This will require users to cd to the directory before running.

OsObjects.cpp - Why do we need COsMove::In(istringstream&) when we already have COsMove::In(istream&)?

pattern/types.h - does changing TConfig from u4 to uint_16 hurt performance? It did last time I tested, but that was on an old 32-bit machine.

n64/utils.cpp - don't wrap scanf in assert, else won't be able to read if NDEBUG.

Other stuff, not requested in the review:

weltyc commented 10 years ago

I didn't notice that about GetModuleFilename - you're right.

It passes visual code review. Because I'm paranoid I still want to run some tests on a compiled version though (to make sure it behaves exactly as the old version).