Re: [Mesa-dev] [PATCH 00/15] i965, mesa, glapi: Build libi965_dri on Android
On Fri, Aug 5, 2011 at 12:27 PM, Chad Versace c...@chad-versace.us wrote: On 08/04/2011 11:25 AM, Chia-I Wu wrote: Hi Chad, On Thu, Aug 4, 2011 at 6:47 PM, Chad Versace c...@chad-versace.us wrote: This is the first step in porting i965 to Android, initially to Gingerbread. Android is ubiquituous now, and the platform's importance can't be denied. The Intel drivers really should support it. The series first builds libglapi, then libmesa, and then libi965_dri. lib965_dri doens't work yet; it doesn't even link to libdrm. This is just the initial work to get it to build. I'm aware that this may be controversial, so I've attempted to integrate the Android makefiles as non-disruptively as possible. - The Android makefiles attempt to duplicate as little as possible from the existing makefiles. In one case, this dictated that some Makefile code be extracted into a shared makefile. - I've avoided modifying the existsing makefiles except when absolutely necessary. When a modification or cleanup was necessary, I have attempted to keep the changes minimal and do them incrementally. - The locations of the Android makefiles mirror those of the existing makefiles. The series does not break the Linux i965 build. I doubt it breaks other drivers' builds, but I haven't tested it. If you want me to check that it doesn't break your build, just let me know the autoconf options. The series lives on my android-gingerbread-i965 branch. If you'd like to try the Android build yourself, you should pull from my personal Android manifest [1], which closely mirrors android-x86, and set `BOARD_USES_I965 := true` in buildspeck.mk. [1] repo init -u git://anongit.freedesktop.git/~chadversary/android.manifeset.git -b gingerbread-i965 Thanks for bringing this up and working on it. Before commenting on the changes to Mesa, I'd like to talk a little bit about the external dependencies. To have a working build of Mesa for the Android Gingerbread source tree, these external changes are needed 1) modify Android's init (pid 1) to create DRI device nodes under /dev 2) make off_t 64-bit so that mmap() a GEM object works 3) figure out how to authenticate a DRM fd 4) add libdrm to Android source tree 5) add drm_gralloc (the DDX driver, sort of) to Android source tree There may be more that I cannot recall now. Of these changes, only 3), 4), and 5) affect how Mesa is built. It is safe to assume 4) and 5) are already there, as we have good relationship with those projects. But how to solve 3) is still an open question. Luckily, some key people who can help us with 3 belong to my team: the author of Wayland, and the authors of Intel's DRM module, i915.ko. I will probe them when we reach that bridge. We can either spend several months discussing 3) with the Android team before landing Android support to Mesa. I have been given a timeframe to complete this, and though I cannot discuss the details, several months is unacceptable. The Mesa port needs to be completed before yesterday. Or we can assume it is solved, land the Android support for Mesa probably over the next few weeks, and initiate the discussion with the Android team with something concrete. Now that we are at this, my preference is the latter. Yes. We should take the latter approach. I forgot to mention that we need to do 1), 2), ..., 5) only when we target for integrating into the vanilla Android tree. When we target for integration with Android-x86, these tasks are either done or worked around. Back to your changes. I am fine with landing your version and is willing to help review them, proivided your work won't stop after i915/i965 is supported. Otherwise, I prefer to prepare my version[1] for upstreaming, which can build all actively maintained drivers including Gallium ones. It would also be really great you can help, and suppose the list is fine with yet another build system for Mesa. [1] http://cgit.freedesktop.org/~olv/mesa/log/?h=android-gingerbread-7.11 You have done a lot of work on the Mesa port, and I want to acknowledge your expertise there. Please don't interpret my submission of this series as a dismissal of that work. My intent in submitting my own patches was just to start the process of getting Android support upstream, incrementally, in a public, code-reviewed forum. Other drivers, of course, should also be supported. I do not want to place any obstacle in way of that goal. At each step of the review process, we should check with each other that the approach taken is compatible with both Gallium and non-Gallium drivers. We will be basically doing everything from scratch, in smaller steps. In that case, I see no reason not to do this based on what is already there. I did plan to upstream my changes. So cleaning it up should not be a nightmare. Are there missing commits in your branch? I tried to build it, but it is almost no-op because
[Mesa-dev] upstreaming Android support [Was: i965, mesa, glapi: Build libi965_dri on Android]
On Fri, Aug 5, 2011 at 3:42 PM, Chia-I Wu olva...@gmail.com wrote: We will be basically doing everything from scratch, in smaller steps. In that case, I see no reason not to do this based on what is already there. I did plan to upstream my changes. So cleaning it up should not be a nightmare. I've pushed a branch here for discussion http://cgit.freedesktop.org/~olv/mesa/log/?h=android-gingerbread-upstream-wip This is still WIP. It still needs to be cleaned up. The goal of the branch is to provide * a clean tree to be merged to master * integration with Android(-x86) Gingerbread * gallium drivers that work well * i915/i965 drivers that work with simple tests The main focus is the integration with Android. Missing features in i915/i965 should be added later. The commits in this branch can be divided into four groups 1) Fix portability issues (the first 5 commits) Android has its own C library called bionic. There are a few places where it is incompatible with GNU C. These five commits adds compile-time tests to define the compatibility layer for Android. 2) Add support for Android to st/egl (the next 3 commits) These should only be interesting to EGL developers. These commits add _EGL_PLATFORM_ANDROID and Android-specific extensions to EGL core. Support for the new platform and extensions is then added to st/egl. 3) Integration with Android build system (the next 2 commits) This first commit here should be killed. Ignore it for now. The second commit[1] adds a bunch of Android.mk's to the source tree. Android.mk's are read by the Android build system to build Mesa. No changes are made to the existing files. IMHO, this is the better way to upstream Andoird support. We know that things work at this point. We know that nothing can break due to this commit. We can then refactor Makefile's, one by one as follow-up commits, until we are satisfied with the results. 4) Add support for Android to egl_dri2/i915/i965 (the last 3 commits) These commits add Android support to egl_dri2. Build rules for i915 and i965 are also added. The most interesting changes should be 3). I prefer the approach I described in it, but I am also fine with interleaving refactoring Makefile's with adding Android.mk's. That could take a while as we need to have a consensus on how to refactor Makefile's systematically. Thoughts? [1] direct link http://cgit.freedesktop.org/~olv/mesa/commit/?h=android-gingerbread-upstream-wipid=90f8f6dc2fa3e121a9e5921fac106d2b058e08cc -- o...@lunarg.com ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Mesa (master): egl/gbm: Fix EGL_DEFAULT_DISPLAY
2011/8/5 Chad Versace c...@chad-versace.us:
This commit *really* needs a more descriptive commit message.
Ok, I thought its clear what is to be done for EGL_DEFAULT_DISPLAY.
But your right, it should have more descriptive. In future I'll try to do so.
Since I cant change the commit message now, let me answer your questions.
- What were the symptoms of the broken EGL_DEFAULT_DISPLAY?
Segfault, because the native egl display is NULL.
- What error in the code caused the problem?
Not handling the above.
- What the hell did you do to fix it? This is not evident due to the length
of the commit.
Well, its really just creating a new gbm device if there is none given,
and properly destroying it, if created by ourself.
And what does this fix?
eglinfo.c in mesa/demos/src/egl/opengl/ with EGL_PLATFORM=drm
P.S. This has nothing to do with hell.
I understand that bad commit messages can upset, but lets keep patient.
For commits of this complexity, write descriptive messages for the sake of
others.
--
Chad Versace
c...@chad-versace.us
On 08/04/2011 05:18 AM, Benjamin Franzke wrote:
Module: Mesa
Branch: master
Commit: 32f4cf38085e4056b8e4a9fc78fea28897a1d05f
URL:
http://cgit.freedesktop.org/mesa/mesa/commit/?id=32f4cf38085e4056b8e4a9fc78fea28897a1d05f
Author: Benjamin Franzke benjaminfran...@googlemail.com
Date: Wed Jun 29 08:49:39 2011 +0200
egl/gbm: Fix EGL_DEFAULT_DISPLAY
---
src/egl/drivers/dri2/egl_dri2.c | 7 ++
src/egl/drivers/dri2/egl_dri2.h | 1 +
src/egl/drivers/dri2/platform_drm.c | 25
+-
src/gallium/state_trackers/egl/drm/native_drm.c | 23
src/gallium/state_trackers/egl/drm/native_drm.h | 4 +++
5 files changed, 53 insertions(+), 7 deletions(-)
diff --git a/src/egl/drivers/dri2/egl_dri2.c
b/src/egl/drivers/dri2/egl_dri2.c
index 0aca929..9a37ea4 100644
--- a/src/egl/drivers/dri2/egl_dri2.c
+++ b/src/egl/drivers/dri2/egl_dri2.c
@@ -592,6 +592,13 @@ dri2_terminate(_EGLDriver *drv, _EGLDisplay *disp)
wl_display_destroy(dri2_dpy-wl_dpy);
break;
#endif
+#ifdef HAVE_DRM_PLATFORM
+ case _EGL_PLATFORM_DRM:
+ if (dri2_dpy-own_gbm_device) {
+ gbm_device_destroy(dri2_dpy-gbm_dri-base.base);
+ }
+ break;
+#endif
default:
break;
}
diff --git a/src/egl/drivers/dri2/egl_dri2.h
b/src/egl/drivers/dri2/egl_dri2.h
index 3854200..a729718 100644
--- a/src/egl/drivers/dri2/egl_dri2.h
+++ b/src/egl/drivers/dri2/egl_dri2.h
@@ -86,6 +86,7 @@ struct dri2_egl_display
#ifdef HAVE_DRM_PLATFORM
struct gbm_dri_device *gbm_dri;
+ int own_gbm_device;
#endif
char *device_name;
diff --git a/src/egl/drivers/dri2/platform_drm.c
b/src/egl/drivers/dri2/platform_drm.c
index 579baf9..04b10e2 100644
--- a/src/egl/drivers/dri2/platform_drm.c
+++ b/src/egl/drivers/dri2/platform_drm.c
@@ -30,6 +30,10 @@
#include string.h
#include xf86drm.h
#include dlfcn.h
+#include sys/types.h
+#include sys/stat.h
+#include fcntl.h
+#include unistd.h
#include egl_dri2.h
@@ -90,6 +94,7 @@ dri2_initialize_drm(_EGLDriver *drv, _EGLDisplay *disp)
{
struct dri2_egl_display *dri2_dpy;
struct gbm_device *gbm;
+ int fd = -1;
int i;
dri2_dpy = malloc(sizeof *dri2_dpy);
@@ -100,7 +105,15 @@ dri2_initialize_drm(_EGLDriver *drv, _EGLDisplay *disp)
disp-DriverData = (void *) dri2_dpy;
- gbm = (struct gbm_device *) disp-PlatformDisplay;
+ gbm = disp-PlatformDisplay;
+ if (gbm == NULL) {
+ fd = open(/dev/dri/card0, O_RDWR);
+ dri2_dpy-own_gbm_device = 1;
+ gbm = gbm_create_device(fd);
+ if (gbm == NULL)
+ return EGL_FALSE;
+ }
+
if (strcmp(gbm_device_get_backend_name(gbm), drm) != 0) {
free(dri2_dpy);
return EGL_FALSE;
@@ -112,7 +125,15 @@ dri2_initialize_drm(_EGLDriver *drv, _EGLDisplay *disp)
return EGL_FALSE;
}
- dri2_dpy-fd = gbm_device_get_fd(gbm);
+ if (fd 0) {
+ fd = dup(gbm_device_get_fd(gbm));
+ if (fd 0) {
+ free(dri2_dpy);
+ return EGL_FALSE;
+ }
+ }
+
+ dri2_dpy-fd = fd;
dri2_dpy-device_name = dri2_get_device_name_for_fd(dri2_dpy-fd);
dri2_dpy-driver_name = dri2_dpy-gbm_dri-base.driver_name;
diff --git a/src/gallium/state_trackers/egl/drm/native_drm.c
b/src/gallium/state_trackers/egl/drm/native_drm.c
index 47910de..c013769 100644
--- a/src/gallium/state_trackers/egl/drm/native_drm.c
+++ b/src/gallium/state_trackers/egl/drm/native_drm.c
@@ -134,8 +134,11 @@ drm_display_destroy(struct native_display *ndpy)
if (drmdpy-device_name)
FREE(drmdpy-device_name);
- if (drmdpy-fd = 0)
- close(drmdpy-fd);
+ if (drmdpy-own_gbm) {
+ gbm_device_destroy(drmdpy-gbmdrm-base.base);
+ if (drmdpy-fd = 0)
+
Re: [Mesa-dev] [PATCH] glsl: empty declarations should be valid
On Fri, Aug 5, 2011 at 1:48 PM, Chad Versace c...@chad-versace.us wrote:
On 08/04/2011 06:18 PM, Chad Versace wrote:
On 08/04/2011 01:29 PM, Eric Anholt wrote:
On Thu, 4 Aug 2011 12:59:35 +0900, Chia-I Wu olva...@gmail.com wrote:
From: Chia-I Wu o...@lunarg.com
Unlike C++, empty declarations such as
float;
should be valid. The spec is not explicit about this actually.
Some apps that generate their shader sources may rely on this. This was
noted when porting one of them to Linux from Windows.
Ew. Looking the GLSL 1.20 spec, I see:
statement:
declaration_statement
declaration_statement:
declaration
declaration:
init_declarator_list SEMICOLON
init_declarator_list:
single_declaration
single_declaration:
fully_specified_type
fully_specified_type IDENTIFIER
so it looks like that is actually valid code. That's awful.
I first suspected that this was a spec grammar bug. But it is still present
in
the GLSL 4.10 spec, so it's unlikely to be a bug.
Surprisingly, C also allows empty declarations. Compiling this
float;
int main() { return 0; }
with `gcc --std=c99` succeeds and emits this warning:
warning: useless type name in empty declaration [enabled by default]
I hate to say this, but I believe the spec grammar intentionally allows empty
declarations. C allows it, and GLSL tries to mimic C.
Even though I don't like empty declarations, this patch is
Reviewed-by: Chad Versace c...@chad-versace.us
Also, please update the commit message to say that `gcc --std=c99` allows
empty
declrations and include the appropriate quotation from the GLSL 1.20 spec's
grammar. Without that extra info, someone may stumble onto this commit and
say WTF.
Oops. I've already committed it...
--
Chad Versace
c...@chad-versace.us
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
--
o...@lunarg.com
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 01/15] mesa: Add toplevel Android.mk
- Original Message -
On 08/04/2011 04:17 PM, Jose Fonseca wrote:
- Original Message -
On Thu, Aug 4, 2011 at 2:47 AM, Chad Versace
c...@chad-versace.us
wrote:
This is the first step in porting libGLES* and libEGL to Android.
The makefile doesn't build anything yet; it just defines common
variables.
The values for MESA_COMMON_C_FLAGS and MESA_COMMON_CPP_FLAGS, I
obtained
by invoking autogen.sh with the options below and then inspecting
MESA_TOP/configs/autoconf. My immediate goal is to port i965 to
Android,
so I used the typical flags for building i965.
--disable-gallium
--disable-glu
--disable-glw
--disable-glut
--enable-32-bit
--enable-egl
--enable-gles2
--enable-gles1
--enable-texture-float
--with-dri-drivers=i965
--with-gallium-drivers=
Note: This is in preparation for porting i965 to Android.
CC: Chia-I Wu o...@lunarg.com,
CC: Chih-Wei Huang cwhu...@android-x86.org
Signed-off-by: Chad Versace c...@chad-versace.us
---
Android.mk | 66
android/mesa_local_vars.mk | 32 +
2 files changed, 98 insertions(+), 0 deletions(-)
create mode 100644 Android.mk
create mode 100644 android/mesa_local_vars.mk
There's quite a bit of new build infrastructure here. What would
it
take to fit this into either the existing autoconf support or add
a
targeted configs/android? Duplicating a pile of make targets and
adding another way to configure/build mesa seems like it might not
be
the best way to go. What are the current constraints that make
building mesa on android difficult?
I agree. I'd prefer not have yet another build infrastructure in
the mix, unless there's a very good reason.
Jose
I also agree. I would prefer to not add another build system to Mesa.
We have
too many as it is.
To address Dan's questions, the Android build cannot be fitted into
autoconf or
configs/android. I explored this option, and discovered that it was
impossible.
The Android build system is... well... interesting. Allow me to
explain.
The entirety of the Android project --- libc, webkit, the window
manager,
*everything* --- exists in a single source tree [1]. And that source
tree is
built with a single, non-recursive invocation of make. Every time I
say that, I
find it hard to believe myself, so I'll say it again: The entirety of
the
Android OS, all core libraries and apps, are built with a single,
non-recursive
invocation of make. (The kernel is the special exception to this
all-encompassing build). The final build artifact is a bootable iso
image.
!!
I know that recursive make is considered evil by many, but reading all those
make includes can't be fast...
[1] http://android.git.kernel.org/
Given this unified design of the Android source tree and build
process, it
requires system components, such as Mesa, to be integrated into its
build
system. If Mesa is going to support Android, the Android.mk's are
necessary.
Fair enough.
To address another concern of Dan's, this will not add another way to
configure
the Mesa build. Android handles the system's build configuration in a
single
location which is outside of Mesa.
I'm aware that the extra set of makefiles is unwelcome, but their
presence will
be innocuous. The only people that will need to touch them are those
maintaining
the Android build.
The most command kind of change to build system is by far
adding/removing/renaming source files, which currently have to be replicated on
each.
Developers typically use only one build system, so it is natural to
forget/ignore updating all, which breaking builds, loosing benefit of
continuous testing, and makeing difficult to bisect in the future.
Spite different build systems, we could try unifying the source lists (have a
.mk which is read by all build systems, including scons) and/or automate source
lists (have one target per dir, and simply glob all *.{c,h,cpp,hpp}).
Jose
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 01/15] mesa: Add toplevel Android.mk
On Thu, Aug 4, 2011 at 7:55 PM, Chad Versace c...@chad-versace.us wrote: On 08/04/2011 04:17 PM, Jose Fonseca wrote: - Original Message - On Thu, Aug 4, 2011 at 2:47 AM, Chad Versace c...@chad-versace.us wrote: This is the first step in porting libGLES* and libEGL to Android. The makefile doesn't build anything yet; it just defines common variables. The values for MESA_COMMON_C_FLAGS and MESA_COMMON_CPP_FLAGS, I obtained by invoking autogen.sh with the options below and then inspecting MESA_TOP/configs/autoconf. My immediate goal is to port i965 to Android, so I used the typical flags for building i965. --disable-gallium --disable-glu --disable-glw --disable-glut --enable-32-bit --enable-egl --enable-gles2 --enable-gles1 --enable-texture-float --with-dri-drivers=i965 --with-gallium-drivers= Note: This is in preparation for porting i965 to Android. CC: Chia-I Wu o...@lunarg.com, CC: Chih-Wei Huang cwhu...@android-x86.org Signed-off-by: Chad Versace c...@chad-versace.us --- Android.mk | 66 android/mesa_local_vars.mk | 32 + 2 files changed, 98 insertions(+), 0 deletions(-) create mode 100644 Android.mk create mode 100644 android/mesa_local_vars.mk There's quite a bit of new build infrastructure here. What would it take to fit this into either the existing autoconf support or add a targeted configs/android? Duplicating a pile of make targets and adding another way to configure/build mesa seems like it might not be the best way to go. What are the current constraints that make building mesa on android difficult? I agree. I'd prefer not have yet another build infrastructure in the mix, unless there's a very good reason. Jose I also agree. I would prefer to not add another build system to Mesa. We have too many as it is. To address Dan's questions, the Android build cannot be fitted into autoconf or configs/android. I explored this option, and discovered that it was impossible. The Android build system is... well... interesting. Allow me to explain. The entirety of the Android project --- libc, webkit, the window manager, *everything* --- exists in a single source tree [1]. And that source tree is built with a single, non-recursive invocation of make. Every time I say that, I find it hard to believe myself, so I'll say it again: The entirety of the Android OS, all core libraries and apps, are built with a single, non-recursive invocation of make. (The kernel is the special exception to this all-encompassing build). The final build artifact is a bootable iso image. [1] http://android.git.kernel.org/ Looking at all those git repos, wouldn't this be more appropriate as an android project? platform/external/mesa or something? I haven't seen any Android.mk files show up in freetype or expat or anything like that. In the same way, mesa doesn't carry a debian folder even though that's how debian and ubuntu build mesa. Certainly if there are fixes to the existing build infrastructure that help get mesa built on android, that should be done, but I don't see why we should carry the android build bits in upstream mesa. -- Dan ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Removing GLw from the main Mesa repository
On Thu, Aug 4, 2011 at 4:26 PM, Kenneth Graunke kenn...@whitecape.org wrote: Hey, I'd like to remove libGLw from the main Mesa repository. It never changes, and almost noone uses it...because GL and Motif is awesome, right? Since Debian still packages it, I pulled it into its own git repository, preserving history, and then autotooled it. You can get it here: git://people.freedesktop.org/~kwg/glw Sounds good to me. Please scan through the Mesa docs for any mention of libGLw and update/remove as needed. There should at least be a pointer to the new home of libGLw too. -Brian ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Removing GLw from the main Mesa repository
On Fri, 2011-08-05 at 07:45 -0600, Brian Paul wrote: On Thu, Aug 4, 2011 at 4:26 PM, Kenneth Graunke kenn...@whitecape.org wrote: Hey, I'd like to remove libGLw from the main Mesa repository. It never changes, and almost noone uses it...because GL and Motif is awesome, right? Since Debian still packages it, I pulled it into its own git repository, preserving history, and then autotooled it. You can get it here: git://people.freedesktop.org/~kwg/glw Sounds good to me. Please scan through the Mesa docs for any mention of libGLw and update/remove as needed. There should at least be a pointer to the new home of libGLw too. While at it, why not to remove libGLU from mesa as well? Best regards, Maxim Levitsky ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Removing GLw from the main Mesa repository
There is nothing compiled since GLW_SOURCES is not substituted by configure: Makefile.am:29 libGLw_la_SOURCES = $(GLW_SOURCES) This would need AC_SUBST([GLW_SOURCES]) in configure.ac, but thats not allowed for _SOURCES variables, see automake output: configure.ac:96: `GLW_SOURCES' includes configure substitution `@GLW_SOURCES@' configure.ac:96: and is referred to from `libGLw_la_SOURCES'; configure.ac:96: configure substitutions are not allowed in _SOURCES variables 2011/8/5 Kenneth Graunke kenn...@whitecape.org: Hey, I'd like to remove libGLw from the main Mesa repository. It never changes, and almost noone uses it...because GL and Motif is awesome, right? Since Debian still packages it, I pulled it into its own git repository, preserving history, and then autotooled it. You can get it here: git://people.freedesktop.org/~kwg/glw --Kenneth ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Mesa (master): egl/gbm: Fix EGL_DEFAULT_DISPLAY
On 08/05/2011 12:30 AM, Benjamin Franzke wrote: 2011/8/5 Chad Versace c...@chad-versace.us: This commit *really* needs a more descriptive commit message. Ok, I thought its clear what is to be done for EGL_DEFAULT_DISPLAY. But your right, it should have more descriptive. In future I'll try to do so. Since I cant change the commit message now, let me answer your questions. - What were the symptoms of the broken EGL_DEFAULT_DISPLAY? Segfault, because the native egl display is NULL. - What error in the code caused the problem? Not handling the above. - What the hell did you do to fix it? This is not evident due to the length of the commit. Well, its really just creating a new gbm device if there is none given, and properly destroying it, if created by ourself. And what does this fix? eglinfo.c in mesa/demos/src/egl/opengl/ with EGL_PLATFORM=drm P.S. This has nothing to do with hell. I understand that bad commit messages can upset, but lets keep patient. Apologies, Ben. I let the moment's emotion get the better of me. Thanks for the taking the time to explain the commit. -- Chad Versace c...@chad-versace.us For commits of this complexity, write descriptive messages for the sake of others. -- Chad Versace c...@chad-versace.us ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH resend] glx/dri2: Paper over errors in DRI2Connect when indirect
On Thu, 4 Aug 2011 12:06:13 +1000, Christopher James Halse Rogers christopher.halse.rog...@canonical.com wrote: DRI2 will throw BadRequest for this when the client is not local, but DRI2 is an implementation detail and not something callers should have to know about. Silently swallow errors in this case, and just propagate the failure through DRI2Connect's return code. Note: This is a candidate for the stable release branches. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=28125 Signed-off-by: Christopher James Halse Rogers christopher.halse.rog...@canonical.com --- This has been sitting on the list for a while; resending to prod it back into attention. Pushed. Thanks! pgpip4YO1J8Qt.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Removing GLw from the main Mesa repository
On Fri, Aug 5, 2011 at 6:45 AM, Brian Paul brian.e.p...@gmail.com wrote: On Thu, Aug 4, 2011 at 4:26 PM, Kenneth Graunke kenn...@whitecape.org wrote: Hey, I'd like to remove libGLw from the main Mesa repository. It never changes, and almost noone uses it...because GL and Motif is awesome, right? Since Debian still packages it, I pulled it into its own git repository, preserving history, and then autotooled it. You can get it here: git://people.freedesktop.org/~kwg/glw Sounds good to me. Please scan through the Mesa docs for any mention of libGLw and update/remove as needed. There should at least be a pointer to the new home of libGLw too. Can we make this mesa/glw eventually to live alongside glut? -- Dan ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Removing GLw from the main Mesa repository
On Fri, Aug 5, 2011 at 7:25 AM, Benjamin Franzke benjaminfran...@googlemail.com wrote: There is nothing compiled since GLW_SOURCES is not substituted by configure: Makefile.am:29 libGLw_la_SOURCES = $(GLW_SOURCES) This would need AC_SUBST([GLW_SOURCES]) in configure.ac, but thats not allowed for _SOURCES variables, see automake output: configure.ac:96: `GLW_SOURCES' includes configure substitution `@GLW_SOURCES@' configure.ac:96: and is referred to from `libGLw_la_SOURCES'; configure.ac:96: configure substitutions are not allowed in _SOURCES variables With automake, we can do this a lot cleaner with an AM_CONDITIONAL. Shortened version: configure.ac: AM_CONDITIONAL([ENABLE_MOTIF], [test $enable_motif = yes]) Makefile.am: libGLw_la_SOURCES = GLwDrawA.c libGLw_la_CFLAGS = $(GLW_CFLAGS) libGLw_la_LIBADD = $(GLW_LIBS) if ENABLE_MOTIF libGLw_la_SOURCES += GLwMDrawA.c libGLw_la_CFLAGS += $(MOTIF_CFLAGS) libGLw_la_LIBADD += $(MOTIF_LIBS) endif -- Dan ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/2] glsl: When linking, emit functions at the tail of the final linked program.
On 4 August 2011 08:33, Eric Anholt e...@anholt.net wrote: On Wed, 3 Aug 2011 17:07:42 -0700, Paul Berry stereotype...@gmail.com wrote: When link_functions.cpp adds a new function to the final linked program, it needs to add it after any global variable declarations that the function refers to, otherwise the IR will be invalid (because variable declarations must occur before variable accesses). The easiest way to do that is to have the linker emit functions to the tail of the final linked program. The linker used to emit functions to the head of the final linked program, in an effort to keep callees sorted before their callers. However, this was not reliable: it didn't work for functions declared or defined in the same compilation unit as main, for diamond-shaped patterns in the call graph, or for some obscure cases involving overloaded functions. And no code currently relies on this sort order. Usually we'd swap the order of these around, so that if we have a testcase that would hit this, you don't get extra failures when a bisect lands on patch 1/2. That's a good point, Eric. I'll switch the order of the patches. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl: empty declarations should be valid
On 4 August 2011 18:18, Chad Versace c...@chad-versace.us wrote:
On 08/04/2011 01:29 PM, Eric Anholt wrote:
On Thu, 4 Aug 2011 12:59:35 +0900, Chia-I Wu olva...@gmail.com wrote:
From: Chia-I Wu o...@lunarg.com
Unlike C++, empty declarations such as
float;
should be valid. The spec is not explicit about this actually.
Some apps that generate their shader sources may rely on this. This was
noted when porting one of them to Linux from Windows.
Ew. Looking the GLSL 1.20 spec, I see:
statement:
declaration_statement
declaration_statement:
declaration
declaration:
init_declarator_list SEMICOLON
init_declarator_list:
single_declaration
single_declaration:
fully_specified_type
fully_specified_type IDENTIFIER
so it looks like that is actually valid code. That's awful.
I first suspected that this was a spec grammar bug. But it is still present in
the GLSL 4.10 spec, so it's unlikely to be a bug.
Surprisingly, C also allows empty declarations. Compiling this
float;
int main() { return 0; }
with `gcc --std=c99` succeeds and emits this warning:
warning: useless type name in empty declaration [enabled by default]
I hate to say this, but I believe the spec grammar intentionally allows empty
declarations. C allows it, and GLSL tries to mimic C.
Even though I don't like empty declarations, this patch is
Reviewed-by: Chad Versace c...@chad-versace.us
--
Chad Versace
c...@chad-versace.us
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Anyone want to volunteer to write a Piglit test for this? It seems
like the kind of obscure corner case that we're likely to regress on
if we don't explicitly test for it.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 39846] can' t compile mesa ‘__u64’problem .
https://bugs.freedesktop.org/show_bug.cgi?id=39846 Soohyun Lee sol...@ucsd.edu changed: What|Removed |Added Status|NEW |ASSIGNED --- Comment #2 from Soohyun Lee sol...@ucsd.edu 2011-08-05 12:12:10 PDT --- (In reply to comment #1) __u64 is supposed to be defined by the userspace headers from the Linux kernel (/usr/include/linux/, /usr/include/asm*). Where are you getting those from? I am using CentOS operating system. I did not checked directly yet (check with gcc -E flag on CentOS), but The __u64 seems to be define only in one place: /usr/include/asm/types.h -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl: empty declarations should be valid
On 08/05/2011 12:41 AM, Chia-I Wu wrote: On Fri, Aug 5, 2011 at 1:48 PM, Chad Versace c...@chad-versace.us wrote: Also, please update the commit message to say that `gcc --std=c99` allows empty declrations and include the appropriate quotation from the GLSL 1.20 spec's grammar. Without that extra info, someone may stumble onto this commit and say WTF. Oops. I've already committed it... Are you sure? I am unable to find it with `git log origin/master src/glsl`. By the way, I've submitted some tests to the Piglit list. -- Chad Versace c...@chad-versace.us signature.asc Description: OpenPGP digital signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl: empty declarations should be valid
On 08/05/2011 11:45 AM, Paul Berry wrote:
On 4 August 2011 18:18, Chad Versace c...@chad-versace.us wrote:
On 08/04/2011 01:29 PM, Eric Anholt wrote:
On Thu, 4 Aug 2011 12:59:35 +0900, Chia-I Wu olva...@gmail.com wrote:
From: Chia-I Wu o...@lunarg.com
Unlike C++, empty declarations such as
float;
should be valid. The spec is not explicit about this actually.
Some apps that generate their shader sources may rely on this. This was
noted when porting one of them to Linux from Windows.
Ew. Looking the GLSL 1.20 spec, I see:
statement:
declaration_statement
declaration_statement:
declaration
declaration:
init_declarator_list SEMICOLON
init_declarator_list:
single_declaration
single_declaration:
fully_specified_type
fully_specified_type IDENTIFIER
so it looks like that is actually valid code. That's awful.
I first suspected that this was a spec grammar bug. But it is still present
in
the GLSL 4.10 spec, so it's unlikely to be a bug.
Surprisingly, C also allows empty declarations. Compiling this
float;
int main() { return 0; }
with `gcc --std=c99` succeeds and emits this warning:
warning: useless type name in empty declaration [enabled by default]
I hate to say this, but I believe the spec grammar intentionally allows empty
declarations. C allows it, and GLSL tries to mimic C.
Even though I don't like empty declarations, this patch is
Reviewed-by: Chad Versace c...@chad-versace.us
--
Chad Versace
c...@chad-versace.us
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Anyone want to volunteer to write a Piglit test for this? It seems
like the kind of obscure corner case that we're likely to regress on
if we don't explicitly test for it.
Tests are posted on the Piglit list.
--
Chad Versace
c...@chad-versace.us
signature.asc
Description: OpenPGP digital signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 01/15] mesa: Add toplevel Android.mk
On 08/05/2011 05:41 AM, Dan Nicholson wrote: On Thu, Aug 4, 2011 at 7:55 PM, Chad Versace c...@chad-versace.us wrote: On 08/04/2011 04:17 PM, Jose Fonseca wrote: - Original Message - On Thu, Aug 4, 2011 at 2:47 AM, Chad Versace c...@chad-versace.us wrote: This is the first step in porting libGLES* and libEGL to Android. The makefile doesn't build anything yet; it just defines common variables. The values for MESA_COMMON_C_FLAGS and MESA_COMMON_CPP_FLAGS, I obtained by invoking autogen.sh with the options below and then inspecting MESA_TOP/configs/autoconf. My immediate goal is to port i965 to Android, so I used the typical flags for building i965. --disable-gallium --disable-glu --disable-glw --disable-glut --enable-32-bit --enable-egl --enable-gles2 --enable-gles1 --enable-texture-float --with-dri-drivers=i965 --with-gallium-drivers= Note: This is in preparation for porting i965 to Android. CC: Chia-I Wu o...@lunarg.com, CC: Chih-Wei Huang cwhu...@android-x86.org Signed-off-by: Chad Versace c...@chad-versace.us --- Android.mk | 66 android/mesa_local_vars.mk | 32 + 2 files changed, 98 insertions(+), 0 deletions(-) create mode 100644 Android.mk create mode 100644 android/mesa_local_vars.mk There's quite a bit of new build infrastructure here. What would it take to fit this into either the existing autoconf support or add a targeted configs/android? Duplicating a pile of make targets and adding another way to configure/build mesa seems like it might not be the best way to go. What are the current constraints that make building mesa on android difficult? I agree. I'd prefer not have yet another build infrastructure in the mix, unless there's a very good reason. Jose I also agree. I would prefer to not add another build system to Mesa. We have too many as it is. To address Dan's questions, the Android build cannot be fitted into autoconf or configs/android. I explored this option, and discovered that it was impossible. The Android build system is... well... interesting. Allow me to explain. The entirety of the Android project --- libc, webkit, the window manager, *everything* --- exists in a single source tree [1]. And that source tree is built with a single, non-recursive invocation of make. Every time I say that, I find it hard to believe myself, so I'll say it again: The entirety of the Android OS, all core libraries and apps, are built with a single, non-recursive invocation of make. (The kernel is the special exception to this all-encompassing build). The final build artifact is a bootable iso image. [1] http://android.git.kernel.org/ Looking at all those git repos, wouldn't this be more appropriate as an android project? platform/external/mesa or something? Mesa already belongs to the Android x86 tree [1] as platform/external/mesa. (The Android x86 project is a fork that closely mirrors upstream Android). It also exists as such in some private trees. [1] http://git.android-x86.org/ I haven't seen any Android.mk files show up in freetype or expat or anything like that. In the same way, mesa doesn't carry a debian folder even though that's how debian and ubuntu build mesa. Certainly if there are fixes to the existing build infrastructure that help get mesa built on android, that should be done, but I don't see why we should carry the android build bits in upstream mesa. -- Dan Dan, you make a strong point. There's no sense in cluttering Mesa with additional makefiles, at least not now. Other projects do not do that for Android, and Mesa doesn't even do it for beloved Debian. The best approach would be to maintain the Android makefiles in a separate branch, perhaps even in a personal repo. To fix the existing build infrastructure to support Android, I would like to extract source lists for some targets into a shared makefile, similiar to what has been done with src/mesa/sources.mak. Jose suggested this, and I think this is the best way to continue. (Jose, I would rather not take the approach of making one target per directory and using glob patters to pick up the source files. I expect conflicts to occur between Gallium and non-Gallium drivers.) -- Chad Versace c...@chad-versace.us signature.asc Description: OpenPGP digital signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl: empty declarations should be valid
On 08/04/2011 09:48 PM, Chad Versace wrote:
On 08/04/2011 06:18 PM, Chad Versace wrote:
On 08/04/2011 01:29 PM, Eric Anholt wrote:
On Thu, 4 Aug 2011 12:59:35 +0900, Chia-I Wu olva...@gmail.com wrote:
From: Chia-I Wu o...@lunarg.com
Unlike C++, empty declarations such as
float;
should be valid. The spec is not explicit about this actually.
Some apps that generate their shader sources may rely on this. This was
noted when porting one of them to Linux from Windows.
Ew. Looking the GLSL 1.20 spec, I see:
statement:
declaration_statement
declaration_statement:
declaration
declaration:
init_declarator_list SEMICOLON
init_declarator_list:
single_declaration
single_declaration:
fully_specified_type
fully_specified_type IDENTIFIER
so it looks like that is actually valid code. That's awful.
I first suspected that this was a spec grammar bug. But it is still present
in
the GLSL 4.10 spec, so it's unlikely to be a bug.
You do know that (at least according to Dan M.) they haven't updated the
grammar in the appendix since GLSL 1.50, right? And it already allows
completely bogus things that don't match the rest of the spec (see the
switch statement grammar).
Surprisingly, C also allows empty declarations. Compiling this
float;
int main() { return 0; }
with `gcc --std=c99` succeeds and emits this warning:
warning: useless type name in empty declaration [enabled by default]
I hate to say this, but I believe the spec grammar intentionally allows empty
declarations. C allows it, and GLSL tries to mimic C.
Even though I don't like empty declarations, this patch is
Reviewed-by: Chad Versace c...@chad-versace.us
Also, please update the commit message to say that `gcc --std=c99` allows
empty
declrations and include the appropriate quotation from the GLSL 1.20 spec's
grammar. Without that extra info, someone may stumble onto this commit and
say WTF.
I still say WTF. Do ATI, nVidia, or Apple support this? If even one of
them doesn't, I say we shouldn't either.
GLSL is so nasty sometimes.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/3] glsl: Make is_lvalue() and variable_referenced() const.
On 08/04/2011 06:55 PM, Chad Versace wrote:
On 08/02/2011 05:38 PM, Paul Berry wrote:
These functions don't modify the target instruction, so it makes sense
to make them const. This allows these functions to be called from ir
validation code (which uses const to ensure that it doesn't
accidentally modify the IR being validated).
---
src/glsl/ir.cpp |4 ++--
src/glsl/ir.h | 18 +-
2 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/src/glsl/ir.cpp b/src/glsl/ir.cpp
index 827fe8e..6f8676e 100644
--- a/src/glsl/ir.cpp
+++ b/src/glsl/ir.cpp
@@ -1096,7 +1096,7 @@
ir_dereference_record::ir_dereference_record(ir_variable *var,
}
bool
-ir_dereference::is_lvalue()
+ir_dereference::is_lvalue() const
{
ir_variable *var = this-variable_referenced();
@@ -1310,7 +1310,7 @@ ir_swizzle::create(ir_rvalue *val, const char *str,
unsigned vector_length)
#undef I
ir_variable *
-ir_swizzle::variable_referenced()
+ir_swizzle::variable_referenced() const
{
return this-val-variable_referenced();
}
diff --git a/src/glsl/ir.h b/src/glsl/ir.h
index 50a9d6e..04fa97b 100644
--- a/src/glsl/ir.h
+++ b/src/glsl/ir.h
@@ -144,7 +144,7 @@ public:
ir_rvalue *as_rvalue_to_saturate();
- virtual bool is_lvalue()
+ virtual bool is_lvalue() const
{
return false;
}
@@ -152,7 +152,7 @@ public:
/**
* Get the variable that is ultimately referenced by an r-value
*/
- virtual ir_variable *variable_referenced()
+ virtual ir_variable *variable_referenced() const
{
return NULL;
}
@@ -1355,7 +1355,7 @@ public:
virtual ir_visitor_status accept(ir_hierarchical_visitor *);
- bool is_lvalue()
+ bool is_lvalue() const
{
return val-is_lvalue() !mask.has_duplicates;
}
@@ -1363,7 +1363,7 @@ public:
/**
* Get the variable that is ultimately referenced by an r-value
*/
- virtual ir_variable *variable_referenced();
+ virtual ir_variable *variable_referenced() const;
ir_rvalue *val;
ir_swizzle_mask mask;
@@ -1387,12 +1387,12 @@ public:
return this;
}
- bool is_lvalue();
+ bool is_lvalue() const;
/**
* Get the variable that is ultimately referenced by an r-value
*/
- virtual ir_variable *variable_referenced() = 0;
+ virtual ir_variable *variable_referenced() const = 0;
};
@@ -1413,7 +1413,7 @@ public:
/**
* Get the variable that is ultimately referenced by an r-value
*/
- virtual ir_variable *variable_referenced()
+ virtual ir_variable *variable_referenced() const
{
return this-var;
}
@@ -1462,7 +1462,7 @@ public:
/**
* Get the variable that is ultimately referenced by an r-value
*/
- virtual ir_variable *variable_referenced()
+ virtual ir_variable *variable_referenced() const
{
return this-array-variable_referenced();
}
@@ -1496,7 +1496,7 @@ public:
/**
* Get the variable that is ultimately referenced by an r-value
*/
- virtual ir_variable *variable_referenced()
+ virtual ir_variable *variable_referenced() const
{
return this-record-variable_referenced();
}
This patch is sensible and straightforward.
Reviwed-by: Chad Versace c...@chad-versace.us
For patch 1:
Reviewed-by: Kenneth Graunke kenn...@whitecape.org
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 00/15] i965, mesa, glapi: Build libi965_dri on Android
On 08/04/2011 11:42 PM, Chia-I Wu wrote: On Fri, Aug 5, 2011 at 12:27 PM, Chad Versace c...@chad-versace.us wrote: On 08/04/2011 11:25 AM, Chia-I Wu wrote: Hi Chad, On Thu, Aug 4, 2011 at 6:47 PM, Chad Versace c...@chad-versace.us wrote: This is the first step in porting i965 to Android, initially to Gingerbread. Android is ubiquituous now, and the platform's importance can't be denied. The Intel drivers really should support it. The series first builds libglapi, then libmesa, and then libi965_dri. lib965_dri doens't work yet; it doesn't even link to libdrm. This is just the initial work to get it to build. I'm aware that this may be controversial, so I've attempted to integrate the Android makefiles as non-disruptively as possible. - The Android makefiles attempt to duplicate as little as possible from the existing makefiles. In one case, this dictated that some Makefile code be extracted into a shared makefile. - I've avoided modifying the existsing makefiles except when absolutely necessary. When a modification or cleanup was necessary, I have attempted to keep the changes minimal and do them incrementally. - The locations of the Android makefiles mirror those of the existing makefiles. The series does not break the Linux i965 build. I doubt it breaks other drivers' builds, but I haven't tested it. If you want me to check that it doesn't break your build, just let me know the autoconf options. The series lives on my android-gingerbread-i965 branch. If you'd like to try the Android build yourself, you should pull from my personal Android manifest [1], which closely mirrors android-x86, and set `BOARD_USES_I965 := true` in buildspeck.mk. [1] repo init -u git://anongit.freedesktop.git/~chadversary/android.manifeset.git -b gingerbread-i965 Thanks for bringing this up and working on it. Before commenting on the changes to Mesa, I'd like to talk a little bit about the external dependencies. To have a working build of Mesa for the Android Gingerbread source tree, these external changes are needed 1) modify Android's init (pid 1) to create DRI device nodes under /dev 2) make off_t 64-bit so that mmap() a GEM object works 3) figure out how to authenticate a DRM fd 4) add libdrm to Android source tree 5) add drm_gralloc (the DDX driver, sort of) to Android source tree There may be more that I cannot recall now. Of these changes, only 3), 4), and 5) affect how Mesa is built. It is safe to assume 4) and 5) are already there, as we have good relationship with those projects. But how to solve 3) is still an open question. Luckily, some key people who can help us with 3 belong to my team: the author of Wayland, and the authors of Intel's DRM module, i915.ko. I will probe them when we reach that bridge. We can either spend several months discussing 3) with the Android team before landing Android support to Mesa. I have been given a timeframe to complete this, and though I cannot discuss the details, several months is unacceptable. The Mesa port needs to be completed before yesterday. Or we can assume it is solved, land the Android support for Mesa probably over the next few weeks, and initiate the discussion with the Android team with something concrete. Now that we are at this, my preference is the latter. Yes. We should take the latter approach. I forgot to mention that we need to do 1), 2), ..., 5) only when we target for integrating into the vanilla Android tree. When we target for integration with Android-x86, these tasks are either done or worked around. Back to your changes. I am fine with landing your version and is willing to help review them, proivided your work won't stop after i915/i965 is supported. Otherwise, I prefer to prepare my version[1] for upstreaming, which can build all actively maintained drivers including Gallium ones. It would also be really great you can help, and suppose the list is fine with yet another build system for Mesa. [1] http://cgit.freedesktop.org/~olv/mesa/log/?h=android-gingerbread-7.11 You have done a lot of work on the Mesa port, and I want to acknowledge your expertise there. Please don't interpret my submission of this series as a dismissal of that work. My intent in submitting my own patches was just to start the process of getting Android support upstream, incrementally, in a public, code-reviewed forum. Other drivers, of course, should also be supported. I do not want to place any obstacle in way of that goal. At each step of the review process, we should check with each other that the approach taken is compatible with both Gallium and non-Gallium drivers. We will be basically doing everything from scratch, in smaller steps. In that case, I see no reason not to do this based on what is already there. I did plan to upstream my changes. So cleaning it up should not be a nightmare. Are there missing commits
Re: [Mesa-dev] Mesa (master): egl/gbm: Fix EGL_DEFAULT_DISPLAY
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 08/04/2011 03:32 PM, Chad Versace wrote: This commit *really* needs a more descriptive commit message. - What were the symptoms of the broken EGL_DEFAULT_DISPLAY? - What error in the code caused the problem? - What the hell did you do to fix it? This is not evident due to the length of the commit. For commits of this complexity, write descriptive messages for the sake of others. I have to agree with Chad. I'm not comfortable with the number of patches happening in the egl subtree that don't have *ANY* Reviewed-by, Acked-by, or Tested-by lines. -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/ iEYEARECAAYFAk48UzAACgkQX1gOwKyEAw/OCACdF3xMucS7urTarYlPOPnCny5j DxUAn2Xvumzn5Eotyjak4DacYBegoZmV =fkmv -END PGP SIGNATURE- ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl: empty declarations should be valid
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
On 08/05/2011 12:41 AM, Chia-I Wu wrote:
On Fri, Aug 5, 2011 at 1:48 PM, Chad Versace c...@chad-versace.us wrote:
On 08/04/2011 06:18 PM, Chad Versace wrote:
On 08/04/2011 01:29 PM, Eric Anholt wrote:
On Thu, 4 Aug 2011 12:59:35 +0900, Chia-I Wu olva...@gmail.com wrote:
From: Chia-I Wu o...@lunarg.com
Unlike C++, empty declarations such as
float;
should be valid. The spec is not explicit about this actually.
Some apps that generate their shader sources may rely on this. This was
noted when porting one of them to Linux from Windows.
Ew. Looking the GLSL 1.20 spec, I see:
statement:
declaration_statement
declaration_statement:
declaration
declaration:
init_declarator_list SEMICOLON
init_declarator_list:
single_declaration
single_declaration:
fully_specified_type
fully_specified_type IDENTIFIER
so it looks like that is actually valid code. That's awful.
I first suspected that this was a spec grammar bug. But it is still present
in
the GLSL 4.10 spec, so it's unlikely to be a bug.
Surprisingly, C also allows empty declarations. Compiling this
float;
int main() { return 0; }
with `gcc --std=c99` succeeds and emits this warning:
warning: useless type name in empty declaration [enabled by default]
I hate to say this, but I believe the spec grammar intentionally allows
empty
declarations. C allows it, and GLSL tries to mimic C.
Even though I don't like empty declarations, this patch is
Reviewed-by: Chad Versace c...@chad-versace.us
Also, please update the commit message to say that `gcc --std=c99` allows
empty
declrations and include the appropriate quotation from the GLSL 1.20 spec's
grammar. Without that extra info, someone may stumble onto this commit and
say WTF.
Oops. I've already committed it...
You have to give a little more time for review. I would like to have
had an opportunity to weigh in, but it's a bit late now.
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
iEYEARECAAYFAk48VBQACgkQX1gOwKyEAw9ZFACdHy2gnNLdszGp5PbDu84Luay1
tpoAoIcgM2pemUdDINNiAvxc/wWw3J8T
=X0eZ
-END PGP SIGNATURE-
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] upstreaming Android support [Was: i965, mesa, glapi: Build libi965_dri on Android]
On 08/05/2011 12:24 AM, Chia-I Wu wrote: On Fri, Aug 5, 2011 at 3:42 PM, Chia-I Wu olva...@gmail.com wrote: We will be basically doing everything from scratch, in smaller steps. In that case, I see no reason not to do this based on what is already there. I did plan to upstream my changes. So cleaning it up should not be a nightmare. I've pushed a branch here for discussion http://cgit.freedesktop.org/~olv/mesa/log/?h=android-gingerbread-upstream-wip This is still WIP. It still needs to be cleaned up. The goal of the branch is to provide * a clean tree to be merged to master * integration with Android(-x86) Gingerbread * gallium drivers that work well * i915/i965 drivers that work with simple tests The main focus is the integration with Android. Missing features in i915/i965 should be added later. The commits in this branch can be divided into four groups 1) Fix portability issues (the first 5 commits) Android has its own C library called bionic. There are a few places where it is incompatible with GNU C. These five commits adds compile-time tests to define the compatibility layer for Android. 2) Add support for Android to st/egl (the next 3 commits) These should only be interesting to EGL developers. These commits add _EGL_PLATFORM_ANDROID and Android-specific extensions to EGL core. Support for the new platform and extensions is then added to st/egl. 3) Integration with Android build system (the next 2 commits) This first commit here should be killed. Ignore it for now. The second commit[1] adds a bunch of Android.mk's to the source tree. Android.mk's are read by the Android build system to build Mesa. No changes are made to the existing files. IMHO, this is the better way to upstream Andoird support. We know that things work at this point. We know that nothing can break due to this commit. We can then refactor Makefile's, one by one as follow-up commits, until we are satisfied with the results. 4) Add support for Android to egl_dri2/i915/i965 (the last 3 commits) These commits add Android support to egl_dri2. Build rules for i915 and i965 are also added. The most interesting changes should be 3). I prefer the approach I described in it, but I am also fine with interleaving refactoring Makefile's with adding Android.mk's. That could take a while as we need to have a consensus on how to refactor Makefile's systematically. Thoughts? [1] direct link http://cgit.freedesktop.org/~olv/mesa/commit/?h=android-gingerbread-upstream-wipid=90f8f6dc2fa3e121a9e5921fac106d2b058e08cc The branch looks like a good candidate for merging. I gave it a quick review. [have comment]mesa: fix !FEATURE_GL build [looks good to me]glsl: remove an unnecessary header include [no comment] gallium: add PIPE_OS_ANDROID support [looks good to me]ralloc: SIZE_MAX is missing on Android [have comment]mesa: fix compile issues on Android [have comment]egl: add _EGL_PLATFORM_ANDROID [looks good to me]egl: add Android-specific extensions [needs discussion]android: add Android.mk's [needs discussion]egl_dri2: allow a platform to specify RGBA masks [have comments] egl_dri2: add support for Android I have comments on some patches, and both of the egl_dri2 patches need to be reviewed by the other EGL devs. Could you post the patch series for review? That way, it will be easier for everyone to post comments inline. Since we (you, me, Jose, and Dan) have not yet decided how to handle the build system issue, when you email the patches please strip the makefiles out. Thanks. -- Chad Versace c...@chad-versace.us signature.asc Description: OpenPGP digital signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] mesa: In validate_program(), initialize errMsg for safety.
On 07/30/2011 09:34 PM, Kenneth Graunke wrote:
validate_program relies on validate_shader_program to fill in errMsg;
empirically, there exist cases where that doesn't happen.
While tracking those down may be worthwhile, initializing the string so
we don't try to ralloc_strdup random garbage also seems wise.
Fixes issues caught by valgrind while running some test case.
NOTE: This is a candidate for stable release branches.
Signed-off-by: Kenneth Graunke kenn...@whitecape.org
---
src/mesa/main/shaderapi.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
index 8df25c3..74997ea 100644
--- a/src/mesa/main/shaderapi.c
+++ b/src/mesa/main/shaderapi.c
@@ -1125,7 +1125,7 @@ static void
validate_program(struct gl_context *ctx, GLuint program)
{
struct gl_shader_program *shProg;
- char errMsg[100];
+ char errMsg[100] = ;
shProg = _mesa_lookup_shader_program_err(ctx, program,
glValidateProgram);
if (!shProg) {
Reviewed-by: Chad Versace c...@chad-versace.us
--
Chad Versace
c...@chad-versace.us
signature.asc
Description: OpenPGP digital signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 39846] can' t compile mesa ‘__u64’problem .
https://bugs.freedesktop.org/show_bug.cgi?id=39846 --- Comment #3 from Chad Versace c...@chad-versace.us 2011-08-05 14:41:32 PDT --- (In reply to comment #2) (In reply to comment #1) __u64 is supposed to be defined by the userspace headers from the Linux kernel (/usr/include/linux/, /usr/include/asm*). Where are you getting those from? I am using CentOS operating system. I did not checked directly yet (check with gcc -E flag on CentOS), but The __u64 seems to be define only in one place: /usr/include/asm/types.h On Fedora, /usr/include/asm/types.h is provided by the kernel-headers package. Since CentOS and Fedora are both RedHat-ish, I assume the package name is the same on CentOS. From the build log, I see you're building libdrm. It's a good idea to install libdrm-devel too. -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/4] glsl: Emit function signatures at toplevel, even for built-ins.
On 2 August 2011 18:02, Ian Romanick i...@freedesktop.org wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 08/02/2011 12:18 PM, Paul Berry wrote: The case this patch is designed to fix is when a built-in function is called from inside the constant integer expression that specifies the length of an array, e.g.: #version 120 float x[cos(3.14159/2.0) 0.5 ? 1 : 2]; This case fails before the patch, because when processing an array length, we are emitting IR into a dummy exec_list (see process_array_type() in ast_to_hir.cpp). process_array_type() later checks (via an assertion) that no instructions were emitted to the dummy exec_list, based on the reasonable assumption that we shouldn't need to emit instructions to calculate the value of a constant. This This is the bit of text that should be in the commit message. :) Now it makes sense. The bit about the dummy exec_list is the important part... that people familiar with the code may have forgotten. Ok, I've updated the commit message to read as follows: glsl: Emit function signatures at toplevel, even for built-ins. The ast-to-hir conversion needs to emit function signatures in two circumstances: when a function declaration (or definition) is encountered, and when a built-in function is encountered. To avoid emitting a function signature in an illegal place (such as inside a function), emit_function() checked whether we were inside a function definition, and if so, emitted the signature before the function definition. However, this didn't cover the case of emitting function signatures for built-in functions when those built-in functions are called from inside the constant integer expression that specifies the length of a global array. This failed because when processing an array length, we are emitting IR into a dummy exec_list (see process_array_type() in ast_to_hir.cpp). process_array_type() later checks (via an assertion) that no instructions were emitted to the dummy exec_list, based on the reasonable assumption that we shouldn't need to emit instructions to calculate the value of a constant. This patch changes emit_function() so that it emits function signatures at toplevel in all cases. This partially fixes bug 38625 (https://bugs.freedesktop.org/show_bug.cgi?id=38625). The remainder of the fix is in the patch that follows. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Removing GLw from the main Mesa repository
On 08/05/2011 06:55 AM, Maxim Levitsky wrote: On Fri, 2011-08-05 at 07:45 -0600, Brian Paul wrote: On Thu, Aug 4, 2011 at 4:26 PM, Kenneth Graunke kenn...@whitecape.org wrote: Hey, I'd like to remove libGLw from the main Mesa repository. It never changes, and almost noone uses it...because GL and Motif is awesome, right? Since Debian still packages it, I pulled it into its own git repository, preserving history, and then autotooled it. You can get it here: git://people.freedesktop.org/~kwg/glw Sounds good to me. Please scan through the Mesa docs for any mention of libGLw and update/remove as needed. There should at least be a pointer to the new home of libGLw too. While at it, why not to remove libGLU from mesa as well? Best regards, Maxim Levitsky Agreed. That's next on the list. --Kenneth ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Removing GLw from the main Mesa repository
On 08/05/2011 10:58 AM, Dan Nicholson wrote: On Fri, Aug 5, 2011 at 6:45 AM, Brian Paul brian.e.p...@gmail.com wrote: On Thu, Aug 4, 2011 at 4:26 PM, Kenneth Graunke kenn...@whitecape.org wrote: Hey, I'd like to remove libGLw from the main Mesa repository. It never changes, and almost noone uses it...because GL and Motif is awesome, right? Since Debian still packages it, I pulled it into its own git repository, preserving history, and then autotooled it. You can get it here: git://people.freedesktop.org/~kwg/glw Sounds good to me. Please scan through the Mesa docs for any mention of libGLw and update/remove as needed. There should at least be a pointer to the new home of libGLw too. Can we make this mesa/glw eventually to live alongside glut? Dan Yes, of course. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Removing GLw from the main Mesa repository
On 08/05/2011 11:05 AM, Dan Nicholson wrote: On Fri, Aug 5, 2011 at 7:25 AM, Benjamin Franzke benjaminfran...@googlemail.com wrote: There is nothing compiled since GLW_SOURCES is not substituted by configure: Makefile.am:29 libGLw_la_SOURCES = $(GLW_SOURCES) This would need AC_SUBST([GLW_SOURCES]) in configure.ac, but thats not allowed for _SOURCES variables, see automake output: configure.ac:96: `GLW_SOURCES' includes configure substitution `@GLW_SOURCES@' configure.ac:96: and is referred to from `libGLw_la_SOURCES'; configure.ac:96: configure substitutions are not allowed in _SOURCES variables D'oh! Thanks for finding that, Ben. As you can see, I haven't actually tried to use libGLw. :) With automake, we can do this a lot cleaner with an AM_CONDITIONAL. Shortened version: configure.ac: AM_CONDITIONAL([ENABLE_MOTIF], [test $enable_motif = yes]) Makefile.am: libGLw_la_SOURCES = GLwDrawA.c libGLw_la_CFLAGS = $(GLW_CFLAGS) libGLw_la_LIBADD = $(GLW_LIBS) if ENABLE_MOTIF libGLw_la_SOURCES += GLwMDrawA.c libGLw_la_CFLAGS += $(MOTIF_CFLAGS) libGLw_la_LIBADD += $(MOTIF_LIBS) endif Dan Thanks Dan! That's -much- nicer. I knew there had to be a way to do this cleanly! I've pushed a rebased branch to ~kwg/glw with these fixes, as well as some glw.pc fixes and configure.ac cleanups. I'm not really sure what we want to do about tarballs. We probably ought to make a release at some point...and then never touch it again. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/3] glsl: Perform implicit type conversions on function call out parameters.
On 2 August 2011 18:27, Ian Romanick i...@freedesktop.org wrote:
+ *
+ * Also, perform implicit conversion of arguments. Note: to
+ * implicitly convert out parameters, we need to place them in a
+ * temporary variable, and do the conversion after the call
+ * takes place. Since we haven't emitted the call yet, we'll
+ * place the post-call conversions in a temporary exec_list, and
+ * emit them later.
It looks like this comment (and the others below) are wrapped to much
less than 80 columns.
They are wrapped to 70 columns, which is the default value used by
Emacs and appeals to me personally. But I care way less about
formatting details like this than I care about not losing time to
debating them, and I think the best way to avoid that is to pick a
standard and document it. What's your preferred line-wrapping width,
and would you be willing to enshrine it in docs/devinfo.html (where it
might, I fear, be subject to a round of bike-shedding by those with
strong feelings about such things)? If so, I'd be glad to update my
Emacs configuration to follow it.
*/
exec_list_iterator actual_iter = actual_parameters-iterator();
exec_list_iterator formal_iter = sig-parameters.iterator();
@@ -185,8 +194,50 @@ match_function_by_name(exec_list *instructions, const
char *name,
}
if (formal-type-is_numeric() || formal-type-is_boolean()) {
- ir_rvalue *converted = convert_component(actual, formal-type);
- actual-replace_with(converted);
+ switch (formal-mode) {
+ case ir_var_in:
+ {
The { should be on the same line as the case, and emacs got the
indentation wonkey because it's not.
For the record, this was actually completely on purpose. I prefer for
an opening brace after a case label to be on its own line, and to
cause an extra level of indent, to emphasize the fact that the brace
exists solely for the purpose of variable scoping, and is not part of
the syntax of the switch statement (this is in contrast with the {
that comes after an if statement, for example, which is part of the
syntax of if and not solely for scoping). Incidentally, the
behavior of indent -br -i3 -npcs --no-tabs (which is recommended by
devinfo.html) seems to agree with my formatting--it doesn't alter this
case block as I submitted it, though it does force the opening braces
of if-statements to be on the same line as the if. And if I force
the { to the same line as the case, the indent command continues to
generate the indentation that appears wonky to you.
I'm guessing from looking at code elsewhere in Mesa that what you
would have preferred would have been this, correct?
switch (formal-mode) {
case ir_var_in: {
ir_rvalue *converted
= convert_component(actual, formal-type);
actual-replace_with(converted);
break;
}
case ir_var_out:
...
}
Normally I wouldn't bike-shed something like this, but what bothers me
about the above is that it puts the closing brace of the case at the
same indentation as the closing brace of the switch statement. That
makes it hard to tell at a glance which closing brace terminates the
switch statement and which closing brace terminates the case, and that
is IMHO the whole point of indentation.
However, having said all that, I realize that I'm still fairly new to
the mesa project and I may just have to suck it up and conform to the
mesa rules. I just wanted to give you a chance to change your mind :)
+ ir_rvalue *converted
+ = convert_component(actual, formal-type);
+ actual-replace_with(converted);
+ }
+ break;
+ case ir_var_out:
+ if (actual-type != formal-type)
+ {
The { should be on the same line as the if.
_This_ was a complete oversight on my part. I'll fix it.
+ /* Create a temporary variable to hold the out
+ * parameter. Don't pass a hashtable to clone()
+ * because we don't want this temporary variable to
+ * be in scope.
+ */
+ ir_variable *tmp = formal-clone(ctx, NULL);
+ tmp-mode = ir_var_auto;
ir_var_auto is only used for user-defined variables, and
ir_var_temporary is used for compiler-generated variables. We'd also
usually do something like:
ir_variable *tmp =
new(mem_ctx) ir_variable(formal-type,
out_parameter_conversion,
ir_var_temporary);
Giving it a name like that makes the origin of the variable clear in
shader dumps. This has proven to be an invaluable debugging tool.
That seems very reasonable. I'll make that change.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
Re: [Mesa-dev] Removing GLw from the main Mesa repository
On 08/05/2011 06:45 AM, Brian Paul wrote: On Thu, Aug 4, 2011 at 4:26 PM, Kenneth Graunke kenn...@whitecape.org wrote: Hey, I'd like to remove libGLw from the main Mesa repository. It never changes, and almost noone uses it...because GL and Motif is awesome, right? Since Debian still packages it, I pulled it into its own git repository, preserving history, and then autotooled it. You can get it here: git://people.freedesktop.org/~kwg/glw Sounds good to me. Please scan through the Mesa docs for any mention of libGLw and update/remove as needed. There should at least be a pointer to the new home of libGLw too. -Brian Thanks, Brian. I've done so in this branch: http://cgit.freedesktop.org/~kwg/mesa/log/?h=die-glw-die ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/4] glsl: Constant-fold built-in functions before outputting IR
On 08/01/2011 04:07 PM, Paul Berry wrote: Rearranged the logic for converting the ast for a function call to hir, so that we constant fold before emitting any IR. Previously we would emit some IR, and then only later detect whether we could constant fold. The unnecessary IR would usually get cleaned up by a later optimization step, however in the case of a builtin function being used to compute an array size, it was causing an assertion. Fixes Piglit test array-size-constant-relational.vert. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=38625 --- src/glsl/ast_function.cpp | 16 ++-- 1 files changed, 14 insertions(+), 2 deletions(-) Much nicer. Nice work. Reviewed-by: Kenneth Graunke kenn...@whitecape.org ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 4/4] glsl: Check array size is const before asserting that no IR was generated.
On 08/01/2011 04:07 PM, Paul Berry wrote: process_array_type() contains an assertion to verify that no IR instructions are generated while processing the expression that specifies the size of the array. This assertion needs to happen _after_ checking whether the expression is constant. Otherwise we may crash on an illegal shader rather than reporting an error. Fixes piglit tests array-size-non-builtin-function.vert and array-size-with-side-effect.vert. Good catch. Reviewed-by: Kenneth Graunke kenn...@whitecape.org ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/2] glsl: When linking, emit functions at the tail of the final linked program.
On 08/03/2011 05:07 PM, Paul Berry wrote: When link_functions.cpp adds a new function to the final linked program, it needs to add it after any global variable declarations that the function refers to, otherwise the IR will be invalid (because variable declarations must occur before variable accesses). The easiest way to do that is to have the linker emit functions to the tail of the final linked program. The linker used to emit functions to the head of the final linked program, in an effort to keep callees sorted before their callers. However, this was not reliable: it didn't work for functions declared or defined in the same compilation unit as main, for diamond-shaped patterns in the call graph, or for some obscure cases involving overloaded functions. And no code currently relies on this sort order. No Piglit regressions with i965 Ironlake. Seems reasonable. Reviewed-by: Kenneth Graunke kenn...@whitecape.org That said, I still dislike all this nonsense. I've really wanted to separate these, so a shader contains three separate lists: 1. A list of global variable declarations (ir_variable *) 2. A list of function declarations (ir_function *) 3. Global initializer instructions (ir_instruction *) Then you wouldn't have any of this crap about emitting functions before variable declarations, functions in functions, etc, etc. Plus you wouldn't need to dynamically check whether things in your top-level list are variables/functions/etc. Quite some time ago I took a stab at this, but wasn't thinking about global initializers, so I made no provisions for them and ended up tossing the code. It'd be nice to resurrect the idea. If you'd like to try, feel free... :) ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/2] glsl: validate IR after linking (debug builds only)
On 08/03/2011 05:07 PM, Paul Berry wrote: At least one of the invariants verified by IR validation concerns the relative ordering of toplevel constructs in the IR: references to global variables must come after the declarations of those global variables. Since linking affects the ordering of toplevel constructs in the IR, it's possible that a bug in the linker will cause invalid IR to be generated, even if all the pre-linked shaders are valid. (In fact, such a bug currently exists. I'll submit a follow-up patch which fixes it.) Bugs like this are easily masked by further optimization passes, particularly inlining. So to make them easier to track down, this patch addes an IR validation step right after linking, and before final optimization occurs. The validation only occurs on debug builds. Really surprised to see that we weren't doing this. It absolutely should happen. Though, I agree with Eric...reorder them so this becomes patch #2. Reviewed-by: Kenneth Graunke kenn...@whitecape.org ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/4] Revert glsl: Skip processing the first function's body in do_dead_functions().
On 08/01/2011 04:07 PM, Paul Berry wrote:
opt_dead_functions contained a shortcut to skip processing the first
function's body, based on the assumption that IR functions are
topologically sorted, with callees always coming before their callers
(therefore the first function cannot contain any calls).
This assumption turns out not to be true in general. For example, the
following code snippet gets translated to IR that violates this
assumption:
void f();
void g();
void f() { g(); }
void g() { ... }
In practice, the shortcut didn't cause bugs because of a coincidence
of the circumstances in which opt_dead_functions is called:
(a) we do inlining right before dead function elimination, and
inlining (when successful) eliminates all calls.
(b) for user-defined functions, inlining is always successful, because
previous optimization passes (during compilation) have reduced
them to a form that is eligible for inlining.
(c) the function that appears first in the IR can't possibly call a
built-in function, because built-in functions are always emitted
before the function that calls them.
It seems unnecessarily fragile to have opt_dead_functions depend on
these coincidences. And the next patch in this series will break (c).
So I'm reverting the shortcut. The consequence will be a slight
increase in link time for complex shaders.
This reverts commit c75427f4c8767e131e5fb3de44fbc9d904cb992d.
Unfortunate, but seems necessary. I like the is_leaf() idea...things
like that ought to get us even more savings -and- clearer code.
Reviewed-by: Kenneth Graunke kenn...@whitecape.org
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] glsl: When assigning to a whole array, mark the array as accessed.
The vs-varying-array-mat2-col-row-wr test writes a mat2[3] constant to
a mat2[3] varying out array, and also statically accesses element 1 of
it on the VS and FS sides. At link time it would get trimmed down to
just 2 elements, and then codegen of the VS would end up generating
assignments to the unallocated last entry of the array. On the new
i965 VS backend, that happened to land on the vertex position.
Some issues remain in this test on softpipe, i965/old-vs and
i965/new-vs on visual inspection, but i965 is passing because only one
green pixel is probed, not the whole split green/red quad.
---
Note: This hasn't seen a full piglit run yet.
src/glsl/ast_to_hir.cpp | 21 +++--
1 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
index 7da1461..fc95f8e 100644
--- a/src/glsl/ast_to_hir.cpp
+++ b/src/glsl/ast_to_hir.cpp
@@ -649,6 +649,16 @@ validate_assignment(struct _mesa_glsl_parse_state *state,
return NULL;
}
+static void
+mark_whole_array_access(ir_rvalue *access)
+{
+ ir_dereference_variable *deref = access-as_dereference_variable();
+
+ if (deref) {
+ deref-var-max_array_access = deref-type-length - 1;
+ }
+}
+
ir_rvalue *
do_assignment(exec_list *instructions, struct _mesa_glsl_parse_state *state,
ir_rvalue *lhs, ir_rvalue *rhs, bool is_initializer,
@@ -709,6 +719,7 @@ do_assignment(exec_list *instructions, struct
_mesa_glsl_parse_state *state,
rhs-type-array_size());
d-type = var-type;
}
+ mark_whole_array_access(lhs);
}
/* Most callers of do_assignment (assign, add_assign, pre_inc/dec,
@@ -769,16 +780,6 @@ ast_node::hir(exec_list *instructions,
return NULL;
}
-static void
-mark_whole_array_access(ir_rvalue *access)
-{
- ir_dereference_variable *deref = access-as_dereference_variable();
-
- if (deref) {
- deref-var-max_array_access = deref-type-length - 1;
- }
-}
-
static ir_rvalue *
do_comparison(void *mem_ctx, int operation, ir_rvalue *op0, ir_rvalue *op1)
{
--
1.7.5.4
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
