8000 MAINT: Make the `NPY_CPU_DISPATCH_CALL` macros expressions not statements by eric-wieser · Pull Request #17201 · numpy/numpy · GitHub
[go: up one dir, main page]

Skip to content

MAINT: Make the NPY_CPU_DISPATCH_CALL macros expressions not statements #17201

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

Merged
merged 1 commit into from
Sep 1, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion numpy/core/code_generators/generate_umath.py
Original file line number Diff line number Diff line change
Expand Up @@ -1042,7 +1042,7 @@ def make_arrays(funcdict):
#ifndef NPY_DISABLE_OPTIMIZATION
#include "{dname}.dispatch.h"
#endif
NPY_CPU_DISPATCH_CALL_XB({name}_functions[{k}] = {tname}_{name})
NPY_CPU_DISPATCH_CALL_XB({name}_functions[{k}] = {tname}_{name});
""").format(
dname=dname, name=name, tname=tname, k=k
))
Expand Down
33 changes: 19 additions & 14 deletions numpy/core/src/common/npy_cpu_dispatch.h
Original file line number Diff line number Diff line change
Expand Up @@ -217,44 +217,49 @@
* func_type the_callee(const int *src, int *dst, func_type *cb)
* {
* // direct call
* NPY_CPU_DISPATCH_CALL(dispatch_me, (src, dst))
* NPY_CPU_DISPATCH_CALL(dispatch_me, (src, dst));
* // assign the pointer
* NPY_CPU_DISPATCH_CALL(*cb = dispatch_me)
* *cb = NPY_CPU_DISPATCH_CALL(dispatch_me);
* // or
* NPY_CPU_DISPATCH_CALL(*cb = dispatch_me);
* // return the pointer
* NPY_CPU_DISPATCH_CALL(return dispatch_me)
* return NPY_CPU_DISPATCH_CALL(dispatch_me);
* }
*/
#define NPY_CPU_DISPATCH_CALL(...) \
if (0) {/*DUMMY*/} \
NPY__CPU_DISPATCH_CALL(NPY_CPU_HAVE, NPY_CPU_DISPATCH_CALL_CB_, __VA_ARGS__) \
NPY__CPU_DISPATCH_BASELINE_CALL(NPY_CPU_DISPATCH_CALL_BASE_CB_, __VA_ARGS__)
// Preprocessor callbacks
#define NPY_CPU_DISPATCH_CALL_CB_(TESTED_FEATURES, TARGET_NAME, LEFT, ...) \
else if (TESTED_FEATURES) { NPY_CAT(NPY_CAT(LEFT, _), TARGET_NAME) __VA_ARGS__; }
(TESTED_FEATURES) ? (NPY_CAT(NPY_CAT(LEFT, _), TARGET_NAME) __VA_ARGS__) :
Copy link
Member

Choose a reason for hiding this comment

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

the if statement is easier to read, why use the conditional expression?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because the conditional expression makes this have a return value

#define NPY_CPU_DISPATCH_CALL_BASE_CB_(LEFT, ...) \
else { LEFT __VA_ARGS__; }
(LEFT __VA_ARGS__)
/**
* Macro NPY_CPU_DISPATCH_CALL_XB(LEFT, ...)
*
* Same as `NPY_CPU_DISPATCH_DECLARE` but exclude the baseline declration even
* if it was provided within the configration statments.
* Same as `NPY_CPU_DISPATCH_DECLARE` but exclude the baseline declaration even
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* Same as `NPY_CPU_DISPATCH_DECLARE` but exclude the baseline declaration even
* Same as `NPY_CPU_DISPATCH_CALL` but exclude the baseline call even

* if it was provided within the configration statements.
* Returns void.
*/
#define NPY_CPU_DISPATCH_CALL_XB_CB_(TESTED_FEATURES, TARGET_NAME, LEFT, ...) \
(TESTED_FEATURES) ? (void) (NPY_CAT(NPY_CAT(LEFT, _), TARGET_NAME) __VA_ARGS__) :
#define NPY_CPU_DISPATCH_CALL_XB(...) \
if (0) {/*DUMMY*/} \
NPY__CPU_DISPATCH_CALL(NPY_CPU_HAVE, NPY_CPU_DISPATCH_CALL_CB_, __VA_ARGS__)
NPY__CPU_DISPATCH_CALL(NPY_CPU_HAVE, NPY_CPU_DISPATCH_CALL_XB_CB_, __VA_ARGS__) \
((void) 0 /* discarded expression value */)
/**
* Macro NPY_CPU_DISPATCH_CALL_ALL(LEFT, ...)
*
* Same as `NPY_CPU_DISPATCH_CALL` but dispatching all the required optimizations for
* the exported functions and variables instead of highest interested one.
* Returns void.
*/
#define NPY_CPU_DISPATCH_CALL_ALL(...) \
NPY__CPU_DISPATCH_CALL(NPY_CPU_HAVE, NPY_CPU_DISPATCH_CALL_ALL_CB_, __VA_ARGS__) \
NPY__CPU_DISPATCH_BASELINE_CALL(NPY_CPU_DISPATCH_CALL_ALL_BASE_CB_, __VA_ARGS__)
(NPY__CPU_DISPATCH_CALL(NPY_CPU_HAVE, NPY_CPU_DISPATCH_CALL_ALL_CB_, __VA_ARGS__) \
NPY__CPU_DISPATCH_BASELINE_CALL(NPY_CPU_DISPATCH_CALL_ALL_BASE_CB_, __VA_ARGS__))
// Preprocessor callbacks
#define NPY_CPU_DISPATCH_CALL_ALL_CB_(TESTED_FEATURES, TARGET_NAME, LEFT, ...) \
if (TESTED_FEATURES) { NPY_CAT(NPY_CAT(LEFT, _), TARGET_NAME) __VA_ARGS__; }
((TESTED_FEATURES) ? (NPY_CAT(NPY_CAT(LEFT, _), TARGET_NAME) __VA_ARGS__) : (void) 0),
#define NPY_CPU_DISPATCH_CALL_ALL_BASE_CB_(LEFT, ...) \
{ LEFT __VA_ARGS__; }
( LEFT __VA_ARGS__ )

#endif // NPY_CPU_DISPATCH_H_
10 changes: 5 additions & 5 deletions numpy/core/src/umath/_umath_tests.c.src
Original file line number Diff line number Diff line change
Expand Up @@ -588,11 +588,11 @@ static PyObject *
UMath_Tests_test_dispatch(PyObject *NPY_UNUSED(dummy), PyObject *NPY_UNUSED(dummy2))
{
const char *highest_func, *highest_var;
NPY_CPU_DISPATCH_CALL(highest_func = _umath_tests_dispatch_func, ())
NPY_CPU_DISPATCH_CALL(highest_var = _umath_tests_dispatch_var)
NPY_CPU_DISPATCH_CALL(highest_func = _umath_tests_dispatch_func, ());
NPY_CPU_DISPATCH_CALL(highest_var = _umath_tests_dispatch_var);
Comment on lines +591 to +592
Copy link
Member Author

Choose a reason for hiding this comment

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

Note that these could now be rewritten

Suggested change
NPY_CPU_DISPATCH_CALL(highest_func = _umath_tests_dispatch_func, ());
NPY_CPU_DISPATCH_CALL(highest_var = _umath_tests_dispatch_var);
highest_func = NPY_CPU_DISPATCH_CALL(_umath_tests_dispatch_func)();
highest_var = NPY_CPU_DISPATCH_CALL(_umath_tests_dispatch_var);

Since this is a test, I decided to leave them as they are for now.

Copy link
Member

Choose a reason for hiding this comment

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

You can just change one of them so we can test both behaviors.

const char *highest_func_xb = "nobase", *highest_var_xb = "nobase";
NPY_CPU_DISPATCH_CALL_XB(highest_func_xb = _umath_tests_dispatch_func, ())
NPY_CPU_DISPATCH_CALL_XB(highest_var_xb = _umath_tests_dispatch_var)
NPY_CPU_DISPATCH_CALL_XB(highest_func_xb = _umath_tests_dispatch_func, ());
NPY_CPU_DISPATCH_CALL_XB(highest_var_xb = _umath_tests_dispatch_var);

PyObject *dict = PyDict_New(), *item;
if (dict == NULL) {
Expand All @@ -610,7 +610,7 @@ UMath_Tests_test_dispatch(PyObject *NPY_UNUSED(dummy), PyObject *NPY_UNUSED(dumm
if (item == NULL || PyDict_SetItemString(dict, "all", item) < 0) {
goto err;
}
NPY_CPU_DISPATCH_CALL_ALL(_umath_tests_dispatch_attach, (item))
NPY_CPU_DISPATCH_CALL_ALL(_umath_tests_dispatch_attach, (item));
if (PyErr_Occurred()) {
goto err;
}
Expand Down
0