WebRTC Code Reviews
Help | Chromium Project | Sign in
(10881)

Issue 28849004: replace inline assembly WebRtcNsx_AnalysisUpdate by intrinsics. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
5 years ago by zhongwei.yao
Modified:
3 years, 8 months ago
Reviewers:
ajm
CC:
webrtc-reviews_webrtc.org, rillian-moz, tterriberry, kwiberg, aluebs, Bjorn
Base URL:
https://chromium.googlesource.com/external/webrtc@master
Visibility:
Public.

Description

replace inline assembly WebRtcNsx_AnalysisUpdate by intrinsics. The modification only uses the unique part of the analysis_update function. Pass byte to byte conformance test on both ARMv7 and AArch64, and the single function performance is similar with original assembly version on different platforms. If not specified, the code is compiled by GCC 4.6. The result is the "X version / C version" ratio, and the less is better. | run 100k times | cortex-a7 | cortex-a9 | cortex-a15 | | use C as the base on each | (1.2Ghz) | (1.0Ghz) | (1.7Ghz) | | CPU target | | | | |----------------------------+-----------+-----------+------------| | Neon asm | 15.61% | 20.15% | 14.89% | | Neon inline asm (LLVM 3.4) | 25.98% | 33.96% | 18.18% | | Neon intrinsics (GCC 4.6) | 22.06% | 27.01% | 19.24% | | Neon intrinsics (GCC 4.8) | 17.28% | 18.23% | 18.55% | | Neon intrinsics (LLVM 3.4) | 21.02% | 19.98% | 16.76% | BUG= R=andrew@webrtc.org Committed: https://code.google.com/p/webrtc/source/detail?r=7596

Patch Set 1 #

Total comments: 6

Patch Set 2 : update according to comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+47 lines, -58 lines) Patch
M webrtc/modules/audio_processing/ns/nsx_core_neon.c View 1 1 chunk +47 lines, -58 lines 0 comments Download
Trybot results:

Messages

Total messages: 6 (0 generated)
zhongwei.yao
Hi, Andrew, please help review this patch. Like the patch in following url: https://webrtc-codereview.appspot.com/23159004/, this ...
5 years ago (2014-10-28 06:36:27 UTC) #1
zhongwei.yao
On 2014/10/28 06:36:27, zhongwei.yao wrote: > Hi, Andrew, please help review this patch. Like the ...
5 years ago (2014-11-03 02:42:08 UTC) #2
ajm
lgtm, just minor stuff. https://webrtc-codereview.appspot.com/28849004/diff/1/webrtc/modules/audio_processing/ns/nsx_core_neon.c File webrtc/modules/audio_processing/ns/nsx_core_neon.c (right): https://webrtc-codereview.appspot.com/28849004/diff/1/webrtc/modules/audio_processing/ns/nsx_core_neon.c#newcode639 webrtc/modules/audio_processing/ns/nsx_core_neon.c:639: int16_t* p_end_out = out + ...
5 years ago (2014-11-03 06:11:21 UTC) #3
zhongwei.yao
https://webrtc-codereview.appspot.com/28849004/diff/1/webrtc/modules/audio_processing/ns/nsx_core_neon.c File webrtc/modules/audio_processing/ns/nsx_core_neon.c (right): https://webrtc-codereview.appspot.com/28849004/diff/1/webrtc/modules/audio_processing/ns/nsx_core_neon.c#newcode639 webrtc/modules/audio_processing/ns/nsx_core_neon.c:639: int16_t* p_end_out = out + inst->anaLen; On 2014/11/03 06:11:21, ...
5 years ago (2014-11-03 07:10:35 UTC) #4
ajm
Committed patchset #2 (id:20001) manually as 7596 (presubmit successful).
5 years ago (2014-11-03 17:17:59 UTC) #5
spoon.reloaded
3 years, 8 months ago (2016-03-07 19:27:13 UTC) #6
Message was sent while issue was closed.
Hi,
I have reported a bug that might be related to this change
(https://bugs.chromium.org/p/webrtc/issues/detail?id=5631). Can you guys take a
look at it?
Thanks
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 245c2c2-tainted