[FFmpeg-devel] [PATCH] AAC: type puns for 16 bit floating point rounding

[Måns Rullgård]

Uoti Urpala <uoti.urpala at pp1.inet.fi> writes:

> On Thu, 2008-12-04 at 17:11 +0000, M?ns Rullg?rd wrote:
>> Uoti Urpala wrote:
>> > On Thu, 2008-12-04 at 14:22 +0000, M?ns Rullg?rd wrote:
>> >> Uoti Urpala wrote:
>> >> > The GCC behavior is more relevant than the standard in this case. The
>> >>
>> >> That is only true in the reverse situation, i.e. when the standard allows
>> >> something but gcc misbehaves.  Then we have no choice but avoiding the
>> >> offending construct.  If gcc happens to generate intuitively correct
>> >> code for something that is undefined by the standard, we should still not
>> >> be using that something.
>> >
>> > It's not that GCC "happens to" generate the correct result, but that it
>> > explicitly defines it will work.
>> 
>> Same thing.  GCC happens to define this one way.  Another compiler may do
>> it differently.  GCC is not the only compiler.
>
> It's not the same thing. There are things undefined by the standard
> which GCC currently happens to implement one way but which are "random"
> and shouldn't be trusted to stay that way or to work the same on other
> compilers. This case is not one of those.

You fail to see the point.  Assuming conforming compilers, all that
matters is the standard.  If specific compilers define specific
behaviour outside the scope of the standard, this behaviour should not
be relied upon, except when the code will never be compiled with
another compiler, e.g. gcc-style inline assembler.

>> >> There is no telling what another compiler or a future version of
>> >> gcc might do.
>> >
>> > You can tell with reasonable certainty. GCC has explicitly defined that
>> > it will work. If you don't rely on that then you shouldn't rely on other
>> > compiler-defined behavior either
>> 
>> I endeavour not to.
>> 
>> > (like right shifting negative numbers, or casting unsigned to
>> > signed to get a negative result).
>> 
>> As I said before, that excludes some Cray systems.
>
> What does that have to do with hardware?

Where did I say anything about hardware.  For the purposes of this
discussion, the "system" includes the compiler and C library.

> A compiler is free to define those any way it wants on any hardware.
> And defining right shifting to produce the same result as first
> casting to unsigned and then shifting is likely as good a match for
> hardware as sign extension.

It is an assumed, and a reasonably so these days, in FFmpeg that
integers use two's complement representation, and that right shift of
a signed integer replicates the sign bit.  We also know these
assumptions to be true for all hardware/compiler variants we wish to
support.  Furthermore, the C99 standard specifies representation of
and operations on signed integers to be implementation-defined,
meaning that a compiler should behave in a consistent, documented
manner.

The effects of violating the aliasing rules are *undefined*.
Implementations are not required to document their behaviour, nor need
the behaviour be consistent.  The GCC documentation specifies a
consistent behaviour for one class of aliasing violations, namely
those involving unions.  Other compilers may differ.  Until you can
point at documentation for all other compilers showing them to handle
this case safely, it should be used with extreme caution if at all.

> Also practically all floating point in FFmpeg relies on sane compiler
> behavior rather than any standard (FFmpeg assumes much more than is
> guaranteed by the C standard, but does not request standard IEEE
> behavior either and is not compiled according to that on x86 with any
> compiler I know of).
>
>> >> If there is any chance at all that it might fail, it should not be done.
>> >
>> > Then you'll get nothing done.
>> 
>> There will of course always be mistakes, and we can never be sure we've
>> corrected them all.  However, we can make a good effort to avoid things
>> we *know* to be undefined.
>
> "Undefined by a particular standard" is not the _only_ measure.

Being defined by a relevant standard is a necessary, but not
sufficient, condition for something to be safely used.  Other
conditions include correct compiler support.

> This case can be expected to work for other reasons

Such as?

>> >> > GCC has the strictest aliasing requirements
>> >>
>> >> What are you comparing it with?
>> >
>> > Other compilers.
>> 
>> Which ones?
>
> Any general-purpose compiler which has some chance of compiling
> programs like FFmpeg. If you think there is some counterexample then
> tell which one.

Did you check all of them?  I very much doubt that.

>> >> > and it seems unlikely that some compiler would use more strict ones in
>> >> > the future
>> >>
>> >> Upon what do you base this assertion?
>> >
>> > Disallowing that would break even more code than the GCC introduction of
>> > strict aliasing did, it's desirable to have some supported mechanism for
>> > type punning, and disallowing it is unlikely to allow any significant
>> > optimizations.
>> 
>> Try for a moment to not see gcc as the centre of the universe.
>
> Nothing I wrote above was specific to GCC.

If that is the case, why do you persist in using gcc-specific
behaviour as proof of the safety of type-punning using unions?

>> Until you've tested *every* compiler, you simply cannot say what
>> works and what doesn't.
>
> You can say a significant amount; compiler writers do not decide things
> completely at random (or if the authors of some compiler do then it's
> probably not worth supporting).

Compilers sometimes do surprising things, sometimes even things that
make sense.

> And testing would be unlikely to give any definitive answers for a
> case like this where there is no easy to test to see whether a
> compiler really supports something or not.

In this context, I read that as advocating the use of methods the
correctness of which can never be verified with certainty, which
strikes me as a very bad idea.

>> > If you want to make optimizations more "reliable" then asm would be a
>> > better place to start. For example there's lots of asm in FFmpeg that
>> > fails to declare SSE register clobbers even though GCC can and does
>> > itself generate code using those registers.
>> 
>> This is definitely something that should be looked into.  I've seen
>> sloppy constraints or clobber lists break things with new gcc versions
>> too many times.
>> 
>> > Fixing this would require splitting files and changing the build
>> > system, as GCC doesn't support the register names at all unless
>> > the corresponding instruction set is enabled (so all code using
>> > SSE registers would have to be in separate files that would
>> > require -msse or higher to compile on 32-bit).
>> 
>> Not at all.  A few simple macros should be enough.
>
> How would you expect to use macros? The only way I see to attempt that
> would be to try detecting which registers are not recognized with
> current flags and omit clobbers for those, hoping that GCC wouldn't
> generate any code using them either. However the details of that get
> ugly. For example the syntax does not allow empty clobber lists so
> arbitrarily dropping entries fails, and functions expecting to keep
> values between asm blocks require separate handling (those would
> probably use the "variables in specified registers" feature). Using the
> registers as an alternative in constraint types also requires -msse or
> similar (see r14743).

I don't know all the details of the situation, but I'm certain
something reasonable could be done.

-- 
M?ns Rullg?rd
mans at mansr.com