[FFmpeg-cvslog] r18936 - in trunk/libavcodec: Makefile mlpdsp.c
Mon May 25 22:46:36 CEST 2009
Reimar D?ffinger wrote:
> On Mon, May 25, 2009 at 04:42:28PM -0300, Ramiro Polla wrote:
>> I still don't really understand what llvm is complaining about, but is
>> there a way to make llvm stop complaining without altering the object
>> code in gcc? There should be a way to correctly specify this...
>> Besides, how bad can a cast to the same type be?
> Sorry, I missed that readval is "r" for 64 bit.
I forgot to include that in my message...
> In that case, I think that's a bug in LLVM, it should ignore nop-casts
> (the types are exactly the same).
> Alternatively, you could change the READVAL macro to include the cast.
> Though I think it would really be a good idea to go over all those
> constraints and check them, because if not plain wrong they are at least
> silly, e.g.
>> @@ -166,7 +167,7 @@
>> /* 2*/"+r"(sample_buffer),
>> /* 3*/RDWRVAL(blocksize)
>> - /* 4*/READVAL((x86_reg)mask),
>> + /* 4*/READVAL(mask_reg),
>> /* 5*/READVAL(firjump),
>> /* 6*/READVAL(iirjump),
> Why force firjump and iirjump into a register ("r") instead of allowing
> both register and reference ("g")?
Because I assume it will be slower to read the value from reference. I
tried to force as much as I could into registers to make the code
faster. Using "g" lead gcc to randomly use reference on x86_64 and run
out of registers for x86_32, so I decided to force both "r" and "m" when
I thought it was best. This is my first attempt at gcc inline asm and
any kind of asm optimization, so I certainly can have chosen a
suboptimal (or even downright stupid =) setting.
>> #if ARCH_X86_64
>> , /* 8*/"r"((int64_t)coeff)
>> , /* 9*/"r"((int64_t)coeff)
>> , /*10*/"r"((int64_t)coeff)
>> #endif /* ARCH_X86_64 */
> Why isn't READVAL used, at least for consistency?
Because these should specifically be "r", and READVAL could have been
something else, but it isn't...
> Why int64_t and not x86_reg?
Good point. I think both this and the previous point could be changed,
but I don't have much of a strong opinion about them.
> Wouldn't it maybe have been more readable to put all the constraints
> that differ under #if ARCH_X86_64 instead of having a RDWRVAL, READVAL
> and COUNTER macro _and_ still needing the if (I admit it is a bit of
> code duplication...)
That's what I tried to avoid. Maybe I obfuscated too much to avoid
duplication... I'll send a patch for this later tonight.
More information about the ffmpeg-cvslog