<p style="white-space: pre-wrap; word-wrap: break-word;">-1 just for the legality of the copyright notice. the other comment is not worth a -1.</p><p>Patch set 1:<span style="border-radius: 3px; display: inline-block; margin: 0 2px; padding: 4px;background-color: #ffd4d4;">Code-Review -1</span></p><p><a href="https://gerrit.osmocom.org/c/libosmocore/+/19372">View Change</a></p><p>3 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/libosmocore/+/19372/1//COMMIT_MSG">Commit Message:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/libosmocore/+/19372/1//COMMIT_MSG@11">Patch Set #1, Line 11:</a> </p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><pre style="font-family: monospace,monospace; white-space: pre-wrap;">Although autodetection according to __ARM_NEON would work because this<br>is only defined if the fpu is neon neon-fp16 neon-vfpv3 eon-vfpv4<br>neon-fp-armv8 crypto-neon-fp-armv8 doing that would lead to a unknown<br>performance impact, so it needs to be enabled manually.<br></pre></blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">I don't like this kind of approach.  We have auto-detection for x86 SIMD instructions, and we should try to align with that for NEON.  The detection would only have to be done once at startup, right?  Projects like ffmpeg are doing run-time detection of CPU features for a very long time.</p><p style="white-space: pre-wrap; word-wrap: break-word;">So without even knowing how bad the impact would be, I'm not sure we should disable it.</p><p style="white-space: pre-wrap; word-wrap: break-word;">The only question would be whether or not we should enable it by default in all our builds anyway.  I'm not sure who is running osmo-bts-trx on an ARM without NEON support.  We have plenty of osmo-bts-sysmo on old cores, probably also -octphy. But we've never heard of an osmo-bts-trx user who doesn't have a modern, NEON enabled core, AFAICT.</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/libosmocore/+/19372/1/src/conv_acc_neon.c">File src/conv_acc_neon.c:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/libosmocore/+/19372/1/src/conv_acc_neon.c@5">Patch Set #1, Line 5:</a> <code style="font-family:monospace,monospace">Copyright (C) 2020 Eric Wild <ewild@sysmocom.de></code></p><p style="white-space: pre-wrap; word-wrap: break-word;">erm, coypright by sysmocom, Author is you. At least as long as it was done during work hours :P</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/libosmocore/+/19372/1/src/conv_acc_neon_impl.h">File src/conv_acc_neon_impl.h:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/libosmocore/+/19372/1/src/conv_acc_neon_impl.h@5">Patch Set #1, Line 5:</a> <code style="font-family:monospace,monospace"> </code></p><p style="white-space: pre-wrap; word-wrap: break-word;">same here of course</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.osmocom.org/c/libosmocore/+/19372">change 19372</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/c/libosmocore/+/19372"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: libosmocore </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: I58ff2cb4ce3514f43390ff0a2121f81e6a4983b5 </div>
<div style="display:none"> Gerrit-Change-Number: 19372 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Hoernchen <ewild@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-Reviewer: fixeria <vyanitskiy@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: laforge <laforge@osmocom.org> </div>
<div style="display:none"> Gerrit-Comment-Date: Thu, 23 Jul 2020 08:26:09 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-Has-Labels: Yes </div>
<div style="display:none"> Gerrit-MessageType: comment </div>