cisco / libsrtp

Library for SRTP (Secure Realtime Transport Protocol)
Other
1.21k stars 474 forks source link

avoid non-existing compiler flag -O4 #492

Closed traud closed 4 years ago

traud commented 4 years ago

That change was introduced because of the compiler Clang. However, even GCC does not know that optimization level, see Gentoo docs and gcc -Q -O4 --help=optimizers. GCC accepts -O4 but turns that into level 3 internally. Minor nit because it does not make any difference. However, not to confuse other programmers, libSRTP should not use a then lowered optimization flag.

Nevertheless, I guess, the libSRTP team was about getting the fastest code. Therefore, the above proposal with -Ofast. An alternative would be to remove that check completely and directly use -O3.

Do what ever you want with it. Just reporting this because I saw it while configuring.

pabuhler commented 4 years ago

@itollefsen, would you care to comment on this?

itollefsen commented 4 years ago

Using -O4 with Clang used to turn on Link TIme Optimization (LTO), but they stopped that around version 3.5 I think. So if you want LTO these days, you'll have to add explicit flags for it.

I'm weary of -Ofast because it allows GCC to drop strict standard compliance. In particular, it turns on -ffast-math. I don't think that's used today (I can't find it at least), so turning that on could potentially break something.

An alternative would be to remove that check completely and directly use -O3.

I think this makes more sense, but don't drop the check. The check is there because not all compilers supports or understands GCC's command line options. Just leave -O3 as the only option in the list.

And then drop -fexpensive-optimizations from the section below. That's turned on at -O2 and higher anyway.

Note for later: Someone(TM) should look into whether -funroll-loops has a positive impact. It may just as well be the opposite. I'm guessing someone profiled it back when it was added, but wondering if it still provides any gain.