Warnings (was: Subsurface libdivecomputer branch merged with upstream..)

Linus Torvalds torvalds at linux-foundation.org
Sun Dec 17 13:00:43 PST 2017


On Sun, Dec 17, 2017 at 4:38 AM, Berthold Stoeger
<bstoeger at mail.tuwien.ac.at> wrote:
>
> I tried to look at the assembler output and the first thing I noted is that we
> don't compile with -O2? Is that on purpose?

I do think we should use -O2. In fact, we *do* use it for release builds:

  CMAKE_CXX_FLAGS_RELEASE:STRING=-O2 -DNDEBUG
  CMAKE_C_FLAGS_RELEASE:STRING=-O2 -DNDEBUG

but the DEBUG builds are done with just "-g". That's a mistake, in my opinion.

Sure, -O2 *can* make debugging a bit harder in that it can remove some
trivial variables, but honestly, that's very rare, and it can go the
other way too.

For example, for the kernel, unoptimized assembly is *so* stupid that
it's actually hard to debug, and we've never even supported it. We
don't have those kinds of low-level issues with subsurface, though.

I don't know what the autoconfig rules are for compiler flags, though.

> Anyway, with -O3, the first case in plannernotes.h produces better code without
> casting to float. You could of course get the same result by replacing 1000.0
> with 1000.0f, but this seems rather obscure.

I would suggest strongly against -O3.

It can improve things. It can also make things _much_ worse, and it's
generally not nearly as well tested as -O2. It tends to be where gcc
starts doing "unproven" optimizations.

> I removed two cases concerning binary representation from the patch and made a
> PR. Perhaps we get Linus to have a look at it.

Generally I agree with the policy of never using "float". The
precision loss can be very noticeable, and with the C rules it's
generally an odd thing to use.

Like "short", the only time "float" makes lots of sense is in
datastructures where size really really matters.

We should hopefully not really even have data structures like that,
since we (very much on purpose) have tried to keep our data structures
with exact types (ie fixed-point integers, eg mbar/mliter/mm/etc)

                 Linus


More information about the subsurface mailing list