BinomialLLC / basis_universal

Basis Universal GPU Texture Codec
Apache License 2.0
2.7k stars 263 forks source link

Compile error in cppspmd_sse.h with C++23 #366

Open MarkCallow opened 11 months ago

MarkCallow commented 11 months ago

When compiling with c++23 the following errors are reported in cppspmd_sse.h:

In file included from /media/dezlow/Drive/Dev/C++/Oneiro/ThirdParty/KTX/lib/basisu/encoder/basisu_kernels_sse.cpp:41:
/media/dezlow/Drive/Dev/C++/Oneiro/ThirdParty/KTX/lib/basisu/encoder/cppspmd_sse.h:496:10: error: non-const lvalue reference to type 'cppspmd_sse41::spmd_kernel::vfloat' cannot bind to a temporary of type 'cppspmd_sse41::spmd_kernel::vfloat'
                return dst;
                       ^~~
/media/dezlow/Drive/Dev/C++/Oneiro/ThirdParty/KTX/lib/basisu/encoder/cppspmd_sse.h:508:10: error: non-const lvalue reference to type 'cppspmd_sse41::spmd_kernel::vfloat' cannot bind to a temporary of type 'cppspmd_sse41::spmd_kernel::vfloat'
                return dst;
                       ^~~

This is with Clang 15 or 16 (--std=c++2b) but probably other c++23 capable compilers as well.

Here is one of the functions

    CPPSPMD_FORCE_INLINE vfloat& store(vfloat&& dst, const vfloat& src)
    {
        dst.m_value = blendv_mask_ps(dst.m_value, src.m_value, _mm_castsi128_ps(m_exec.m_mask));
        return dst;
    }

Writing to an r_value dst seems to have no purpose. In order for this overload to be called you either have to pass a constant as dst or use std_move(dst_param) when calling it which means dst_param will be thrown away. I think the proper fix is

    CPPSPMD_FORCE_INLINE vfloat& store(vfloat&& dst, const vfloat& src)
    {
                vfloat ret = dst;
        ret.m_value = blendv_mask_ps(dst.m_value, src.m_value, _mm_castsi128_ps(m_exec.m_mask));
        return ret;
    }

but I do not understand the real purpose of this code. Please advise on the correct fix.

richgel999 commented 10 months ago

Thanks, I'll reproduce it here.