pitrou commented on code in PR #51340:
URL: https://github.com/apache/arrow/pull/51340#discussion_r4387051696


##########
cpp/cmake_modules/SetupCxxFlags.cmake:
##########
@@ -547,6 +565,24 @@ if(ARROW_CPU_FLAG STREQUAL "aarch64")
     message(WARNING "ARROW_SIMD_LEVEL=${ARROW_SIMD_LEVEL} not supported by 
Arm.")
   endif()
 endif()
+if(ARROW_CPU_FLAG STREQUAL "riscv64")
+  if(ARROW_SIMD_LEVEL MATCHES "RVV")
+    if(NOT CXX_SUPPORTS_RVV)
+      message(FATAL_ERROR "RVV required but compiler doesn't support it.")
+    endif()
+    set(CXX_COMMON_FLAGS "${CXX_COMMON_FLAGS} -march=${ARROW_RVV_MARCH}")
+    string(REGEX MATCH "[0-9]+" RVV_VECTOR_BITS ${ARROW_SIMD_LEVEL})
+    if(RVV_VECTOR_BITS)
+      set(ARROW_HAVE_RVV${RVV_VECTOR_BITS} ON)
+      add_definitions(-DARROW_HAVE_RVV${RVV_VECTOR_BITS})
+    else()
+      set(ARROW_HAVE_RVV ON)

Review Comment:
   Perhaps call it `ARROW_HAVE_RVV_SIZELESS` just like SVE above?



##########
cpp/cmake_modules/SetupCxxFlags.cmake:
##########
@@ -547,6 +565,24 @@ if(ARROW_CPU_FLAG STREQUAL "aarch64")
     message(WARNING "ARROW_SIMD_LEVEL=${ARROW_SIMD_LEVEL} not supported by 
Arm.")
   endif()
 endif()
+if(ARROW_CPU_FLAG STREQUAL "riscv64")

Review Comment:
   Nit: add line break as above.
   
   ```suggestion
   
   if(ARROW_CPU_FLAG STREQUAL "riscv64")
   ```



##########
cpp/cmake_modules/SetupCxxFlags.cmake:
##########
@@ -547,6 +565,24 @@ if(ARROW_CPU_FLAG STREQUAL "aarch64")
     message(WARNING "ARROW_SIMD_LEVEL=${ARROW_SIMD_LEVEL} not supported by 
Arm.")
   endif()
 endif()
+if(ARROW_CPU_FLAG STREQUAL "riscv64")
+  if(ARROW_SIMD_LEVEL MATCHES "RVV")
+    if(NOT CXX_SUPPORTS_RVV)
+      message(FATAL_ERROR "RVV required but compiler doesn't support it.")
+    endif()
+    set(CXX_COMMON_FLAGS "${CXX_COMMON_FLAGS} -march=${ARROW_RVV_MARCH}")
+    string(REGEX MATCH "[0-9]+" RVV_VECTOR_BITS ${ARROW_SIMD_LEVEL})
+    if(RVV_VECTOR_BITS)
+      set(ARROW_HAVE_RVV${RVV_VECTOR_BITS} ON)
+      add_definitions(-DARROW_HAVE_RVV${RVV_VECTOR_BITS})

Review Comment:
   Shouldn't we also enable a compiler option for the given vector size, as in 
SVE above?



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