make compile pass

[megazone87]

1. add some missing functions
2. add cmake file
3. add header guards
4. fix typos
5. format code as google style

[megazone87]

The missing functions are declared only, not implemented.

[danpovey]

Great work! Merging.

[danpovey]

Thanks a lot!

On Wed, Nov 27, 2019 at 1:51 PM Meixu Song notifications@github.com wrote:

@songmeixu commented on this pull request.

In lilcom/fixed_math.h https://github.com/danpovey/lilcom/pull/21#discussion_r351105570:

@@ -283,7 +274,7 @@ void CopyFromScalar64(const Scalar64 a, int i, Vector64 y);

void MulScalar64(const Scalar64 a, const Scalar64 b, Scalar64 *y);

/ does: y := a. Just copies all the elements of the struct. /

-extern inline void CopyScalar64(const Scalar64 a, Scalar64 y) {

+static inline void CopyScalar64(const Scalar64 a, Scalar64 y) {

Hi dan,

About making "inline" static, i just find it work, so i used it. After you asked, I google it and find it's a tricky problem. Ref: http://gudok.xyz/inline/

Reasons in brief:

1. Inline funtion is linked diffenently in C and C++, C: internal link, C++: external link. (In C, make definition of inline func be extern, will easily lead to the func duplicated defined)
2. In C, "-O0" don't inline the function actually, but "-O3" leads to real "inline" (except the func cannot be inlined). Thus, with "-O3", the inline func don't need to be "static".
3. But with fake inline("-O0"), the compiler ignores inline function body, thus that missing during linking.

Solutions for C:

1. add “static” to inline func
2. or, explicitly instantiate this function once, and make it extern. (It is a inferior solution.)

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/danpovey/lilcom/pull/21?email_source=notifications&email_token=AAZFLO46WSXH7JIRDBXNBXLQVYDHBA5CNFSM4JRYNQT2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCNDVH7A#discussion_r351105570, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAZFLO5WIOYWZ6FER7MEIHTQVYDHBANCNFSM4JRYNQTQ .

[danpovey]

After reading more about static/extern, it seems the only really correct way would be to declare it as 'inline' in the header, say foo.h, but use the preprocessor so that when included from exactly one translation unit (say, foo.c), it shows up as "extern inline".

On Wed, Nov 27, 2019 at 1:58 PM Daniel Povey dpovey@gmail.com wrote:

Thanks a lot!

On Wed, Nov 27, 2019 at 1:51 PM Meixu Song notifications@github.com wrote:

@songmeixu commented on this pull request.

In lilcom/fixed_math.h https://github.com/danpovey/lilcom/pull/21#discussion_r351105570:

@@ -283,7 +274,7 @@ void CopyFromScalar64(const Scalar64 a, int i, Vector64 y);

void MulScalar64(const Scalar64 a, const Scalar64 b, Scalar64 *y);

/ does: y := a. Just copies all the elements of the struct. /

-extern inline void CopyScalar64(const Scalar64 a, Scalar64 y) {

+static inline void CopyScalar64(const Scalar64 a, Scalar64 y) {

Hi dan,

About making "inline" static, i just find it work, so i used it. After you asked, I google it and find it's a tricky problem. Ref: http://gudok.xyz/inline/

Reasons in brief:

1. Inline funtion is linked diffenently in C and C++, C: internal link, C++: external link. (In C, make definition of inline func be extern, will easily lead to the func duplicated defined)
2. In C, "-O0" don't inline the function actually, but "-O3" leads to real "inline" (except the func cannot be inlined). Thus, with "-O3", the inline func don't need to be "static".
3. But with fake inline("-O0"), the compiler ignores inline function body, thus that missing during linking.

Solutions for C:

1. add “static” to inline func
2. or, explicitly instantiate this function once, and make it extern. (It is a inferior solution.)

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/danpovey/lilcom/pull/21?email_source=notifications&email_token=AAZFLO46WSXH7JIRDBXNBXLQVYDHBA5CNFSM4JRYNQT2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCNDVH7A#discussion_r351105570, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAZFLO5WIOYWZ6FER7MEIHTQVYDHBANCNFSM4JRYNQTQ .