Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
MarkCallow opened this issue Nov 28, 2023 · 1 comment
Open

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

MarkCallow opened this issue Nov 28, 2023 · 1 comment

Comments

@MarkCallow
Copy link
Contributor

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
Copy link
Contributor

Thanks, I'll reproduce it here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants