"Ronald S. Bultje" <[email protected]> writes: > 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: >>>>> 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. -- Måns Rullgård [email protected] _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
