Hi,

2012/3/19 Måns Rullgård <[email protected]>:
> "Ronald S. Bultje" <[email protected]> writes:
>> 2012/3/19 Måns Rullgård <[email protected]>:
>>> "Ronald S. Bultje" <[email protected]> writes:
>>>> 2012/3/19 Måns Rullgård <[email protected]>:
>>>>> "Ronald S. Bultje" <[email protected]> writes:
>>>>>> 2012/3/19 Måns Rullgård <[email protected]>:
>>>>>>> "Ronald S. Bultje" <[email protected]> writes:
>>>>>>>> On Sat, Mar 17, 2012 at 11:57 AM, Ronald S. Bultje 
>>>>>>>> <[email protected]> wrote:
>>>>>>>>> 2012/3/17 Måns Rullgård <[email protected]>:
>>>>>>>>>> "Ronald S. Bultje" <[email protected]> writes:
>>>>>>>>>>
>>>>>>>>>>> ---
>>>>>>>>>>>  libavcodec/x86/cabac.h |   17 ++++++++++-------
>>>>>>>>>>>  1 files changed, 10 insertions(+), 7 deletions(-)
>>>>>>>>>>>
>>>>>>>>>>> diff --git a/libavcodec/x86/cabac.h b/libavcodec/x86/cabac.h
>>>>>>>>>>> index 3c3652d..c4832c3 100644
>>>>>>>>>>> --- a/libavcodec/x86/cabac.h
>>>>>>>>>>> +++ b/libavcodec/x86/cabac.h
>>>>>>>>>>> @@ -105,8 +105,8 @@ static av_always_inline int 
>>>>>>>>>>> get_cabac_bypass_sign_x86(CABACContext *c, int val)
>>>>>>>>>>>  {
>>>>>>>>>>>      x86_reg tmp;
>>>>>>>>>>>      __asm__ volatile(
>>>>>>>>>>> -        "movl %4, %k1                           \n\t"
>>>>>>>>>>> -        "movl %2, %%eax                         \n\t"
>>>>>>>>>>> +        "movl %c5(%2), %k1                      \n\t"
>>>>>>>>>>> +        "movl %c3(%2), %%eax                    \n\t"
>>>>>>>>>>
>>>>>>>>>> %c5?  Last I checked, the code to get a plain number was 'a'.
>>>>>>>>>
>>>>>>>>> Fixed.
>>>>>>>>>
>>>>>>>>>>> -        :"+c"(val), "=&r"(tmp), "+m"(c->low), "+m"(c->bytestream)
>>>>>>>>>>> -        :"m"(c->range)
>>>>>>>>>>> -        : "%eax", "%edx"
>>>>>>>>>>> +        : "+c"(val), "=&r"(tmp)
>>>>>>>>>>> +        : "r"(c),
>>>>>>>>>>> +          "i"(offsetof(CABACContext, low)),
>>>>>>>>>>> +          "i"(offsetof(CABACContext, bytestream)),
>>>>>>>>>>> +          "i"(offsetof(CABACContext, range))
>>>>>>>>>>> +        : "%eax", "%edx", "memory"
>>>>>>>>>>
>>>>>>>>>> We changed this to use "m" operands to avoid the memory clobber.  I 
>>>>>>>>>> know
>>>>>>>>>> why you're doing this, but I think it's the wrong approach.
>>>>>>>>>
>>>>>>>>> It generates better code (less instructions for e.g.
>>>>>>>>> decode_cabac_mb_mvd()) with gcc-4.2.1 (which is shipped with XCode).
>>>>>>>>> Does it generate worse code anywhere? (It's true that later on it adds
>>>>>>>>> instructions for the overread protection again, but this commit in
>>>>>>>>> isolation makes things better, not worse.)
>>>>>>>>
>>>>>>>> Ping, are there any practical concerns left?
>>>>>>>
>>>>>>> We're still no closer to understanding what really is going on here.
>>>>>>
>>>>>> You'll have to be more practical than "I don't get it, so let's do
>>>>>> nothing". Do something to understand it. This patchset improves things
>>>>>> on my end (better code, compiler doesn't bomb out on adding extra
>>>>>> argument such as bytestream_end), which is more than sufficient.
>>>>>
>>>>> Your compiler seems to be the only one where it gives better code.
>>>>> There is no guarantee that your compiler will keep doing this next time
>>>>> you upgrade it.  Since I can't reproduce the problem, I'm not in a very
>>>>> good position to figure out why it happens.
>>>>
>>>> My compiler has been like that for years.
>>>>
>>>>> You can, and you're the one
>>>>> pushing for these patches, so the work falls to you.  Tough luck.
>>>>
>>>> You're not very clear on what you want. You want the holy grail? You
>>>> want a time machine? You want a better pension? What falls on me? I've
>>>> written code that is (if I understand you correctly) the same for you,
>>>> and better for me. That's fantastic! So does that mean we agree I can
>>>> commit it? If not, what exactly is your problem with this code?
>>>
>>> You've made changes that have very unexpected results.  This is never a
>>> good thing unless the reasons are understood.
>>
>> Yes: the compiler screwed up, and I fixed it.
>
> No, you did not fix it.  You randomly hacked around until it by chance
> did what you wanted.
>
>> Now, this isn't going anywhere. What are you looking for? I need a
>> concrete thing that you intend me to do,
>
> I want to understand what is causing the compiler to screw up in the
> first place.  If we figure that out, we might find a clean solution.
> Usually the first step is to reduce the problem to a smaller test case.
> The function where this is happening isn't very large, so this should be
> fairly easy.

I'm waiting. This is not concrete. What is your plan? You're currently
holding this up. I need an ETA.

Ronald
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to