You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@impala.apache.org by "Youwei Wang (Code Review)" <ge...@cloudera.org> on 2016/09/01 12:41:16 UTC

[Impala-ASF-CR] Impala-4058: unaligned memory access issue in SIMDed byteswap.

Youwei Wang has uploaded a new change for review.

  http://gerrit.cloudera.org:8080/4204

Change subject: Impala-4058: unaligned memory access issue in SIMDed byteswap.
......................................................................

Impala-4058: unaligned memory access issue in SIMDed byteswap.

Substitute the direct memory assignments using _mm_loadu_si128/_mm_storeu_si128
to prevent the compiler from generating aligned memory access instructions which
may cause crash.
Add new test cases in the expr-test.cc and re-run the bswap-benchmark.
No performance regression observed.

Change-Id: I2986991f8b695d68af465fd0aff293d3d03709a0
---
M be/src/exprs/expr-test.cc
M be/src/util/bit-util.cc
2 files changed, 24 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/04/4204/1
-- 
To view, visit http://gerrit.cloudera.org:8080/4204
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I2986991f8b695d68af465fd0aff293d3d03709a0
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Youwei Wang <yo...@intel.com>

[Impala-ASF-CR] Impala-4058: unaligned memory access issue in SIMDed byteswap.

Posted by "Jim Apple (Code Review)" <ge...@cloudera.org>.
Jim Apple has posted comments on this change.

Change subject: Impala-4058: unaligned memory access issue in SIMDed byteswap.
......................................................................


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/4204/1//COMMIT_MSG
Commit Message:

Line 9: Substitute the direct memory assignments using _mm_loadu_si128/_mm_storeu_si128
Please wrap commit comment lines at 72 characters


http://gerrit.cloudera.org:8080/#/c/4204/1/be/src/exprs/expr-test.cc
File be/src/exprs/expr-test.cc:

Line 1935:   TestStringValue("reverse('abcdefghijklmnop')", "ponmlkjihgfedcba");
I am not sure that these exercise the unaligned loads and stores, or that it will do so on all executions.


-- 
To view, visit http://gerrit.cloudera.org:8080/4204
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I2986991f8b695d68af465fd0aff293d3d03709a0
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Youwei Wang <yo...@intel.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] Impala-4058: unaligned memory access issue in SIMDed byteswap.

Posted by "Jim Apple (Code Review)" <ge...@cloudera.org>.
Jim Apple has abandoned this change.

Change subject: Impala-4058: unaligned memory access issue in SIMDed byteswap.
......................................................................


Abandoned

https://gerrit.cloudera.org/#/c/4205

-- 
To view, visit http://gerrit.cloudera.org:8080/4204
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: abandon
Gerrit-Change-Id: I2986991f8b695d68af465fd0aff293d3d03709a0
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Youwei Wang <yo...@intel.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Youwei Wang <yo...@intel.com>

[Impala-ASF-CR] Impala-4058: unaligned memory access issue in SIMDed byteswap.

Posted by "Youwei Wang (Code Review)" <ge...@cloudera.org>.
Youwei Wang has posted comments on this change.

Change subject: Impala-4058: unaligned memory access issue in SIMDed byteswap.
......................................................................


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/4204/1//COMMIT_MSG
Commit Message:

Line 9: Substitute the direct memory assignments using _mm_loadu_si128/_mm_storeu_si128
> Please wrap commit comment lines at 72 characters
Hi Jim. Thank you for your review.
If you don't mind, this patch will be abandoned soon since we are working on a similar one.


http://gerrit.cloudera.org:8080/#/c/4204/1/be/src/exprs/expr-test.cc
File be/src/exprs/expr-test.cc:

Line 1935:   TestStringValue("reverse('abcdefghijklmnop')", "ponmlkjihgfedcba");
> I am not sure that these exercise the unaligned loads and stores, or that i
Hi Jim. Simply speaking, my idea is to prove the string reverse function can handle such "long" strings using new implementation. And yes, the memory address of these strings are not necessarily aligned or unaligned. So if you think these test cases not necessary to keep, simply abandoning this patch is okay.


-- 
To view, visit http://gerrit.cloudera.org:8080/4204
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I2986991f8b695d68af465fd0aff293d3d03709a0
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Youwei Wang <yo...@intel.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Youwei Wang <yo...@intel.com>
Gerrit-HasComments: Yes