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

[Måns Rullgård]

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.

> 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.

> 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.

> Where it doesnt affect readability and speed i dont mind if people change
> the code to the latest gnu style, though i consider the whole a huge amount
> of wasted time. In many cases its not the code but the compiler that needs
> fixing.
>
> source code is supposed to be readable and maintainable, show me one
> developer who considers the code after this patch more readable than before!

I do.  When pointer types actually match it is much easier to see the
intent of the code.

> I guess you would also replace the c code by a base64 encoded string of a
> gif file if part of the c spec required it.

If that's what the spec required, of course.  Now please send your
straw man back wherever it came from.

> 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.

THERE IS NO BUG IN THE COMPILER!

> With a critical mass of compilers supporting this chances are probably good
> to also get it into the C standard one day. And that would lead to more
> readable and simpler code.

Strict aliasing IS A GOOD THING.  You yourself often complain about
gcc not optimising fully in some cases where potential aliasing
prevents it.  The recent discussion about src/dst in dct functions is
a good example.  Stronger aliasing constraints would allow the split
src/dst to be optimised just as much as the inplace version.

> 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.

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