CRC32 PPC Fast Zero Optimization #2966
Conversation
There was a problem hiding this comment.
@siying has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
@siying Please review the changes |
|
@siying's on leave. Not sure who else on our team can review the patch. |
| @@ -7,6 +7,7 @@ | |||
| // COPYING file in the root directory of this source tree. | |||
There was a problem hiding this comment.
BSD is not compatible with patents. I'm wondering if it is okay for us to include this header in our code.
There was a problem hiding this comment.
@yiwu-arbug The BSD licenses were approved by facebook legal in the past, for earlier drops that @kamasubb has submitted.
Links to prior discussion on that topic:
The pull requests show the discussion. The commit shows 2716 was merged with these BSD licenses previously. This new drop provides a performance enhancement to that previous crc work if the buffer to be crc'ed is zero filled. The caller indicates the buffer is zero filled by passing in a null data pointer and a non-zero buffer length.
| // All rights reserved. | ||
| // This source code is licensed under the BSD-style license found in the | ||
| // LICENSE file in the root directory of this source tree. An additional grant | ||
| // of patent rights can be found in the PATENTS file in the same directory. |
|
@yiwu-arbug If there are no more comments, can you please approve the changes |
| @@ -1733,6 +1736,9 @@ jl/crc32c_ppc.o: util/crc32c_ppc.c | |||
|
|
|||
| jl/crc32c_ppc_asm.o: util/crc32c_ppc_asm.S | |||
| $(AM_V_CC)$(CC) $(CFLAGS) -c $< -o $@ | |||
|
|
|||
| jl/crc32c_ppc_fast_zero_asm.o: util/crc32c_ppc_fast_zero_asm.S | |||
There was a problem hiding this comment.
can you also make sure the build target rocksdbjavastatic works? It builds everything under "jls/" not "jl/"
There was a problem hiding this comment.
@ajkr I have verified that the target rocksdbjavastatic builds everything under jls and not jl.
There was a problem hiding this comment.
I agree with that but still don't know whether the rocksdbjavastatic build target will work after this change. Have you tested it? My concern is we may need to add this target: "jls/util/crc32c_ppc_fast_zero_asm.o".
|
@kamasubb has updated the pull request. View: changes, changes since last import |
|
I lost my memory for this PR. Where are we? |
|
Was waiting for confirmation that |
Fast Zero optimization for CRC32 computation in PowerPC