saigegit / SAIGE

Development for SAIGE and SAIGE-GENE(+)
GNU General Public License v3.0
66 stars 29 forks source link

Build failure on arm64: savvy cannot build / run on arm64 #14

Closed pettyalex closed 2 years ago

pettyalex commented 2 years ago

SAIGE needs to update to a newer version of savvy to enable arm64 running / building.

From the Savvy issue: Savvy can't build or run on arm64 right now, because char is unsigned on arm64 and this code assumes that char will be signed. The C/C++ specification says nothing about the signed-ness of char by default, so if char is signed varies by compiler / architecture / implementation.

Some quick ways to work around this would be to either specify that these types are signed chars, or to update the template so that it works on unsigned types. I haven't investigated deeper and barely know C++, so I'd appreciate your help with a solution.

Specifically, I see that typed_value.hpp we have some char vectors https://github.com/statgen/savvy/blob/master/include/savvy/typed_value.hpp#L1712

    std::vector<char> off_data_;
    std::vector<char> val_data_;

and then expect to be able to operate on them with this template https://github.com/statgen/savvy/blob/master/include/savvy/typed_value.hpp#L125:

    template<typename T>
    static typename std::enable_if<std::is_signed<T>::value, std::uint8_t>::type
    type_code(const T& val);

Stack trace from build failure on arm64:

include/savvy/typed_value.hpp: In instantiation of ‘void savvy::typed_value::thin_types_fn::operator()(T*, T*, savvy::typed_value*) [with T = char]’:
include/savvy/typed_value.hpp:712:11:   required from ‘bool savvy::typed_value::apply_dense(Fn, Args ...) [with Fn = savvy::typed_value::thin_types_fn; Args = {savvy::typed_value*}]’
include/savvy/typed_value.hpp:1412:42:   required from here
include/savvy/typed_value.hpp:1331:44: error: no matching function for call to ‘savvy::typed_value::type_code(char&)’
 1331 |           new_val_type = std::max(type_code(max_val), type_code(min_val));
      |                                   ~~~~~~~~~^~~~~~~~~
In file included from include/savvy/site_info.hpp:12,
                 from include/savvy/region.hpp:10,
                 from include/savvy/s1r.hpp:11,
                 from include/savvy/sav_reader.hpp:12,
                 from include/sav/sort.hpp:10,
                 from src/sav/main.cpp:14:
include/savvy/typed_value.hpp:122:32: note: candidate: ‘template<class T> static uint8_t savvy::typed_value::type_code()’
  122 |     inline static std::uint8_t type_code();
      |                                ^~~~~~~~~
include/savvy/typed_value.hpp:122:32: note:   template argument deduction/substitution failed:
In file included from include/savvy/site_info.hpp:12,
                 from include/savvy/region.hpp:10,
                 from include/savvy/s1r.hpp:11,
                 from include/savvy/sav_reader.hpp:12,
                 from include/sav/sort.hpp:10,
                 from src/sav/main.cpp:14:
include/savvy/typed_value.hpp:1331:44: note:   candidate expects 0 arguments, 1 provided
 1331 |           new_val_type = std::max(type_code(max_val), type_code(min_val));
      |                                   ~~~~~~~~~^~~~~~~~~
include/savvy/typed_value.hpp:1864:73: note: candidate: ‘template<class T> static typename std::enable_if<std::is_signed<_Tp>::value, unsigned char>::type savvy::typed_value::type_code(const T&)’
 1864 |   typename std::enable_if<std::is_signed<T>::value, std::uint8_t>::type typed_value::type_code(const T& val)
      |                                                                         ^~~~~~~~~~~
include/savvy/typed_value.hpp:1864:73: note:   template argument deduction/substitution failed:
include/savvy/typed_value.hpp: In substitution of ‘template<class T> static typename std::enable_if<std::is_signed<_Tp>::value, unsigned char>::type savvy::typed_value::type_code(const T&) [with T = char]’:
include/savvy/typed_value.hpp:1331:44:   required from ‘void savvy::typed_value::thin_types_fn::operator()(T*, T*, savvy::typed_value*) [with T = char]’
include/savvy/typed_value.hpp:712:11:   required from ‘bool savvy::typed_value::apply_dense(Fn, Args ...) [with Fn = savvy::typed_value::thin_types_fn; Args = {savvy::typed_value*}]’
include/savvy/typed_value.hpp:1412:42:   required from here
include/savvy/typed_value.hpp:1864:73: error: no type named ‘type’ in ‘struct std::enable_if<false, unsigned char>’
pettyalex commented 2 years ago

Closing, I meant to create this on savvy. Apologies!

SAIGE can't build right now on arm64 because of Savvy's problem.

pettyalex commented 2 years ago

Actually, I'm going to re-open this to track the update to a fixed version of Savvy

pettyalex commented 2 years ago

This was resolved by the update to Savvy v2.1.0