kou commented on code in PR #49580:
URL: https://github.com/apache/arrow/pull/49580#discussion_r2975391348


##########
cpp/src/arrow/util/bpacking_simd_kernel_internal.h:
##########
@@ -200,14 +201,16 @@ auto left_shift(const xsimd::batch<Int, Arch>& batch,
     return xsimd::bitwise_cast<Int>(shifted0 | shifted1);
   }
 
-  // TODO(xsimd) bug fixed likely in xsimd>14.0.0
+  // TODO(xsimd) bug fixed in xsimd 14.1.0

Review Comment:
   Can we remove `TODO` because the bug was fixed?
   
   ```suggestion
     // For a bug fixed in xsimd 14.1.0
   ```



##########
cpp/src/arrow/util/bpacking_simd_kernel_internal.h:
##########
@@ -237,7 +240,8 @@ auto right_shift_by_excess(const xsimd::batch<Int, Arch>& 
batch,
   constexpr auto IntSize = sizeof(Int);
 
   // Architecture for which there is no variable right shift but a larger 
fallback exists.
-  /// TODO(xsimd) Tracking for Avx2 in 
https://github.com/xtensor-stack/xsimd/pull/1220
+  // TODO(xsimd) Tracking for Avx2 in 
https://github.com/xtensor-stack/xsimd/pull/1220

Review Comment:
   It seems that the PR is also merged. Do we need `#if ...` for this too?



##########
cpp/src/arrow/util/bpacking_simd_kernel_internal.h:
##########
@@ -265,14 +269,16 @@ auto right_shift_by_excess(const xsimd::batch<Int, Arch>& 
batch,
     return xsimd::bitwise_rshift<kMaxRShift>(left_shift(batch, kLShifts));
   }
 
-  // TODO(xsimd) bug fixed likely in xsimd>14.0.0
+  // TODO(xsimd) bug fixed in xsimd 14.1.0

Review Comment:
   Can we remove `TODO` because the bug was fixed?
   
   ```suggestion
     // Fox a bug fixed in xsimd 14.1.0
   ```



-- 
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]

Reply via email to