[FFmpeg-devel] [patch v4, v5] libpostproc: mmx code uses stack below %esp

[Yuriy Kaminskiy]

On 02.02.2010 23:12, Michael Niedermayer wrote:
> On Tue, Feb 02, 2010 at 11:08:01PM +0300, Yuriy Kaminskiy wrote:
>> On 02.02.2010 22:20, Michael Niedermayer wrote:
>>> On Tue, Feb 02, 2010 at 04:47:44PM +0300, Yuriy Kaminskiy wrote:
>>> I think after reading over this again the best solution would be to use the
>>> context as temporary space, we have a "m"(c->pQPb) anyway so if we put a
>>> pointer into a register to the context we could address pQPb and the temp
>>> aligned space easily.
>>> That way we dont need to create any aligned space on the stack ...
>> Eww, but context here is actually "aligned space on stack" (I think that's why
>> some code can work at all: else gcc would die with "not enough registers" even
>> without -fPIC):
>> ==== cut postprocess_template.c:3163 ===
>> static void RENAME(postProcess)(const uint8_t src[], int srcStride, uint8_t
>> dst[], int dstStride, int width, int height,
>>                                 const QP_STORE_T QPs[], int QPStride, int
>> isColor, PPContext *c2)
>> {
>>     DECLARE_ALIGNED(8, PPContext, c)= *c2; //copy to stack for faster access
> hmm, i forgot that ...
> anyway does gcc add additional instructions with your code to align the new
> variable or not?
hmm. no, it seems uses "aligned" offset, but does not /realign/ stack
so if stack /was/ misaligned (($esp % 8)) - it will be bad
on other hand, DECLARE_ALIGNED (with same alignment) already used for this
ppcontext copy, so i doubt it will make situation worse (and i doubt it is
possible - such misaligned stack would be bad for double variables [I know that
larger alignment - 16 bytes for SSE - is certainly problematic on some OSes])