Add alignment, SM_INLINE, OpenMP and GLFW example support#1
Open
joneqdaniel wants to merge 1 commit into
Open
Conversation
joneqdaniel
force-pushed
the
master
branch
5 times, most recently
from
2c38922 to
a5391b0
Compare
joneqdaniel
changed the title
joneqdaniel
force-pushed
the
master
branch
6 times, most recently
from
c806259 to
53dae22
Compare
slendidev
requested changes
Jul 2, 2026
slendidev
left a comment
slendidev
left a comment
Owner
There was a problem hiding this comment.
Lots and lots of constexpr was removed. What is up with that? There's also other issues, and I don't think that there needs to be an alignment parameter, especially as a template parameter. I also don't see any tangible benefits from it.
Comment on lines
+24
to
+64
| if(NOT CMAKE_GENERATOR MATCHES "^Visual Studio") | ||
| if( NOT CMAKE_BUILD_TYPE STREQUAL Debug ) | ||
| add_compile_options(-O3) | ||
| add_link_options(-Wl,--gc-sections) | ||
| add_link_options(-Wl,--print-gc-sections) | ||
| add_link_options(-Wl,-s) | ||
| else() | ||
| add_compile_options(-O0 -g) | ||
| endif() | ||
| else() | ||
| if( NOT CMAKE_BUILD_TYPE STREQUAL Debug ) | ||
| add_compile_options(/O2) | ||
| else() | ||
| add_compile_options(/Od /Zi) | ||
| endif() | ||
| endif() | ||
|
|
||
| if (NOT CMAKE_GENERATOR MATCHES "^Visual Studio") | ||
| if ( NOT CMAKE_CROSS_COMPILING ) | ||
| add_compile_options(-march=native) | ||
| if( CMAKE_GENERATOR_PLATFORM STREQUAL "x64" OR CMAKE_GENERATOR_PLATFORM STREQUAL "x86_64" OR CMAKE_GENERATOR_PLATFORM STREQUAL "AMD64" ) | ||
| add_compile_options(-mfpmath=sse) | ||
| endif() | ||
| endif() | ||
| add_compile_options(-fopenmp) | ||
| add_compile_options(-fopenmp-simd) | ||
| add_compile_options(-ftree-vectorize) | ||
| add_compile_options(-Wno-gnu-alignof-expression) | ||
| add_compile_options(-D_FILE_OFFSET_BITS=64) | ||
| add_compile_options(-fdata-sections) | ||
| add_compile_options(-ffunction-sections) | ||
| add_compile_options(-fpermissive) | ||
| add_compile_options(-Wno-array-bounds) | ||
| add_compile_options(-Wno-unused-parameter) | ||
| add_compile_options(-Wno-pedantic) | ||
| add_compile_options(-Wno-narrowing) | ||
| add_compile_options(-Wno-attributes) | ||
| else() | ||
| add_compile_options(/openmp:experimental) | ||
| endif() | ||
|
|
|
|
||
| export using ::smath::Vec2; | ||
| export using ::smath::Vec3; | ||
| export using ::smath::AVec3 |
Comment on lines
+58
to
+60
| template<std::size_t N, typename T = float, std::size_t A = ((N == std::bit_ceil<std::size_t>(N)) ? N : 1) * alignof(T), std::size_t N_POW2 = std::bit_ceil<std::size_t>(N)> | ||
| requires std::is_arithmetic_v<T> | ||
| struct Vec; | ||
| struct alignas(A) Vec; |
Owner
There was a problem hiding this comment.
I don't think A should be a template parameter, complicates API and causes type mismatches. If we truly want aligned storage this should be an internal implementation detail and not part of the public type signature.
Comment on lines
+52
to
+54
| template <class S> | ||
| constexpr SM_INLINE std::pair<S,S> sincos(S arg) { return { std::sin(arg), std::cos(arg) }; } | ||
|
|
Owner
There was a problem hiding this comment.
This should be inside the smath namespace alongside the other math functions.
| #define SM_INLINE __attribute__((always_inline,flatten,nothrow)) inline | ||
| #define SM_CONST __attribute__((const)) | ||
| #endif | ||
|
|
Owner
There was a problem hiding this comment.
Use C++ style attribute syntax. Also I don't think __flatten is available on MSVC.
Owner
There was a problem hiding this comment.
Also I think it should be consistently named with the rest of the macros, aka SMATH_* and not SM_*.
Comment on lines
+759
to
762
| template<std::size_t R, std::size_t C, typename T = float, std::size_t A = std::bit_ceil<std::size_t>(alignof(Vec<R,T>) * C)> | ||
| requires std::is_arithmetic_v<T> | ||
| struct Mat; | ||
| struct alignas(A) Mat; | ||
|
|
Owner
There was a problem hiding this comment.
Same issue with the addition of A in Vec
Comment on lines
+48
to
+61
| add_compile_options(-fopenmp) | ||
| add_compile_options(-fopenmp-simd) | ||
| add_compile_options(-ftree-vectorize) | ||
| add_compile_options(-Wno-gnu-alignof-expression) | ||
| add_compile_options(-D_FILE_OFFSET_BITS=64) | ||
| add_compile_options(-fdata-sections) | ||
| add_compile_options(-ffunction-sections) | ||
| add_compile_options(-fpermissive) | ||
| add_compile_options(-Wno-array-bounds) | ||
| add_compile_options(-Wno-unused-parameter) | ||
| add_compile_options(-Wno-pedantic) | ||
| add_compile_options(-Wno-narrowing) | ||
| add_compile_options(-Wno-attributes) | ||
| else() |
Owner
There was a problem hiding this comment.
Check the compiler instead, not the cmake generator. MSVC can be used with other generators so that would end up with those flags being passed to MSVC.
| endif() | ||
| endif() | ||
|
|
||
| if (NOT CMAKE_GENERATOR MATCHES "^Visual Studio") |
Comment on lines
+142
to
+163
| if(SMATH_BUILD_GLFW_EXAMPLES) | ||
| find_package(OpenGL REQUIRED) | ||
| find_package(glfw3 REQUIRED) | ||
| file(GLOB GLFW_EXAMPLE_SOURCES "${CMAKE_SOURCE_DIR}/examples/glfw/*.cpp") | ||
| foreach(EXAMPLE_FILE ${GLFW_EXAMPLE_SOURCES}) | ||
| get_filename_component(EXAMPLE_NAME ${EXAMPLE_FILE} NAME_WE) | ||
| add_executable("glfw_${EXAMPLE_NAME}" ${EXAMPLE_FILE}) | ||
| target_link_libraries("glfw_${EXAMPLE_NAME}" PUBLIC smath::smath glfw OpenGL OpenGL::GLU) | ||
| endforeach() | ||
| endif() | ||
|
|
||
| if(SMATH_BUILD_GLUT_EXAMPLES) | ||
| find_package(OpenGL REQUIRED) | ||
| find_package(GLUT REQUIRED) | ||
| file(GLOB GLUT_EXAMPLE_SOURCES "${CMAKE_SOURCE_DIR}/examples/glut/*.cpp") | ||
| foreach(EXAMPLE_FILE ${GLUT_EXAMPLE_SOURCES}) | ||
| get_filename_component(EXAMPLE_NAME ${EXAMPLE_FILE} NAME_WE) | ||
| add_executable("glut_${EXAMPLE_NAME}" ${EXAMPLE_FILE}) | ||
| target_link_libraries("glut_${EXAMPLE_NAME}" PUBLIC smath::smath OpenGL OpenGL::GLU GLUT::GLUT) | ||
| endforeach() | ||
| endif() | ||
|
|
Owner
There was a problem hiding this comment.
Was this vibe coded? There's no examples/glfw or examples/glut!
Comment on lines
+1348
to
1354
| res[0, 0] = 2 / (right - left); | ||
| res[1, 1] = 2 / (top - bottom); | ||
| res[2, 2] = -2 / (far - near); | ||
| res[0, 3] = -(right + left) / (right - left); | ||
| res[1, 3] = -(top + bottom) / (top - bottom); | ||
| res[2, 3] = -(far + near) / (far - near); | ||
| res[2, 3] = -(far + near) / (far - near); | ||
| res[3, 3] = 1; |
Owner
There was a problem hiding this comment.
Unrelated formatting changes. Run clang-format.
There was a problem hiding this comment.
Pull request overview
This PR expands smath with alignment-focused vector types and additional build/example support, aiming to enable more aggressive inlining/SIMD and add optional GLFW/GLUT example builds.
Changes:
- Adds aligned
Vecvariants (AVec3,AVec3d) and alignment-related unit tests. - Introduces
SM_INLINE, adds OpenMP SIMD pragmas across vector/matrix operations, and addsposition()/direction()helpers for 3D vectors. - Updates CMake to add OpenMP-related flags and adds optional GLFW/GLUT example targets.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 9 comments.
| File | Description |
|---|---|
include/smath/smath.hpp |
Adds alignment/inlining macros, aligned vector aliases, OpenMP SIMD pragmas, and new Vec helpers. |
src/modules/smath.cppm |
Re-exports new public API symbols from the module interface. |
tests/vec.cpp |
Adds/updates tests to validate alignment behavior and aligned cross-product results. |
CMakeLists.txt |
Adds OpenMP flags and optional GLFW/GLUT example build targets. |
Comments suppressed due to low confidence (1)
include/smath/smath.hpp:214
- The scalar-fill constructor is no longer
constexpr, but there are stillconstexproperators (e.g.,operator-(T, Vec const&)) that callVec(s). Aconstexprfunction that unconditionally calls a non-constexprconstructor/function is ill-formed and can break compilation.
explicit Vec(T const &s) noexcept
{
for (auto &v : *this)
v = s;
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
13
to
16
| export using ::smath::Vec2; | ||
| export using ::smath::Vec3; | ||
| export using ::smath::AVec3 | ||
| export using ::smath::Vec4; |
Comment on lines
+44
to
+50
| #ifdef _MSC_VER | ||
| #define SM_INLINE __force_inline __flatten __declspec(nothrow) inline | ||
| #define SM_CONST __declspec(noalias) | ||
| #else | ||
| #define SM_INLINE __attribute__((always_inline,flatten,nothrow)) inline | ||
| #define SM_CONST __attribute__((const)) | ||
| #endif |
Comment on lines
25
to
35
| #include <algorithm> | ||
| #include <array> | ||
| #include <cmath> | ||
| #include <cstddef> | ||
| #include <cstdint> | ||
| #include <format> | ||
| #include <numbers> | ||
| #include <optional> | ||
| #include <type_traits> | ||
| #include <bit> | ||
|
|
Comment on lines
685
to
688
| using Vec4 = Vec<4>; | ||
| /** @brief 3D aligned float vector alias. */ | ||
| using AVec3 = Vec<3, float, alignof(Vec4)>; | ||
|
|
Comment on lines
481
to
+486
| requires(N == 3) | ||
| /** @brief 3D cross product. */ | ||
| constexpr auto cross(Vec const &r) const noexcept -> Vec | ||
| constexpr SM_INLINE auto cross(Vec const &r) const noexcept -> Vec<3,U,alignof(U) * 4> | ||
| { | ||
| return { | ||
| (*this)[1] * r[2] - (*this)[2] * r[1], | ||
| (*this)[2] * r[0] - (*this)[0] * r[2], | ||
| (*this)[0] * r[1] - (*this)[1] * r[0], | ||
| }; | ||
| return (Vec<3,U,alignof(U) * 4>)swizzle<"yzx">((*this)) * (Vec<3,U,alignof(U) * 4>)swizzle<"zxy">(r) - | ||
| (Vec<3,U,alignof(U) * 4>)swizzle<"zyx">((*this)) * (Vec<3,U,alignof(U) * 4>)swizzle<"yzx">(r); |
Comment on lines
+20
to
+22
| if( NOT CMAKE_BUILD_TYPE ) | ||
| set( CMAKE_BUILD_TYPE Release CACHE STRING "Choose the type of build." FORCE ) | ||
| endif() |
Comment on lines
+41
to
+63
| if (NOT CMAKE_GENERATOR MATCHES "^Visual Studio") | ||
| if ( NOT CMAKE_CROSS_COMPILING ) | ||
| add_compile_options(-march=native) | ||
| if( CMAKE_GENERATOR_PLATFORM STREQUAL "x64" OR CMAKE_GENERATOR_PLATFORM STREQUAL "x86_64" OR CMAKE_GENERATOR_PLATFORM STREQUAL "AMD64" ) | ||
| add_compile_options(-mfpmath=sse) | ||
| endif() | ||
| endif() | ||
| add_compile_options(-fopenmp) | ||
| add_compile_options(-fopenmp-simd) | ||
| add_compile_options(-ftree-vectorize) | ||
| add_compile_options(-Wno-gnu-alignof-expression) | ||
| add_compile_options(-D_FILE_OFFSET_BITS=64) | ||
| add_compile_options(-fdata-sections) | ||
| add_compile_options(-ffunction-sections) | ||
| add_compile_options(-fpermissive) | ||
| add_compile_options(-Wno-array-bounds) | ||
| add_compile_options(-Wno-unused-parameter) | ||
| add_compile_options(-Wno-pedantic) | ||
| add_compile_options(-Wno-narrowing) | ||
| add_compile_options(-Wno-attributes) | ||
| else() | ||
| add_compile_options(/openmp:experimental) | ||
| endif() |
Comment on lines
+148
to
+150
| add_executable("glfw_${EXAMPLE_NAME}" ${EXAMPLE_FILE}) | ||
| target_link_libraries("glfw_${EXAMPLE_NAME}" PUBLIC smath::smath glfw OpenGL OpenGL::GLU) | ||
| endforeach() |
Comment on lines
+159
to
+161
| add_executable("glut_${EXAMPLE_NAME}" ${EXAMPLE_FILE}) | ||
| target_link_libraries("glut_${EXAMPLE_NAME}" PUBLIC smath::smath OpenGL OpenGL::GLU GLUT::GLUT) | ||
| endforeach() |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
SM_INLINEwith(always_inline,flatten,nothrow) inlineVectoN * alignof(T)ifN==std::bit_ceil(N)otherwise use defaultalignof(T)CMAKE_BUILD_TYPE != DebugGNUInstallDirsandFindPkgConfigfor non MSVCalignof(Vec3)andalignof(Vec4)AVec3andAVec3dsincosconstexprposition()anddirection()toVecwithN = 3