[FFmpeg-cvslog] r18936 - in trunk/libavcodec: Makefile mlpdsp.c

Ramiro Polla ramiro.polla
Thu May 28 21:28:44 CEST 2009


On Mon, May 25, 2009 at 5:46 PM, Ramiro Polla <ramiro at lisha.ufsc.br> wrote:
> 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[0])
>>> ? ? ? ?, /* 9*/"r"((int64_t)coeff[1])
>>> ? ? ? ?, /*10*/"r"((int64_t)coeff[2])
>>> #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.

Sorry for the delay...

What do you think about attached patch? It removes macros and should
make llvm work. And then another commit for (int64_t) -> (x86_reg).

Ramiro Polla
-------------- next part --------------
A non-text attachment was scrubbed...
Name: x86_mlpdsp_less_macros.diff
Type: text/x-diff
Size: 1624 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-cvslog/attachments/20090528/2bc9ba75/attachment.diff>



More information about the ffmpeg-cvslog mailing list