Fix compile errors #3318

TobyRoseman · 2020-09-11T23:14:34Z

No description provided.

TobyRoseman · 2020-09-14T17:23:52Z

Internal test pass.

@hoytak - do you have any concerns merging these changes?

src/core/storage/fileio/fs_utils.cpp

src/core/storage/fileio/general_fstream.cpp

hoytak · 2020-09-14T18:07:34Z

src/core/storage/serialization/iarchive.hpp

@@ -117,7 +117,7 @@ namespace turi {
    template <typename T>
    inline void read_into(T& c) {
      if (buf) {
-        memcpy(&c, buf + off, sizeof(T));
+        memcpy(reinterpret_cast<void*>(&c), buf + off, sizeof(T));


This should not be necessary. A cast to void* is automatic for any trivial types (i.e. no inheritance, no pointers, etc., and this memcpy is valid only for those types. The so the lack of an explicit cast here is a guard against that (though it could be made more explicit with a

static_assert(std::is_trivial::value, "Only trivial types allowed for hash.");

Here is the error message I get without the reinterpret_cast:

[ 14%] Building CXX object src/model_server/CMakeFiles/unity.dir/server/registration.cpp.o In file included from /home/toby/Documents/turicreate/src/core/storage/serialization/serialize.hpp:8, from /home/toby/Documents/turicreate/src/core/storage/serialization/serialization_includes.hpp:6, from /home/toby/Documents/turicreate/src/core/parallel/atomic.hpp:12, from /home/toby/Documents/turicreate/src/core/data/flexible_type/flexible_type.hpp:15, from /home/toby/Documents/turicreate/src/model_server/lib/toolkit_class_specification.hpp:10, from /home/toby/Documents/turicreate/src/model_server/lib/toolkit_class_registry.hpp:8, from /home/toby/Documents/turicreate/src/model_server/server/registration.hpp:9, from /home/toby/Documents/turicreate/src/model_server/server/registration.cpp:6: /home/toby/Documents/turicreate/src/core/storage/serialization/iarchive.hpp: In instantiation of ‘void turi::iarchive::read_into(T&) [with T = boost::multiprecision::number<boost::multiprecision::backends::cpp_int_backend<128, 128, boost::multiprecision::unsigned_magnitude, boost::multiprecision::unchecked, void> >]’: /home/toby/Documents/turicreate/src/core/storage/serialization/iarchive.hpp:243:9: required from ‘static void turi::archive_detail::deserialize_impl<InArcType, T, true>::exec(InArcType&, T&) [with InArcType = turi::iarchive; T = boost::multiprecision::number<boost::multiprecision::backends::cpp_int_backend<128, 128, boost::multiprecision::unsigned_magnitude, boost::multiprecision::unchecked, void> >]’ /home/toby/Documents/turicreate/src/core/storage/serialization/iarchive.hpp:258:65: required from ‘turi::iarchive& turi::operator>>(turi::iarchive&, T&) [with T = boost::multiprecision::number<boost::multiprecision::backends::cpp_int_backend<128, 128, boost::multiprecision::unsigned_magnitude, boost::multiprecision::unchecked, void> >]’ /home/toby/Documents/turicreate/src/core/util/hash_value.hpp:65:39: required from here /home/toby/Documents/turicreate/src/core/storage/serialization/iarchive.hpp:121:8: error: ‘void* memcpy(void*, const void*, size_t)’ writing to an object of type ‘class boost::multiprecision::number<boost::multiprecision::backends::cpp_int_backend<128, 128, boost::multiprecision::unsigned_magnitude, boost::multiprecision::unchecked, void> >’ with no trivial copy-assignment; use copy-assignment or copy-initialization instead [-Werror=class-memaccess] 121 | memcpy(&c, buf + off, sizeof(T)); | ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~ In file included from /home/toby/Documents/turicreate/src/external/boost/boost_1_68_0/boost/multiprecision/cpp_int.hpp:12, from /home/toby/Documents/turicreate/src/core/util/int128_types.hpp:44, from /home/toby/Documents/turicreate/src/core/data/flexible_type/flexible_type.hpp:16, from /home/toby/Documents/turicreate/src/model_server/lib/toolkit_class_specification.hpp:10, from /home/toby/Documents/turicreate/src/model_server/lib/toolkit_class_registry.hpp:8, from /home/toby/Documents/turicreate/src/model_server/server/registration.hpp:9, from /home/toby/Documents/turicreate/src/model_server/server/registration.cpp:6: /home/toby/Documents/turicreate/src/external/boost/boost_1_68_0/boost/multiprecision/number.hpp:41:7: note: ‘class boost::multiprecision::number<boost::multiprecision::backends::cpp_int_backend<128, 128, boost::multiprecision::unsigned_magnitude, boost::multiprecision::unchecked, void> >’ declared here 41 | class number | ^~~~~~ cc1plus: error: unrecognized command line option ‘-Wno-unknown-warning-option’ [-Werror] cc1plus: error: unrecognized command line option ‘-Wno-parentheses-equality’ [-Werror] cc1plus: error: unrecognized command line option ‘-Wno-constant-logical-operand’ [-Werror] cc1plus: error: unrecognized command line option ‘-Wno-mismatched-tags’ [-Werror] cc1plus: error: unrecognized command line option ‘-Wno-deprecated-register’ [-Werror] cc1plus: error: unrecognized command line option ‘-Wno-unused-command-line-argument’ [-Werror] cc1plus: all warnings being treated as errors make[2]: *** [src/model_server/CMakeFiles/unity.dir/build.make:63: src/model_server/CMakeFiles/unity.dir/server/registration.cpp.o] Error 1 make[1]: *** [CMakeFiles/Makefile2:3952: src/model_server/CMakeFiles/unity.dir/all] Error 2 make: *** [Makefile:152: all] Error 2

The following program prints out non-trivial:

#include <iostream> #include <boost/multiprecision/number.hpp> #include <boost/multiprecision/cpp_int.hpp> int main() { const bool is_trivial = std::is_trivial< boost::multiprecision::number<boost::multiprecision::backends::cpp_int_backend<128, 128, boost::multiprecision::unsigned_magnitude, boost::multiprecision::unchecked, void>> >::value; if(is_trivial) std::cout<<"trivial"<<std::endl; else std::cout<<"non-trivial"<<std::endl; return 0; }

Yes, exactly, as it should. Only trivial types can be used in that serialization function. Boost multiprecision is the fallback type when no native 128 bit integer types are detected at build time (maybe it should error instead?). These are detected by the Find128BitInteger.cmake routines under cmake/. It appears these routines are failing. That is the root cause of this problem.

What's the output of cmake config log after a clean cmake run?

Here is the relevant section of the logs:

Testing for 128 bit integer support with __int128_t. Found: Enabling support for 128 bit integers using __int128_t. Testing for presence of 128 bit unsigned integer hash function for __int128_t. std::hash<__int128_t> defined. Testing for 128 bit unsigned integer support with __uint128_t.

Output for debug and release is the same.

So for both signed and unsigned, it seems to be finding the first things it's looking for.

Sorry to drop the ball on this -- let's dig into it tomorrow. There's some typo somewhere.

src/core/storage/sframe_data/sframe_index_file.cpp

src/core/system/lambda/lambda_master.cpp

hoytak

See comments.

hoytak · 2020-09-15T13:51:07Z

src/core/storage/sgraph_data/sgraph_triple_apply.cpp

@@ -854,7 +854,7 @@ namespace {
                                 g.get_vertex_fields(),
                                 g.get_edge_fields(),
                                 m_srcid_column, m_dstid_column);
-      } catch (cppipc::ipcexception e) {
+      } catch (cppipc::ipcexception& e) {


Need const in all these. Sorry, thought that was implied.

hoytak · 2020-09-15T13:51:12Z

src/core/storage/sgraph_data/sgraph_triple_apply.cpp

@@ -943,7 +943,7 @@ namespace {
      try {
        mutated_edge_data = m_evaluator->proxy->eval_triple_apply(all_edge_data, m_src_partition,
                                                                  m_dst_partition, mutated_edge_field_ids);
-      } catch (cppipc::ipcexception e) {
+      } catch (cppipc::ipcexception& e) {


hoytak · 2020-09-15T13:51:17Z

src/core/system/lambda/lambda_master.cpp

@@ -134,7 +134,7 @@ static lambda_master* instance_ptr = nullptr;
    // catch and reinterpret comm failure
    try {
      out = worker->proxy->bulk_eval(lambda_hash, args, skip_undefined, seed);
-    } catch (cppipc::ipcexception e) {
+    } catch (cppipc::ipcexception& e) {


hoytak · 2020-09-15T13:51:22Z

src/core/system/lambda/lambda_master.cpp

@@ -236,7 +236,7 @@ static lambda_master* instance_ptr = nullptr;
        logstream(LOG_WARNING) << "Unexpected SHMIPC failure. Falling back to CPPIPC" << std::endl;
      }
      out = worker->proxy->bulk_eval_rows(lambda_hash, args, skip_undefined, seed);
-    } catch (cppipc::ipcexception e) {
+    } catch (cppipc::ipcexception& e) {


hoytak · 2020-09-15T13:51:28Z

src/core/system/lambda/lambda_master.cpp

@@ -252,7 +252,7 @@ static lambda_master* instance_ptr = nullptr;
    // catch and reinterpret comm failure
    try {
      out = worker->proxy->bulk_eval_dict(lambda_hash, keys, values, skip_undefined, seed);
-    } catch (cppipc::ipcexception e) {
+    } catch (cppipc::ipcexception& e) {


hoytak · 2020-09-15T13:51:33Z

src/core/system/lambda/lambda_master.cpp

@@ -289,7 +289,7 @@ static lambda_master* instance_ptr = nullptr;
        logstream(LOG_WARNING) << "Unexpected SHMIPC failure. Falling back to CPPIPC" << std::endl;
      }
      out = worker->proxy->bulk_eval_dict_rows(lambda_hash, keys, rows, skip_undefined, seed);
-    } catch (cppipc::ipcexception e) {
+    } catch (cppipc::ipcexception& e) {


hoytak · 2020-09-15T13:51:40Z

src/core/system/lambda/worker_pool.hpp

@@ -385,7 +385,7 @@ class worker_pool {
      parallel_for(0, m_num_workers, [&](size_t i) {
        try {
          ret[i] = f(temp_workers[i]->proxy);
-        } catch (cppipc::ipcexception e) {
+        } catch (cppipc::ipcexception& e) {


Fix compile errors

a9de42e

TobyRoseman requested review from hoytak and nickjong September 11, 2020 23:14

shantanuchhabra approved these changes Sep 11, 2020

View reviewed changes

hoytak reviewed Sep 14, 2020

View reviewed changes

src/core/storage/fileio/fs_utils.cpp Outdated Show resolved Hide resolved

hoytak reviewed Sep 14, 2020

View reviewed changes

src/core/storage/fileio/general_fstream.cpp Outdated Show resolved Hide resolved

hoytak reviewed Sep 14, 2020

View reviewed changes

src/core/storage/sframe_data/sframe_index_file.cpp Outdated Show resolved Hide resolved

hoytak reviewed Sep 14, 2020

View reviewed changes

src/core/system/lambda/lambda_master.cpp Outdated Show resolved Hide resolved

hoytak suggested changes Sep 14, 2020

View reviewed changes

Code review feedback. Use const references.

296821b

hoytak reviewed Sep 15, 2020

View reviewed changes

pradkrish mentioned this pull request Nov 11, 2020

Building turicreate is broken with GCC 5.4.0 #3243

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix compile errors #3318

Fix compile errors #3318

TobyRoseman commented Sep 11, 2020

TobyRoseman commented Sep 14, 2020 •

edited

Loading

hoytak Sep 14, 2020

TobyRoseman Sep 14, 2020

TobyRoseman Sep 14, 2020

hoytak Sep 15, 2020

hoytak Sep 15, 2020

TobyRoseman Sep 15, 2020

hoytak Sep 28, 2020

hoytak left a comment

hoytak Sep 15, 2020

hoytak Sep 15, 2020

hoytak Sep 15, 2020

hoytak Sep 15, 2020

hoytak Sep 15, 2020

hoytak Sep 15, 2020

hoytak Sep 15, 2020

Fix compile errors #3318

Are you sure you want to change the base?

Fix compile errors #3318

Conversation

TobyRoseman commented Sep 11, 2020

TobyRoseman commented Sep 14, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hoytak left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TobyRoseman commented Sep 14, 2020 •

edited

Loading