From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTP id 77EF046495 for ; Sat, 16 Sep 2023 21:46:22 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 63F0968C718; Sun, 17 Sep 2023 00:46:19 +0300 (EEST) Received: from mail8.parnet.fi (mail8.parnet.fi [77.234.108.134]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 45DCB68BB63 for ; Sun, 17 Sep 2023 00:46:11 +0300 (EEST) Received: from mail9.parnet.fi (mail9.parnet.fi [77.234.108.21]) by mail8.parnet.fi with ESMTP id 38GLk870022752-38GLk871022752; Sun, 17 Sep 2023 00:46:08 +0300 Received: from foo.martin.st (host-97-144.parnet.fi [77.234.97.144]) by mail9.parnet.fi (Postfix) with ESMTPS id 322BBA1450; Sun, 17 Sep 2023 00:46:07 +0300 (EEST) Date: Sun, 17 Sep 2023 00:46:07 +0300 (EEST) From: =?ISO-8859-15?Q?Martin_Storsj=F6?= To: "Logan.Lyu" In-Reply-To: <32aa76c9-1f18-252c-1e13-4f18a1cbbe50@myais.com.cn> Message-ID: References: <33af9c88-c31d-e11e-58a3-7f9a05718c8f@myais.com.cn> <7c2a5ea9-ccc9-d07c-88a0-af8c37e3a5c@martin.st> <32aa76c9-1f18-252c-1e13-4f18a1cbbe50@myais.com.cn> MIME-Version: 1.0 X-FE-Policy-ID: 3:14:2:SYSTEM Subject: Re: [FFmpeg-devel] [PATCH 1/4] lavc/aarch64: new optimization for 8-bit hevc_epel_uni_v X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Cc: jb@videolan.org, jdek@itanimul.li, FFmpeg development discussions and patches Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: On Thu, 14 Sep 2023, Logan.Lyu wrote: > Hi Martin, > > You can try the attached patchset. If that doesn't work, My code branch > address is https://github.com/myais2023/FFmpeg/tree/hevc-aarch64 Thanks for the patches. Functionally, they seem to work, and the issues i saw in the code are relatively minor. Unfortunately, some of the issues are issues that we've been through in many earlier patches, so I would hope that you would pay attention to them in the future before posting more patches. In patch 1, you've got a bunch of sxtw instructions for src/dst stride parameters that have the type ptrdiff_t - that shouldn't be necessary? In patch 2, you're moving the macros calc_epelh, calc_epelh2, load_epel_filterh - can you split out the move into a separate commit? (This isn't strictly necessary but would make things even clearer.) In patch 2, you're storing below the stack, then decrementing it afterwards - e.g. like this: > + stp x0, x30, [sp, #-16] > + stp x1, x2, [sp, #-32] > + stp x3, x4, [sp, #-48] > + stp x5, x6, [sp, #-64]! Please change that so that you're first predecrementing the whole area, then storing the other elements above that stack pointer, e.g. like this: stp x0, x30, [sp, #-64]! stp x1, x2, [sp, #16] stp x3, x4, [sp, #32] etc. The same issue also appears in variouos places within functions like this: > + stp x0, x1, [sp, #-16] > + stp x4, x6, [sp, #-32] > + stp xzr, x30, [sp, #-48]! Please fix all of these cases - you can search through your patches for anything related to storing on the stack. Also, storing xzr here seems superfluous - if you've got an odd number of registers to store, just make one instruction str instead of stp (but keep the stack aligned). Then in patch 4, you've got yet another pattern for doing these stores, where you have superfluous consecutive stack decrements like this: > + stp x6, x30, [sp, #-16]! > + mov x7, #16 > + stp x0, x1, [sp, #-16]! > + stp x2, x3, [sp, #-16]! > + stp x4, x5, [sp, #-16]! Please just do one stack decrement covering all the stack space you need. I believe these issues have been raised in earlier reviews as well. // Martin _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".