On 19/06/2015 00:36, Julien Isorce wrote:
On 18 June 2015 at 19:46, Emil Velikov <emil.l.veli...@gmail.com> wrote:
On 18 June 2015 at 06:53, Julien Isorce <julien.iso...@gmail.com> wrote:
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90908
I don't think it's necessary to create a bugzilla entry just to link to
it in your patch.
A bit more commentary about what this patch is doing and why it is right
would help review it.
This patch seems to be doing 2 things: Changing xform4.S to use
assyntax.h, and turning on the use of asm for darwin, so might be best
broken in two.
diff --git a/src/mesa/x86-64/xform4.S b/src/mesa/x86-64/xform4.S
index c185f62..17eb7fa 100644
--- a/src/mesa/x86-64/xform4.S
+++ b/src/mesa/x86-64/xform4.S
@@ -24,14 +24,15 @@
#ifdef USE_X86_64_ASM
+#include "x86/assyntax.h"
#include "matypes.h"
.text
.align 16
-.globl _mesa_x86_64_cpuid
-.hidden _mesa_x86_64_cpuid
-_mesa_x86_64_cpuid:
+GLOBL GLNAME(_mesa_x86_64_cpuid)
+HIDDEN(_mesa_x86_64_cpuid)
+GLNAME(_mesa_x86_64_cpuid):
Whilst the use of HIDDEN is necessary, I'm don't think there are any
x86_64 platforms which use a leading underscore, so I'm not sure if
using GLNAME makes sense in an x86_64 specific file.
-
+
+#if defined (__ELF__) && defined (__linux__)
.section .rodata
+#endif
This looks wrong. I don't see why this is linux/ELF specific.
Perhaps something like:
#ifdef __APPLE__
.const
#else
.section .rodata
#endif
maybe that needs to be conditional on __MACH__?
maybe that needs to explicitly use a .section directive with segment and
section?
--- a/src/mesa/x86/assyntax.h
+++ b/src/mesa/x86/assyntax.h
@@ -255,7 +255,7 @@
#endif /* ACK_ASSEMBLER */
-#if defined(__QNX__) || defined(Lynx) || (defined(SYSV) ||
defined(SVR4)) && !defined(ACK_ASSEMBLER) || defined(__ELF__) ||
defined(__GNU__) || defined(__GNUC__) && !defined(__MINGW32__)
+#if defined(__QNX__) || defined(Lynx) || (defined(SYSV) ||
defined(SVR4)) && !defined(ACK_ASSEMBLER) || defined(__ELF__) ||
defined(__GNU__) || defined(__GNUC__) && !defined(__MINGW32__) &&
!defined(__APPLE__)
#define GLNAME(a) a
#else
#define GLNAME(a) CONCAT(_,a)
Considering that this is a fragile area in mesa, can you confirm what
kind of testing you've done ? Would be nice to avoid breaking things
in various subtle ways.
Very minimal tests: only glxgears and es2gears_x11, on osx.
Also "make check" succeeds until it reaches glx-test which fails to build
(link error). I need to submit a bug.
In my OSX dev enviroment (which I haven't used for a year :D), I have
CFLAGS containing "-arch i386 -arch x86_64" (because that's what [1]
says to use). With this patch applied, that fails to build for i386.
That's a tricky problem to solve within the mesa build system. I don't
know if building for OSX i386 is useful anymore?
[1] http://xquartz.macosforge.org/trac/wiki/DeveloperInfo
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
