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

Issue 23349004: replace inline assembly WebRtcAecm_CalcLinearEnergiesNeon by intrinsics. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 years, 8 months ago by zhongwei.yao
Modified:
4 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 WebRtcAecm_CalcLinearEnergiesNeon by intrinsics. The modification only uses the unique part of the CalcLinearEnergies function. Pass byte to byte conformance test both on ARMv7 and ARM64, 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 | 19.48% | 19.26% | 13.68% | | Neon inline | 27.90% | 28.87% | 17.79% | | Neon intrinsics (GCC 4.8) | 18.69% | 20.18% | 14.69% | | Neon intrinsics (LLVM 3.4) | 18.52% | 21.15% | 13.56% | BUG=3580 R=andrew@webrtc.org Committed: https://code.google.com/p/webrtc/source/detail?r=7686

Patch Set 1 #

Total comments: 4

Patch Set 2 : Changing the macro to inline function and clearing code #

Total comments: 3

Patch Set 3 : Updating function name. #

Patch Set 4 : Updating to comment. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+65 lines, -52 lines) Patch
M webrtc/modules/audio_processing/aecm/aecm_core_neon.c View 1 2 3 1 chunk +65 lines, -52 lines 0 comments Download
Trybot results:

Messages

Total messages: 16 (0 generated)
zhongwei.yao
Hi, Andrew, This patch passed module_unittests. But I don't know what is the correct option ...
4 years, 8 months ago (2014-11-04 09:24:04 UTC) #1
ajm
On 2014/11/04 09:24:04, zhongwei.yao wrote: > Hi, Andrew, > This patch passed module_unittests. But I ...
4 years, 8 months ago (2014-11-05 05:52:19 UTC) #2
ajm
https://webrtc-codereview.appspot.com/23349004/diff/1/webrtc/modules/audio_processing/aecm/aecm_core_neon.c File webrtc/modules/audio_processing/aecm/aecm_core_neon.c (right): https://webrtc-codereview.appspot.com/23349004/diff/1/webrtc/modules/audio_processing/aecm/aecm_core_neon.c#newcode231 webrtc/modules/audio_processing/aecm/aecm_core_neon.c:231: #define WEBRTCAECM_ADD_LANES(ptr, v) \ Any reason you can't add ...
4 years, 8 months ago (2014-11-05 05:53:10 UTC) #3
zhongwei.yao
On 2014/11/05 05:52:19, ajm wrote: > On 2014/11/04 09:24:04, zhongwei.yao wrote: > > Hi, Andrew, ...
4 years, 8 months ago (2014-11-05 11:13:58 UTC) #4
zhongwei.yao
https://webrtc-codereview.appspot.com/23349004/diff/1/webrtc/modules/audio_processing/aecm/aecm_core_neon.c File webrtc/modules/audio_processing/aecm/aecm_core_neon.c (right): https://webrtc-codereview.appspot.com/23349004/diff/1/webrtc/modules/audio_processing/aecm/aecm_core_neon.c#newcode231 webrtc/modules/audio_processing/aecm/aecm_core_neon.c:231: #define WEBRTCAECM_ADD_LANES(ptr, v) \ On 2014/11/05 05:53:10, ajm wrote: ...
4 years, 8 months ago (2014-11-05 11:25:14 UTC) #5
ajm
On 2014/11/05 11:13:58, zhongwei.yao wrote: > Hi, Andrew, > I've rebased to current master. But ...
4 years, 8 months ago (2014-11-05 17:04:28 UTC) #6
ajm
https://webrtc-codereview.appspot.com/23349004/diff/1/webrtc/modules/audio_processing/aecm/aecm_core_neon.c File webrtc/modules/audio_processing/aecm/aecm_core_neon.c (right): https://webrtc-codereview.appspot.com/23349004/diff/1/webrtc/modules/audio_processing/aecm/aecm_core_neon.c#newcode231 webrtc/modules/audio_processing/aecm/aecm_core_neon.c:231: #define WEBRTCAECM_ADD_LANES(ptr, v) \ On 2014/11/05 11:25:14, zhongwei.yao wrote: ...
4 years, 8 months ago (2014-11-05 17:05:49 UTC) #7
zhongwei.yao
On 2014/11/05 17:05:49, ajm wrote: > https://webrtc-codereview.appspot.com/23349004/diff/1/webrtc/modules/audio_processing/aecm/aecm_core_neon.c > File webrtc/modules/audio_processing/aecm/aecm_core_neon.c (right): > > https://webrtc-codereview.appspot.com/23349004/diff/1/webrtc/modules/audio_processing/aecm/aecm_core_neon.c#newcode231 > ...
4 years, 8 months ago (2014-11-06 07:57:48 UTC) #8
ajm
On 2014/11/06 07:57:48, zhongwei.yao wrote: > The difference is not big. And I also can ...
4 years, 8 months ago (2014-11-06 16:10:30 UTC) #9
zhongwei.yao
On 2014/11/05 17:04:28, ajm wrote: > On 2014/11/05 11:13:58, zhongwei.yao wrote: > > Hi, Andrew, ...
4 years, 8 months ago (2014-11-07 07:13:12 UTC) #10
ajm
On 2014/11/07 07:13:12, zhongwei.yao wrote: > Hi, Andrew, > > When rebased to latest master, ...
4 years, 8 months ago (2014-11-10 17:01:23 UTC) #11
ajm
lgtm with one change. https://webrtc-codereview.appspot.com/23349004/diff/20001/webrtc/modules/audio_processing/aecm/aecm_core_neon.c File webrtc/modules/audio_processing/aecm/aecm_core_neon.c (right): https://webrtc-codereview.appspot.com/23349004/diff/20001/webrtc/modules/audio_processing/aecm/aecm_core_neon.c#newcode231 webrtc/modules/audio_processing/aecm/aecm_core_neon.c:231: static inline void webrtcaecm_add_lanes(uint32_t* ptr, ...
4 years, 8 months ago (2014-11-10 17:04:23 UTC) #12
zhongwei.yao
https://webrtc-codereview.appspot.com/23349004/diff/20001/webrtc/modules/audio_processing/aecm/aecm_core_neon.c File webrtc/modules/audio_processing/aecm/aecm_core_neon.c (right): https://webrtc-codereview.appspot.com/23349004/diff/20001/webrtc/modules/audio_processing/aecm/aecm_core_neon.c#newcode231 webrtc/modules/audio_processing/aecm/aecm_core_neon.c:231: static inline void webrtcaecm_add_lanes(uint32_t* ptr, uint32x4_t v) { On ...
4 years, 8 months ago (2014-11-11 06:31:48 UTC) #13
ajm
https://webrtc-codereview.appspot.com/23349004/diff/20001/webrtc/modules/audio_processing/aecm/aecm_core_neon.c File webrtc/modules/audio_processing/aecm/aecm_core_neon.c (right): https://webrtc-codereview.appspot.com/23349004/diff/20001/webrtc/modules/audio_processing/aecm/aecm_core_neon.c#newcode231 webrtc/modules/audio_processing/aecm/aecm_core_neon.c:231: static inline void webrtcaecm_add_lanes(uint32_t* ptr, uint32x4_t v) { On ...
4 years, 8 months ago (2014-11-11 07:19:02 UTC) #14
zhongwei.yao
On 2014/11/11 07:19:02, ajm wrote: > https://webrtc-codereview.appspot.com/23349004/diff/20001/webrtc/modules/audio_processing/aecm/aecm_core_neon.c > File webrtc/modules/audio_processing/aecm/aecm_core_neon.c (right): > > https://webrtc-codereview.appspot.com/23349004/diff/20001/webrtc/modules/audio_processing/aecm/aecm_core_neon.c#newcode231 > ...
4 years, 8 months ago (2014-11-11 07:50:54 UTC) #15
ajm
4 years, 8 months ago (2014-11-11 19:34:18 UTC) #16
Message was sent while issue was closed.
Committed patchset #4 (id:60001) manually as 7686 (presubmit successful).
Sign in to reply to this message.

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