[FFmpeg-devel] [PATCH] strict-aliasing-safe aes.c
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
>> > 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!
> 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
> 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
>> > 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:
> 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
mans at mansr.com
More information about the ffmpeg-devel