[FFmpeg-devel] [PATCH] strict-aliasing-safe aes.c

Måns Rullgård mans
Tue Jun 29 13:28:50 CEST 2010

Michael Niedermayer <michaelni at gmx.at> writes:

> On Tue, Jun 29, 2010 at 02:02:41AM +0100, M?ns Rullg?rd wrote:
>> Michael Niedermayer <michaelni at gmx.at> writes:
>> > On Mon, Jun 28, 2010 at 10:13:31PM +0200, Reimar D?ffinger wrote:
>> >> On Mon, Jun 28, 2010 at 10:02:15PM +0200, Michael Niedermayer wrote:
>> >> > On Mon, Jun 28, 2010 at 05:55:30PM +0200, Reimar D?ffinger wrote:
>> >> > > Hello,
>> >> > > this uses AV_WN32A, AV_WN64A etc. macros.
>> >> > > The generated code is the same on x86_64 (assuming I did not mess up that test).
>> >> > 
>> >> > >  aes.c |   20 +++++++++++++-------
>> >> > >  1 file changed, 13 insertions(+), 7 deletions(-)
>> >> > > 76705b63949d4abbe51b0d7d59537045ae91179a  aesalias.diff
>> >> > 
>> >> > this makes the code unreadable, iam thus against it.
>> >> 
>> >> Well, what is the alternative? The current code seems to not work with some compilers.
>> >
>> > Fix the compiler.
>> There is nothing wrong with the compiler.  It implements the C99
>> language exactly as specified.
> The C99 standard isnt everything, a mpeg2 encoder also could return
> nothing but a black picture independant of the input, yet it wouldnt be a
> reasonable mpeg encoder or a usefull one.

It would still have to output a bitstream compatible with the

> I know you are the one who prefered the mpeg demuxer to comply to the spec
> even at the cost of not demuxing actual existing files.

I fail to see the relevance.

>> > This language lawyering and pedantic gnu style compliance crussade is
>> This has nothing to do with GNU and everything to do with the C99 standard.
> yes, that comment was about something else

Then why bring it up here?

>> > really annoying. Anything that gnu considers to be worth warning
>> > about makes people run and change their code, and often to the
>> > worse.
>> In this particular case the code is in violation of the spec.  That
>> the compiler only recently got the ability to detect this does not
>> make it less of a violation.
> again the comment is not about aliassing violations but warnings in general

But here we have some very real errors that gcc has been warning about
for YEARS.

>> > what i think should be done is: people should post a feature
>> > request on the <put affected compiler here> bug tracker about it
>> > acting a bit more reasonable and handle such trivial aliassing
>> > cases. And everyone who considers this important should reply
>> > there and express his oppinon.
> please send your strawman back as well, i never said theres a bug.
> I said the compiler behaves unreasonable and i clearly was talking about
> a feature request.

The compiler behaves according the C spec.  What is unreasonable about

> we arent talking about the same thing it seems.

How about we start doing that then?  I'm talking about the patch for
aes.c Reimar sent.

> I guess we agree that the compiler should generate fast code.

Yes, but it _also_ must be correct code.  The generated code must be
correct for any possible intent of the programmer.  This means some
pointers must be assumed to alias.

> I guess we agree that fast code needs types sometimes accessed
> through different types.


> I guess we agree that there must be some way for the programmer to
> specify how pointers can alias and some default rules about aliasing
> in absence.

Yes, and the defaults must be chosen to allow a decent level of
performance without detailed 

> What iam saying is that a compiler should detect 
> *(uint32_t*)p
> and accept it as correct code

This _is_ correct code provided whatever "p" points to is always
accessed as uint32_t.  If it is ever accessed as another type, this is
a strict aliasing violation.

> like it per spec has to accept ((magic_union_type*)p)->u32

The union trick exists so type punning can be done in a safe way
without sacrificing the optimisations possible with strict aliasing

> I also think that the aliassing model used by gcc is severly lacking.
> It for example lacks the possibility of the programmer specifying how
> 2 pointers can alias exactly. The issue just came up with the dct
> optimization and forced us to adapt our design of it.
> I dont like having to design our code around shortcommings of the compiler

You are not adequately distinguishing between the language spec and
the compiler.

>> > I dont mind us working around it here if theres some effort
>> > toward fixing this where the problem is (aka compilers and the C
>> > spec) even if that effort is small and unlikely to be successfull
>> > but if everything thats done is directed toward turning every
>> > file into a unmaintaunable mess with not one second spend on
>> > solving the actual problem then i dont think i will approve such
>> > workarounds. Id like to have at least some hope even if small
>> > that this red tape around every bit of optimized code one day
>> > would become unneeded
>> The main author of libswscale is in no position to speak about an
>> unmaintainable mess.
> I have less problem maintaining it

Then why don't you fix it?

> besides instead of continuing to reply to your language lawyering trolling
> ill refer you to linus torvalds:
> http://kerneltrap.org/mailarchive/linux-kernel/2007/10/26/359162
> http://lkml.org/lkml/2003/2/26/158
> Ive seen better quotes from him about it but i cant find them atm

This is the opposite of our situation.  Linus dislikes optimisations
that are allowed by the spec but cause trouble for real-world code.
Not performing these optimisations would never break anything.

Your sloppy aliasing would break perfectly valid code, and that is not

M?ns Rullg?rd
mans at mansr.com

More information about the ffmpeg-devel mailing list