Skip to content

Conversation

@SwayamInSync
Copy link
Member

ops.hpp was originally a C file, but got refactored into CPP implementation to use the templates (which indeed simplified a lot of code)
I noticed the functions are still declared with C style static inline (this leads each TU to own a private copy of that function) which is not desireable here (C's semantics of inline differ with C++). Switching all of them to just inline to allow linker to share those objects rather making private copies. Although most of the functions are small enough to be get inlined by the compiler anyway but this can help in debugging and will reduce some code bloat (2% on my machine) as we use these ops in multiple C++ TUs (see below)

Details
(quaddtype) ➜  numpy-quaddtype git:(code-bloat) ✗ rg "ops.hpp"                              
src/csrc/scalar_ops.cpp
17:#include "ops.hpp"

src/csrc/umath/unary_props.cpp
21:#include "ops.hpp"

src/csrc/casts.cpp
29:#include "ops.hpp"

src/csrc/umath/comparison_ops.cpp
21:#include "ops.hpp"

src/csrc/umath/matmul.cpp
22:#include "ops.hpp"

src/csrc/umath/binary_ops.cpp
20:#include "ops.hpp"

src/csrc/umath/unary_ops.cpp
21:#include "ops.hpp"

src/csrc/umath/umath.cpp
21:#include "ops.hpp"
(quaddtype) ➜  numpy-quaddtype git:(code-bloat) ✗ 

I proposed this earlier and one more time here, Ideally it'll be better to replace all explicit use of SLEEF routines to these ops abstractions (this will require porting the remaining C files C++) but will make the code more extendable.

Copy link
Contributor

@juntyr juntyr left a comment

Choose a reason for hiding this comment

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

LGTM

@SwayamInSync SwayamInSync merged commit 1c9451d into numpy:main Jan 27, 2026
12 checks passed
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.

2 participants