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

Etheory patch gcc narrowing fix + MSVC support #3

Merged
merged 10 commits into from
Apr 27, 2017

Conversation

etheory
Copy link
Contributor

@etheory etheory commented Mar 1, 2017

Fixes issue for more strict compilation in GCC (also works in Clang).
Tested in a VFX production environment on Linux.

The previous definition of v4sil was trying to cast to a 64bit type, not not exactly matching the expected underlying type (long long).

Under c++11, this fails to compile with a narrowing warning.
i.e. in gcc when using the flags "-std=c++11 -Wnarrowing"

By casting to a "long long" instead of "unsigned long long" above, we match the type of the underlying definition, and also the v2dil cast then work correctly to widen the 2 64bit type to the expected final 128bit type.

  • Additionally added MSVC compatibility for 2015+
  • As far as I know it will now compile and run in MSVC directly out of the box (it does for me now)

@pmineiro @romeric

P.S. to verify go to godbolt, and paste the following:

#include <stdint.h>
#include <emmintrin.h>
#include <smmintrin.h>
#include <cmath>

#define __forceinline __attribute__((always_inline))

typedef __m128 v4sf;
typedef __m128i v4si;

#define v4si_to_v4sf _mm_cvtepi32_ps
#define v4sf_to_v4si _mm_cvttps_epi32

#define v4sfl(x) ((const v4sf) { (x), (x), (x), (x) })

#define v2dil(x) ((const v4si) { (x), (x) })
#define v4sil(x) v2dil((((long long) (x)) << 32) | (long long) (x))

v4si test()
{
  v4si result = v4sil(0x807FFFFF);
  return result;
}

int main()
{
  volatile v4si val = test();
  return 0;
}

Then compile with a gcc target, with the following flags:

-O3 -ffast-math -std=c++11 -Wnarrowing

Note that without this change, it won't compile.

The previous definition of v4sil was trying to cast to a 64bit type, not not exactly matching the expected underlying type (long long).

Under c++11, this fails to compile with a narrowing warning.
i.e. in gcc when using the flags "-std=c++11 -Wnarrowing"

By casting to a "long long" instead of "unsigned long long" above, we match the type of the underlying definition, and also the v2dil cast then work correctly to widen the 2 64bit type to the expected final 128bit type.
Same as previous file change.
It spews out error C4576: a parenthesized type followed by an initializer list is a non-standard explicit type conversion syntax unless you make this change.
It spews out error C4576: a parenthesized type followed by an initializer list is a non-standard explicit type conversion syntax unless you make this change.
Update to provide MSVC compatibility
Update for MSVC compatibility
Update to add vector operators for MSVC
Add MSVC vector operators
fixed v4sf_fabs for MSVC
Fixed v4sf_fabs for MSVC
@etheory etheory changed the title Etheory patch gcc narrowing fix Etheory patch gcc narrowing fix + MSVC support Mar 1, 2017
@pmineiro
Copy link
Contributor

pmineiro commented Mar 1, 2017

Nice!

@romeric romeric merged commit e729add into romeric:master Apr 27, 2017
@romeric
Copy link
Owner

romeric commented Apr 27, 2017

@etheory Good job. This should address #1

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

Successfully merging this pull request may close these issues.

3 participants