-
Notifications
You must be signed in to change notification settings - Fork 25
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Intel flags are applied to PowerPC and break the build: cc1: error: unrecognized command line option "-mavx" #56
Comments
Notice this happens despite |
Yeah, this is the bug: Lines 140 to 146 in 00890a6
This gonna break the build for any OS on PowerPC, MIPS etc. |
This is intentional to support the cross-compiling workflow for the FreeDV application. From the top-level
You should be able to explicitly specify BTW I'm not sure if there are any PPC machines that could satisfactorily run this version of LPCNet or FreeDV 2020 modes in general. (I only have access to Intel and ARM based Macs here for development.) Definitely let us know if your testing does prove that they can. |
@tmiw Thank you for responding. Since existing code breaks the build for several archs, I believe it is wrong, regardless of the fact that it facilitates something else. It can be written in a way cross-compiling works, and PPC (or RISC-V, MIPS etc.) are not broken. Requiring the user to manually set flags (which is also undocumented, apparently) is a suboptimal solution. Especially given that people will expect
So far I only confirmed it builds fine (after CMakeLists are fixed), but I can do testing, if there are proper tests. Macports turns off testing for this port even on Intel, with a note that tests are Linux-only (I have no idea if that is accurate). P. S. If it will be determined for sure that PPC (or any other arch for that matter) cannot work in principle, it should be a) documented and b) the build should err out at configure with a clear message why it fails. Until proven otherwise, however, I would assume it either works (at least on Linux PPC) or can work with some fixes. |
@tmiw
|
@tmiw I looked into the source of At the moment, this is the only test which fails. For the reference, test log: |
Hi @barracuda156 - using your target machine, can you pls post the output of
So 31 seconds for a 49 second wave file on my machine. So the key question is will the code run in real time on PPC etc. This is a experimental fork of LPCNet that we have been using to try out Neural speech coding with Ham radio. We're not pitching it at widespread use outside of the FreeDV community at this time. It may be deprecated in the next few years as our project and neural speech coding evolves. What is your use case for this repo? |
@drowe67 Thank you for responding! Here is the output:
Not fast, I understand :) But we use non-vectorized code, and this is 2005 machine. Re use case: we got two ports which depend on LPCNet: https://ports.macports.org/port/lpcnetfreedv/details |
So unfortunately the code in this repo can't run in real time so will be of no use to your end users - FreeDV decodes off air signal in real time. Can I suggest you build codec2 without the LPCNet option (see codec2/README.md)? @tmiw - does freedv-gui still build without LPCNet? |
IIRC it required LPCNet a while back. I haven't tried recently, though. Regardless, there is logic in the freedv-gui code to prevent use of 2020/2020B if AVX isn't available (x86 only) or if the 2020 decode is slower than twice the speed of real time (i.e. it needs to take ~0.5s* or less to decode a 1s audio sample). * I don't think it's exactly 0.5s anymore; I remember adding a buffer a while back per user feedback but I don't remember what it got set to.
I think the lack of documentation for this is a fair point. This could perhaps be renamed to I do have a few concerns about PR #57 that was opened for this issue:
That said, I took a look at how LPCNet is being built for Windows and macOS and
So I could be worrying too much about the |
Actually I feel this is the key to this Issue. Rather than relying on a run time test, and committing our team to support and maintenance on platforms where it can't possibly work - I'd prefer to not build LPCNet for platforms where it can't possibly run, and have the higher layer build systems deal with that at build time (like codec2 does). |
@drowe67 I will address other points when I am not on a mobile app, but couple of things:
|
FWIW, I was able to get freedv-gui to compile without LPCNet, but it definitely needed code and CMakeLists.txt changes to do so. The good news is that it needed fewer changes than I thought it would, at least for the proof of concept (a neater solution would of course need more work). One issue is that Speaking of the runtime test, I still think one is necessary even if we conditionally compile LPCNet. For example, there are ARM systems that have NEON but aren't necessarily powerful enough to satisfactorily decode 2020 (e.g. Raspberry Pi). |
I would assume some CoreDuo would not work either, since they are presumably slower that the last G5s. Or if they do, then G5 may also work, once AltiVec backend is added. |
@tmiw Regarding universal builds on macOS. The problem here is that GNU GCC does not support universal builds at all – at least until upstream fixes its Now, Macports has its own implementation for universal builds which does work with modern GCC, however it has issues with combining Intel with PowerPC. Long story short, current ports for GCC do not build as genuinely universal and won’t support x86_86+ppc(64) targets. They do support i386+x86_64 and ppc+ppc64 though. Clang is broken for PowerPC, so if anyone is building on macOS PowerPC, they will be using GCC. Having all this in mind, I think it is a sane choice to make it either Intel+ARM (which would require Clang) or ppc+ppc64 (gonna work with Macports environment only – so do not force-enable universal in this case). |
@barracuda156, thanks for that feedback. We'll need to discuss further as a PLT how this will be resolved (if at all). Once we have any further updates, we'll update this issue accordingly. |
@tmiw Thank you! P. S. Just to be clear, universal builds issue is of very low priority, and I am fine with leaving it as is (i.e. keeping Intel+ARM only). This we can fix in Macports locally, and that kind of patch will be easy to carry, be it needed. I am also not particularly concerned about deployment target setting, as Macports overrides it with correct setting. (However it makes no sense to set it to 10.9 for PowerPC, since any SDK after 10.6 lacks PPC slices.) |
I reimplemented the previously proposed PR in what should hopefully be a lower impact manner: #59 |
@tmiw Thank you very much! |
The text was updated successfully, but these errors were encountered: