• wolenetz@chromium.org's avatar
    Change prototypes to yuv_convert_simd_x86 yasm and equivalent C to portably sign extend int args · d4be6380
    wolenetz@chromium.org authored
    Inspection of stack prepared by caller shows it is indeed copying dword, not qword, for 32-bit arguments on Win64, and the background stack is not zeroed.
    
    Incorrect function result, access violations or worse (not-immediately caught side effect of corrupted memory within process address space) could easily result from current win64 usage of these routines.
    
    One solution, implemented here, is to change the function prototypes for the yasm routines to take ptrdiff_t arguments instead of int args.  The C version of the equivalent code needs matching prototype (due to the choose proc routines), so they are updated too.
    
    Another, probably slightly less performant, solution would be to change the asm routines to conditionally sign-extend when loading int (32-bit) parameters on x64.  None of these routines currently use int passed in directly as register; I  don't know if zero-extension of registers would be required if they did.
    
    Yet another solution would be to switch to VS2012 and redo the yasm routines using compiler intrinsics completely (not supported in VS2010).
    
    Note, some other media_unittests, namely those involving ffmpeg, currently give access violation on Win64.  There is a separate ffmpeg fix in progress https://gerrit.chromium.org/gerrit/#/c/43063/.  With PS1 or PS2 applied along with the ffmpeg fix, Win64 media_unittests pass.
    
    Detail:
    The following are sign-extended in prototype:
    simd/convert_rgb_to_yuv_ssse3.asm:%define SYMBOL          ConvertARGBToYUVRow_SSSE3
    simd/convert_rgb_to_yuv_ssse3.asm:%define SYMBOL          ConvertRGBToYUVRow_SSSE3
    simd/convert_yuv_to_rgb_mmx.asm:%define SYMBOL ConvertYUVToRGB32Row_MMX
    simd/convert_yuv_to_rgb_sse.asm:%define SYMBOL ConvertYUVToRGB32Row_SSE
    simd/scale_yuv_to_rgb_mmx.asm:%define SYMBOL ScaleYUVToRGB32Row_MMX
    simd/scale_yuv_to_rgb_sse.asm:%define SYMBOL ScaleYUVToRGB32Row_SSE
    simd/scale_yuv_to_rgb_sse2_x64.asm:%define SYMBOL ScaleYUVToRGB32Row_SSE2_X64
    simd/linear_scale_yuv_to_rgb_mmx.asm:%define SYMBOL LinearScaleYUVToRGB32Row_MMX
    simd/linear_scale_yuv_to_rgb_sse.asm:%define SYMBOL LinearScaleYUVToRGB32Row_SSE
    simd/linear_scale_yuv_to_rgb_mmx_x64.asm:%define SYMBOL LinearScaleYUVToRGB32Row_MMX_X64
    
    The following C versions and the corresponding typedefs also needed updating to match:
    ConvertYUVToRGB32Row_C
    ScaleYUVToRGB32Row_C
    LinearScaleYUVToRGB32Row_C
    
    The following yasm routines have no C prototype, nor do they appear to be used anywhere.  I updated their comments though to match the same prototype change that would be necessary.
    simd/convert_rgb_to_yuv_ssse3.asm:%define SYMBOL          ConvertARGBToYUVEven_SSSE3
    simd/convert_rgb_to_yuv_ssse3.asm:%define SYMBOL          ConvertARGBToYUVOdd_SSSE3
    simd/convert_rgb_to_yuv_ssse3.asm:%define SYMBOL          ConvertRGBToYUVEven_SSSE3
    simd/convert_rgb_to_yuv_ssse3.asm:%define SYMBOL          ConvertRGBToYUVOdd_SSSE3
    
    BUG=174160
    TEST=with https://gerrit.chromium.org/gerrit/#/c/43063/ also applied, all win64 media_unittests non-disabled test cases pass
    R=hclam@chromium.org
    R=dalecurtis@chromium.org
    R=rbultje@chromium.org
    CC=jschuh@chromium.org
    CC=scherkus@chromium.org
    
    Review URL: https://chromiumcodereview.appspot.com/12211067
    
    git-svn-id: svn://svn.chromium.org/chrome/trunk/src@182407 0039d316-1c4b-4281-b951-d872f2087c98
    d4be6380
convert_yuv_to_rgb_mmx.asm 717 Bytes