Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15531 )
Change subject: IMPALA-9544 Replace Intel's SSE instructions with ARM's NEON instructions ...................................................................... Patch Set 41: Code-Review+2 (2 comments) Thanks for all the code cleanup. I think it's not ideal that the files are slightly different from Kudu but I understand the difference now and I think we can move forward. http://gerrit.cloudera.org:8080/#/c/15531/40/be/src/kudu/util/block_bloom_filter.cc File be/src/kudu/util/block_bloom_filter.cc: PS40: > Hi, Tim. This file is same with kudu's, I already compare them. I compared with commit 1c5a04f7774f348b3c9fc4fd39de98c858c87c5f of kudu and it seems to be different? tarmstrong@tarmstrong-box2:~/Impala/impala$ diff -r {be/src/kudu/util/,~/repos/kudu/src/kudu/util/}block_bloom_filter.cc 21,24c21,24 < #include "sse2neon.h" < #else < #include <emmintrin.h> < #include <mm_malloc.h> --- > #include "kudu/util/sse2neon.h" > #else //__aarch64__ > #include <emmintrin.h> > #include <mm_malloc.h> 189a190,195 > #ifdef __aarch64__ > // IWYU pragma: no_include <arm_neon.h> > uint8x16_t new_bucket_neon = vreinterpretq_u8_u32(vld1q_u32(new_bucket + > 4 * i)); > uint8x16_t* existing_bucket = > reinterpret_cast<uint8x16_t*>(&directory_[bucket_idx][4 * i]); > *existing_bucket = vorrq_u8(*existing_bucket, new_bucket_neon); > #else 193a200 > #endif http://gerrit.cloudera.org:8080/#/c/15531/41/be/src/kudu/util/block_bloom_filter.cc File be/src/kudu/util/block_bloom_filter.cc: http://gerrit.cloudera.org:8080/#/c/15531/41/be/src/kudu/util/block_bloom_filter.cc@189 PS41, Line 189: for (int i = 0; i < 2; ++i) { Kudu has some neon-specific code here. I guess the newer version of sse2neon has all the functions that are needed? If so, this is OK, but can you file a Kudu JIRA to clean this up? -- To view, visit http://gerrit.cloudera.org:8080/15531 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id7dfe17125b2910ece54e7dd18b4e4b25d7de8b9 Gerrit-Change-Number: 15531 Gerrit-PatchSet: 41 Gerrit-Owner: Anonymous Coward <[email protected]> Gerrit-Reviewer: Anonymous Coward <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Thu, 13 Aug 2020 03:23:10 +0000 Gerrit-HasComments: Yes
