kpu / mtplz

Code for the paper Faster Phrase-Based Decoding by Refining Feature State
http://kheafield.com/professional/
Other
14 stars 12 forks source link

Build fails on PPC: error: class 'util::FixedArray<T>' does not have any field named 'allocated_end_' and other errors #3

Closed barracuda156 closed 1 year ago

barracuda156 commented 1 year ago

On macOS PPC the build fails with:

In file included from /opt/local/var/macports/build/_opt_PPCRosettaPorts_textproc_mtplz/mtplz/work/mtplz-dace2e61ae402eb8fac8b8fb09aa22bec0dc768b/util/double-conversion/bignum-dtoa.h:31,
                 from /opt/local/var/macports/build/_opt_PPCRosettaPorts_textproc_mtplz/mtplz/work/mtplz-dace2e61ae402eb8fac8b8fb09aa22bec0dc768b/util/double-conversion/bignum-dtoa.cc:30:
/opt/local/var/macports/build/_opt_PPCRosettaPorts_textproc_mtplz/mtplz/work/mtplz-dace2e61ae402eb8fac8b8fb09aa22bec0dc768b/util/double-conversion/utils.h:71:2: error: #error Target architecture was not detected as supported by Double-Conversion.
   71 | #error Target architecture was not detected as supported by Double-Conversion.
      |  ^~~~~
make[2]: *** [util/CMakeFiles/kenlm_util.dir/double-conversion/bignum-dtoa.cc.o] Error 1
make[2]: Leaving directory `/opt/local/var/macports/build/_opt_PPCRosettaPorts_textproc_mtplz/mtplz/work/build'
make[1]: *** [util/CMakeFiles/kenlm_util.dir/all] Error 2
barracuda156 commented 1 year ago

Okay, that one was due to a wrong define in macro, but further down the road there are more serious problems:

In file included from /opt/local/var/macports/build/_opt_PPCRosettaPorts_textproc_mtplz/mtplz/work/mtplz-dace2e61ae402eb8fac8b8fb09aa22bec0dc768b/lm/interpolate/bounded_sequence_encoding.cc:1:
/opt/local/var/macports/build/_opt_PPCRosettaPorts_textproc_mtplz/mtplz/work/mtplz-dace2e61ae402eb8fac8b8fb09aa22bec0dc768b/lm/interpolate/bounded_sequence_encoding.hh:14:2: warning: #warning The interpolation code assumes little endian for now. [-Wcpp]
   14 | #warning The interpolation code assumes little endian for now.
      |  ^~~~~~~
In file included from /opt/local/var/macports/build/_opt_PPCRosettaPorts_textproc_mtplz/mtplz/work/mtplz-dace2e61ae402eb8fac8b8fb09aa22bec0dc768b/lm/interpolate/merge_probabilities.hh:5,
                 from /opt/local/var/macports/build/_opt_PPCRosettaPorts_textproc_mtplz/mtplz/work/mtplz-dace2e61ae402eb8fac8b8fb09aa22bec0dc768b/lm/interpolate/merge_probabilities.cc:1:
/opt/local/var/macports/build/_opt_PPCRosettaPorts_textproc_mtplz/mtplz/work/mtplz-dace2e61ae402eb8fac8b8fb09aa22bec0dc768b/lm/interpolate/bounded_sequence_encoding.hh:14:2: warning: #warning The interpolation code assumes little endian for now. [-Wcpp]
   14 | #warning The interpolation code assumes little endian for now.
      |  ^~~~~~~
In file included from /opt/local/var/macports/build/_opt_PPCRosettaPorts_textproc_mtplz/mtplz/work/mtplz-dace2e61ae402eb8fac8b8fb09aa22bec0dc768b/lm/interpolate/bounded_sequence_encoding.hh:11:
/opt/local/var/macports/build/_opt_PPCRosettaPorts_textproc_mtplz/mtplz/work/mtplz-dace2e61ae402eb8fac8b8fb09aa22bec0dc768b/util/fixed_array.hh: In constructor 'util::FixedArray<T>::FixedArray(util::FixedArray<T>&&)':
/opt/local/var/macports/build/_opt_PPCRosettaPorts_textproc_mtplz/mtplz/work/mtplz-dace2e61ae402eb8fac8b8fb09aa22bec0dc768b/util/fixed_array.hh:79:9: error: class 'util::FixedArray<T>' does not have any field named 'allocated_end_'
   79 |         allocated_end_(from.allocated_end_)
      |         ^~~~~~~~~~~~~~
kpu commented 1 year ago

Woah code I haven't touched in 6 years. Does it work now? Did I break it?

barracuda156 commented 1 year ago

Woah code I haven't touched in 6 years. Does it work now? Did I break it?

@kpu I do not know yet, haha. If it builds at least, I will update you. The first error is trivially fixed, but I am not sure about big endian support and allocated_end_.

kpu commented 1 year ago

Are you running the bin/interpolate program to log-linearly interpolate n-gram language models? I suspect not. If not, ignore the warnings about the interpolation code.

barracuda156 commented 1 year ago

allocated_end_ is inside a macro #ifdef NDEBUG. So apparently it is invoked without the macro being defined.

Hmm, looks like NDEBUG is nowhere defined. I can add a cppflag and see what happens.

kpu commented 1 year ago

Is allocated_end_ working as of b80a72dcca0ce378814a9d44130ab65f889cbecf? I updated that.

kpu commented 1 year ago

NDEBUG is defined by cmake when compiling for release.

barracuda156 commented 1 year ago

Is allocated_end_ working as of b80a72d? I updated that.

Let me try that.

P. S. Thank you for fixing defines for PPC! We could also use defined(__POWERPC__) instead of defined(__ppc__) || defined(__ppc64__), since the former includes both. However, unless we care about BeOS support, these are equivalent.

barracuda156 commented 1 year ago

@kpu If fails further now:

/opt/local/var/macports/build/_opt_PPCRosettaPorts_textproc_mtplz/mtplz/work/mtplz-b80a72dcca0ce378814a9d44130ab65f889cbecf/util/usage.cc:203:31: error: aggregate 'util::PrintUsage(std::ostream&)::mach_task_basic_info t_info' has incomplete type and cannot be defined
  203 |   struct mach_task_basic_info t_info;
      |                               ^~~~~~
/opt/local/var/macports/build/_opt_PPCRosettaPorts_textproc_mtplz/mtplz/work/mtplz-b80a72dcca0ce378814a9d44130ab65f889cbecf/util/usage.cc:207:41: error: 'MACH_TASK_BASIC_INFO_COUNT' was not declared in this scope; did you mean 'TASK_BASIC_INFO_COUNT'?
  207 |   mach_msg_type_number_t t_info_count = MACH_TASK_BASIC_INFO_COUNT;
      |                                         ^~~~~~~~~~~~~~~~~~~~~~~~~~
      |                                         TASK_BASIC_INFO_COUNT
/opt/local/var/macports/build/_opt_PPCRosettaPorts_textproc_mtplz/mtplz/work/mtplz-b80a72dcca0ce378814a9d44130ab65f889cbecf/util/usage.cc:208:31: error: 'MACH_TASK_BASIC_INFO' was not declared in this scope; did you mean 'TASK_BASIC_INFO'?
  208 |   task_info(mach_task_self(), MACH_TASK_BASIC_INFO, (task_info_t)&t_info, &t_info_count);
      |                               ^~~~~~~~~~~~~~~~~~~~
      |                               TASK_BASIC_INFO
make[2]: *** [util/CMakeFiles/kenlm_util.dir/usage.cc.o] Error 1

Gimme a min, I think I have a fix for this in another port.

barracuda156 commented 1 year ago

@kpu Will this work? I did that for folly earlier: https://github.com/macports/macports-ports/blob/2e67a9d4cba2eb408d036dcf0facb712f195b5e2/devel/folly/files/patch-older-systems.diff#L18

kpu commented 1 year ago

Feel free to make a PR for the MACH_TASK_BASIC_INFO stuff; I have no way to test it.

Regarding "We could also use defined(POWERPC) instead of defined(ppc) || defined(ppc64), since the former includes both.", the relevant code is copied from https://github.com/google/double-conversion

barracuda156 commented 1 year ago

Fixed with https://github.com/kpu/mtplz/commit/5eb053b41856dfd4bbbf1b58f03372107aff1a95 Thank you!