BUG: shift operator cycles, fixes #2449 #7473
Conversation
|
|
||
| #define LEFT_SHIFT_OP \ | ||
| do { \ | ||
| if (NPY_LIKELY(in2 < sizeof(@type@) * 8)) { \ |
There was a problem hiding this comment.
I think there are number of bits macros defined somewhere... yep, in npy_common.h. The have the form NPY_BITSOF_LONGLONG etc.
There was a problem hiding this comment.
Probably already included here.
There was a problem hiding this comment.
Does this actually buy speed over ...? ... : ...?
There was a problem hiding this comment.
A cleaner version modulo signedness might be
#define LEFT_SHIFT = do {*out = NPY_LIKELY(in2 < NPY_BITSOF_@TYPE@) ? in1 << in2 : 0} while(0)
Although I don't think we actually need the do ... while(0) bit here.
There was a problem hiding this comment.
Macro removed in #13739. Kept the if statements because I think they're clearer. Did not use NPY_BITS_OF_LONG and friends because they don't exist for NPY_BITS_OF_ULONG etc.
|
Might want to also check the scalar math in |
| { | ||
| if (IS_BINARY_REDUCE) { | ||
| BINARY_REDUCE_LOOP(@type@) { | ||
| @type@ ip2_val = *(@type@ *)ip2; |
There was a problem hiding this comment.
Could remove the reduce specialization, I suspect reduce using the shift operators would be rare and BINARY_LOOP_FAST should have adequate performance.
|
Be good to finish this up for 1.12. |
|
@jaimefrio How is your time looking? |
|
I'll take a look at it during this week/weekend. |
|
@jaimefrio Waiting... If you don't have the time let me know and I'll try to fix it up. |
|
☔ The latest upstream changes (presumably #7980) made this pull request unmergeable. Please resolve the merge conflicts. |
|
I wanted to get this into 1.12 but don't have the time at the moment, so pushing off to 1.13. |
|
Pushed off to 1.15. |
|
@jaimefrio - ping? |
|
Pushing off again. |
|
Pushed off again. |
|
I don't have permissions to push fixups to this branch, so I fixed the conflicts and addressed comments in gh-13739 |
|
@eric-wieser OK, closing this. |
This: * Inlines the macros in loops.c.src * Replaces 8 with `CHAR_BIT `. The `NPY_SIZEOF_*` macros are not used here because it's too much work to apply them to the signed types, and they expand to the same thing anyway. * Removes the reduce loop specializations which likely no one cares about * Uses pytest.mark.parametrize to shorten the test
#2449