rmuir commented on code in PR #14896:
URL: https://github.com/apache/lucene/pull/14896#discussion_r2204957249


##########
lucene/core/src/java/org/apache/lucene/util/Constants.java:
##########
@@ -152,6 +163,17 @@ private static boolean hasFastScalarFMA() {
     return false;
   }
 
+  private static boolean hasFastCompress() {
+    if (OS_ARCH.equals("aarch64") && MAC_OS_X == false && HAS_SVE) {
+      return true;
+    }
+
+    if (OS_ARCH.equals("amd64") && HAS_AVX2 && MAX_VECTOR_SIZE >= 32) {
+      return true;
+    }

Review Comment:
   ```suggestion
       if (OS_ARCH.equals("aarch64") && HAS_SVE) {
         return true;
       }
   
       if (OS_ARCH.equals("amd64") && HAS_AVX2) {
         return true;
       }
   ```
   
   Looks good, I think we can simplify a bit the conditions:
   * for amd64: AVX2 always supports 32 bit vectors
   * for aarch64: we can remove the macos. If a future mac gets SVE, then it 
should be able to use it.
   
   With your new HAS_SVE/HAS_AVX2 we can maybe simplify some checks elsewhere, 
we were using vector size as a substitute for a proper HAS_AVX2 before.



-- 
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: issues-unsubscr...@lucene.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org

Reply via email to