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

Add Clang Vector Extension and swich to xyzw quat #2025

Merged
merged 7 commits into from
Dec 7, 2024

Conversation

jopadan
Copy link
Contributor

@jopadan jopadan commented Nov 15, 2024

  • use ext_vector_type for -DCMAKE_C_COMPILER=clang
  • pass vector argument as static instance of type not address (they are all const anyway)
  • use xyzw components instead of wxyz for quat
  • add [src/]CMakeFiles to .gitignore

TODO

  • Assert Gyro Gamepad functioning as before (I was not able to get a Gyro capable device)

DONE

  • build/run with gcc/clang
  • assert non-gyro gamepad function

@jopadan jopadan changed the title Add CLang Vector Extension and swich to xyzw quat Add Clang Vector Extension and swich to xyzw quat Nov 15, 2024
Copy link
Owner

@fabiangreffrath fabiangreffrath left a comment

Choose a reason for hiding this comment

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

Thank you very much for your contribution! While it looks very interesting and promising, I am not sure which changes are really required and which may just be considered drive-by (i.e. the change to C23).

cmake/WoofSettings.cmake Outdated Show resolved Hide resolved
src/m_vector.h Outdated Show resolved Hide resolved
src/doomtype.h Outdated Show resolved Hide resolved
src/m_vector.h Outdated Show resolved Hide resolved
src/doomtype.h Outdated Show resolved Hide resolved
src/doomtype.h Outdated Show resolved Hide resolved
.gitignore Show resolved Hide resolved
src/doomtype.h Outdated Show resolved Hide resolved
@gendlin
Copy link
Contributor

gendlin commented Nov 16, 2024

Would now be a good time to start compiling with -ffast-math?

@jopadan jopadan force-pushed the master branch 2 times, most recently from 4614b5a to 68ce564 Compare November 17, 2024 12:03
@fabiangreffrath
Copy link
Owner

I think you should move the feature check to the build system like this:

woof/CMakeLists.txt

Lines 68 to 77 in e832180

check_c_source_compiles("
#include <immintrin.h>
int main()
{
int tmp = _div64(4, 2, NULL);
return 0;
}
"
HAVE__DIV64
)

@rfomin
Copy link
Collaborator

rfomin commented Nov 17, 2024

Would now be a good time to start compiling with -ffast-math?

I think we still have too few float calculations.

@fabiangreffrath
Copy link
Owner

fabiangreffrath commented Nov 21, 2024

I think you should move the feature check to the build system like this:

Could you please apply something like this (untested) and see if it works for you?

--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -75,6 +75,16 @@ check_c_source_compiles("
   "
   HAVE__DIV64
 )
+check_c_source_compiles("
+    typedef float vec __attribute__((ext_vector_type(4)));
+    int main()
+    {
+        vec a = (vec){0, 1, 2, 3};
+        return 0;
+    }
+  "
+  HAVE_EXT_VECTOR_TYPE
+)

 option(CMAKE_FIND_PACKAGE_PREFER_CONFIG
        "Lookup package config files before using find modules" ON)
--- a/src/m_vector.h
+++ b/src/m_vector.h
@@ -20,9 +20,10 @@

 #include <math.h>

+#include "config.h"
 #include "doomtype.h"

-#if defined(__has_attribute) && defined(__clang__) && __has_attribute(ext_vector_type)
+#if defined(HAVE_EXT_VECTOR_TYPE)
 typedef float vec __attribute__((ext_vector_type(3)));
 #else
 typedef struct
@@ -33,7 +34,7 @@ typedef struct
 } vec;
 #endif

-#if defined(__has_attribute) && defined(__clang__) && __has_attribute(ext_vector_type)
+#if defined(HAVE_EXT_VECTOR_TYPE)
 typedef float quat __attribute__((ext_vector_type(4)));
 #else
 typedef struct

Copy link
Owner

@fabiangreffrath fabiangreffrath left a comment

Choose a reason for hiding this comment

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

LGTM, I don't care that much about the .gitignore changes.

Copy link
Collaborator

@ceski-1 ceski-1 left a comment

Choose a reason for hiding this comment

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

LGTM

@rfomin rfomin merged commit 89aae9c into fabiangreffrath:master Dec 7, 2024
@rfomin
Copy link
Collaborator

rfomin commented Dec 7, 2024

Thanks!

fabiangreffrath pushed a commit that referenced this pull request Dec 7, 2024
* Add Clang Vector Extension and switch to xyzw quat

* Formatting

---------

Co-authored-by: Jon Daniel <[email protected]>
Co-authored-by: ceski <[email protected]>
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.

5 participants