arm64: clang assembler compatability

arm64: clang assembler compatability

Fixes to compile with clang (3.7)

- Fix fmul syntax:
 fmul  v0.2s,v1.2s,v2.2s[0] ->  v0.2s,v1.2s,v2.s[0]
- lowercase RSB macro call to -> rsb
- add w modifier and use U32 in and out for fastlog2()

With these changes openmax_dl compiles as part of chromium on arm64
 with both gcc 5.2 and clang 3.7. Unfortunately there was no
instructions how to run the included tests. The generated .o
files looks same before and after (minor changes in debug sections)

BUG=5090
R=andrew@webrtc.org, rtoy@google.com

Committed: https://chromium.googlesource.com/external/webrtc/deps/third_party/openmax/+/4636f5bb744c0828ac853e1a513e375886fcd424

[Riku Voipio, 2015-10-27 14:21:10]

On 2015/10/27 14:16:55, riku.voipio wrote:

Sorry, the bug link was supposed to go to:
https://code.google.com/p/webrtc/issues/detail?id=5090

[Raymond Toy (Google), 2015-10-30 22:38:46]

https://codereview.webrtc.org/1420973006/diff/1/dl/sp/api/armSP.h
File dl/sp/api/armSP.h (right):

https://codereview.webrtc.org/1420973006/diff/1/dl/sp/api/armSP.h#newcode96
dl/sp/api/armSP.h:96: static inline OMX_U32 fastlog2(OMX_U32 x) {
Why the change from long to OMX_U32?

https://codereview.webrtc.org/1420973006/diff/1/dl/sp/api/armSP.h#newcode99
dl/sp/api/armSP.h:99: "sub %w0, %w0, #63\n\t"
If %w0 means a 32-bit register than clz is probably only returning a max value
of 32, so subtracting 63 is probably wrong.

https://codereview.webrtc.org/1420973006/diff/1/dl/sp/src/arm/arm64/ComplexTo...
File dl/sp/src/arm/arm64/ComplexToRealFixup.S (right):

https://codereview.webrtc.org/1420973006/diff/1/dl/sp/src/arm/arm64/ComplexTo...
dl/sp/src/arm/arm64/ComplexToRealFixup.S:157: fmov     v0.2s, #0.5
I think you should change the definition of half instead of using v0.2s here. 
Or something that makes it clear that you're storing 0.5 into the register half
that is used in other parts of the code below.

https://codereview.webrtc.org/1420973006/diff/1/dl/sp/src/arm/arm64/armSP_FFT...
File dl/sp/src/arm/arm64/armSP_FFTInv_CCSToR_F32_preTwiddleRadix2_s.S (right):

https://codereview.webrtc.org/1420973006/diff/1/dl/sp/src/arm/arm64/armSP_FFT...
dl/sp/src/arm/arm64/armSP_FFTInv_CCSToR_F32_preTwiddleRadix2_s.S:118: fmov   
v13.2s, 0.5
This makes it really hard to follow that half below actually contains 0.5.  Try
to reuse half or change the definition or add a new one.

[Riku Voipio, 2015-11-02 12:24:12]

Thanks for review, I'll provide an updated patch.

https://codereview.webrtc.org/1420973006/diff/1/dl/sp/api/armSP.h
File dl/sp/api/armSP.h (right):

https://codereview.webrtc.org/1420973006/diff/1/dl/sp/api/armSP.h#newcode96
dl/sp/api/armSP.h:96: static inline OMX_U32 fastlog2(OMX_U32 x) {
On 2015/10/30 22:38:46, Raymond Toy (Google) wrote:
> Why the change from long to OMX_U32?

clang throws a warning:
../../third_party/openmax_dl/dl/sp/api/armSP.h:101:14: warning: value size does
not match register size specified by
 the constraint and modifier [-Wasm-operand-widths]
      : "=r"(out)

The only users of this fastlog2 call it with OMX_U32 value (spec->N). So instead
of changing the just the out value to long, I changed the whole function to
32bit. 

I'll update the patch to only change the return type.

https://codereview.webrtc.org/1420973006/diff/1/dl/sp/api/armSP.h#newcode99
dl/sp/api/armSP.h:99: "sub %w0, %w0, #63\n\t"
On 2015/10/30 22:38:46, Raymond Toy (Google) wrote:
> If %w0 means a 32-bit register than clz is probably only returning a max value
> of 32, so subtracting 63 is probably wrong.

Will be fixed in the next iteration by keeping the function 64bit.

https://codereview.webrtc.org/1420973006/diff/1/dl/sp/src/arm/arm64/ComplexTo...
File dl/sp/src/arm/arm64/ComplexToRealFixup.S (right):

https://codereview.webrtc.org/1420973006/diff/1/dl/sp/src/arm/arm64/ComplexTo...
dl/sp/src/arm/arm64/ComplexToRealFixup.S:157: fmov     v0.2s, #0.5
On 2015/10/30 22:38:46, Raymond Toy (Google) wrote:
> I think you should change the definition of half instead of using v0.2s here. 
> Or something that makes it clear that you're storing 0.5 into the register
half
> that is used in other parts of the code below.

Ok, I'll define half = v0.2s, and halfs = v0.s to keep in line with rest of the
definitions (dX0 and dX0s for example). This will result a bit bigger patch.

https://codereview.webrtc.org/1420973006/diff/1/dl/sp/src/arm/arm64/armSP_FFT...
File dl/sp/src/arm/arm64/armSP_FFTInv_CCSToR_F32_preTwiddleRadix2_s.S (right):

https://codereview.webrtc.org/1420973006/diff/1/dl/sp/src/arm/arm64/armSP_FFT...
dl/sp/src/arm/arm64/armSP_FFTInv_CCSToR_F32_preTwiddleRadix2_s.S:118: fmov   
v13.2s, 0.5
On 2015/10/30 22:38:46, Raymond Toy (Google) wrote:
> This makes it really hard to follow that half below actually contains 0.5. 
Try
> to reuse half or change the definition or add a new one.

Likewise, will keep half as it used be and use halfs

[Riku Voipio, 2015-11-02 14:56:14]

Updated patch with more uniform use of half and corrected fastlog2.

[Raymond Toy (Google), 2015-11-03 21:08:55]

lgtm.

Before landing, it would be good to run the tests.  To do that, run gyp on
dl/sp/src/test/test_fft.gyp.  This should generate several executables for
testing.  Just run the executables and the output should tell if you if the
tests pass or not.

I'll try to run the tests soon; it's been a while since I've done an arm64
build.

[Riku Voipio, 2015-11-05 09:59:17]

On 2015/11/03 21:08:55, Raymond Toy (Google) wrote:
> lgtm.
> 
> Before landing, it would be good to run the tests.  To do that, run gyp on
> dl/sp/src/test/test_fft.gyp.  This should generate several executables for
> testing.  Just run the executables and the output should tell if you if the
> tests pass or not.

All tests passed, both when compiled with gcc 4.9 and clang-3.7. The
test_fft_time suggests
clang made slightly faster code (testing on cortex-a53).

> I'll try to run the tests soon; it's been a while since I've done an arm64
> build.

Do you want to verify the tests yourself before landing?

[Raymond Toy (Google), 2015-11-05 21:44:52]

On 2015/11/05 at 09:59:17, riku.voipio wrote:
> On 2015/11/03 21:08:55, Raymond Toy (Google) wrote:
> > lgtm.
> > 
> > Before landing, it would be good to run the tests.  To do that, run gyp on
> > dl/sp/src/test/test_fft.gyp.  This should generate several executables for
> > testing.  Just run the executables and the output should tell if you if the
> > tests pass or not.
> 
> All tests passed, both when compiled with gcc 4.9 and clang-3.7. The
test_fft_time suggests
> clang made slightly faster code (testing on cortex-a53).
> 
> > I'll try to run the tests soon; it's been a while since I've done an arm64
> > build.
> 
> Do you want to verify the tests yourself before landing?

Thanks for running the tests!

lgtm.

Over to ajm@ for final approval.

[Andrew MacDonald, 2015-11-05 23:33:06]

lgtm. I'll land this.

[Andrew MacDonald, 2015-11-05 23:34:43]

Committed patchset #2 (id:20001) manually as
4636f5bb744c0828ac853e1a513e375886fcd424 (presubmit successful).

[Andrew MacDonald, 2015-11-05 23:39:38]

Riku, patch to add you to AUTHORS up here:
https://codereview.webrtc.org/1430973006

Please have a look.