mbutrovich commented on code in PR #11580:
URL: https://github.com/apache/arrow-rs/pull/11580#discussion_r4314714663
##########
arrow-buffer/src/bigint/div.rs:
##########
@@ -59,6 +59,29 @@ fn bits(arr: &[u64]) -> usize {
/// Division of numerator by a u64 divisor
fn div_rem_small<const N: usize>(numerator: &[u64; N], divisor: u64) -> ([u64;
N], [u64; N]) {
+ // x86_64 divides by one word with `div`. On 32-bit targets, including
wasm32, each
+ // 64x64-to-128-bit product in the reciprocal path would call `__multi3`
+ #[cfg(any(
+ all(target_arch = "x86_64", not(miri)),
+ not(target_pointer_width = "64")
+ ))]
+ {
+ div_rem_small_word(numerator, divisor)
+ }
+
+ // Here `div_rem_word` would call `__udivti3` for every digit
+ #[cfg(all(target_pointer_width = "64", any(not(target_arch = "x86_64"),
miri)))]
+ {
+ div_rem_small_reciprocal(numerator, divisor)
+ }
Review Comment:
> The cfg attributes makes it harder to understand the fallback here, maybe
add a a comment which is the fallback and move it to be the last like most
people assume
Good idea. The reciprocal path is now first, with a comment that says what
its `cfg` selects: the 64-bit targets where `div_rem_word` is a `u128` division
rather than the `div` instruction. That condition is the same as the one in
`div_rem_word`. The `div_rem_word` path is last, and its `cfg` is `not(...)` of
the first one's, so it reads as the fallback.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
