Skip to content
This repository has been archived by the owner on Apr 2, 2021. It is now read-only.

Update casts for NEON #27

Merged
merged 1 commit into from
Jan 16, 2018
Merged

Update casts for NEON #27

merged 1 commit into from
Jan 16, 2018

Conversation

QuLogic
Copy link
Contributor

@QuLogic QuLogic commented Jan 16, 2018

The problem is that the comparisons return uint32x4_t and MASK is int32x4_t; converting comparison results to float just causes an error trying to convert back to int. The second commit adds some other casts to make unsigned/signed explicit (and now works without -flax-vector-conversions).

This fixes the other problem in #23. Still not able to build on armv7 because it needs android_getCpuFeatures and cpu-features.c doesn't yet build.

@@ -190,7 +191,7 @@ static SIMDf VECTORCALL FUNC(FLOOR)(SIMDf a)
{
SIMDf fval = SIMDf_CONVERT_TO_FLOAT(SIMDi_CONVERT_TO_INT(a));

return vsubq_f32(fval, SIMDf_AND(SIMDf_LESS_THAN(a, fval), SIMDf_NUM(1)));
return vsubq_f32(fval, SIMDf_CAST_TO_FLOAT(vandq_s32(SIMDf_LESS_THAN(a, fval), SIMDi_NUM(1))));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm only unsure whether this is right; the inputs should be int, but I'm not sure if SIMDi_NUM does the right thing.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This won't work, it is meant to conditionally subtract 1 if a < fval. You are subtracting an int 1 instead of a float 1 these are bit casted not converted

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, that's what I thought; I think it should be correct now.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You shouldn't need to change the floor function. See my latest commit

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You haven't pushed any new commits.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My bad, looks like our changes are the same anyway, apart from the floor function

Copy link
Contributor Author

@QuLogic QuLogic Jan 16, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change to floor is still necessary; on master:

In file included from FastNoiseSIMD/FastNoiseSIMD_neon.cpp:37:0:
FastNoiseSIMD/FastNoiseSIMD_internal.cpp: In function ‘SIMDf L5_FUNC_FLOOR(SIMDf)’:
FastNoiseSIMD/FastNoiseSIMD_internal.cpp:184:77: error: cannot convert ‘int32x4_t {aka __vector(4) int}’ to ‘float32x4_t {aka __vector(4) float}’ for argument ‘1’ to ‘int32x4_t vreinterpretq_s32_f32(float32x4_t)’
 #define SIMDf_AND(a,b) SIMDf_CAST_TO_FLOAT(vandq_s32(vreinterpretq_s32_f32(a),vreinterpretq_s32_f32(b)))
                                                                             ^
FastNoiseSIMD/FastNoiseSIMD_internal.cpp:154:54: note: in definition of macro ‘SIMDf_CAST_TO_FLOAT’
 #define SIMDf_CAST_TO_FLOAT(a) vreinterpretq_f32_s32(a)
                                                      ^
FastNoiseSIMD/FastNoiseSIMD_internal.cpp:193:25: note: in expansion of macro ‘SIMDf_AND’
  return vsubq_f32(fval, SIMDf_AND(SIMDf_LESS_THAN(a, fval), SIMDf_NUM(1)));
                         ^~~~~~~~~

and it works on this branch.

@QuLogic QuLogic force-pushed the arm-casts branch 2 times, most recently from 86bae84 to 691eaf8 Compare January 16, 2018 21:27
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants