[opus] Patch cleaning up Opus x86 intrinsics configury

[Jonathan Lennox]

That works for me.  Thanks!

On Mar 7, 2015, at 1:30 PM, Viswanath Puttagunta <viswanath.puttagunta at linaro.org> wrote:

> Hello Jonathan,
> 
> Just FYI, I started doing review of your patch and will get back to
> you in few days. After review, I would like to rebase your patch (as
> necessary) myself and do some testing.. and re-submit.
> 
> Regards,
> Vish
> 
> On 4 March 2015 at 09:00, Viswanath Puttagunta
> <viswanath.puttagunta at linaro.org> wrote:
>> 
>> On 3 March 2015 at 22:17, Jonathan Lennox <jonathan at vidyo.com> wrote:
>>> 
>>> On Mar 3, 2015, at 11:08 PM, Viswanath Puttagunta
>>> <viswanath.puttagunta at linaro.org> wrote:
>>> 
>>> 
>>> 
>>> On 3 March 2015 at 21:59, Jonathan Lennox <jonathan at vidyo.com> wrote:
>>>> 
>>>> Viswenath,
>>>> 
>>>> My patch should be against the tip, but it’s the very recent tip,
>>>> including some changes this past Friday (27 Feb).  I mentioned in the IRC
>>>> room a problem I discovered in creating my patch, and then later improved
>>>> the fix Tim had made for the problem.  Where do you get conflicts merging it
>>>> to tip?
>>>> 
>>>> In terms of merging, you posted your patch before I posted mine, so
>>>> probably I should be the one on the hook to rebase after your fix goes in.
>>>> Looking over your patch quickly, I don’t think any of my changes should be
>>>> that difficult to merge with yours.
>>> 
>>>>> Yeah.. trivial merge issues.. shouldn't take too long to resolve.
>>>> 
>>>> 
>>>> I haven’t studied your patch in depth yet.  Does Ne10 use its own RTCD
>>>> code, or do you use Opus’s?
>>> 
>>>>> Not using Ne10's RTCD feature at the moment.. using Opus's rtcd. libopus
>>>>> RTCD for linux/armv7 will not work for aarch64. It needs to be updated (or
>>>>> just disabled if system built for aarch64 since Neon support is
>>>>> mandatory)... It is in my todo list..
>>> 
>>> 
>>> My patch should handle aarch64 properly.  One of the things it does is it
>>> discovers (in configure) whether a little test program using Neon intrinsics
>>> can be compiled without -mfpu=neon — if so, it disables RTCD for Neon, and
>>> sets a #define OPUS_ARM_PRESUME_NEON_INTR.  (This is analogous to the
>>> existing code for determining whether Neon assembly instructions can be
>>> assembled by default.)
>>> 
>>> This thus handles not only aarch64, but also iOS armv7.
>> This sounds good to me. I wanted to do more thorough code review for
>> your patch, but am waiting until it applies cleanly on tip.. But now
>> that we agree that your patch should be rebased on top of my patch,
>> I'll either wait for Timothy/opus-dev to take in my patch, and your
>> patch to be rebased? And if this is taking too long for my patch to go
>> in (Due to opus-dev maintainers time availability), I'll rebase
>> (retaining your authorship ofcourse) your patch on my submitted
>> patch... and put in my future work on top of your patch.
>> 
>> But I really hope we can get some cycles from Timothy/Jean to get the
>> patches into opus. It seems they are really busy for last month or so.
>> 
>>> 
>>> I also have a number of aarch64 patches pending — mine are entirely for
>>> fixed-point though.  Are yours all floating-point?
>> Yes, so far they are floating point. Can you post your fixed-point
>> optimizations? Also, are any of your patches using NE10 library?
>> 
>> So far, what I submitted is to optimize opus_fft using NE10 (float
>> only). I also modified mdct_forward to take advantage of this
>> optimization.
>> 
>> My next action plan:
>> - use opus_ifft to optimize mdct_backwards (float via Ne10)
>> - Re-work/add opus_fft and opus_ifft for fixed (again via Ne10)
>> - Enable for Aarch64 and test (I'm hoping your patch already does this)
>> - float-to-int and int-to-float conversions using neon
>> - etc.
>> 
>> 
>>> 
>>>> On Mar 3, 2015, at 5:18 PM, Viswanath Puttagunta
>>>> <viswanath.puttagunta at linaro.org> wrote:
>>>> 
>>>>> Hello Jonathan,
>>>>> 
>>>>> I am unable to apply your patch cleanly on tip.
>>>>> 
>>>>> Timothy/opus-dev,
>>>>> 
>>>>> This patch has some conflicts with my ARM patch that does fft
>>>>> optimizations
>>>>> http://lists.xiph.org/pipermail/opus/2015-March/002904.html
>>>>> http://lists.xiph.org/pipermail/opus/2015-March/002905.html
>>>>> 
>>>>> One of us probably has to rebase depending on which patch goes into opus
>>>>> first.
>>>>> 
>>>>> Regards,
>>>>> Vish
>>>>> 
>>>>> 
>>>>> On 1 March 2015 at 20:47, Jonathan Lennox <jonathan at vidyo.com> wrote:
>>>>>> The attached patch cleans up Opus's x86 intrinsics configury.
>>>>>> 
>>>>>> It:
>>>>>> * Makes —enable-intrinsics work with clang and other non-GCC compilers
>>>>>> * Enables RTCD for the floating-point-mode SSE code in Celt.
>>>>>> * Disables use of RTCD in cases where the compiler targets an
>>>>>> instruction
>>>>>> set by default.
>>>>>> * Enables the SSE4.1 Silk optimizations that apply to the common parts
>>>>>> of
>>>>>> Silk when Opus is built in floating-point mode, not just in fixed-point
>>>>>> mode.
>>>>>> * Enables the SSE intrinsics (with RTCD when appropriate) in the Win32
>>>>>> build.
>>>>>> * Fixes a case where GCC would compile SSE2 code as SSE4.1, causing a
>>>>>> crash
>>>>>> on non-SSE4.1 CPUs.
>>>>>> * Allows configuration with compilers with non-GCC-flavor flags for
>>>>>> enabling
>>>>>> architecture options.
>>>>>> * Hopefully makes the configuration and ifdef’s easier to follow and
>>>>>> understand.
>>>>>> 
>>>>>> This does not yet switch —enable-intrinsics to be enabled by default on
>>>>>> supported architectures, but I think it’d be ready to do so.
>>>>>> 
>>>>>> Comments are welcome!
>>>>>> 
>>>>>> 
>>>>>> 
>>>>>> —
>>>>>> Jonathan Lennox
>>>>>> jonathan at vidyo.com
>>>>>> 
>>>>>> 
>>>>>> _______________________________________________
>>>>>> opus mailing list
>>>>>> opus at xiph.org
>>>>>> http://lists.xiph.org/mailman/listinfo/opus
>>>>>> 
>>>> 
>>> 
>>>