<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=Windows-1252">
</head>
<body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; color: rgb(0, 0, 0); font-size: 14px; font-family: Calibri, sans-serif;">
<div>Yes, regarding the unsigned to signed conversion you are right, it is implementation defined. I just had an issue a couple of years ago with a compiler which incorrectly treated unsigned overflow as undefined rather than implementation defined…</div>
<div><br>
</div>
<div>Regarding the 64 bit profiling: I looked at the disassembly (gcc –c –S –O2 ../opus/silk/sum_sqr_shift.c –I../opus/include –I../opus/celt) of the 64 bit accumulator version (unrolled twice like the current code) and found that, as well as having only one
loop, the loop has 12 instructions per iteration.</div>
<div><br>
</div>
<div>The current version (after fixing the bug) gives 12 instructions per iteration until shift becomes non zero (first loop). After that there are 16 instructions per iteration in the second loop, and that without counting the additional instructions we jump
to when we need to handle the overflow and increase the shift.</div>
<div>Although this is not proper profiling, it is good enough for me. And considering the increase in code clarity, I would definitely go for the 64 bit version.</div>
<div><br>
</div>
<div>Just for kicks, I also made an experimental version that uses 32 bits but leaves 8 bits margin before accumulating, and that shaves 2 instructions per loop, making it 10. All in all, I am not sure it is worth the hassle though considering this is only
about 1/order of the cost of the autocorrelation…</div>
<div><br>
</div>
<div>Marcello</div>
<div><br>
</div>
<div><br>
</div>
<span id="OLK_SRC_BODY_SECTION">
<div style="font-family:Calibri; font-size:11pt; text-align:left; color:black; BORDER-BOTTOM: medium none; BORDER-LEFT: medium none; PADDING-BOTTOM: 0in; PADDING-LEFT: 0in; PADDING-RIGHT: 0in; BORDER-TOP: #b5c4df 1pt solid; BORDER-RIGHT: medium none; PADDING-TOP: 3pt">
<span style="font-weight:bold">From: </span>KOEN VOS <<a href="mailto:koenvos74@gmail.com">koenvos74@gmail.com</a>><br>
<span style="font-weight:bold">Date: </span>Friday, 20 June 2014 22:04<br>
<span style="font-weight:bold">To: </span>"Timothy B. Terriberry" <<a href="mailto:tterribe@xiph.org">tterribe@xiph.org</a>><br>
<span style="font-weight:bold">Cc: </span>"<a href="mailto:opus@xiph.org">opus@xiph.org</a>" <<a href="mailto:opus@xiph.org">opus@xiph.org</a>>, Marcello Caramma <<a href="mailto:mcaramma@cisco.com">mcaramma@cisco.com</a>><br>
<span style="font-weight:bold">Subject: </span>Re: [opus] Alleged bug in Silk codec<br>
</div>
<div><br>
</div>
<div>
<div>
<div dir="ltr">
<div>
<div>
<div>Yes those instructions exist, although they're a bit slower than the basic 16x16->32 with 32-bit accumulation (SMLABB). So I'd be surprised if the function with 64 bit accumulation would run as fast as the current code. Don't know how much we care about
16-bit platforms. And accuracy should not matter.<br>
</div>
<br>
On the other hand, a 64-bit implementation is much cleaner/shorter, which is always a good argument :-)<br>
</div>
All in all, no strong preference from my side.<br>
</div>
koen.<br>
</div>
<div class="gmail_extra"><br>
<br>
<div class="gmail_quote">On Fri, Jun 20, 2014 at 12:40 PM, Timothy B. Terriberry <span dir="ltr">
<<a href="mailto:tterribe@xiph.org" target="_blank">tterribe@xiph.org</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="">KOEN VOS wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
current implementation is faster on a 32 or 16 bit processor. If you<br>
would find the opposite to be true (ie that a 64 bit implementation is<br>
faster on, say, a 32 bit ARM CPU) then perhaps we should reconsider.<br>
</blockquote>
<br>
</div>
Doesn't ARMv6 have a dual signed 16x16->32 multiply with a 64-bit accumulator (SMLALD)? Even v5E should have a single 16x16->32 with a 64-bit accumulator (SMLALBB). I would think a 64-bit version could be made pretty fast on 32-bit ARM, without even resorting
to SIMD.<br>
</blockquote>
</div>
<br>
</div>
</div>
</div>
</span>
</body>
</html>