After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 584174 - [deinterlace] Problems with tvtime assembler
[deinterlace] Problems with tvtime assembler
Status: RESOLVED INCOMPLETE
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
0.10.15
Other opensolaris
: Normal normal
: NONE
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2009-05-29 00:53 UTC by Brian Cameron
Modified: 2015-05-15 13:56 UTC
See Also:
GNOME target: ---
GNOME version: 2.25/2.26


Attachments
patch fixing issue (2.17 KB, patch)
2009-05-29 00:54 UTC, Brian Cameron
needs-work Details | Review
updated patch (1.92 KB, patch)
2009-06-01 18:47 UTC, Brian Cameron
committed Details | Review

Description Brian Cameron 2009-05-29 00:53:57 UTC
In the new 0.10.15 release of gst-plugins-good, the deinterlace plugin was added.
The mmx.h assembler code fails to compile because some "movl" statements include a ":" character.  In talking with the Solaris compiler team this is an error, and the code compiles okay if the ":" characters are removed.

I also needed to change the function mm_support to be "inline int" instead of "inline extern int" otherwise I get these errors:

        "/tmp/ube_sGAA.28938.ekaqH4", line 3809 : Multiply defined label: "TryAMD"
        "/tmp/ube_sGAA.28938.ekaqH4", line 3818 : Multiply defined label: "TryCyrix"
        "/tmp/ube_sGAA.28938.ekaqH4", line 3839 : Multiply defined label: "EMMXSupported"
        "/tmp/ube_sGAA.28938.ekaqH4", line 3843 : Multiply defined label: "AMD"
        "/tmp/ube_sGAA.28938.ekaqH4", line 3858 : Multiply defined label: "ThreeDNowSupported"
        "/tmp/ube_sGAA.28938.ekaqH4", line 3862 : Multiply defined label: "Intel"
        "/tmp/ube_sGAA.28938.ekaqH4", line 3863 : Multiply defined label: "MMXtest"
        "/tmp/ube_sGAA.28938.ekaqH4", line 3872 : Multiply defined label: "NotSupported1"
        "/tmp/ube_sGAA.28938.ekaqH4", line 3876 : Multiply defined label: "NotSupported2"
        "/tmp/ube_sGAA.28938.ekaqH4", line 3880 : Multiply defined label: "NotSupported3"
        "/tmp/ube_sGAA.28938.ekaqH4", line 3884 : Multiply defined label: "NotSupported4"
        "/tmp/ube_sGAA.28938.ekaqH4", line 3888 : Multiply defined label: "NotSupported5"
        "/tmp/ube_sGAA.28938.ekaqH4", line 3892 : Multiply defined label: "NotSupported6"
        "/tmp/ube_sGAA.28938.ekaqH4", line 3896 : Multiply defined label: "NotSupported7"
        "/tmp/ube_sGAA.28938.ekaqH4", line 3901 : Multiply defined label: "Return"

Looking at the code, I think that simply changing it to "inline int" should be reasonable.

Adding a patch which fixes these compile issues.
Comment 1 Brian Cameron 2009-05-29 00:54:42 UTC
Created attachment 135532 [details] [review]
patch fixing issue
Comment 2 David Schleef 2009-05-29 01:34:12 UTC
Yeah, those colons look wrong.  I'm surprised gcc manages to compile it.

For some reason, gcc doesn't like changing to 'inline extern int':

tvtime/mmx.h: In function ‘mm_support’:
tvtime/mmx.h:111: error: PIC register ‘ebx’ clobbered in ‘asm’
Comment 3 Brian Cameron 2009-06-01 18:47:53 UTC
Created attachment 135758 [details] [review]
updated patch


Here is an updated patch which just fixes the colon issue.  I've pinged the Sun Studio compiler team to ask what the best solution for the "inline extern int" issue.  Lets deal with that as a separate bug if it turns out there is any need to change the GStreamer code upstream to fix this.

I also note that the "inline extern int" issue is only a problem with the Sun Studio compiler if optimization is set to "-xO3" or higher.  I suspect the problem is caused by the compiler unrolling loops in a way that causes the labels to be defined more than once when "-xO3" or higher is used.  So, another workaround is to simply compile this file with "-xO2".
Comment 4 David Schleef 2009-06-01 19:02:52 UTC
Committed:

commit 09fbeee42d4ce76840d144a5eb03a9192fc1f4dd
Author: Brian Cameron <brian.cameron@sun.com>
Date:   Mon Jun 1 11:58:21 2009 -0700

    deinterlace: Fix spurious colons in asm code
    
    Fixes #584174.
    
    Signed-off-by: David Schleef <ds@schleef.org>


Leaving bug open on the 'inline extern int' issue.
Comment 5 Brian Cameron 2009-06-02 22:26:17 UTC
Ignore the "inline extern int" issue for now.  Based on discussions with the Sun Studio compiler team, I am fairly sure this is a Sun Studio compiler bug.

However, we are having more problems compiling the deinterlace assembler code.
When compiling linear.c, I see these errors:

ube: Sun Compiler Common 12.1 SunOS_i386 2009/04/15

assertion failed in function gra_renumber_instrs() @ gra.c:7962
assert(mit_dest_reg_kind_(lf_op_(curr_instr)) == B_REGISTER)

cc: ube failed for linear.c

In talking with the Sun Studio team, they suspect that this problem is caused by these lines in deinterlace_scanline_linear_mmxext:

    movntq_r2m (mm0, *out);
    movntq_r2m (mm2, *(out + 8));
    movntq_r2m (mm4, *(out + 16));
    movntq_r2m (mm6, *(out + 24));

This gets translated to lines like:

    __asm__ __volatile__ ( "movntq" " %%" "mm0" ", %0" : "=X" ( * out ) : );

Note that X-constraint means "Any operand whatsoever is allowed." (from gcc.gnu.org)

This is not true: "movntq" accept a *memory* operand only.

The fact that gcc generate correct code is accidental: if you try to build absolutely equivalent code then you will get the same errors even with gcc
                
    char ch_out = *out;
    __asm__ __volatile__ ( "movntq" " %%" "mm0" ", %0" : "=X" ( ch_out ) : );
    *out = ch_out;

We have different behavior with gcc and SunStudio because SunStudio in some cases does not organize access to arguments optimally.

This is way above my head, and I am hoping that the GStreamer maintainers may have an idea about how to fix this?Thanks for taking the time to report this bug.
Comment 6 Sebastian Dröge (slomo) 2011-05-20 06:34:12 UTC
Would using "m" as a contraint here work and does it fix the build with SunStudio?
Comment 7 David Schleef 2011-05-20 17:22:06 UTC
We removed the mmx code.  However, I'm guessing there might be a similar problem in the asm that is remaining in the tvtime code.
Comment 8 Tobias Mueller 2011-08-18 14:07:03 UTC
Reopening as I don't see any open non-developer issue.
Comment 9 Edward Hervey 2013-08-14 08:15:08 UTC
Brian, David, Jan, can someone with opensolari confirm this is still an issue with 1.x ?
Comment 10 Tim-Philipp Müller 2015-05-15 13:56:09 UTC
Closing this bug report as no further information has been provided. Please feel free to reopen this bug report if you can provide the information that was asked for in a previous comment.
Thanks!