Opened 5 years ago
Closed 5 years ago
#60403 closed defect (fixed)
lpcnetfreedv: incorrect optimization when building universal
Reported by: | ryandesign (Ryan Carsten Schmidt) | Owned by: | ra1nb0w |
---|---|---|---|
Priority: | Normal | Milestone: | |
Component: | ports | Version: | 2.6.2 |
Keywords: | Cc: | ||
Port: | lpcnetfreedv |
Description
The lpcnetfreedv portfile contains this block:
pre-configure { # enable optimization on all Intel hardwares if {${build_arch} eq "i386"} { configure.args-append \ -DCMAKE_C_FLAGS="-mssse3 -msse4.1" } elseif {${build_arch} eq "x86_64"} { configure.args-append \ -DAVX=ON } }
This will not be correct when building with the universal variant, because you will be basing the optimization choice on the arch of the build machine rather than choosing it correctly for each arch that you're building. If you need to use different flags for each arch, as appears to be the case here, you'll want to investigate the muniversal portgroup.
As an aside, you don't really ever want to do configure.args-append -DCMAKE_C_FLAGS=...
. Instead, you'll want to do configure.cflags-append ...
and let the cmake portgroup handle how that gets communicated to cmake.
Also, if you're going to check whether a variable equals one value, and if not then check if it equals another value, etc., the proper construct is not a chain of if
statements but rather a switch statement.
Change History (4)
comment:1 Changed 5 years ago by ra1nb0w
comment:2 Changed 5 years ago by ryandesign (Ryan Carsten Schmidt)
Thanks. I haven't tested it but it looks like that should work.
The only other thing I'm wondering about is that right before the block you added, there's this:
# disable AVX and AVX2 for compatibility configure.args-append \ -DDISABLE_CPU_OPTIMIZATION=ON
And right after your block, there's this:
# select native cpu flags variant native description {Enable auto selection of cpu flags like avx/avx2/neon} { configure.args-delete -DDISABLE_CPU_OPTIMIZATION=ON }
Is the optimization that's enabled by your -DAVX=ON
different from the AVX and AVX2 flags mentioned in this existing +native variant?
comment:3 Changed 5 years ago by ra1nb0w
Yes, they are different. with DISABLE_CPU_OPTIMIZATION=OFF
cmake try to discover the best options available; therefore can be: AVX, AVX2 and/or AVX512.
Ok. I merge. Thank you!
comment:4 Changed 5 years ago by Davide Gerhard <ra1nb0w@…>
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
Sorry for the delay.
this is the change https://github.com/ra1nb0w/macports-ports/commit/4c9b9c99c28ec74677a8d8faaacc30e1b4557ce2
is it ok?
thanks for the tip about the switch; generally I use if/else(if) when I have only a few checks because they are generally faster or the compiler optimize the switch to if/else(if). Surelly the readability is better with switch.