Youwei Wang has posted comments on this change. Change subject: IMPALA-3604: Clean up SSE handling ......................................................................
Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/3227/1/be/src/util/sse-util.h File be/src/util/sse-util.h: Line 85: /// CPU requirement is raised to at least the targeted instruction set. > Please do not create empty #if blocks. Hi Jim. Since this becomes a blank #if block after its internal #error is commented out, and the remained comment is contradictory to IMPALA-3604, can I remove this entire #if block for I guess it is no longer needed here? Line 97: static inline __m128i SSE4_cmpestrm(__m128i str1, int len1, __m128i str2, int len2) { > Why did you remove the comment that was here? Sorry, it seems I removed them by accident. Line 99: __asm__ volatile ("pcmpestrm %5, %2, %1" > Can clang use the intrinsics? Clang doesn't support __attribute__ for clang version < 3.8. (https://godbolt.org/g/PUcrhU) In this case, we have to modify the build configuration if we add some SSE4.2 intrinsics here. But this is not allowed. So clang itself can use intrinsics. However, due to missing support for the __attribute__, we can't use the intrinsics directly here for clang like the GCC code. -- To view, visit http://gerrit.cloudera.org:8080/3227 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id346187f3248922ec2a323548de4bdeaccd7a771 Gerrit-PatchSet: 1 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Youwei Wang <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Jim Apple <[email protected]> Gerrit-Reviewer: Mostafa Mokhtar <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Reviewer: Youwei Wang <[email protected]> Gerrit-HasComments: Yes
