[FFmpeg-devel] [PATCH 06/10] avfilter/avgblur_vulkan: call av_vkfmt_from_pixfmt only one time

[Wu, Jianhua]

Andreas Rheinhardt:
> Sent: Thursday, November 18, 2021 1:24 PM
> To: ffmpeg-devel at ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH 06/10] avfilter/avgblur_vulkan: call
> av_vkfmt_from_pixfmt only one time
> 
> Wu, Jianhua:
> > Andreas Rheinhardt wrote:
> >>
> >> Wu Jianhua:
> >>> There is a loop in av_vkfmt_from_pixfmt. And, even though some
> >>> compilers will optimize it when the optimization option is on, to
> >>> consider more situations, just call the function only one time here.
> >>>
> >>> Signed-off-by: Wu Jianhua <jianhua.wu at intel.com>
> >>> ---
> >>
> >> What compilers optimize this? And how? After all, this function is
> >> not marked as const.
> >
> > When there is some same operations in the codes the compiler will help
> > the developer to reuse the same result when the optimization is on,
> > like -O3 options in GCC. However the -O# was not used, the function
> > would be called again and again. And I check the assembly codes, it
> > did. If someone compiled with the optimizations, this commit didn't
> > changed anything in assembly codes. But it did help when there is no
> optimizations compiling.
> >
> 
> That is very surprising: Without a pure/const attribute the compiler is not
> allowed to optimize multiple calls to the same functions into one; and even
> with said attribute it is only allowed to do so if it can prove that the argument
> (and in case of pure also the whole state of the
> program) didn't change. I don't see how the compiler could know this, so
> regardless of whether you add the attribute it would be a compiler bug if the
> compiler actually optimized the code in that way.
> 

It seemed that it's just the compiler know how we call the functions because we
expose the implementation codes. If we repeatedly call a function from an opaque
binary API, the multiple calling would not be optimized at all.

> > I am more prefer this way to just call the function only one time
> > literally. I am not sure if anybody like this so I put it on the back
> > of the patch set. And I check this function and it did return a const
> > VkFormat * that supposed to mean we should not change the content.
> >
> I also prefer to not emit unnecessary calls; it's just that the commit message
> is surprising/unbelievable.
> 

Yeah. Bad description perhaps. The commit title is enough, I'm supposed to remove the description.