[Mesa-dev] Shader-cache status and transition
Hi folks, I've pushed the latest version of my shader-cache work to a branch named shader-cache at: git://people.freedesktop.org/~cworth/mesa I've rebased this against the latest master branch and verified that it at least compiles and works with at least some trivial test programs. I'm not attaching the code as patches sent via email, because things really aren't ready for that yet, but instead will need a bit of attention from someone else. There are a few FIXME comments in the code, most of which should be quite trivial to address. Here are a couple of less-trivial things that still need to be done: 1. Code needs to be added to fallback to recompile everything from source when there's a cache miss. The scenario here is that glCompileShader sees a sha1 of some GLSL source code that has been seen before so optimistically doesn't do any compilation. But then, (due to some intervening state change), the shader cache may miss when it actually does a lookup based on the composite sha1 that references the program keys, etc. In this case, we need code added to do the full recompile. 2. The functions brw_upload_programs() and upload_cached_program(), (in brw_state_upload.c and brw_shader_cache.c) need some refactoring. As-is, in the patch series, there are two significant problems here: i. The upload_cached_program function makes calls to the expensive functions brw_stage_populate_key. These functions are also called subsequently by the various brw_upload_stage_prog() functions. This should be refactored so that the populate_key functions are never called more than once for a single call to brw_upload_programs. ii. The upload_cached_program has a really cheesy 64-entry array named been_there which is an attempt to avoid excessive checks of the on-disk cache. This array should be eliminated. In its place, the code from brw_upload_programs down should be refactored to find compiled binaries in the following order: 1st: The in-memory BO cache, (which is poorly named here---it's more a stash of every program seen before, there's no replacement happening so it's really not acting like a cache). 2nd: If not found there, look in the on-disk cache The current been_there array exists only because the code is structured to look at the on-disk cache first, and that will be really wasteful if the program happens to be in memory already. The right fix is to simply do the expensive checks only if the cheap checks fail. I had made several preliminary attempts at this particular refactoring, but wasn't able to get any to work completely. Ken should be up to speed on what I was doing there. Beyond that, there's obviously a lot of testing that will be needed before we have assurance that the shader cache is working well, (such as getting piglit to all pass). I've only been testing with trivial programs so far. So I anticipate that there are lots of little pieces of state that will needed to be saved and restored that are not currently happening. The hardest part should be tracking down each of these. Hopefully the actual code required in each case should be nearly trivial. Fortunately, the hard part should be very parallelizable, (everyone grab your favorite piglit test). I really had wanted to get this code into better shape before now. But the sad reality is that I don't anticipate being able to spend any more direct time on this. I will be quite happy to answer any questions that come up from whoever takes this code on. Thanks for everything. I've had a lot of fun playing with mesa the last few years, and I'm sure we'll all keep bumping into each other in various places. -Carl pgpe44380JXmV.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/5] glcpp: Allow arithmetic integer expressions in #line
On Tue, Jun 09 2015, Ian Romanick wrote: From section 3.4 (Preprocessor) of the GLSL ES 3.00 specification: #line must have, after macro substitution, one of the following forms: #line line #line line source-string-number where line and source-string-number are constant integral expressions. ... From section 4.3.3 (Constant Expressions) of the same specification: A constant integral expression is a constant expression that evaluates to a scalar signed or unsigned integer. Yes. That's an extremely unfortunate piece of the specification. This, together with unary operators introduces inherent ambiguity into the grammar. Just think about things like: #line 2-1+5 #line 2 -1+5 #line 2-1 +5 #line 2-1+5 3 #line 2 -1+5 3 #line 2-1 +5 3 That's off the top of my head. I'll dig through some old branches to see if I have some other gems for testing this stuff. FFS. I can't believe they have a test for this. For what it's worth, this makes the grammar non-LALR. Not too long ago it came up for a vote to remove this since it does not work on *ANY* desktop OpenGL implementation. While there was a majority vote to remove it, it was not a large enough majority (by a single vote). It is also a deviation from C / C++ preprocessors. Carl spent some time on this, and he couldn't find a way to make it work without adding significant bison warnings or have it fail for some cases. I would definitely push back against anyone enforcing this piece of the specification in a test suite. It's language that really doesn't belong in the specification. But I'll also take a look at this patch. Thanks for bringing it to my attention, Ian. -Carl pgpo0L7QdI8dN.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [RFC: PATCH 4/4] i965: Refactor all upload_stage_prog functions up into brw_upload_programs
Yes. That patch was severely broken in several ways. In its place, here is a pair of patches intending to do the same thing in two steps. Thanks to Ken for helpful review and guidance to get to this point. At this point, I'm quite confident that the logic of the code is undisturbed by the refactoring of the following two pages. And to the extent that our jenkins-based testing with piglit over a variety of platforms is accurate, it agrees. That said, there are some pieces of the final code that look somewhat ugly, (see the need_ff_gs condition or any of the four continue statements within the loop within brw_upload_programs). I don't think any of this ugliness is a reason to reject these patches---the exact ugliness exists already, (but is a bit more hidden since this code is split across a handful of files). I do think the ugliness could help motivate some further cleanups here if anyone is interested. (I would have done more, but I'm running out of my depth to some extent. It was easy for me to leave the logic unchanged, and it would be harder for me to change the logic in ways that would look cleaner, but would leave the final effects of the code equivalent.) For example, Ken looked at the final result and pondered whether it would make sense to merge the FF_GS and GS stages somehow, (notice that the need_ff_gs condition ensures that only one or the other is used here, never both). Aside from those cleanups, though, as I discuss in the commit message of the second patch, for the shader-cache, what I still need it to be able to pull out the per_stage_codegen from the loop in brw_upload_porograms and place it into its own loop after the first. Between the loops, then I would have the perfect place to insert a call for shader-cache lookups. It's the code within the per_stage_vue_map_update function that makes it non-trivial to do this last bit of refactoring. Ken has some ideas about moving the vue_map calculation away from here and to link-time instead. It would be great if he could do that. In the meantime, I may experiment with a less-invasive change to the vue_map code that would allow me to refactor this function to allow the shader-cache to live here like I want it to. -Carl ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/2] i965: Refactor brw_upload_programs by inlining per-stage upload functions
In this commit, the function bodies of each of the
brw_upload_stage_prog functions are manually inlined into
brw_upload_programs. This commit is intended to have no functional
change.
The resulting function body of brw_upload_programs is fairly messy,
and is expected to be cleaned up by subsequent commits that continue
the refactoring.
---
src/mesa/drivers/dri/i965/brw_ff_gs.c| 37 +---
src/mesa/drivers/dri/i965/brw_ff_gs.h| 7 +-
src/mesa/drivers/dri/i965/brw_gs.c | 59 +---
src/mesa/drivers/dri/i965/brw_gs.h | 6 +-
src/mesa/drivers/dri/i965/brw_state_upload.c | 129 +--
src/mesa/drivers/dri/i965/brw_vs.c | 43 +
src/mesa/drivers/dri/i965/brw_vs.h | 6 +-
src/mesa/drivers/dri/i965/brw_wm.c | 31 +--
src/mesa/drivers/dri/i965/brw_wm.h | 6 +-
9 files changed, 153 insertions(+), 171 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_ff_gs.c
b/src/mesa/drivers/dri/i965/brw_ff_gs.c
index f72f37f..4c438e5 100644
--- a/src/mesa/drivers/dri/i965/brw_ff_gs.c
+++ b/src/mesa/drivers/dri/i965/brw_ff_gs.c
@@ -149,7 +149,7 @@ brw_codegen_ff_gs_prog(struct brw_context *brw,
ralloc_free(mem_ctx);
}
-static bool
+bool
brw_ff_gs_state_dirty(struct brw_context *brw)
{
return brw_state_dirty(brw,
@@ -159,7 +159,7 @@ brw_ff_gs_state_dirty(struct brw_context *brw)
BRW_NEW_VS_PROG_DATA);
}
-static void
+void
brw_ff_gs_populate_key(struct brw_context *brw,
struct brw_ff_gs_prog_key *key)
{
@@ -231,36 +231,3 @@ brw_ff_gs_populate_key(struct brw_context *brw,
brw-primitive == _3DPRIM_LINELOOP);
}
}
-
-/* Calculate interpolants for triangle and line rasterization.
- */
-void
-brw_upload_ff_gs_prog(struct brw_context *brw)
-{
- struct brw_ff_gs_prog_key key;
-
- if (!brw_ff_gs_state_dirty(brw))
- return;
-
- /* Populate the key:
-*/
- brw_ff_gs_populate_key(brw, key);
-
- if (brw-ff_gs.prog_active != key.need_gs_prog) {
- brw-ctx.NewDriverState |= BRW_NEW_FF_GS_PROG_DATA;
- brw-ff_gs.prog_active = key.need_gs_prog;
- }
-
- if (brw-ff_gs.prog_active) {
- if (!brw_search_cache(brw-cache, BRW_CACHE_FF_GS_PROG,
- key, sizeof(key),
- brw-ff_gs.prog_offset, brw-ff_gs.prog_data)) {
- brw_codegen_ff_gs_prog(brw, key);
- }
- }
-}
-
-void gen6_brw_upload_ff_gs_prog(struct brw_context *brw)
-{
- brw_upload_ff_gs_prog(brw);
-}
diff --git a/src/mesa/drivers/dri/i965/brw_ff_gs.h
b/src/mesa/drivers/dri/i965/brw_ff_gs.h
index 9e016b8..dca6405 100644
--- a/src/mesa/drivers/dri/i965/brw_ff_gs.h
+++ b/src/mesa/drivers/dri/i965/brw_ff_gs.h
@@ -110,10 +110,13 @@ void brw_ff_gs_lines(struct brw_ff_gs_compile *c);
void gen6_sol_program(struct brw_ff_gs_compile *c,
struct brw_ff_gs_prog_key *key,
unsigned num_verts, bool check_edge_flag);
-void gen6_brw_upload_ff_gs_prog(struct brw_context *brw);
+
+bool
+brw_ff_gs_state_dirty(struct brw_context *brw);
void
-brw_upload_ff_gs_prog(struct brw_context *brw);
+brw_ff_gs_populate_key(struct brw_context *brw,
+ struct brw_ff_gs_prog_key *key);
void
brw_codegen_ff_gs_prog(struct brw_context *brw,
diff --git a/src/mesa/drivers/dri/i965/brw_gs.c
b/src/mesa/drivers/dri/i965/brw_gs.c
index 52c7303..71765f9 100644
--- a/src/mesa/drivers/dri/i965/brw_gs.c
+++ b/src/mesa/drivers/dri/i965/brw_gs.c
@@ -290,7 +290,7 @@ brw_codegen_gs_prog(struct brw_context *brw,
return true;
}
-static bool
+bool
brw_gs_state_dirty(struct brw_context *brw)
{
return brw_state_dirty(brw,
@@ -300,7 +300,7 @@ brw_gs_state_dirty(struct brw_context *brw)
BRW_NEW_VUE_MAP_VS);
}
-static void
+void
brw_gs_populate_key(struct brw_context *brw,
struct brw_gs_prog_key *key)
{
@@ -324,61 +324,6 @@ brw_gs_populate_key(struct brw_context *brw,
key-input_varyings = brw-vue_map_vs.slots_valid;
}
-void
-brw_upload_gs_prog(struct brw_context *brw)
-{
- struct gl_context *ctx = brw-ctx;
- struct gl_shader_program **current = ctx-_Shader-CurrentProgram;
- struct brw_stage_state *stage_state = brw-gs.base;
- struct brw_gs_prog_key key;
- /* BRW_NEW_GEOMETRY_PROGRAM */
- struct brw_geometry_program *gp =
- (struct brw_geometry_program *) brw-geometry_program;
-
- if (!brw_gs_state_dirty(brw))
- return;
-
- if (gp == NULL) {
- /* No geometry shader. Vertex data just passes straight through. */
- if (brw-ctx.NewDriverState BRW_NEW_VUE_MAP_VS) {
- brw-vue_map_geom_out = brw-vue_map_vs;
- brw-ctx.NewDriverState |= BRW_NEW_VUE_MAP_GEOM_OUT;
- }
-
- if (brw-gen == 6
- (brw-ctx.NewDriverState BRW_NEW_TRANSFORM_FEEDBACK)) {
- gen6_brw_upload_ff_gs_prog(brw);
- return;
- }
-
-
[Mesa-dev] [PATCH 2/2] i965: Refactor brw_upload_programs with a loop over each stage
This refactor idetnfies as much common code as possible across the various
stages within brw_upload_programs. The resulting code is a loop over all
relevant stages and various accessory functions (per_stage_state_dirty,
per_stage_populate_key, per_stage_codegen, and per_stage_vue_map_update),
whenever the code needs to vary from one stage to another.
This code is intended to have no functional change, and has been verified with
piglit across several Intel platforms.
From here, any remaining conditionals within brw_upload_programs indicate code
that could benefit from some simplification, (such as combinining the FF_GS
and GS stages or otherwise reducing some special cases).
This refactoring is intended to progress toward a point where on-disk
shader-cache lookups can be inserted after brw_search_cache has been called
for each stage, but before the per_stage_codegen function has been called for
any stage. But before we can do that, the vue_map_update code will need to be
reworked in some form or another. (The current code hangs all of the vue_map
state off of prog_data such that it cannot be performed until after codegen,
but the vue_map update code also modifies state that will be examined by the
state_dirty calls of subsequent stages. So this will need to be disentangled
before we can make further progress to prepare for the shader-cache here.)
---
src/mesa/drivers/dri/i965/brw_state_upload.c | 304 ++-
1 file changed, 207 insertions(+), 97 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_state_upload.c
b/src/mesa/drivers/dri/i965/brw_state_upload.c
index 7fa434f..c75eef0 100644
--- a/src/mesa/drivers/dri/i965/brw_state_upload.c
+++ b/src/mesa/drivers/dri/i965/brw_state_upload.c
@@ -615,138 +615,248 @@ brw_print_dirty_count(struct dirty_bit_map *bit_map)
}
}
-static inline void
-brw_upload_programs(struct brw_context *brw,
-enum brw_pipeline pipeline)
+static bool
+per_stage_state_dirty(struct brw_context *brw,
+ enum brw_cache_id stage)
+{
+ switch (stage) {
+ case BRW_CACHE_VS_PROG:
+ return brw_vs_state_dirty(brw);
+ case BRW_CACHE_FF_GS_PROG:
+ return brw_ff_gs_state_dirty(brw);
+ case BRW_CACHE_GS_PROG:
+ return brw_gs_state_dirty(brw);
+ case BRW_CACHE_FS_PROG:
+ return brw_wm_state_dirty(brw);
+ default:
+ unreachable(not reached);
+ }
+}
+
+struct key_block
{
- struct gl_context *ctx = brw-ctx;
- struct gl_shader_program **current = ctx-_Shader-CurrentProgram;
- struct gl_shader_program *current_fp =
ctx-_Shader-_CurrentFragmentProgram;
struct brw_vs_prog_key vs_key;
struct brw_ff_gs_prog_key ff_gs_key;
struct brw_gs_prog_key gs_key;
struct brw_wm_prog_key wm_key;
- struct brw_stage_state *stage_state = brw-gs.base;
- struct brw_vertex_program *vp =
- (struct brw_vertex_program *) brw-vertex_program;
- struct brw_geometry_program *gp =
- (struct brw_geometry_program *) brw-geometry_program;
- struct brw_fragment_program *fp =
- (struct brw_fragment_program *) brw-fragment_program;
+};
- if (pipeline == BRW_RENDER_PIPELINE) {
+static void
+per_stage_populate_key(struct brw_context *brw,
+ enum brw_cache_id stage,
+ struct key_block *key_block,
+ void **key_ret,
+ size_t *key_size_ret,
+ uint32_t **prog_offset_ret,
+ void **prog_data_ret)
+{
+ switch (stage) {
+ case BRW_CACHE_VS_PROG:
+ brw_vs_populate_key(brw, key_block-vs_key);
+ *key_ret = key_block-vs_key;
+ *key_size_ret = sizeof(key_block-vs_key);
+ *prog_offset_ret = brw-vs.base.prog_offset;
+ *prog_data_ret = brw-vs.prog_data;
+ break;
+ case BRW_CACHE_FF_GS_PROG:
+ brw_ff_gs_populate_key(brw, key_block-ff_gs_key);
+ *key_ret = key_block-ff_gs_key;
+ *key_size_ret = sizeof(key_block-ff_gs_key);
+ *prog_offset_ret = brw-ff_gs.prog_offset;
+ *prog_data_ret = brw-ff_gs.prog_data;
+ break;
+ case BRW_CACHE_GS_PROG:
+ brw_gs_populate_key(brw, key_block-gs_key);
+ *key_ret = key_block-gs_key;
+ *key_size_ret = sizeof(key_block-gs_key);
+ *prog_offset_ret = brw-gs.base.prog_offset;
+ *prog_data_ret = brw-gs.prog_data;
+ break;
+ case BRW_CACHE_FS_PROG:
+ brw_wm_populate_key(brw, key_block-wm_key);
+ *key_ret = key_block-wm_key;
+ *key_size_ret = sizeof(key_block-wm_key);
+ *prog_offset_ret = brw-wm.base.prog_offset;
+ *prog_data_ret = brw-wm.prog_data;
+ break;
+ default:
+ unreachable(not reached);
+ break;
+ }
+}
- if (brw_vs_state_dirty(brw)) {
+static bool
+per_stage_codegen(struct brw_context *brw,
+ enum brw_cache_id stage,
+ struct key_block *key_block)
+{
+ struct gl_context *ctx = brw-ctx;
- brw_vs_populate_key(brw, vs_key);
+ switch (stage)
Re: [Mesa-dev] [PATCH] Fix automatic indentation mode for recent emacs, use fewer columns in .git
On Wed, Apr 08 2015, Neil Roberts wrote: It seems a bit strange that this has stopped working for you. Yes. I don't understand exactly what's going on. mode. The C and C++ modes both inherit from prog-mode, as well as a bunch of other ones such as Python and lisp files. That's what I guessed, (given that we have prog-mode in our files). I tried investigating a little bit, but didn't get too far. From an editor session editing an emacs file, (whether my standard environment or with emacs -q), M-x describe-mode says: C/l mode: Major mode for editing KR and ANSI C code. Which looks pretty standard to me. But I don't know what the identifier for this mode would be to specify it in the .dir-locals.el file nor what modes it inherits from. If you are using a non-standard mode for C files it would be surprising if it doesn't also inherit from prog-mode. I have just tested this with emacs -q (to prevent it from loading my personal config) on Emacs 24.3.1 and it does work as is. No non-standard mode here, (at least not intentionally). And I also verified the behavior is the same with emacs -q. Maybe there's a Debian-specific bug that I'm hitting here? I don't think the patch would break anything for me since you explicitly set the fill-column back to 70 for commit messages so I don't care enough to complain if you want to commit it anyway, but it does seem like something fishy is going on and the reasoning in the commit message doesn't add up. I won't disagree there. I don't know the actual root cause, but since this fixes an actual problem for me, and we haven't identified any negative side effects, I'll plan to commit this change. And if anyone can diagnose the root cause and improve .dir-locals.el further, that will be fine too. -Carl pgpD6Q3OM6D8Y.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] Fix automatic indentation mode for recent emacs, use fewer columns in .git
On Fri, Apr 03 2015, Jordan Justen wrote: - (fill-column . 78) Do we want to remove this? Or does it match the default? The default is actually 70. I didn't mean to have that part in the commit. Thanks for noticing. + (.git (nil (fill-column . 70))) Should the commit subject line be under 70 characters? I notice that yours is 74. :) Right. I don't use auto-fill, and I don't always hit the fill-paragraph key if I'm only typing a single line. https://www.kernel.org/doc/Documentation/SubmittingPatches says the subject line should be limited to 70-75 characters. Phew! I at least fit within the kernel guideline. Maybe the .git part should be moved that into a separate patch? I thought about that, but in one sense, all of this mucking around with trying to get emacs to behave in a sane way is somewhat off-topic for mesa, so I thought keeping it in just one commit was more polite. Not a huge deal though, so, Reviewed-by: Jordan Justen jordan.l.jus...@intel.com Thanks. -Carl pgpkwl9DXKwgq.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] Fix automatic indentation mode for recent emacs, use fewer columns in .git
I recently noticed (after upgrading to emacs 24?) that I was no longer getting automatic C-style settings in emacs like I was accustomed to getting. That is, I was now getting a default indentation of 8 and indentation with tabs instead of spaces. It appears that the .dir-locals.el file is no longer taking effect. Presumably, emacs was previously using prog-mode for C and C++ source files but is now using a mode with some other name? I didn't chase down the name of the current mode, but just using nil makes these variables get set on all files, (which should be mostly harmless), and should be compatible with both old and new emacs. I did verify that the later change in this file (to indent with tabs when in makefile-mode) still takes precendence as desired. While editing these files, I've also set things up to use a smaller value for fill-column when editing a file within the .git directory. This will help avoid commit messages getting wrapped when git log adds some extra indentation. Note: If this change causes .dir-locals.el to take effect for someone when it never had before, then emacs may prompt about the potentially unsafe eval block here. User can reply to that prompt with ! to permanently whitelist this particular eval block as safe so that prompt will not be seen again in the future. --- .dir-locals.el| 4 ++-- src/gallium/drivers/freedreno/.dir-locals.el | 2 +- src/gallium/drivers/r600/.dir-locals.el | 2 +- src/gallium/drivers/radeon/.dir-locals.el | 2 +- src/gallium/drivers/radeonsi/.dir-locals.el | 2 +- src/gallium/drivers/vc4/.dir-locals.el| 2 +- src/gallium/drivers/vc4/kernel/.dir-locals.el | 2 +- src/gallium/winsys/radeon/.dir-locals.el | 2 +- src/mesa/drivers/dri/nouveau/.dir-locals.el | 2 +- 9 files changed, 10 insertions(+), 10 deletions(-) diff --git a/.dir-locals.el b/.dir-locals.el index d95eb48..f44d964 100644 --- a/.dir-locals.el +++ b/.dir-locals.el @@ -1,12 +1,12 @@ -((prog-mode +((nil (indent-tabs-mode . nil) (tab-width . 8) (c-basic-offset . 3) (c-file-style . stroustrup) - (fill-column . 78) (eval . (progn (c-set-offset 'innamespace '0) (c-set-offset 'inline-open '0))) ) + (.git (nil (fill-column . 70))) (makefile-mode (indent-tabs-mode . t)) ) diff --git a/src/gallium/drivers/freedreno/.dir-locals.el b/src/gallium/drivers/freedreno/.dir-locals.el index aa20d49..c26578b 100644 --- a/src/gallium/drivers/freedreno/.dir-locals.el +++ b/src/gallium/drivers/freedreno/.dir-locals.el @@ -1,4 +1,4 @@ -((prog-mode +((nil (indent-tabs-mode . true) (tab-width . 4) (c-basic-offset . 4) diff --git a/src/gallium/drivers/r600/.dir-locals.el b/src/gallium/drivers/r600/.dir-locals.el index 4e35c12..8be6a30 100644 --- a/src/gallium/drivers/r600/.dir-locals.el +++ b/src/gallium/drivers/r600/.dir-locals.el @@ -1,4 +1,4 @@ -((prog-mode +((nil (indent-tabs-mode . true) (tab-width . 8) (c-basic-offset . 8) diff --git a/src/gallium/drivers/radeon/.dir-locals.el b/src/gallium/drivers/radeon/.dir-locals.el index 4e35c12..8be6a30 100644 --- a/src/gallium/drivers/radeon/.dir-locals.el +++ b/src/gallium/drivers/radeon/.dir-locals.el @@ -1,4 +1,4 @@ -((prog-mode +((nil (indent-tabs-mode . true) (tab-width . 8) (c-basic-offset . 8) diff --git a/src/gallium/drivers/radeonsi/.dir-locals.el b/src/gallium/drivers/radeonsi/.dir-locals.el index 4e35c12..8be6a30 100644 --- a/src/gallium/drivers/radeonsi/.dir-locals.el +++ b/src/gallium/drivers/radeonsi/.dir-locals.el @@ -1,4 +1,4 @@ -((prog-mode +((nil (indent-tabs-mode . true) (tab-width . 8) (c-basic-offset . 8) diff --git a/src/gallium/drivers/vc4/.dir-locals.el b/src/gallium/drivers/vc4/.dir-locals.el index ac94242..ed10dc2 100644 --- a/src/gallium/drivers/vc4/.dir-locals.el +++ b/src/gallium/drivers/vc4/.dir-locals.el @@ -1,4 +1,4 @@ -((prog-mode +((nil (indent-tabs-mode . nil) (tab-width . 8) (c-basic-offset . 8) diff --git a/src/gallium/drivers/vc4/kernel/.dir-locals.el b/src/gallium/drivers/vc4/kernel/.dir-locals.el index 49403de..2e58e90 100644 --- a/src/gallium/drivers/vc4/kernel/.dir-locals.el +++ b/src/gallium/drivers/vc4/kernel/.dir-locals.el @@ -1,4 +1,4 @@ -((prog-mode +((nil (indent-tabs-mode . t) (tab-width . 8) (c-basic-offset . 8) diff --git a/src/gallium/winsys/radeon/.dir-locals.el b/src/gallium/winsys/radeon/.dir-locals.el index d5f0f04..0e937bb 100644 --- a/src/gallium/winsys/radeon/.dir-locals.el +++ b/src/gallium/winsys/radeon/.dir-locals.el @@ -1,4 +1,4 @@ -((prog-mode +((nil (indent-tabs-mode . nil) (tab-width . 8) (c-basic-offset . 4) diff --git a/src/mesa/drivers/dri/nouveau/.dir-locals.el b/src/mesa/drivers/dri/nouveau/.dir-locals.el index 774f023..29735e8 100644 --- a/src/mesa/drivers/dri/nouveau/.dir-locals.el +++ b/src/mesa/drivers/dri/nouveau/.dir-locals.el @@ -1,4 +1,4 @@ -((prog-mode +((nil (indent-tabs-mode . true) (tab-width . 8)
Re: [Mesa-dev] [PATCH 3/4] i965: Rename do_stage_prog to brw_stage_compile
On Fri, Mar 20 2015, Chris Forbes wrote: I think that having both the existing `struct brw_vs_compile` and a function with the same name is going to cause confusion. (same with the other non-fs stages) In an earlier version of the patch I had brw_vs_do_compile, (there is a do precedent in the code being replaced here). I could go back to that if it helps. -Carl pgpmHnTz9onNG.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/4] i965: Split out brw_stage_populate_key into their own functions
On Fri, Mar 20 2015, Ian Romanick wrote: +brw_gs_populate_key(struct brw_context *brw, +struct brw_gs_prog_key *key) Tabs. There may be some in other places too. Thunderbird's editor isn't too smart to be able to search for tabs... it matches any whitespace. Sorry about that! I don't know what changed, but recently the .dir-locals.el file stopped having any effect on emacs for me like it should, (and like it used to). Does any other emacs user have an idea for me? So I've been manually changing the indentation from 8 to 3, but I hadn't realized I also needed to be changing the setting to use spaces instead of tabs. I'll fix that for the series, (and hopefully fix this to get set automatically once again). -Carl pgpxwIrzDk1ZL.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [RFC: PATCH 4/4] i965: Refactor all upload_stage_prog functions up into brw_upload_programs
This completes the refactoring being prepared for in the previous
commits. The function bodies of all brw_upadload_stage_prog
functions are pulled up into brw_upload_programs where there are
unified with loops and switch statements.
The purpose of this refactoring is to allow for new code to
be added after all programs have been queried from the in-memory
cache, but before any programs have been recompiled. It's exactly at
that point that we want to add the check of the on-disk cache.
(Previously, there was no such single point in the
execution---instead, for each stage there was a point in
brw_upload_stage_prog after the in-memory-cache query and before the
recompile.)
This commit is intended to have no functional change, but whether that
is actually achieved is not as clearly obvious as in the previous few
commits.
---
On this patch in particular, I would appreciate some review.
This may be entirely broken, (or a freak failure happened which
aborted the test run I tried---I'm not sure which). I'm only sending
this out ine spite of the inconclusive testing because I'm about to be
gone on vacation for a week, and I could use some input here.
With this patch, it would be possible to have the new code act 100%
identical to the code before, but I don't think that would be
useful. Keeping all of the ordering, etc. exactly the same would
complicate the code more than necesary, (especially when the final
results don't actually depend on the order, etc.).
But since I'm not the most familiar with this code and all of its side
effects, I could use some input from others who might know it
better. So if you've got any high-level input on how you think the
code should look, I'd like to hear it.
Most of the state complexity is dealing with the fixed-function GS
stuff. Take a look if you can.
Thanks,
-Carl
src/mesa/drivers/dri/i965/brw_ff_gs.c| 39 +
src/mesa/drivers/dri/i965/brw_ff_gs.h| 11 +-
src/mesa/drivers/dri/i965/brw_gs.c | 62 +--
src/mesa/drivers/dri/i965/brw_gs.h | 13 +-
src/mesa/drivers/dri/i965/brw_state_upload.c | 251 ++-
src/mesa/drivers/dri/i965/brw_vs.c | 45 +
src/mesa/drivers/dri/i965/brw_vs.h | 12 +-
src/mesa/drivers/dri/i965/brw_wm.c | 32 +---
src/mesa/drivers/dri/i965/brw_wm.h | 6 +-
9 files changed, 295 insertions(+), 176 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_ff_gs.c
b/src/mesa/drivers/dri/i965/brw_ff_gs.c
index 8bc0a1c..6ae7ffa 100644
--- a/src/mesa/drivers/dri/i965/brw_ff_gs.c
+++ b/src/mesa/drivers/dri/i965/brw_ff_gs.c
@@ -45,7 +45,7 @@
#include util/ralloc.h
-static void
+void
brw_ff_gs_compile(struct brw_context *brw,
struct brw_ff_gs_prog_key *key)
{
@@ -148,7 +148,7 @@ brw_ff_gs_compile(struct brw_context *brw,
ralloc_free(mem_ctx);
}
-static bool
+bool
brw_ff_gs_state_dirty(struct brw_context *brw)
{
return brw_state_dirty(brw,
@@ -158,7 +158,7 @@ brw_ff_gs_state_dirty(struct brw_context *brw)
BRW_NEW_VS_PROG_DATA);
}
-static void
+void
brw_ff_gs_populate_key(struct brw_context *brw,
struct brw_ff_gs_prog_key *key)
{
@@ -230,36 +230,3 @@ brw_ff_gs_populate_key(struct brw_context *brw,
brw-primitive == _3DPRIM_LINELOOP);
}
}
-
-/* Calculate interpolants for triangle and line rasterization.
- */
-void
-brw_upload_ff_gs_prog(struct brw_context *brw)
-{
- struct brw_ff_gs_prog_key key;
-
- if (!brw_ff_gs_state_dirty(brw))
- return;
-
- /* Populate the key:
-*/
- brw_ff_gs_populate_key(brw, key);
-
- if (brw-ff_gs.prog_active != key.need_gs_prog) {
- brw-state.dirty.brw |= BRW_NEW_FF_GS_PROG_DATA;
- brw-ff_gs.prog_active = key.need_gs_prog;
- }
-
- if (brw-ff_gs.prog_active) {
- if (!brw_search_cache(brw-cache, BRW_CACHE_FF_GS_PROG,
- key, sizeof(key),
- brw-ff_gs.prog_offset, brw-ff_gs.prog_data)) {
-brw_ff_gs_compile(brw, key);
- }
- }
-}
-
-void gen6_brw_upload_ff_gs_prog(struct brw_context *brw)
-{
- brw_upload_ff_gs_prog(brw);
-}
diff --git a/src/mesa/drivers/dri/i965/brw_ff_gs.h
b/src/mesa/drivers/dri/i965/brw_ff_gs.h
index e4afdab..1f831a2 100644
--- a/src/mesa/drivers/dri/i965/brw_ff_gs.h
+++ b/src/mesa/drivers/dri/i965/brw_ff_gs.h
@@ -110,9 +110,16 @@ void brw_ff_gs_lines(struct brw_ff_gs_compile *c);
void gen6_sol_program(struct brw_ff_gs_compile *c,
struct brw_ff_gs_prog_key *key,
unsigned num_verts, bool check_edge_flag);
-void gen6_brw_upload_ff_gs_prog(struct brw_context *brw);
+
+bool
+brw_ff_gs_state_dirty(struct brw_context *brw);
+
+void
+brw_ff_gs_populate_key(struct brw_context *brw,
+ struct brw_ff_gs_prog_key *key);
void
-brw_upload_ff_gs_prog(struct brw_context *brw);
+brw_ff_gs_compile(struct brw_context *brw,
+
[Mesa-dev] [PATCH 1/4] i965: Split out brw_stage_populate_key into their own functions
This commit splits portions of the existing brw_upload_vs_prog and
brw_upload_gs_prog function into new brw_vs_populate_key and
brw_gs_populate_key functions. This follows the same style as is
already present for all other stages, (see brw_wm_populate_key, etc.).
This commit is intended to have no functional change. It exists in
preparation for some upcoming code movement in preparation for the
shader cache.
---
src/mesa/drivers/dri/i965/brw_ff_gs.c | 7 +++--
src/mesa/drivers/dri/i965/brw_gs.c| 39 ++-
src/mesa/drivers/dri/i965/brw_vs.c| 58 +--
3 files changed, 64 insertions(+), 40 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_ff_gs.c
b/src/mesa/drivers/dri/i965/brw_ff_gs.c
index 828e383..1dec2ab 100644
--- a/src/mesa/drivers/dri/i965/brw_ff_gs.c
+++ b/src/mesa/drivers/dri/i965/brw_ff_gs.c
@@ -147,8 +147,9 @@ static void compile_ff_gs_prog(struct brw_context *brw,
ralloc_free(mem_ctx);
}
-static void populate_key(struct brw_context *brw,
- struct brw_ff_gs_prog_key *key)
+static void
+brw_ff_gs_populate_key(struct brw_context *brw,
+ struct brw_ff_gs_prog_key *key)
{
static const unsigned swizzle_for_offset[4] = {
BRW_SWIZZLE4(0, 1, 2, 3),
@@ -235,7 +236,7 @@ brw_upload_ff_gs_prog(struct brw_context *brw)
/* Populate the key:
*/
- populate_key(brw, key);
+ brw_ff_gs_populate_key(brw, key);
if (brw-ff_gs.prog_active != key.need_gs_prog) {
brw-state.dirty.brw |= BRW_NEW_FF_GS_PROG_DATA;
diff --git a/src/mesa/drivers/dri/i965/brw_gs.c
b/src/mesa/drivers/dri/i965/brw_gs.c
index 45c157a..3458df3 100644
--- a/src/mesa/drivers/dri/i965/brw_gs.c
+++ b/src/mesa/drivers/dri/i965/brw_gs.c
@@ -288,6 +288,30 @@ do_gs_prog(struct brw_context *brw,
return true;
}
+static void
+brw_gs_populate_key(struct brw_context *brw,
+ struct brw_gs_prog_key *key)
+{
+ struct gl_context *ctx = brw-ctx;
+ struct brw_stage_state *stage_state = brw-gs.base;
+ struct brw_geometry_program *gp =
+ (struct brw_geometry_program *) brw-geometry_program;
+ struct gl_program *prog = gp-program.Base;
+
+ memset(key, 0, sizeof(*key));
+
+ key-base.program_string_id = gp-id;
+ brw_setup_vue_key_clip_info(brw, key-base,
+ gp-program.Base.UsesClipDistanceOut);
+
+ /* _NEW_TEXTURE */
+ brw_populate_sampler_prog_key_data(ctx, prog, stage_state-sampler_count,
+ key-base.tex);
+
+ /* BRW_NEW_VUE_MAP_VS */
+ key-input_varyings = brw-vue_map_vs.slots_valid;
+}
+
void
brw_upload_gs_prog(struct brw_context *brw)
{
@@ -327,20 +351,7 @@ brw_upload_gs_prog(struct brw_context *brw)
return;
}
- struct gl_program *prog = gp-program.Base;
-
- memset(key, 0, sizeof(key));
-
- key.base.program_string_id = gp-id;
- brw_setup_vue_key_clip_info(brw, key.base,
- gp-program.Base.UsesClipDistanceOut);
-
- /* _NEW_TEXTURE */
- brw_populate_sampler_prog_key_data(ctx, prog, stage_state-sampler_count,
- key.base.tex);
-
- /* BRW_NEW_VUE_MAP_VS */
- key.input_varyings = brw-vue_map_vs.slots_valid;
+ brw_gs_populate_key(brw, key);
if (!brw_search_cache(brw-cache, BRW_CACHE_GS_PROG,
key, sizeof(key),
diff --git a/src/mesa/drivers/dri/i965/brw_vs.c
b/src/mesa/drivers/dri/i965/brw_vs.c
index ba2c23d..b8e91c1 100644
--- a/src/mesa/drivers/dri/i965/brw_vs.c
+++ b/src/mesa/drivers/dri/i965/brw_vs.c
@@ -401,64 +401,76 @@ brw_setup_vue_key_clip_info(struct brw_context *brw,
}
}
-void
-brw_upload_vs_prog(struct brw_context *brw)
+static void
+brw_vs_populate_key(struct brw_context *brw,
+struct brw_vs_prog_key *key)
{
struct gl_context *ctx = brw-ctx;
- struct brw_vs_prog_key key;
/* BRW_NEW_VERTEX_PROGRAM */
struct brw_vertex_program *vp =
(struct brw_vertex_program *)brw-vertex_program;
struct gl_program *prog = (struct gl_program *) brw-vertex_program;
int i;
- if (!brw_state_dirty(brw,
-_NEW_BUFFERS |
-_NEW_LIGHT |
-_NEW_POINT |
-_NEW_POLYGON |
-_NEW_TEXTURE |
-_NEW_TRANSFORM,
-BRW_NEW_VERTEX_PROGRAM |
-BRW_NEW_VS_ATTRIB_WORKAROUNDS))
- return;
-
- memset(key, 0, sizeof(key));
+ memset(key, 0, sizeof(*key));
/* Just upload the program verbatim for now. Always send it all
* the inputs it asks for, whether they are varying or not.
*/
- key.base.program_string_id = vp-id;
- brw_setup_vue_key_clip_info(brw, key.base,
+ key-base.program_string_id = vp-id;
+ brw_setup_vue_key_clip_info(brw, key-base,
vp-program.Base.UsesClipDistanceOut);
/* _NEW_POLYGON
[Mesa-dev] [PATCH 2/4] i965: Split out per-stage dirty-bit checking into separate functions
The dirty-bit checking from each brw_upload_stage_prog function is
split out into its a new brw_stage_state_dirty function.
This commit is intended to have no functional change. It exists in
preparation for some upcoming code movement in preparation for the
shader cache.
---
src/mesa/drivers/dri/i965/brw_ff_gs.c | 16 ++-
src/mesa/drivers/dri/i965/brw_gs.c| 16 ++-
src/mesa/drivers/dri/i965/brw_vs.c| 24 +-
src/mesa/drivers/dri/i965/brw_wm.c| 38 ---
4 files changed, 59 insertions(+), 35 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_ff_gs.c
b/src/mesa/drivers/dri/i965/brw_ff_gs.c
index 1dec2ab..c589171 100644
--- a/src/mesa/drivers/dri/i965/brw_ff_gs.c
+++ b/src/mesa/drivers/dri/i965/brw_ff_gs.c
@@ -147,6 +147,16 @@ static void compile_ff_gs_prog(struct brw_context *brw,
ralloc_free(mem_ctx);
}
+static bool
+brw_ff_gs_state_dirty(struct brw_context *brw)
+{
+ return brw_state_dirty(brw,
+ _NEW_LIGHT,
+ BRW_NEW_PRIMITIVE |
+ BRW_NEW_TRANSFORM_FEEDBACK |
+ BRW_NEW_VS_PROG_DATA);
+}
+
static void
brw_ff_gs_populate_key(struct brw_context *brw,
struct brw_ff_gs_prog_key *key)
@@ -227,11 +237,7 @@ brw_upload_ff_gs_prog(struct brw_context *brw)
{
struct brw_ff_gs_prog_key key;
- if (!brw_state_dirty(brw,
-_NEW_LIGHT,
-BRW_NEW_PRIMITIVE |
-BRW_NEW_TRANSFORM_FEEDBACK |
-BRW_NEW_VS_PROG_DATA))
+ if (!brw_ff_gs_state_dirty(brw))
return;
/* Populate the key:
diff --git a/src/mesa/drivers/dri/i965/brw_gs.c
b/src/mesa/drivers/dri/i965/brw_gs.c
index 3458df3..c45e217 100644
--- a/src/mesa/drivers/dri/i965/brw_gs.c
+++ b/src/mesa/drivers/dri/i965/brw_gs.c
@@ -288,6 +288,16 @@ do_gs_prog(struct brw_context *brw,
return true;
}
+static bool
+brw_gs_state_dirty(struct brw_context *brw)
+{
+ return brw_state_dirty(brw,
+ _NEW_TEXTURE,
+ BRW_NEW_GEOMETRY_PROGRAM |
+ BRW_NEW_TRANSFORM_FEEDBACK |
+ BRW_NEW_VUE_MAP_VS);
+}
+
static void
brw_gs_populate_key(struct brw_context *brw,
struct brw_gs_prog_key *key)
@@ -322,11 +332,7 @@ brw_upload_gs_prog(struct brw_context *brw)
struct brw_geometry_program *gp =
(struct brw_geometry_program *) brw-geometry_program;
- if (!brw_state_dirty(brw,
-_NEW_TEXTURE,
-BRW_NEW_GEOMETRY_PROGRAM |
-BRW_NEW_TRANSFORM_FEEDBACK |
-BRW_NEW_VUE_MAP_VS))
+ if (!brw_gs_state_dirty(brw))
return;
if (gp == NULL) {
diff --git a/src/mesa/drivers/dri/i965/brw_vs.c
b/src/mesa/drivers/dri/i965/brw_vs.c
index b8e91c1..2c76d25 100644
--- a/src/mesa/drivers/dri/i965/brw_vs.c
+++ b/src/mesa/drivers/dri/i965/brw_vs.c
@@ -401,6 +401,20 @@ brw_setup_vue_key_clip_info(struct brw_context *brw,
}
}
+static bool
+brw_vs_state_dirty(struct brw_context *brw)
+{
+ return brw_state_dirty(brw,
+ _NEW_BUFFERS |
+ _NEW_LIGHT |
+ _NEW_POINT |
+ _NEW_POLYGON |
+ _NEW_TEXTURE |
+ _NEW_TRANSFORM,
+ BRW_NEW_VERTEX_PROGRAM |
+ BRW_NEW_VS_ATTRIB_WORKAROUNDS);
+}
+
static void
brw_vs_populate_key(struct brw_context *brw,
struct brw_vs_prog_key *key)
@@ -459,15 +473,7 @@ brw_upload_vs_prog(struct brw_context *brw)
struct brw_vertex_program *vp =
(struct brw_vertex_program *)brw-vertex_program;
- if (!brw_state_dirty(brw,
-_NEW_BUFFERS |
-_NEW_LIGHT |
-_NEW_POINT |
-_NEW_POLYGON |
-_NEW_TEXTURE |
-_NEW_TRANSFORM,
-BRW_NEW_VERTEX_PROGRAM |
-BRW_NEW_VS_ATTRIB_WORKAROUNDS))
+ if (!brw_vs_state_dirty(brw))
return;
brw_vs_populate_key(brw, key);
diff --git a/src/mesa/drivers/dri/i965/brw_wm.c
b/src/mesa/drivers/dri/i965/brw_wm.c
index a0eda3a8..e9cb133 100644
--- a/src/mesa/drivers/dri/i965/brw_wm.c
+++ b/src/mesa/drivers/dri/i965/brw_wm.c
@@ -421,6 +421,27 @@ brw_populate_sampler_prog_key_data(struct gl_context *ctx,
}
}
+static bool
+brw_wm_state_dirty (struct brw_context *brw)
+{
+ return brw_state_dirty(brw,
+ _NEW_BUFFERS |
+ _NEW_COLOR |
+ _NEW_DEPTH |
+ _NEW_FRAG_CLAMP |
+ _NEW_HINT |
+ _NEW_LIGHT |
+ _NEW_LINE |
+
[Mesa-dev] [PATCH 3/4] i965: Rename do_stage_prog to brw_stage_compile
The rename here is in preparation for these functions to be exported
to other files.
This commit is intended to have no functional change. It exists in
preparation for some upcoming code movement in preparation for the
shader cache.
---
src/mesa/drivers/dri/i965/brw_ff_gs.c | 7 ---
src/mesa/drivers/dri/i965/brw_fs.cpp | 2 +-
src/mesa/drivers/dri/i965/brw_gs.c| 14 +++---
src/mesa/drivers/dri/i965/brw_vs.c| 14 +++---
src/mesa/drivers/dri/i965/brw_wm.c| 14 --
src/mesa/drivers/dri/i965/brw_wm.h| 8
6 files changed, 31 insertions(+), 28 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_ff_gs.c
b/src/mesa/drivers/dri/i965/brw_ff_gs.c
index c589171..8bc0a1c 100644
--- a/src/mesa/drivers/dri/i965/brw_ff_gs.c
+++ b/src/mesa/drivers/dri/i965/brw_ff_gs.c
@@ -45,8 +45,9 @@
#include util/ralloc.h
-static void compile_ff_gs_prog(struct brw_context *brw,
- struct brw_ff_gs_prog_key *key)
+static void
+brw_ff_gs_compile(struct brw_context *brw,
+ struct brw_ff_gs_prog_key *key)
{
struct brw_ff_gs_compile c;
const GLuint *program;
@@ -253,7 +254,7 @@ brw_upload_ff_gs_prog(struct brw_context *brw)
if (!brw_search_cache(brw-cache, BRW_CACHE_FF_GS_PROG,
key, sizeof(key),
brw-ff_gs.prog_offset, brw-ff_gs.prog_data)) {
-compile_ff_gs_prog( brw, key );
+brw_ff_gs_compile(brw, key);
}
}
}
diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp
b/src/mesa/drivers/dri/i965/brw_fs.cpp
index 780be80..24eb076 100644
--- a/src/mesa/drivers/dri/i965/brw_fs.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
@@ -4177,7 +4177,7 @@ brw_fs_precompile(struct gl_context *ctx,
uint32_t old_prog_offset = brw-wm.base.prog_offset;
struct brw_wm_prog_data *old_prog_data = brw-wm.prog_data;
- bool success = do_wm_prog(brw, shader_prog, bfp, key);
+ bool success = brw_wm_compile(brw, shader_prog, bfp, key);
brw-wm.base.prog_offset = old_prog_offset;
brw-wm.prog_data = old_prog_data;
diff --git a/src/mesa/drivers/dri/i965/brw_gs.c
b/src/mesa/drivers/dri/i965/brw_gs.c
index c45e217..a0da919 100644
--- a/src/mesa/drivers/dri/i965/brw_gs.c
+++ b/src/mesa/drivers/dri/i965/brw_gs.c
@@ -35,10 +35,10 @@
static bool
-do_gs_prog(struct brw_context *brw,
- struct gl_shader_program *prog,
- struct brw_geometry_program *gp,
- struct brw_gs_prog_key *key)
+brw_gs_compile(struct brw_context *brw,
+ struct gl_shader_program *prog,
+ struct brw_geometry_program *gp,
+ struct brw_gs_prog_key *key)
{
struct brw_stage_state *stage_state = brw-gs.base;
struct brw_gs_compile c;
@@ -363,8 +363,8 @@ brw_upload_gs_prog(struct brw_context *brw)
key, sizeof(key),
stage_state-prog_offset, brw-gs.prog_data)) {
bool success =
- do_gs_prog(brw, ctx-_Shader-CurrentProgram[MESA_SHADER_GEOMETRY],
gp,
-key);
+ brw_gs_compile(brw,
ctx-_Shader-CurrentProgram[MESA_SHADER_GEOMETRY],
+ gp, key);
assert(success);
(void)success;
}
@@ -400,7 +400,7 @@ brw_gs_precompile(struct gl_context *ctx,
*/
key.input_varyings = gp-Base.InputsRead;
- success = do_gs_prog(brw, shader_prog, bgp, key);
+ success = brw_gs_compile(brw, shader_prog, bgp, key);
brw-gs.base.prog_offset = old_prog_offset;
brw-gs.prog_data = old_prog_data;
diff --git a/src/mesa/drivers/dri/i965/brw_vs.c
b/src/mesa/drivers/dri/i965/brw_vs.c
index 2c76d25..e5a55a7 100644
--- a/src/mesa/drivers/dri/i965/brw_vs.c
+++ b/src/mesa/drivers/dri/i965/brw_vs.c
@@ -188,10 +188,10 @@ brw_vs_prog_data_compare(const void *in_a, const void
*in_b)
}
static bool
-do_vs_prog(struct brw_context *brw,
- struct gl_shader_program *prog,
- struct brw_vertex_program *vp,
- struct brw_vs_prog_key *key)
+brw_vs_compile(struct brw_context *brw,
+ struct gl_shader_program *prog,
+ struct brw_vertex_program *vp,
+ struct brw_vs_prog_key *key)
{
GLuint program_size;
const GLuint *program;
@@ -482,8 +482,8 @@ brw_upload_vs_prog(struct brw_context *brw)
key, sizeof(key),
brw-vs.base.prog_offset, brw-vs.prog_data)) {
bool success =
- do_vs_prog(brw, ctx-_Shader-CurrentProgram[MESA_SHADER_VERTEX], vp,
-key);
+ brw_vs_compile(brw,ctx-_Shader-CurrentProgram[MESA_SHADER_VERTEX],
+ vp, key);
(void) success;
assert(success);
}
@@ -524,7 +524,7 @@ brw_vs_precompile(struct gl_context *ctx,
(prog-OutputsWritten (VARYING_BIT_COL0 | VARYING_BIT_COL1 |
VARYING_BIT_BFC0 | VARYING_BIT_BFC1));
- success = do_vs_prog(brw, shader_prog, bvp, key);
+
Re: [Mesa-dev] [PATCH 1/3] mesa: Replace _mesa_round_to_even() with _mesa_roundeven().
On Wed, Mar 18 2015, Matt Turner wrote:
This patch implements _mesa_roundeven{f,}, a function similar to the
roundeven function added by a yet unimplemented technical specification
(ISO/IEC TS 18661-1:2014), with a small difference in behavior -- we
don't bother raising the inexact exception, which I don't think we care
about anyway.
All looks good to me, Matt.
For the series:
Reviewed-by: Carl Worth cwo...@cworth.org
-Carl
pgpJ8HMM5zZAh.pgp
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965: Mark paths in linear - tiled functions as unreachable().
On Mon, Mar 16 2015, Matt Turner wrote: textdata bss dec hex filename 9663 0 0966325bf intel_tiled_memcpy.o before 8215 0 082152017 intel_tiled_memcpy.o after Reviewed-by: Carl Worth cwo...@cworth.org -Carl pgp1BOJRVmNZE.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/3] mesa: Replace _mesa_round_to_even() with _mesa_roundeven().
On Fri, Mar 13 2015, Matt Turner wrote: Perhaps something like the following? Round \x to the nearest even integer (returned in floating-point format). I didn't read this closely enough the first time, but if I had I think I could have short circuited a lot of the confusion. Well, I didn't write that carefully enough either. I knew that the even part only referred to the halfway cases, but I botched that in my description. But I think even with my mistake, I did make my point, which is simply that this function should be documented to describe what it's actually intended to do[*]. -Carl pgpT5_9KFsRTd.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/2] i965/fs: Make an emit_discard_jump() function to reduce duplication.
On Fri, Mar 13 2015, Kenneth Graunke wrote: This is already copied in two places, and I want to copy it to a third place. Reviewed-by: Carl Worth cwo...@cworth.org -Carl -- carl.d.wo...@intel.com pgp5HqiGlRi7H.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/3] mesa: Replace _mesa_round_to_even() with _mesa_roundeven().
On Thu, Mar 12 2015, Matt Turner wrote: I think you misread. rint() *does* provide the behavior we want (round-to-nearest, half to even) when the rounding mode is the default round-to-nearest. Thanks. I did at least verify that behaviorally as I just mentioned in a separate mail. As Eric noted in his original patch, the man pages for rint(3)/nearbyint(3) don't describe the half-to-even behavior but round(3) does: Ah. Yes, I was using the man pages, so that was the source of my confusion. Maybe I should send a patch to the Linux man-pages project too. That definitely wouldn't hurt. assert (fegetround() == FE_TONEAREST); I don't really want to do this. We rely on the default rounding mode everywhere. I don't think asserting here is useful. What's useful is increasing the likelihood that when somebody does violate the assumption, they are alterted to the issue, rather than having Mesa quietly misbehave in hard-to-predict ways. It seems obvious to me that we should improve robustness when we've already identified exactly where (at least one piece) of fragility lies in Mesa. Also it requires adding code for MSVC since it doesn't have fegetround(). Not code, no. I'd be happy with just the #ifndef to hide the call from MSVC. I won't ask you find equivalent functionality for MSVC. -Carl pgpWezL81msfM.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/3] mesa: Replace _mesa_round_to_even() with _mesa_roundeven().
On Thu, Mar 12 2015, Matt Turner wrote:
+/* The C standard library has functions round()/rint()/nearbyint() that round
+ * their arguments according to the rounding mode set in the floating-point
+ * control register. While there are trunc()/ceil()/floor() functions that do
+ * a specific operation without modifying the rounding mode, there is no
+ * roundeven() in any version of C.
+ *
+ * Technical Specification 18661 (ISO/IEC TS 18661-1:2014) adds roundeven(),
+ * but it's unfortunately not implemented by glibc.
+ *
+ * This implementation differs in that it does not raise the inexact
exception.
+ */
This documentation needs the actual description of the behavior of the
function. Most of the above comment is commentary on the implementation
itself.
Perhaps something like the following?
Round \x to the nearest even integer (returned in floating-point
format).
Note: This function is similar to roundeven() as specified by
Technical Specification 18661 (ISO/IEC TS 18661-1:2014),
differing only in that _mesa_roundeven() won't necessarily raise
the inexact exception as roundeven() would.
Then, (within the function body itself?), it makes sense to have a
comment on the implementation:
When glibc eventually implements the standardized roundeven() we
could use it directly. Until then, the available C standard
library functions have the following limitations:
round()/rint()/nearbyint(): Behavior varies depending on
the rounding mode set in the floating-point control
register.
trunc()/ceil()/floor(): These specify rounding without
relying on the rounding mode, but none give the rounding
behavior desired for _mesa_roundeven().
But since none of those work??? See my questions below...
+static inline float
+_mesa_roundevenf(float x)
+{
+ float ret;
+ /* Assume that the floating-point rounding mode has not been changed from
+* the default (Round to nearest).
+*/
+ ret = rintf(x);
+ return ret;
+}
But after all that commentary about how rint() doesn't provide what's
needed, here the implementation is just using it anyway?
The comment here, (and even any history of other parts of mesa make the
same assumption), doesn't give the code adequate robustness. So at
least an assert is needed here. Is this correct: ?
assert (fegetround() == FE_TONEAREST);
But beyond that, I'm actually confused---how can the default rounding
mode (rounding toward nearest number, right?) give us an adequate
implementation for roundeven()?
[Thanks for sending the split patch. It made this question very
apparent, where I wasn't asking it about the combined series. Thanks
also for adding the test and the SSE optimization as a separate
commit. Those look good to me.]
-Carl
pgpKohQEREqPH.pgp
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/3] mesa: Replace _mesa_round_to_even() with _mesa_roundeven().
On Thu, Mar 12 2015, Carl Worth wrote: But beyond that, I'm actually confused---how can the default rounding mode (rounding toward nearest number, right?) give us an adequate implementation for roundeven()? Of course, I had asked for (and received) a patch with a test. And the test verifies that rint() with the default rounding is working. (I also verified that with an explicit call to fesetround(FE_TONEAREST) in the test, the test still passes, and similarly that a call to fesetround with any of the other values makes it fail.) So I'll clarify my confusion separately, but it doesn't necessarily point to any problem in the code. -Carl PS. And with that testing, here's my official: Reviewed-by: Carl Worth cwo...@cworth.org for the testing patch. pgpnpX_hGkdI4.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/2] glsl: Expose built-in packing functions under GLSL 4.2.
ARB_shading_language_packing is part of GLSL 4.2, not 4.0 as I mistakenly believed. The following functions are available only with ARB_shading_language_packing, GLSL 4.2 (not GLSL 4.0), or ES 3.0: I'll trust you that you're correct on the specification version, so: Reviewed-by: Carl Worth cwo...@cworth.org (for both patches). -Carl pgpATr8uN7x4W.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [RFC PATCH] mesa: Replace _mesa_round_to_even() with roundeven().
On Wed, Mar 11 2015, Matt Turner wrote:
Eric's initial patch adding constant expression evaluation for
ir_unop_round_even used nearbyint...
Hi Matt,
It's great to see this commit, (and with such a detailed message).
Rounding is one of those things that can be surprisingly difficult to
get correct, and there are a lot of moving pieces here, (software
specifications, hardware behavior, performance penalties of changing
modes, exceptions, etc.). So this is definitely an area worth spending
the effort to get things right.
I do have a feeling that there's something we should add to this commit.
My first difficulty in trying to review it was in determining what the
behavioral change in the patch actually is. Most of the commit message
is history (which is useful) but it was hard to find a succinct
description of here's what's changed.
Reading between the lines of the history, I can guess the following
list of things are happening in this commit:
Worse, IROUND() is implemented using the trunc(x + 0.5) trick which
fails for x = nextafterf(0.5, 0.0).
1. Fix _mesa_round_to_even to return a correct value in this case.
Since you've done the careful work to identify this boundary
case, (which was subtle enough that it was missed by the
original implementation), I think this should be verified with a
unit test.
Still worse, _mesa_round_to_even unexpectedly returns an int.
2. Fix _mesa_round_to_even to return a floating-point value
This sounds logically independent from the above
change. Usually, I'd say that justifies separate commits, but
maybe combining these two in one is fine. What would at least be
nice if the first paragraph of the commit message could list
exactly what's changed, before all the motivation.
rint() and nearbyint() implement the round-half-to-even behavior we want
when the rounding mode is set to the default round-to-nearest. The only
difference between them is that nearbyint() raises the inexact
exception.
This patch implements roundeven{f,}, a function added by a yet
unimplemented technical specification (ISO/IEC TS 18661-1:2014), with a
small difference in behavior -- we don't bother raising the inexact
exception, which I don't think we care about anyway.
3. Rename _mesa_round_to_even to roundeven
I'm less confident about this change. If we're using the name of
a standardized function, (that may appear in glibc at any point
in the future), shouldn't we at least have some configure
machinery in place so that we can actually use the system
version if available?
If we do indeed want the don't-raise-the-inexact-exception behavior,
maybe we should just keep the _mesa_round_to_even name?
If we care and don't want the exception, then yes, we'd better not use
the standard name. But even if we don't care and could live with the
exception, I don't think we should use a standardized name for a
function that we know differs in its behavior.
If we do decide to keep the rename, I propose at least splitting this
part into a separate commit.
The SSE 4.1 ROUND instructions let us implement roundeven directly.
Otherwise we assume that the rounding mode has not been modified (as we
do in the rest of Mesa) and use rint().
4. Optimize this function (whatever its name) with SSE4.1 ROUND
This looks like a reasonable optimization, but should be a
separate commit, (and this commit would benefit from the unit
tests mentioned before).
So I think I'd like to see at least three or four commits here:
1. Change _mesa_round_to_even to return a float
2. Fix rounding bug in _mesa_round_to_even
3. Add unit test for recent rounding-bug fix
4. Optimize _mesa_round_to_even with SSE 4.1 ROUND if available
With that: Reviewed-by: Carl Worth cwo...@cworth.org
-Carl
--
carl.d.wo...@intel.com
pgp9C0kDV4b3E.pgp
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 5/6] i965: Silence unused parameter warning
On Fri, Feb 27 2015, Ian Romanick wrote: All dd functions take a gl_context as the first parameter. Instead of removing it, just silence the warning. For code using gcc, I really prefer the __attribute__((__unused__)) style, (though, obviously hidden in a reasonable looking macro). That results in cleaner looking code than these weird unused expressions being cast to void. Does MSVC have an equivalent? Or, does it not emit the warning in the first place such that we could just define the macro as empty outside of gcc? So there's room for investigation here. In the meantime: Reviewed-by: Carl Worth cwo...@cworth.org -Carl pgpSkpdVeZKDc.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 4/6] i965: Silence many 'static' is not at beginning of declaration warnings
On Fri, Feb 27 2015, Ian Romanick wrote: From: Ian Romanick ian.d.roman...@intel.com What a useful warning. #ThanksGCC Reviewed-by: Carl Worth cwo...@cworth.org Sarcasm-by: Ian Romanick? ;-) -Carl pgph3XoYbTHGG.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 5/6] i965: Silence unused parameter warning
On Sat, Feb 28 2015, Ilia Mirkin wrote: Another clean alternative is to leave the name of the variable out, i.e. function(struct gl_context *) Wow. Less is more! I hadn't realized that's a solution for this, but it's really elegant. -Carl pgpHapVflSAmW.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] Revert use of Mesa IR optimizer for ARB_fragment_programs
Commit f82f2fb3dc770902f1657ab1c22e6004faa3afab added use of the Mesa IR optimizer for both ARB_fragment_program and ARB_vertex_program, but only justified the vertex-program portions with measured performance improvements. Meanwhile, the optimizer was seen to generate hundreds of unused immediates without discarding them, causing failures. Discard the use of the optimizer for now to fix the regression. (In the future, we anticpate things moving from Mesa IR to NIR for better optimization anyway.) Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82477 --- src/mesa/program/arbprogparse.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/mesa/program/arbprogparse.c b/src/mesa/program/arbprogparse.c index 7dec399..53a6f37 100644 --- a/src/mesa/program/arbprogparse.c +++ b/src/mesa/program/arbprogparse.c @@ -85,9 +85,6 @@ _mesa_parse_arb_fragment_program(struct gl_context* ctx, GLenum target, return; } - if ((ctx-_Shader-Flags GLSL_NO_OPT) == 0) - _mesa_optimize_program(ctx, prog); - free(program-Base.String); /* Copy the relevant contents of the arb_program struct into the -- 2.1.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] i965: Perform program state upload outside of atom handling
Across the board of the various generations, the intial few atoms in
all of the atom lists are basically the same, (performing uploads for
the various programs). The only difference is that prior to gen6
there's an ff_gs upload in place of the later gs upload.
In this commit, instead of using the atom lists for this program state
upload, we add a new function brw_upload_programs that performs the
same state checks and calls the various upload functions as necessary.
This commit is intended to have no functional change. The motivation
is that future code, (such as the shader cache), wants to have a
single function within which to perform various operations before and
after program upload, (with some local variables holding state across
the upload).
---
src/mesa/drivers/dri/i965/brw_ff_gs.c| 12 +
src/mesa/drivers/dri/i965/brw_ff_gs.h| 3 ++
src/mesa/drivers/dri/i965/brw_gs.c | 15 +-
src/mesa/drivers/dri/i965/brw_gs.h | 5 ++
src/mesa/drivers/dri/i965/brw_state_upload.c | 75 ++--
src/mesa/drivers/dri/i965/brw_vs.c | 20 +---
src/mesa/drivers/dri/i965/brw_vs.h | 3 ++
src/mesa/drivers/dri/i965/brw_wm.c | 26 +-
src/mesa/drivers/dri/i965/brw_wm.h | 3 ++
9 files changed, 78 insertions(+), 84 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_ff_gs.c
b/src/mesa/drivers/dri/i965/brw_ff_gs.c
index 653c4b6..4f15511 100644
--- a/src/mesa/drivers/dri/i965/brw_ff_gs.c
+++ b/src/mesa/drivers/dri/i965/brw_ff_gs.c
@@ -221,7 +221,7 @@ static void populate_key(struct brw_context *brw,
/* Calculate interpolants for triangle and line rasterization.
*/
-static void
+void
brw_upload_ff_gs_prog(struct brw_context *brw)
{
struct brw_ff_gs_prog_key key;
@@ -247,13 +247,3 @@ void gen6_brw_upload_ff_gs_prog(struct brw_context *brw)
{
brw_upload_ff_gs_prog(brw);
}
-
-const struct brw_tracked_state brw_ff_gs_prog = {
- .dirty = {
- .mesa = _NEW_LIGHT,
- .brw = BRW_NEW_PRIMITIVE |
- BRW_NEW_TRANSFORM_FEEDBACK |
- BRW_NEW_VS_PROG_DATA,
- },
- .emit = brw_upload_ff_gs_prog
-};
diff --git a/src/mesa/drivers/dri/i965/brw_ff_gs.h
b/src/mesa/drivers/dri/i965/brw_ff_gs.h
index a538948..e4afdab 100644
--- a/src/mesa/drivers/dri/i965/brw_ff_gs.h
+++ b/src/mesa/drivers/dri/i965/brw_ff_gs.h
@@ -112,4 +112,7 @@ void gen6_sol_program(struct brw_ff_gs_compile *c,
unsigned num_verts, bool check_edge_flag);
void gen6_brw_upload_ff_gs_prog(struct brw_context *brw);
+void
+brw_upload_ff_gs_prog(struct brw_context *brw);
+
#endif
diff --git a/src/mesa/drivers/dri/i965/brw_gs.c
b/src/mesa/drivers/dri/i965/brw_gs.c
index c7ebe5f..a7b9384 100644
--- a/src/mesa/drivers/dri/i965/brw_gs.c
+++ b/src/mesa/drivers/dri/i965/brw_gs.c
@@ -292,8 +292,7 @@ do_gs_prog(struct brw_context *brw,
return true;
}
-
-static void
+void
brw_upload_gs_prog(struct brw_context *brw)
{
struct gl_context *ctx = brw-ctx;
@@ -358,18 +357,6 @@ brw_upload_gs_prog(struct brw_context *brw)
}
}
-
-const struct brw_tracked_state brw_gs_prog = {
- .dirty = {
- .mesa = _NEW_TEXTURE,
- .brw = BRW_NEW_GEOMETRY_PROGRAM |
- BRW_NEW_TRANSFORM_FEEDBACK |
- BRW_NEW_VUE_MAP_VS,
- },
- .emit = brw_upload_gs_prog
-};
-
-
bool
brw_gs_precompile(struct gl_context *ctx,
struct gl_shader_program *shader_prog,
diff --git a/src/mesa/drivers/dri/i965/brw_gs.h
b/src/mesa/drivers/dri/i965/brw_gs.h
index 5a15fa9..5f7c437 100644
--- a/src/mesa/drivers/dri/i965/brw_gs.h
+++ b/src/mesa/drivers/dri/i965/brw_gs.h
@@ -26,6 +26,8 @@
#include stdbool.h
+#include brw_context.h
+
#ifdef __cplusplus
extern C {
#endif
@@ -36,6 +38,9 @@ struct gl_program;
bool brw_gs_prog_data_compare(const void *a, const void *b);
+void
+brw_upload_gs_prog(struct brw_context *brw);
+
#ifdef __cplusplus
} /* extern C */
#endif
diff --git a/src/mesa/drivers/dri/i965/brw_state_upload.c
b/src/mesa/drivers/dri/i965/brw_state_upload.c
index 52d96f4..36f44f3 100644
--- a/src/mesa/drivers/dri/i965/brw_state_upload.c
+++ b/src/mesa/drivers/dri/i965/brw_state_upload.c
@@ -36,17 +36,17 @@
#include drivers/common/meta.h
#include intel_batchbuffer.h
#include intel_buffers.h
+#include brw_vs.h
+#include brw_ff_gs.h
+#include brw_gs.h
+#include brw_wm.h
static const struct brw_tracked_state *gen4_atoms[] =
{
- brw_vs_prog, /* must do before GS prog, state base address. */
- brw_ff_gs_prog, /* must do before state base address */
-
brw_interpolation_map,
brw_clip_prog, /* must do before state base address */
brw_sf_prog, /* must do before state base address */
- brw_wm_prog, /* must do before state base address */
/* Once all the programs are done, we know how large urb entry
* sizes need to be and can decide if we need to change the urb
@@ -107,10 +107,6 @@ static const struct
Re: [Mesa-dev] [PATCH] meta: Fix saving the results of the current occlusion query
On Tue, Nov 25 2014, Neil Roberts wrote: This patch fixes it by making it actually wait for the query object to be ready before grabbing the previous result. The downside of doing this is that it could introduce a stall but I think this situation is unlikely so it might not matter too much. Thanks for the fix, Neil. The analysis and patch look sound to me, and thanks for contributing a piglit test as well. Additionally, we ran the patch through some benchmarks and didn't notice any negative performance impact. Reviewed-by: Carl Worth cwo...@cworth.org -Carl pgpUYUpkJL7HF.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2] i965: Perform program state upload outside of atom handling
Across the board of the various generations, the intial few atoms in
all of the atom lists are basically the same, (performing uploads for
the various programs). The only difference is that prior to gen6
there's an ff_gs upload in place of the later gs upload.
In this commit, instead of using the atom lists for this program state
upload, we add a new function brw_upload_programs that performs the
same state checks and calls the various upload functions as necessary.
This commit is intended to have no functional change. The motivation
is that future code, (such as the shader cache), wants to have a
single function within which to perform various operations before and
after program upload, (with some local variables holding state across
the upload).
---
In this revision of the patch, I've changed things so that the lists
of flags remain in the same files they were in before, (near the code
performing the operations that the flags control). So I think this
aspect is a bit cleaner.
I'm still not a huge fan of the resulting conditional expressions,
(and the number of parentheses involved in them). It would not be too
hard to imagine one of those getting edited incorrectly.
So feedback would be most welcome on improving those.
Maybe:
int dirty = 0;
dirty |= (brw-state.dirty.mesa (FLAG | FLAG2 | FLAG3);
dirty |= (brw-state.dirty.brw (BRW_FLAG | BRW_FLAG2);
if (dirty == 0)
return;
?
src/mesa/drivers/dri/i965/brw_ff_gs.c| 21 +++--
src/mesa/drivers/dri/i965/brw_ff_gs.h| 3 ++
src/mesa/drivers/dri/i965/brw_gs.c | 23 ++
src/mesa/drivers/dri/i965/brw_gs.h | 5
src/mesa/drivers/dri/i965/brw_state_upload.c | 35 --
src/mesa/drivers/dri/i965/brw_vs.c | 32 +---
src/mesa/drivers/dri/i965/brw_vs.h | 3 ++
src/mesa/drivers/dri/i965/brw_wm.c | 45 +---
src/mesa/drivers/dri/i965/brw_wm.h | 3 ++
9 files changed, 86 insertions(+), 84 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_ff_gs.c
b/src/mesa/drivers/dri/i965/brw_ff_gs.c
index 653c4b6..fd68aa3 100644
--- a/src/mesa/drivers/dri/i965/brw_ff_gs.c
+++ b/src/mesa/drivers/dri/i965/brw_ff_gs.c
@@ -221,10 +221,19 @@ static void populate_key(struct brw_context *brw,
/* Calculate interpolants for triangle and line rasterization.
*/
-static void
+void
brw_upload_ff_gs_prog(struct brw_context *brw)
{
struct brw_ff_gs_prog_key key;
+
+ if (((brw-state.dirty.mesa _NEW_LIGHT) |
+ (brw-state.dirty.brw (BRW_NEW_PRIMITIVE |
+BRW_NEW_TRANSFORM_FEEDBACK |
+BRW_NEW_VS_PROG_DATA))) == 0)
+ {
+ return;
+ }
+
/* Populate the key:
*/
populate_key(brw, key);
@@ -247,13 +256,3 @@ void gen6_brw_upload_ff_gs_prog(struct brw_context *brw)
{
brw_upload_ff_gs_prog(brw);
}
-
-const struct brw_tracked_state brw_ff_gs_prog = {
- .dirty = {
- .mesa = _NEW_LIGHT,
- .brw = BRW_NEW_PRIMITIVE |
- BRW_NEW_TRANSFORM_FEEDBACK |
- BRW_NEW_VS_PROG_DATA,
- },
- .emit = brw_upload_ff_gs_prog
-};
diff --git a/src/mesa/drivers/dri/i965/brw_ff_gs.h
b/src/mesa/drivers/dri/i965/brw_ff_gs.h
index a538948..e4afdab 100644
--- a/src/mesa/drivers/dri/i965/brw_ff_gs.h
+++ b/src/mesa/drivers/dri/i965/brw_ff_gs.h
@@ -112,4 +112,7 @@ void gen6_sol_program(struct brw_ff_gs_compile *c,
unsigned num_verts, bool check_edge_flag);
void gen6_brw_upload_ff_gs_prog(struct brw_context *brw);
+void
+brw_upload_ff_gs_prog(struct brw_context *brw);
+
#endif
diff --git a/src/mesa/drivers/dri/i965/brw_gs.c
b/src/mesa/drivers/dri/i965/brw_gs.c
index c7ebe5f..0fa06bb 100644
--- a/src/mesa/drivers/dri/i965/brw_gs.c
+++ b/src/mesa/drivers/dri/i965/brw_gs.c
@@ -292,8 +292,7 @@ do_gs_prog(struct brw_context *brw,
return true;
}
-
-static void
+void
brw_upload_gs_prog(struct brw_context *brw)
{
struct gl_context *ctx = brw-ctx;
@@ -303,6 +302,14 @@ brw_upload_gs_prog(struct brw_context *brw)
struct brw_geometry_program *gp =
(struct brw_geometry_program *) brw-geometry_program;
+ if (((brw-state.dirty.mesa _NEW_TEXTURE) |
+ (brw-state.dirty.brw (BRW_NEW_GEOMETRY_PROGRAM |
+BRW_NEW_TRANSFORM_FEEDBACK |
+BRW_NEW_VUE_MAP_VS))) == 0)
+ {
+ return;
+ }
+
if (gp == NULL) {
/* No geometry shader. Vertex data just passes straight through. */
if (brw-state.dirty.brw BRW_NEW_VUE_MAP_VS) {
@@ -358,18 +365,6 @@ brw_upload_gs_prog(struct brw_context *brw)
}
}
-
-const struct brw_tracked_state brw_gs_prog = {
- .dirty = {
- .mesa = _NEW_TEXTURE,
- .brw = BRW_NEW_GEOMETRY_PROGRAM |
- BRW_NEW_TRANSFORM_FEEDBACK |
- BRW_NEW_VUE_MAP_VS,
- },
- .emit = brw_upload_gs_prog
-};
-
-
bool
[Mesa-dev] [PATCH v2] glsl: Add initial functions to implement an on-disk cache
This code provides for an on-disk cache of objects. Objects are stored and retrieved via names that are arbitrary 20-byte sequences, (intended to be SHA-1 hashes of something identifying for the content). The directory used for the cache can be specified by means of environment variables in the following priority order: $MESA_GLSL_CACHE_DIR $XDG_CACHE_HOME/mesa user-home-directory/.cache/mesa By default the cache will be limited to a maximum size of 1GB. The environment variable: $MESA_GLSL_CACHE_MAX_SIZE can be set (at the time of GL context creation) to choose some other size. This variable is a number that can optionally be followed by 'K', 'M', or 'G' to select a size in kilobytes, megabytes, or gigabytes. By default, an unadorned value will be interpreted as gigabytes. The cache will be entirely disabled at runtime if the variable MESA_GLSL_CACHE_DISABLE is set at the time of GL context creation. This patch is intended to support a future implementation of a shader cache. But for now, the only code using this cache is the unit tests included here. Many thanks to Kristian Høgsberg k...@bitplanet.net for the initial implementation of code that led to this patch. In particular, the idea of using an mmapped file, (indexed by a portion of the SHA-1), for the efficent implementation of cache_has_key was entirely his idea. Kristian also provided some very helpful advice in discussions regarding various race conditions to be avoided in this code. --- This is an update of the patch series I sent last week. Here is a summary of the major changes: * All squashed into one commit, since rebasing changes through the series was getting too annyoing, (and there wasn't too much value in having things broken out). * The cache has now been split to have two separate notions: 1. Storing objects named by a hash 2. Storing a hash along with no other data These two notions are supported by the separate APIs of cache_put/cache_get and cache_put_key/cache_get_key. (The difference from before was that the equivalent of the new cache_get_key would also return true if that same key had previously been used for cache_put). Having these two notions separated means all the code that needs the efficiency of cache_has_key still gets it, but several difficult-to-fix race conditions that were discussed previously are no longer possible. (This was the discussion of a few lingering FIXME comments around index updates and unlink.) * The cache is now size-limited and controllable via the MESA_GLSL_CACHE_MAX_SIZE environment variable. * The patch now includes unit tests. Here are the two major things I know that I could use in review of this code: 1. Portability For the size-limited cache, this code now calls readdir() which wasn't previously called in Mesa. I don't know if an MSVC environment provides the d_type field in the resulting struct dirent entries, or if the code will need some additional stat() calls. And there could, of course, be other portability problems I'm not aware of. 2. Concurrency issues Kristian and I have discussed quite a number of potential race-related issues from multiple processed accessing the cache simultaneously. I'm not aware of cases we haven't thought through, but of course that's very weak as a guarantee of correctness. There are comments throughout the relevant parts of the code, but here is an overview to guide review: The two primary shared resources are the index file which is mmap()ped with MAP_SHARED and the filesystem itself where cached files are written and unlinked. The syncrhonization primitive used for the index file is atomic addition. This is used to protect the number that tracks the total size of the cache. The other writes to the index file are SHA-1 signatures. These are not protected from multiple writers. The thought there is that a corrupt signature from multiple writeres should be effectually identical to the default all-zeros---in either case no real SHA-1 signature for a GLSL source file is expected to match. For the filesystem, when the code as a cache miss on a file named sha1 and goes off to compile and link it, it then comes back and opens sha1.tmp and attempts an flock with LOCK_EX. If this fails, it gives up, (trusting that the process with the active lock will write the identical contents that this process would have written). If the lock succeeds, it also next checks whether the file sha1 now exists, (in case it was written by a racing process between the cache miss and now). Only if it does not exist at this point, does the process then perform the following operations: * Write contents to sha1.tmp * Rename from sha1.tmp to sha1 * Perform
Re: [Mesa-dev] [PATCH] util/u_atomic: Add new macro p_atomic_add
On Mon, Feb 09 2015, Jose Fonseca wrote: Just one more tweak to InterlockedExchangeAdd64 as per patch attached. .. With that u_test_atomic builds and passes for me both on 32 and 64bits. Excellent. Thanks for the fix and for the testing report. Sorry for the delay. And thanks for your help in keeping MSVC support on par. No problem with the delay. The timing is just right since I'm about to send to the list an updated cache implementation that relies on this new atomic_add, and that also includes some unit tests that I'm hoping people can help test. (But, fair warning, there's a bit of file-system manipulation code there that I think will probably need some attention to work on non-Linux systems.) -Carl pgppFCoguqri0.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] util/u_atomic: Add new macro p_atomic_add
This provides for atomic addition, which will be used by an upcoming shader-cache patch. A simple test is added to make check as well. Note: The various O/S functions differ on whether they return the original value or the value after the addition, so I did not provide an add_return() macro which would be sensitive to that difference. --- Note: I referenced the MSDN web pages and some online Solaris man pages to update the relevant sections, (but I have neither compiled nor tested this new macro on those operating systems). I would appreciate any relvant review or reports from testing. src/util/u_atomic.h | 16 src/util/u_atomic_test.c | 5 + 2 files changed, 21 insertions(+) diff --git a/src/util/u_atomic.h b/src/util/u_atomic.h index cf7fff3..4eb0ec6 100644 --- a/src/util/u_atomic.h +++ b/src/util/u_atomic.h @@ -39,6 +39,7 @@ #define p_atomic_dec_zero(v) (__sync_sub_and_fetch((v), 1) == 0) #define p_atomic_inc(v) (void) __sync_add_and_fetch((v), 1) #define p_atomic_dec(v) (void) __sync_sub_and_fetch((v), 1) +#define p_atomic_add(v, i) (void) __sync_add_and_fetch((v), (i)) #define p_atomic_inc_return(v) __sync_add_and_fetch((v), 1) #define p_atomic_dec_return(v) __sync_sub_and_fetch((v), 1) #define p_atomic_cmpxchg(v, old, _new) \ @@ -60,6 +61,7 @@ #define p_atomic_dec_zero(_v) (p_atomic_dec_return(_v) == 0) #define p_atomic_inc(_v) ((void) p_atomic_inc_return(_v)) #define p_atomic_dec(_v) ((void) p_atomic_dec_return(_v)) +#define p_atomic_add(_v, _i) (*(_v) = *(_v) + (_i)) #define p_atomic_inc_return(_v) (++(*(_v))) #define p_atomic_dec_return(_v) (--(*(_v))) #define p_atomic_cmpxchg(_v, _old, _new) (*(_v) == (_old) ? (*(_v) = (_new), (_old)) : *(_v)) @@ -144,6 +146,13 @@ char _InterlockedCompareExchange8(char volatile *Destination8, char Exchange8, c sizeof *(_v) == sizeof(__int64) ? InterlockedDecrement64 ((__int64 *)(_v)) : \ (assert(!should not get here), 0)) +#define p_atomic_add(_v, _i) (\ + sizeof *(_v) == sizeof(short) ? _InterlockedExchangeAdd8 ((char *) (_v), (_i)) : \ + sizeof *(_v) == sizeof(short) ? _InterlockedExchangeAdd16((short *) (_v), (_i)) : \ + sizeof *(_v) == sizeof(long)? _InterlockedExchangeAdd ((long *) (_v), (_i)) : \ + sizeof *(_v) == sizeof(__int64) ? _InterlockedExchangeAdd64((__int64 *)(_v), (_i)) : \ + (assert(!should not get here), 0)) + #define p_atomic_cmpxchg(_v, _old, _new) (\ sizeof *(_v) == sizeof(char)? _InterlockedCompareExchange8 ((char *) (_v), (char) (_new), (char) (_old)) : \ sizeof *(_v) == sizeof(short) ? _InterlockedCompareExchange16((short *) (_v), (short) (_new), (short) (_old)) : \ @@ -198,6 +207,13 @@ char _InterlockedCompareExchange8(char volatile *Destination8, char Exchange8, c sizeof(*v) == sizeof(uint64_t) ? atomic_dec_64_nv((uint64_t *)(v)) : \ (assert(!should not get here), 0)) +#define p_atomic_add(v, i) ((void) \ + sizeof(*v) == sizeof(uint8_t) ? atomic_add_8 ((uint8_t *)(v), (i)) : \ + sizeof(*v) == sizeof(uint16_t) ? atomic_add_16((uint16_t *)(v), (i)) : \ + sizeof(*v) == sizeof(uint32_t) ? atomic_add_32((uint32_t *)(v), (i)) : \ + sizeof(*v) == sizeof(uint64_t) ? atomic_add_64((uint64_t *)(v), (i)) : \ +(assert(!should not get here), 0)) + #define p_atomic_cmpxchg(v, old, _new) ((typeof(*v)) \ sizeof(*v) == sizeof(uint8_t) ? atomic_cas_8 ((uint8_t *)(v), (uint8_t )(old), (uint8_t )(_new)) : \ sizeof(*v) == sizeof(uint16_t) ? atomic_cas_16((uint16_t *)(v), (uint16_t)(old), (uint16_t)(_new)) : \ diff --git a/src/util/u_atomic_test.c b/src/util/u_atomic_test.c index 4845e75..c506275 100644 --- a/src/util/u_atomic_test.c +++ b/src/util/u_atomic_test.c @@ -97,6 +97,11 @@ assert(v == ones p_atomic_dec_return); \ assert(r == v p_atomic_dec_return); \ \ + v = 23; \ + p_atomic_add(v, 42); \ + r = p_atomic_read(v); \ + assert(r == 65 p_atomic_add); \ + \ (void) r; \ (void) b; \ } -- 2.1.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] util/u_atomic: Add new macro p_atomic_add
On Fri, Feb 06 2015, Aaron Watry wrote: Ignore me if this is a stupid question, but should those both be sizeof(short)? I'd expect the first to be sizeof(char). Not a stupid question. That was a copy-and-paste (kill-and-yank ?) bug of mine. Thanks for your attention to detail. I've fixed this in my tree. -Carl pgp78403rKWFH.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Low-level infrastructure for the shader cache
On Wed, Feb 04 2015, Carl Worth wrote: First, when mapping the index file: /* FIXME: We map this shared, which is a start, but we need to think about * how to make it multi-process safe. */ cache-index = (unsigned char *) mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0); Then, when reading an entry from the cache file, (and potentially unlinking the old file being replaced): /* FIXME: We'll need an fsync here and think about races... maybe even need * an flock to avoid leaking files. Or maybe fsync, then read back and * verify the entry is still ours, delete it if somebody else overwrote * it. */ So please follow up in replies if you have good ideas on how to best address these comments. Kristian and I just had a good conversation about these issues. The key insight is that the shader cache implementation uses this cache for two distinct things: 1. Here's a SHA-1 corresponding to some GLSL source that I just compiled. Hold onto this hash, so that when I see this source again in the future, I can quickly know that this source compiles. 2. Here's a SHA-1 that I'm using to name a compiled and linked binary blob. Hold onto this blob so that I can retrieve it later with this same hash. For case 1, we want quick lookups, and the mmapped index file provides this nicely. But for case 2, the index-file approach has some problems: * As Kristian mentions in his comment above, there is a race condition where two processes can update the same entry with different hashes, (and neither seeing the other's first). This will result in files leaking. * As discussed elsewhere in the thread, a replacement policy based on a maximum number of cached entries isn't what we want. And interestingly, the one big benefit of the index file, (fast lookups for presence in the cache), is not needed at all for case 2. If we're trying to load a binary from disk we can instead just open() the file with the hash as its filename. If the open succeeds, proceed to load the data. If not, we go off to compile-and-link, (which is expensive enough that we need not worry about the cost of the open()). So my plan now is to entirely separate the above two cases in the cache API. Case 1, (have we seen this shader source?) will be implemented with the index-file idea. This can be a fixed number of entries, (there's no associated data so we don't need a size-based knob for configuration). And without any unlink() being involved we don't have races to worry about. [There is the potential for two processes to write to the same entry at the same time and corrupt it, but we have a cryptographic improbability that the resulting corrupted entry will ever be valid for any future GLSL source.] Then, case 2, (what's the linked binary blob for this hash?) will be implemented by simply reading and writing blobs to files based on the hash filenames, (without going through the index). And for this, I'll implement a maximum size for the cache. I plan to do that at cache_create() time by noticing a too-large cache and freeing up some space by deleting a sufficient number of random entries. I'll make this concrete in patches soon. As always, any comments are welcome. -Carl pgp3zENd3wNCs.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 3/7] glsl: Add initial functions to implement an on-disk cache
From: Kristian Høgsberg k...@bitplanet.net
This code provides for an on-disk cache of objects. Objects are stored
and retrieved (in ~/.cache/mesa) via names that are arbitrary 20-byte
sequences, (intended to be SHA-1 hashes of something identifying for
the content).
The cache is limited to a maximum number of entries (1024 in this
patch), and uses random replacement. These attributes are managed via
an index file that is stored in the cache directory and mmapped. This
file is indexed by the low-order bytes of the cached object's names
and each entry stores the complete name. So a quick comparison of the
index entry verifies whether the cache has an item, or whether an
existing item should be replaced.
Note: Some FIXME comments are still present in this commit. These will
be addressed in subsequent commits, (and before any of this code gets
any active use).
---
src/glsl/Makefile.am | 4 +
src/glsl/Makefile.sources | 3 +
src/glsl/cache.c | 230 ++
3 files changed, 237 insertions(+)
create mode 100644 src/glsl/cache.c
diff --git a/src/glsl/Makefile.am b/src/glsl/Makefile.am
index 01123bc..604af51 100644
--- a/src/glsl/Makefile.am
+++ b/src/glsl/Makefile.am
@@ -137,6 +137,10 @@ libglsl_la_SOURCES =
\
$(LIBGLSL_FILES)\
$(NIR_FILES)
+if ENABLE_SHADER_CACHE
+libglsl_la_SOURCES += $(LIBGLSL_SHADER_CACHE_FILES)
+endif
+
glsl_compiler_SOURCES = \
$(top_srcdir)/src/mesa/main/imports.c \
$(top_srcdir)/src/mesa/program/prog_hash_table.c \
diff --git a/src/glsl/Makefile.sources b/src/glsl/Makefile.sources
index 8375f6e..c5b742c 100644
--- a/src/glsl/Makefile.sources
+++ b/src/glsl/Makefile.sources
@@ -179,6 +179,9 @@ LIBGLSL_FILES = \
$(GLSL_SRCDIR)/s_expression.cpp \
$(GLSL_SRCDIR)/s_expression.h
+LIBGLSL_SHADER_CACHE_FILES = \
+ $(GLSL_SRCDIR)/cache.c
+
# glsl_compiler
GLSL_COMPILER_CXX_FILES = \
diff --git a/src/glsl/cache.c b/src/glsl/cache.c
new file mode 100644
index 000..fd087db
--- /dev/null
+++ b/src/glsl/cache.c
@@ -0,0 +1,230 @@
+/*
+ * Copyright © 2014 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the Software),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+#include string.h
+#include stdlib.h
+#include stdio.h
+#include sys/types.h
+#include sys/stat.h
+#include sys/mman.h
+#include unistd.h
+#include fcntl.h
+#include pwd.h
+
+#include util/mesa-sha1.h
+
+#include cache.h
+
+#define INDEX_SIZE 1024
+struct program_cache {
+ unsigned char *index;
+ char path[256];
+};
+
+struct program_cache *
+cache_create(void)
+{
+ struct program_cache *cache;
+ char index_file[256], buffer[512];
+ struct stat sb;
+ size_t size;
+ int fd;
+ struct passwd pwd, *result;
+
+ getpwuid_r(getuid(), pwd, buffer, sizeof buffer, result);
+ if (result == NULL)
+ return NULL;
+ snprintf(index_file, sizeof index_file,
+%s/.cache/mesa/index, pwd.pw_dir);
+
+ fd = open(index_file, O_RDWR | O_CREAT | O_CLOEXEC, 0644);
+ if (fd == -1) {
+ /* FIXME: Check for ENOENT and mkdir on demand */
+ return NULL;
+ }
+
+ if (fstat(fd, sb) == -1) {
+ close(fd);
+ return NULL;
+ }
+
+ size = INDEX_SIZE * CACHE_KEY_SIZE;
+ if (sb.st_size == 0) {
+ if (ftruncate(fd, size) == -1) {
+ close(fd);
+ return NULL;
+ }
+ fsync(fd);
+ }
+
+ cache = (struct program_cache *) malloc(sizeof *cache);
+ if (cache == NULL) {
+ close(fd);
+ return NULL;
+ }
+
+ snprintf(cache-path, sizeof cache-path,
+%s/.cache/mesa, pwd.pw_dir);
+
+ /* FIXME: We map this shared, which is a start, but we need to think about
+* how to make it multi-process safe. */
+ cache-index = (unsigned char *)
+ mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
+ close(fd);
+ if (cache-index == MAP_FAILED) {
[Mesa-dev] [PATCH 1/7] glsl: Add cache.h, defining an API for a persistent cache of objects
This API forms the base infrastructure for the future shader cache. At
this point, the cache is simply a persistent, on-disk store of objects
stored and retrieved by 20-byte keys.
---
src/glsl/cache.h | 121 +++
1 file changed, 121 insertions(+)
create mode 100644 src/glsl/cache.h
diff --git a/src/glsl/cache.h b/src/glsl/cache.h
new file mode 100644
index 000..5e9b3a8
--- /dev/null
+++ b/src/glsl/cache.h
@@ -0,0 +1,121 @@
+/*
+ * Copyright © 2014 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the Software),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+#pragma once
+#ifndef CACHE_H
+#define CACHE_H
+
+#ifdef __cplusplus
+extern C {
+#endif
+
+#include stdint.h
+
+/* These functions implement a persistent, (on disk), cache of objects.
+ *
+ * Objects are stored and retrieved from the cache using keys which are each a
+ * sequence of 20 arbitrary bytes. This 20-byte size is chosen to allow for a
+ * SHA-1 signature to be used as they key, (but nothing in cache.c actually
+ * computes or relies on the keys being SHA-1). See mesa-sha1.h and
+ * _mesa_sha1_compute for assistance in computing SHA-1 signatures.
+ */
+
+/* Size of cache keys in bytes. */
+#define CACHE_KEY_SIZE 20
+
+typedef uint8_t cache_key[CACHE_KEY_SIZE];
+
+/**
+ * Create a new cache object.
+ *
+ * This function creates the handle necessary for all subsequent cache_*
+ * functions.
+ */
+struct program_cache *
+cache_create(void);
+
+/**
+ * Store an item in the cache under the name \key.
+ *
+ * The item can be retrieved later with cache_get(), (unless the item has
+ * been evicted).
+ *
+ * Any call to cache_put() may cause an existing, random, item to be evicted
+ * from the cache.
+ */
+void
+cache_put(struct program_cache *cache, cache_key key,
+ const void *data, size_t size);
+
+/**
+ * Mark the cache name \key as used in the cache, (without storing any
+ * associated data).
+ *
+ * A call to cache_mark() is conceptually the same as a call to cache_put()
+ * but without any associated data. Following such a call, cache_get()
+ * cannot be usefully used, (since there is no data to return), but
+ * cache_probe() can be used to check whether key has been marked.
+ *
+ * Any call to cache_mark() may cause an existing, random, item to be evicted
+ * from the cache.
+ */
+void
+cache_mark(struct program_cache *cache, cache_key key);
+
+/**
+ * Return an item previously stored in the cache with the name key.
+ *
+ * The item must have been previously stored with a call to cache_put().
+ *
+ * If \size is non-NULL, then, on successful return, it will be set to the
+ * size of the object.
+ *
+ * \return A pointer to the stored object, (or NULL if the object is not
+ * found, or if any error occurs such memory allocation failure or a
+ * filesystem error). The returned data is malloc'ed so the caller should call
+ * free() when done with it.
+ */
+uint8_t *
+cache_get(struct program_cache *cache, cache_key key, size_t *size);
+
+/**
+ * A lightweight test whether the given key is currently in the cache.
+ *
+ * This test is lightweight in that it makes no syscalls and will not hit the
+ * disk. It is implemented via a small array of \key signatures.
+ *
+ * Return value: True if the item is in the cache (at this instant), false
+ * otherwise.
+ *
+ * Note: After cache_has(), a subsequent call to cache_get()
+ * might fail, (if another user caused the item to be evicted in the
+ * meantime).
+ */
+int
+cache_has(struct program_cache *cache, cache_key key);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* CACHE_H */
--
2.1.4
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] Low-level infrastructure for the shader cache
Hi folks, This series adds a layer of code to store a cache of objects on disk. Thanks to Kristian Høgsberg for the initial proof-of-concept implementation here. I've take his original code and added my own cleanups and documentation to the cache API. I've also fixed up a couple of the items he had left as FIXMEs. In this series, my cleaned-up API arrives first, so you won't see the changes I've made there. But, for the implementation fixes, I've left his code as originally written and have separate commits for my fixes. I think this is the history we want for sake of review and maintainability. The code compiles at all points, (and is never used), so it's not like there are any regressions left lingering due to the unsquashed history. Like I said, None of the code here is actually used yet, (that will be a future series of patches, where we'll finally have an actual shader cache working). But I think the cache API and implementation can be productively reviewed now. There is one FIXME comment in the implementation here that will still need to be addressed. I'm soliciting some thoughts from others on how to do it. The issue is around the mmapped index file used for random-cache replacement, and how to ensure things stay consistent with multiple processes. Kristian's comments in this area are as follows. First, when mapping the index file: /* FIXME: We map this shared, which is a start, but we need to think about * how to make it multi-process safe. */ cache-index = (unsigned char *) mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0); Then, when reading an entry from the cache file, (and potentially unlinking the old file being replaced): /* FIXME: We'll need an fsync here and think about races... maybe even need * an flock to avoid leaking files. Or maybe fsync, then read back and * verify the entry is still ours, delete it if somebody else overwrote * it. */ So please follow up in replies if you have good ideas on how to best address these comments. Other than that, I think the code in this series is sound. Please do let me know what you see that I've missed. -Carl ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 5/7] glsl: Make cache directory if it does not already exist
With this patch, there are now three different options for the shader
cache directory, (considered in order until the first variable is
set):
$MESA_GLSL_CACHE_DIR
$XDG_CACHE_HOME/mesa
user-home-directory/.cache/mesa
Also with this patch, once the desired path is determined, the
directory is created if it does not exist, (but this code will not
create an arbitrary number of parent directories if they don't exist).
---
src/glsl/cache.c | 170 +++
1 file changed, 147 insertions(+), 23 deletions(-)
diff --git a/src/glsl/cache.c b/src/glsl/cache.c
index da71868..79e8afb 100644
--- a/src/glsl/cache.c
+++ b/src/glsl/cache.c
@@ -30,76 +30,200 @@
#include unistd.h
#include fcntl.h
#include pwd.h
+#include errno.h
#include util/mesa-sha1.h
+#include util/ralloc.h
+#include main/errors.h
#include cache.h
#define INDEX_SIZE 1024
struct program_cache {
unsigned char *index;
- char path[256];
+ char *path;
};
+/* Create a directory named 'path' if it does not already exist.
+ *
+ * Returns: 0 if path already exists as a directory or if created.
+ * -1 in all other cases.
+ */
+static int
+mkdir_if_needed(char *path)
+{
+ struct stat sb;
+
+ /* If the path exists already, then our work is done if it'sa directory,
+* but it's an error if it is not.
+*/
+ if (stat(path, sb) == 0) {
+ if (S_ISDIR(sb.st_mode)) {
+ return 0;
+ } else {
+ _mesa_warning(NULL,
+ Cannot use %s for shader cache (not a directory)
+ ---disabling.\n, path);
+ return -1;
+ }
+ }
+
+ if (mkdir(path, 0755) == 0)
+ return 0;
+
+ _mesa_warning(NULL,
+ Failed to create %s for shader cache (%s)---disabling.\n,
+ path, strerror(errno));
+
+ return -1;
+}
+
+/* Concatenate an existing path and a new name to form a new path. If the new
+ * path does not exist as a directory, create it then return the resulting
+ * name of the new path (ralloc'ed off of 'ctx').
+ *
+ * Returns NULL on any error, such as:
+ *
+ * path does not exist or is not a directory
+ * path/name exists but is not a directory
+ * path/name cannot be created as a directory
+ */
+static char *
+concatenate_and_mkdir(void *ctx, char *path, char *name)
+{
+ char *new_path;
+ struct stat sb;
+
+ if (stat(path, sb) != 0 || ! S_ISDIR(sb.st_mode))
+ return NULL;
+
+ new_path = ralloc_asprintf(ctx, %s/%s, path, name);
+
+ if (mkdir_if_needed(new_path) == 0)
+ return new_path;
+ else
+ return NULL;
+}
+
struct program_cache *
cache_create(void)
{
- struct program_cache *cache;
- char index_file[256], buffer[512];
+ void *ctx = ralloc_context(NULL);
+ struct program_cache *cache = NULL;
+ char *path, *index_file;
struct stat sb;
size_t size;
- int fd;
- struct passwd pwd, *result;
+ int fd = -1;
/* At user request, disable shader cache entirely. */
if (getenv(MESA_GLSL_CACHE_DISABLE))
- return NULL;
+ goto done;
+
+ /* Determine path for cache based on the first defined name as follows:
+*
+* $MESA_GLSL_CACHE_DIR
+* $XDG_CACHE_HOME/mesa
+* pwd.pw_dir/.cache/mesa
+*/
+ path = getenv(MESA_GLSL_CACHE_DIR);
+ if (path mkdir_if_needed(path) == -1) {
+ goto done;
+ }
- getpwuid_r(getuid(), pwd, buffer, sizeof buffer, result);
- if (result == NULL)
- return NULL;
- snprintf(index_file, sizeof index_file,
-%s/.cache/mesa/index, pwd.pw_dir);
+ if (path == NULL) {
+ char *xdg_cache_home = getenv(XDG_CACHE_HOME);
+
+ if (xdg_cache_home) {
+ if (mkdir_if_needed(xdg_cache_home) == -1)
+goto done;
+
+ path = concatenate_and_mkdir(ctx, xdg_cache_home, mesa);
+ if (path == NULL)
+goto done;
+ }
+ }
+
+ if (path == NULL) {
+ char *buf;
+ size_t buf_size;
+ struct passwd pwd, *result;
+
+ buf_size = sysconf(_SC_GETPW_R_SIZE_MAX);
+ if (buf_size == -1)
+ buf_size = 512;
+
+ /* Loop until buf_size is large enough to query the directory */
+ while (1) {
+ buf = ralloc_size(ctx, buf_size);
+
+ getpwuid_r(getuid(), pwd, buf, buf_size, result);
+ if (result)
+break;
+
+ if (errno == ERANGE) {
+ralloc_free(buf);
+buf = NULL;
+buf_size *= 2;
+ } else {
+goto done;
+ }
+ }
+
+ path = concatenate_and_mkdir(ctx, pwd.pw_dir, .cache);
+ if (path == NULL)
+ goto done;
+
+ path = concatenate_and_mkdir(ctx, path, mesa);
+ if (path == NULL)
+ goto done;
+ }
+
+ index_file = ralloc_asprintf(ctx, %s/%s, path, index);
fd = open(index_file, O_RDWR | O_CREAT | O_CLOEXEC, 0644);
if (fd == -1) {
- /* FIXME: Check for ENOENT and mkdir on demand */
- return NULL;
+
[Mesa-dev] [PATCH 7/7] glsl/cache: Write newly cached files atomically via rename()
Instead of writing directly to the desired filename, with this patch
we instead first write to filename.tmp and then use rename() to
atomically rename from filename.tmp to filename. This ensures that
any process that opens filename for reading will never see any
partially written file.
---
src/glsl/cache.c | 41 +
1 file changed, 25 insertions(+), 16 deletions(-)
diff --git a/src/glsl/cache.c b/src/glsl/cache.c
index 0bbf659..e0cdc1a 100644
--- a/src/glsl/cache.c
+++ b/src/glsl/cache.c
@@ -323,9 +323,9 @@ cache_put(struct program_cache *cache,
uint32_t *s = (uint32_t *) key;
int i = *s (INDEX_SIZE - 1);
unsigned char *entry;
- int fd, ret;
+ int fd = -1, ret;
size_t len;
- char *filename;
+ char *filename = NULL, *filename_tmp = NULL;
const char *p = (const char *) data;
/* FIXME: We'll need an fsync here and think about races... maybe even need
@@ -336,40 +336,49 @@ cache_put(struct program_cache *cache,
entry = cache-index[i * CACHE_KEY_SIZE];
filename = get_cache_file(cache, entry);
if (filename == NULL)
- return;
+ goto done;
unlink(filename);
ralloc_free(filename);
+ filename = NULL;
memcpy(entry, key, CACHE_KEY_SIZE);
if (data == NULL)
- return;
-
- /* FIXME: We should write the file to a name like sha1-foo, close it and
-* then rename(2) it to sha1 to make sure some other mesa process doesn't
-* open it and gets a partial result. Racing with another mesa writing the
-* same file is ok, since they'll both write the same contents, and whoever
-* finishes first will move the complete file in place. */
+ goto done;
filename = get_cache_file(cache, key);
if (filename == NULL)
- return;
+ goto done;
- fd = open(filename, O_WRONLY | O_CLOEXEC | O_CREAT, 0644);
- ralloc_free(filename);
+ filename_tmp = ralloc_asprintf(cache, %s.tmp, filename);
+ if (filename_tmp == NULL)
+ goto done;
+
+ fd = open(filename_tmp, O_WRONLY | O_CLOEXEC | O_CREAT, 0644);
if (fd == -1)
- return;
+ goto done;
for (len = 0; len size; len += ret) {
ret = write(fd, p + len, size - len);
if (ret == -1) {
- unlink(filename);
- break;
+ unlink(filename_tmp);
+ goto done;
}
}
close(fd);
+ fd = -1;
+
+ rename(filename_tmp, filename);
+
+ done:
+ if (filename_tmp)
+ ralloc_free(filename_tmp);
+ if (filename)
+ ralloc_free(filename);
+ if (fd != -1)
+ close(fd);
}
void
--
2.1.4
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 6/7] glsl/cache.c: Dynamically allocate filenames internal to cache
The user can put the cache directory anywhere, so it's not safe to use
fixed-size arrays to store filenames. Instead, allocate the cache
pointer itself as a ralloc context and use that to dynamically
allocate all filenames.
While making this change, simplify the error handling in cache_get
with a new goto FAIL block so the cleanup code exists in a single
place, rather than being spread throughout the function over and over.
---
src/glsl/cache.c | 87 +++-
1 file changed, 54 insertions(+), 33 deletions(-)
diff --git a/src/glsl/cache.c b/src/glsl/cache.c
index 79e8afb..0bbf659 100644
--- a/src/glsl/cache.c
+++ b/src/glsl/cache.c
@@ -197,14 +197,14 @@ cache_create(void)
fsync(fd);
}
- cache = (struct program_cache *) malloc(sizeof *cache);
+ cache = ralloc(NULL, struct program_cache);
if (cache == NULL) {
goto done;
}
cache-path = strdup(path);
if (cache-path == NULL) {
- free (cache);
+ ralloc_free (cache);
cache = NULL;
goto done;
}
@@ -214,7 +214,7 @@ cache_create(void)
cache-index = (unsigned char *)
mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
if (cache-index == MAP_FAILED) {
- free(cache);
+ ralloc_free(cache);
cache = NULL;
goto done;
}
@@ -227,16 +227,18 @@ cache_create(void)
return cache;
}
+/* Return a filename within the cache's directory corresponding to 'key'. The
+ * returned filename is ralloced with 'cache' as the parent context.
+ *
+ * Returns NULL if out of memory.
+ */
static char *
-get_cache_file(struct program_cache *cache,
- char *buffer, size_t size, cache_key key)
+get_cache_file(struct program_cache *cache, cache_key key)
{
char buf[41];
- snprintf(buffer, size, %s/%s,
-cache-path, _mesa_sha1_format(buf, key));
-
- return buffer;
+ return ralloc_asprintf(cache, %s/%s, cache-path,
+ _mesa_sha1_format(buf, key));
}
int
@@ -261,59 +263,69 @@ cache_has(struct program_cache *cache, cache_key key)
uint8_t *
cache_get(struct program_cache *cache, cache_key key, size_t *size)
{
- int fd, ret, len;
+ int fd = -1, ret, len;
struct stat sb;
- char filename[256], *data;
+ char *filename = NULL;
+ uint8_t *data = NULL;
if (size)
*size = 0;
if (!cache_has(cache, key))
- return NULL;
+ goto fail;
- get_cache_file(cache, filename, sizeof filename, key);
+ filename = get_cache_file(cache, key);
+ if (filename == NULL)
+ goto fail;
fd = open(filename, O_RDONLY | O_CLOEXEC);
if (fd == -1)
- return NULL;
+ goto fail;
- if (fstat(fd, sb) == -1) {
- close(fd);
- return NULL;
- }
+ if (fstat(fd, sb) == -1)
+ goto fail;
- data = (char *) malloc(sb.st_size);
- if (data == NULL) {
- close(fd);
- return NULL;
- }
+ data = malloc(sb.st_size);
+ if (data == NULL)
+ goto fail;
for (len = 0; len sb.st_size; len += ret) {
ret = read(fd, data + len, sb.st_size - len);
- if (ret == -1) {
- free(data);
- close(fd);
- return NULL;
- }
+ if (ret == -1)
+ goto fail;
}
+ ralloc_free(filename);
close(fd);
if (size)
*size = sb.st_size;
- return (void *) data;
+ return data;
+
+ fail:
+ if (data)
+ free(data);
+ if (filename)
+ ralloc_free(filename);
+ if (fd != -1)
+ close(fd);
+
+ return NULL;
}
void
-cache_put(struct program_cache *cache, cache_key key, const void *data, size_t
size)
+cache_put(struct program_cache *cache,
+ cache_key key,
+ const void *data,
+ size_t size)
{
uint32_t *s = (uint32_t *) key;
int i = *s (INDEX_SIZE - 1);
unsigned char *entry;
int fd, ret;
size_t len;
- char filename[256];
+ char *filename;
const char *p = (const char *) data;
/* FIXME: We'll need an fsync here and think about races... maybe even need
@@ -322,8 +334,13 @@ cache_put(struct program_cache *cache, cache_key key,
const void *data, size_t s
* it. */
entry = cache-index[i * CACHE_KEY_SIZE];
- get_cache_file(cache, filename, sizeof filename, entry);
+ filename = get_cache_file(cache, entry);
+ if (filename == NULL)
+ return;
+
unlink(filename);
+ ralloc_free(filename);
+
memcpy(entry, key, CACHE_KEY_SIZE);
if (data == NULL)
@@ -335,8 +352,12 @@ cache_put(struct program_cache *cache, cache_key key,
const void *data, size_t s
* same file is ok, since they'll both write the same contents, and whoever
* finishes first will move the complete file in place. */
- get_cache_file(cache, filename, sizeof filename, key);
+ filename = get_cache_file(cache, key);
+ if (filename == NULL)
+ return;
+
fd = open(filename, O_WRONLY | O_CLOEXEC | O_CREAT, 0644);
+ ralloc_free(filename);
if (fd == -1)
return;
[Mesa-dev] [PATCH 2/7] glsl: Add stubs for the case of --disable-shader-cache
If Mesa is being compiled with no shader cache, (whether due to
explicit user request or due to a missing library dependency), then we
want to incur no cost on the implementation. To achieve this with as
little fuss as possible, (that is, without sprinkling #ifdef
throughout every call into cache functions), we implement inlined
stubs to make all of the called functions do nothing.
For these stubs, the configure script is updated to provide a new
ENABLE_SHADER_CACHE preprocessor definition that can be queried at
compile time.
---
configure.ac | 3 +++
src/glsl/Makefile.sources | 1 +
src/glsl/cache.h | 39 +++
3 files changed, 43 insertions(+)
diff --git a/configure.ac b/configure.ac
index a4c5c74..c081b7b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1062,6 +1062,9 @@ if test x$with_sha1 = x; then
fi
fi
AM_CONDITIONAL([ENABLE_SHADER_CACHE], [test x$enable_shader_cache = xyes])
+if test x$enable_shader_cache = xyes; then
+ AC_DEFINE([ENABLE_SHADER_CACHE], [1], [Enable shader cache])
+fi
# Check for libdrm
PKG_CHECK_MODULES([LIBDRM], [libdrm = $LIBDRM_REQUIRED],
diff --git a/src/glsl/Makefile.sources b/src/glsl/Makefile.sources
index 6237627..8375f6e 100644
--- a/src/glsl/Makefile.sources
+++ b/src/glsl/Makefile.sources
@@ -67,6 +67,7 @@ LIBGLSL_FILES = \
$(GLSL_SRCDIR)/ast_type.cpp \
$(GLSL_SRCDIR)/blob.c \
$(GLSL_SRCDIR)/blob.h \
+ $(GLSL_SRCDIR)/cache.h \
$(GLSL_SRCDIR)/builtin_functions.cpp \
$(GLSL_SRCDIR)/builtin_type_macros.h \
$(GLSL_SRCDIR)/builtin_types.cpp \
diff --git a/src/glsl/cache.h b/src/glsl/cache.h
index 5e9b3a8..51be663 100644
--- a/src/glsl/cache.h
+++ b/src/glsl/cache.h
@@ -45,6 +45,10 @@ extern C {
typedef uint8_t cache_key[CACHE_KEY_SIZE];
+/* Provide inlined stub functions if the shader cache is disabled. */
+
+#ifdef ENABLE_SHADER_CACHE
+
/**
* Create a new cache object.
*
@@ -114,6 +118,41 @@ cache_get(struct program_cache *cache, cache_key key,
size_t *size);
int
cache_has(struct program_cache *cache, cache_key key);
+#else
+
+static inline struct program_cache *
+cache_create(void)
+{
+ return NULL;
+}
+
+static inline void
+cache_put(struct program_cache *cache, cache_key key,
+ const void *data, size_t size)
+{
+ return 0;
+}
+
+static inline void
+cache_mark(struct program_cache *cache, cache_key key)
+{
+ return;
+}
+
+static inline uint8_t *
+cache_get(struct program_cache *cache, cache_key key, size_t *size)
+{
+ return NULL;
+}
+
+static inline int
+cache_has(struct program_cache *cache, cache_key key)
+{
+ return 0;
+}
+
+#endif /* ENABLE_SHADER_CACHE */
+
#ifdef __cplusplus
}
#endif
--
2.1.4
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 4/7] glsl: Add environment variable to disable shader cache
This patch adds support for a new variable, MESA_GLSL_CACHE_DISABLE. If this variable is set, then all use of the shader cache will be disabled at run time. --- docs/envvars.html | 1 + src/glsl/cache.c | 4 2 files changed, 5 insertions(+) diff --git a/docs/envvars.html b/docs/envvars.html index 31d14a4..65db3a9 100644 --- a/docs/envvars.html +++ b/docs/envvars.html @@ -94,6 +94,7 @@ This is only valid for versions gt;= 3.0. glGetString(GL_SHADING_LANGUAGE_VERSION). Valid values are integers, such as 130. Mesa will not really implement all the features of the given language version if it's higher than what's normally reported. (for developers only) +liMESA_GLSL_CACHE_DISABLE - if set, disables the GLSL shader cache liMESA_GLSL - a href=shading.html#envvarsshading language compiler options/a /ul diff --git a/src/glsl/cache.c b/src/glsl/cache.c index fd087db..da71868 100644 --- a/src/glsl/cache.c +++ b/src/glsl/cache.c @@ -51,6 +51,10 @@ cache_create(void) int fd; struct passwd pwd, *result; + /* At user request, disable shader cache entirely. */ + if (getenv(MESA_GLSL_CACHE_DISABLE)) + return NULL; + getpwuid_r(getuid(), pwd, buffer, sizeof buffer, result); if (result == NULL) return NULL; -- 2.1.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/7] glsl: Add initial functions to implement an on-disk cache
On Wed, Feb 04 2015, Kenneth Graunke wrote: The cache will need to be much larger than 1024 entries - perhaps by an order of magnitude. Thanks for the feedback. I had meant to add a comment next to that 1024 in the code along the lines of This value was chosen arbitrarily. An appropriate value will need to be found with testing. We should respect $XDG_CACHE_HOME from the XDG base directory spec: http://standards.freedesktop.org/basedir-spec/basedir-spec-latest.html Also, the snprintf makes me a bit nervous See patch 5/7. It fixes both of these problems. (My apologies for setting you up to review code that was replaced later in the series---but I'm always a little hesitant to squash things to much when only some of the code is actually mine). Typo: unnecessary compile (missing 'n'). Also, */ goes on its own line. Thanks. I've fixed both of these now. -Carl pgpRw1mcWIDW5.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 6/7] glsl/cache.c: Dynamically allocate filenames internal to cache
On Wed, Feb 04 2015, Matt Turner wrote: Is there some reason we should do this as a separate patch instead of just squashing it? Not really. I've just squashed it, (while also fixing the space before the parenthesis---I should probably add a pre-commit check for those to help me break that habit. That, and the '*/' on its own line as well. I'm never going to get used to that without help). -Carl pgpccczBBcOl8.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/7] glsl: Add initial functions to implement an on-disk cache
On Wed, Feb 04 2015, Matt Turner wrote: Rebase needed. I removed GLSL_SRCDIR a week and a half ago. Was this the removal of all those subdir-objects warnings? Thank you! I don't think it makes a difference to autotools, but I think I'd list cache.h here instead of in LIBGLSL_FILES. Actually, that was sort of intentional. The cache.c file is only conditionally compiled, but cache.h has the inline stub implementations that we want to compile unconditionally. So it felt right to put the .h in the unconditional list. Of course, maybe that doesn't matter at all. It's the .c files that include the .h file that are going to result in the inline stubs being provided. Now that we're on the subject, what does automake even do with .h files that are listed in these lists? Do they need to be there for non-srcdir builds to work or something? If it truly doesn't matter, then I would prefer to see the cache.h next to the cache.c, yes. + cache = (struct program_cache *) malloc(sizeof *cache); Don't cast malloc. I'll blame krh on this one, (and he can in turn blame it on bad habits From programming in C++ too much). :-) + return (void *) data; I don't think you need this cast either? Just removing that cast doesn't quite do the trick because there's a potential signedness difference between char and uint8_t. But the cast is not the way I want to fix this, so thanks for pointing that out. (I have gone back and forth on whether this function should return a uint8_t* or a void *. There's a later patch in the next series that still expects a direct assignment from cache_get() to some data-structure pointer to work without a cast. So I may change cache_get back to returning a void *. But even then, the cast above won't be needed.). Thanks for your attention to detail. -Carl pgpaAlNnBcoKs.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] mesa: Add mesa SHA-1 functions
On Fri, Jan 16 2015, Jose Fonseca wrote: Oh, just saying that in fact I got it to build that module on Windows. Thanks. That's good to know. I appreciate that testing. I've just pushed this series along with your fixes. And for the sake of everyone that contributed to the discussion on the SHA-1 dependency, here's where things stand currently: With autotools: If a supported SHA-1 library is detected, the resulting Mesa library will be linked against it. If no supported library is detected, Mesa will still compile, but without any of the code that might require a SHA-1 implementation, (none today, but a shader cache is expected in the near future). In either case, the dependency can be avoided by running configure with the option --disable-shader-cache. With scons: Regardless of the presence or absence of SHA-1 libraries, Mesa will not be linked against any, nor will any of the SHA-1-using code be compiled. This is effectively the same situation one would get with --disable-shader-cache in the autotools case. In the future, I expect the situation with scons to match that of autotools, (so that the shader cache could be made available in scons builds). José volunteered to write the scons magic to detect SHA-1 implementations as time permits. Additionally, some people mentioned the possibility of adding a SHA-1 implementation directly to Mesa, (to allow for the shader-cache to be used without any external dependency). If someone wants to pursue that, that's still a possibility. But that's not something I plan to pursue myself. -Carl pgp41DBd0mvsR.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] mesa: Add mesa SHA-1 functions
On Thu, Jan 08 2015, Jose Fonseca wrote: Note that Windows build is only supported with SCons. Never with autobuild. OK. That's good for me to learn. I've requested that the folks doing our automated build testing here will also start testing scons builds, so hopefully on our end we can avoid breaking scons builds in the future. In fact, SCons build was broken on that branch, with Windows or Linux. I've pushed a few fixes to http://cgit.freedesktop.org/~jrfonseca/mesa/log/?h=sha1 Thanks. I've incorporated these fixes into my commits and rebased everything onto the latest master. You can see what I have here: http://cgit.freedesktop.org/~cworth/mesa/log/?h=sha1 The only change I didn't pull in was the || defined(_WIN32) to force compilation of the CryptoAPI SHA-1 support on a Windows build, (since this file won't even be compiled). This is enough to get things building without failing. And in fact I Looks like you didn't finish that sentence. Perhaps there was nothing important there. So my recommendation is: disable the shader cache on all scons builds, get this merged and working on master with automake, then I'll add the HAVE_SHA1 * checks on SCons and enable this as time permits. That sounds like a good plan. I can push what I've got above unless there are any objections from anybody. -Carl pgpDoNtiab9ey.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/2] configure: Add machinery for --enable-shader-cache (and --disable-shader-cache)
On Thu, Jan 15 2015, Matt Turner wrote: I wouldn't put the spaces after and before the [ ] (there's an occurrence of this in the previous patch as well, that gets removed in this one). OK. I'll fix that. Both are: Reviewed-by: Matt Turner matts...@gmail.com Thanks! -Carl pgpPQ3Jww2upP.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] mesa: Add mesa SHA-1 functions
On Wed, Jan 07 2015, Jose Fonseca wrote:
I lost bit track of email over the Christmas period. Just noticed I had
flagged this one for replay. Sorry.
No worries. Thanks for following up now. :-)
Do you still need me to test anything on Windows? If so are the patches
in some pull-able git repos by any chance?
Yes, some testing on Windows would be great. I've got these patches
here:
git://people.freedesktop.org/~cworth/mesa
And testing that the build works fine with or without one of the
potential Windows crypto libraries available would be great. Look for
lines like the following in the configure output:
Shader cache:yes
With SHA1 from: libnettle
And you can manually control this by passing options such as:
./configure --disable-shader-cache
or:
./configure --enable-shader-cache --with-sha1=CryptoAPI
The possible values for --with-sha1 are listed in ./configure --help
and include the following:
libc, libmd, libnettle, libgcrypt, libcrypto, libsha1,
CommonCrypto, CryptoAPI
As I said earlier in the thread, I've tested libnettle, libgcrypt, and
libcrypto on my Linux machine. So any touch testing of any of the other
options, (particularly those available only on Windows), would be great.
In that branch, there's not actually any code that calls into any of the
sha1 functions. So you'll basically just be testing configuration,
building, and linking. If you'd like to go the extra step and verify
that the code can be called and actually do something, then you could
use something like the attached patch which simply prints the computed
sha1 for any compiled shader.
Please let me know if you have any questions, and what testing results
you get.
Thanks,
-Carl
From 07bd85f5c620361ad0ea358f01a8a0b5139f1239 Mon Sep 17 00:00:00 2001
From: Carl Worth cwo...@cworth.org
Date: Wed, 7 Jan 2015 11:07:59 -0800
Subject: [PATCH] Exercise the recently-added sha1 code.
This commit is not intended to be pushed upstream. It simply adds a
print message for each compiled shader, giving the computed sha1 of
the shader source. (This is intended to provide minimal testing that
the sha1 code detected by the configure script actually links and
runs.)
---
src/glsl/glsl_parser_extras.cpp | 7 +++
1 file changed, 7 insertions(+)
diff --git a/src/glsl/glsl_parser_extras.cpp b/src/glsl/glsl_parser_extras.cpp
index 7bfc39e..39a4749 100644
--- a/src/glsl/glsl_parser_extras.cpp
+++ b/src/glsl/glsl_parser_extras.cpp
@@ -33,6 +33,7 @@ extern C {
}
#include util/ralloc.h
+#include util/sha1.h
#include ast.h
#include glsl_parser_extras.h
#include glsl_parser.h
@@ -1449,11 +1450,17 @@ _mesa_glsl_compile_shader(struct gl_context *ctx, struct gl_shader *shader,
struct _mesa_glsl_parse_state *state =
new(shader) _mesa_glsl_parse_state(ctx, shader-Stage, shader);
const char *source = shader-Source;
+ unsigned char sha1[20];
+ char sha1_str[41];
if (ctx-Const.GenerateTemporaryNames)
(void) p_atomic_cmpxchg(ir_variable::temporaries_allocate_names,
false, true);
+ _mesa_sha1_compute(source, strlen(source), sha1);
+ _mesa_sha1_format(sha1_str, sha1);
+ printf(Computed sha1 of GLSL source string: %s\n, sha1_str);
+
state-error = glcpp_preprocess(state, source, state-info_log,
ctx-Extensions, ctx);
--
2.1.4
pgpz8TYx0Gn3A.pgp
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] mesa: Add mesa SHA-1 functions
On Fri, Dec 12 2014, Jose Fonseca wrote: Yes, ideally we'd have something small that we could bundle into mesa source tree, for sake of non Linux OSes. Ken has pointed to an implementation that might be suitable for this. I haven't reviewed that code myself, nor am I signing up to maintain a SHA-1 implementation within Mesa. But if somebody does want to do that review and maintenance, then it would be a very simple extension of the code I've provided to allow a builtin implementation as well as all of the external implementations that are deteted. If Windows was the only concern, we could use its Crypto API, http://msdn.microsoft.com/en-us/library/windows/desktop/aa382379.aspx and avoid depending on anything else, but some of the above mention libraries are not trivial to install. I believe that the second patch I posted, (as well as what I'm about to follow up with), does have support for CryptoAPI. Would you be available to test that? The other alternative is to disable shader cache when no suitable dependency is found. That is, make this an optional dependency. That makes a lot of sense and should make this feature quite non-controversial, since it doesn't force any new dependency requirement on anyone who can't easily satisfy it. I've now written the configure machinery to do exactly this. I've also tested, (and fixed), the code I posted earlier to detect and choose from among several different SHA-1 implementations. Specifically, I've tested that it works correctly with --with-sha1=libnettle, --with-sha1=libgcrypt, or --with-sha1=libcrypto, (three libraries that I happened to have installed). I haven't tested the others, but I'm optimistic that they should work unchanged from the xserver implementation. I'll follow to this email with my two latest patches. -Carl pgpFekw9JDxhc.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/2] mesa: Add mesa SHA-1 functions
libcrypto
+AC_CHECK_LIB([crypto], [SHA1_Init], [HAVE_LIBCRYPTO=yes])
+PKG_CHECK_MODULES([OPENSSL], [openssl], [HAVE_OPENSSL_PKC=yes],
+ [HAVE_OPENSSL_PKC=no])
+if test x$HAVE_LIBCRYPTO = xyes || test x$HAVE_OPENSSL_PKC = xyes; then
+ if test x$with_sha1 = x; then
+ with_sha1=libcrypto
+ fi
+else
+ if test x$with_sha1 = xlibcrypto; then
+ AC_MSG_ERROR([OpenSSL libcrypto requested but not found])
+ fi
+fi
+if test x$with_sha1 = xlibcrypto; then
+ if test x$HAVE_LIBCRYPTO = xyes; then
+ SHA1_LIBS=-lcrypto
+ else
+ SHA1_LIBS=$OPENSSL_LIBS
+ SHA1_CFLAGS=$OPENSSL_CFLAGS
+ fi
+fi
+AC_MSG_CHECKING([for SHA1 implementation])
+AM_CONDITIONAL([HAVE_SHA1], [ test x$with_sha1 != x ])
+AC_MSG_RESULT([$with_sha1])
+AC_SUBST(SHA1_LIBS)
+AC_SUBST(SHA1_CFLAGS)
+
# Check for libdrm
PKG_CHECK_MODULES([LIBDRM], [libdrm = $LIBDRM_REQUIRED],
[have_libdrm=yes], [have_libdrm=no])
diff --git a/src/util/Makefile.am b/src/util/Makefile.am
index 8d5f90e..ee3c57f 100644
--- a/src/util/Makefile.am
+++ b/src/util/Makefile.am
@@ -31,12 +31,15 @@ libmesautil_la_CPPFLAGS = \
-I$(top_srcdir)/src \
-I$(top_srcdir)/src/mapi \
-I$(top_srcdir)/src/mesa \
+ $(SHA1_CFLAGS) \
$(VISIBILITY_CFLAGS)
libmesautil_la_SOURCES = \
$(MESA_UTIL_FILES) \
$(MESA_UTIL_GENERATED_FILES)
+libmesautil_la_LIBADD = $(SHA1_LIBS)
+
BUILT_SOURCES = $(MESA_UTIL_GENERATED_FILES)
CLEANFILES = $(BUILT_SOURCES)
diff --git a/src/util/Makefile.sources b/src/util/Makefile.sources
index 9e27424..39f87e4 100644
--- a/src/util/Makefile.sources
+++ b/src/util/Makefile.sources
@@ -1,10 +1,17 @@
+if HAVE_SHA1
+MESA_UTIL_SHA1_FILES := \
+ sha1.c \
+ sha1.h
+endif
+
MESA_UTIL_FILES := \
hash_table.c\
ralloc.c \
register_allocate.c \
register_allocate.h \
rgtc.c \
- strtod.cpp
+ strtod.cpp \
+$(MESA_UTIL_SHA1_FILES)
MESA_UTIL_GENERATED_FILES = \
format_srgb.c
diff --git a/src/util/sha1.c b/src/util/sha1.c
new file mode 100644
index 000..1edc5fe
--- /dev/null
+++ b/src/util/sha1.c
@@ -0,0 +1,316 @@
+/* Copyright © 2007 Carl Worth
+ * Copyright © 2009 Jeremy Huddleston, Julien Cristau, and Matthieu Herrb
+ * Copyright © 2009-2010 Mikhail Gusarov
+ * Copyright © 2012 Yaakov Selkowitz and Keith Packard
+ * Copyright © 2014 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the Software),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ */
+
+#include sha1.h
+
+#if defined(HAVE_SHA1_IN_LIBMD) /* Use libmd for SHA1 */ \
+ || defined(HAVE_SHA1_IN_LIBC) /* Use libc for SHA1 */
+
+#include sha1.h
+
+struct mesa_sha1 *
+_mesa_sha1_init(void)
+{
+ SHA1_CTX *ctx = malloc(sizeof(*ctx));
+
+ if (!ctx)
+ return NULL;
+
+ SHA1Init(ctx);
+ return (struct mesa_sha1 *) ctx;
+}
+
+int
+_mesa_sha1_update(struct mesa_sha1 *ctx, const void *data, int size)
+{
+ SHA1_CTX *sha1_ctx = (SHA1_CTX *) ctx;
+
+ SHA1Update(sha1_ctx, data, size);
+ return 1;
+}
+
+int
+_mesa_sha1_final(struct mesa_sha1 *ctx, unsigned char result[20])
+{
+ SHA1_CTX *sha1_ctx = (SHA1_CTX *) ctx;
+
+ SHA1Final(result, sha1_ctx);
+ free(sha1_ctx);
+ return 1;
+}
+
+#elif defined(HAVE_SHA1_IN_COMMONCRYPTO)/* Use CommonCrypto for SHA1 */
+
+#include CommonCrypto/CommonDigest.h
+
+struct mesa_sha1 *
+_mesa_sha1_init(void)
+{
+ CC_SHA1_CTX *ctx = malloc(sizeof(*ctx));
+
+ if (!ctx)
+ return NULL;
+
+ CC_SHA1_Init(ctx);
+ return (struct mesa_sha1 *) ctx;
+}
+
+int
+_mesa_sha1_update(struct mesa_sha1 *ctx, const void *data, int size)
+{
+ CC_SHA1_CTX *sha1_ctx = (CC_SHA1_CTX *) ctx;
+
+ CC_SHA1_Update(sha1_ctx, data, size);
+ return 1;
+}
+
+int
+_mesa_sha1_final(struct mesa_sha1 *ctx, unsigned char result[20])
+{
+ CC_SHA1_CTX *sha1_ctx = (CC_SHA1_CTX *) ctx
[Mesa-dev] [PATCH 2/2] configure: Add machinery for --enable-shader-cache (and --disable-shader-cache)
We don't actually have the code for the shader cache just yet, but this configure machinery puts everything in place so that the shader cache can be optionally compiled in. Specifically, if the user passes no option (neither --disable-shader-cache, nor --enable-shader-cache), then this feature will be automatically detected based on the presence of a usable SHA-1 library. If no suitable library can be found, then the shader cache will be automatically disabled, (and reported in the final output from configure). The user can force the shader-cache feature to not be compiled, (even if a SHA-1 library is detected), by passing --disable-shader-cache. This will prevent the compiled Mesa libraries from depending on any library for SHA-1 implementation. Finally, the user can also force the shader cache on with --enable-shader-cache. This will cause configure to trigger a fatal error if no sutiable SHA-1 implementation can be found for the shader-cache feature. --- configure.ac | 23 ++- src/util/Makefile.sources | 6 +++--- 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/configure.ac b/configure.ac index 9697e9f..0361893 100644 --- a/configure.ac +++ b/configure.ac @@ -1026,11 +1026,26 @@ if test x$with_sha1 = xlibcrypto; then fi fi AC_MSG_CHECKING([for SHA1 implementation]) -AM_CONDITIONAL([HAVE_SHA1], [ test x$with_sha1 != x ]) AC_MSG_RESULT([$with_sha1]) AC_SUBST(SHA1_LIBS) AC_SUBST(SHA1_CFLAGS) +# Allow user to configure out the shader-cache feature +AC_ARG_ENABLE([shader-cache], +AS_HELP_STRING([--disable-shader-cache], [Disable binary shader cache]), +[enable_shader_cache=$enableval], +[if test x$with_sha1 != x; then +enable_shader_cache=yes + else +enable_shader_cache=no + fi]) +if test x$with_sha1 = x; then +if test x$enable_shader_cache = xyes; then +AC_MSG_ERROR([Cannot enable shader cache (no SHA-1 implementation found)]) +fi +fi +AM_CONDITIONAL([ENABLE_SHADER_CACHE], [ test x$enable_shader_cache = xyes ]) + # Check for libdrm PKG_CHECK_MODULES([LIBDRM], [libdrm = $LIBDRM_REQUIRED], [have_libdrm=yes], [have_libdrm=no]) @@ -2476,6 +2491,12 @@ else echo Gallium: no fi +dnl Shader cache +echo +echo Shader cache:$enable_shader_cache +if test x$enable_shader_cache = xyes; then +echo With SHA1 from: $with_sha1 +fi dnl Libraries echo diff --git a/src/util/Makefile.sources b/src/util/Makefile.sources index 39f87e4..d76ab13 100644 --- a/src/util/Makefile.sources +++ b/src/util/Makefile.sources @@ -1,5 +1,5 @@ -if HAVE_SHA1 -MESA_UTIL_SHA1_FILES := \ +if ENABLE_SHADER_CACHE +MESA_UTIL_SHADER_CACHE_FILES := \ sha1.c \ sha1.h endif @@ -11,7 +11,7 @@ MESA_UTIL_FILES :=\ register_allocate.h \ rgtc.c \ strtod.cpp \ -$(MESA_UTIL_SHA1_FILES) +$(MESA_UTIL_SHADER_CACHE_FILES) MESA_UTIL_GENERATED_FILES = \ format_srgb.c -- 2.1.3 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] glsl: Add unit tests for blob.c
In addition to exercising all of the functions in blob.h, this
includes a stress test that forces some reallocing, and also tests to
verify the alignment and overrun-detection code in blob.c.
---
src/glsl/Makefile.am | 7 +
src/glsl/tests/.gitignore | 1 +
src/glsl/tests/blob_test.c | 320 +
3 files changed, 328 insertions(+)
create mode 100644 src/glsl/tests/blob_test.c
diff --git a/src/glsl/Makefile.am b/src/glsl/Makefile.am
index 0ccc81d..bf42ac8 100644
--- a/src/glsl/Makefile.am
+++ b/src/glsl/Makefile.am
@@ -34,6 +34,7 @@ include Makefile.sources
TESTS = glcpp/tests/glcpp-test \
glcpp/tests/glcpp-test-cr-lf\
+ tests/blob-test \
tests/general-ir-test \
tests/optimization-test \
tests/sampler-types-test\
@@ -47,12 +48,18 @@ noinst_LTLIBRARIES = libglsl.la libglcpp.la
check_PROGRAMS = \
glcpp/glcpp \
glsl_test \
+ tests/blob-test \
tests/general-ir-test \
tests/sampler-types-test\
tests/uniform-initializer-test
noinst_PROGRAMS = glsl_compiler
+tests_blob_test_SOURCES = \
+ tests/blob_test.c
+tests_blob_test_LDADD =\
+ $(top_builddir)/src/glsl/libglsl.la
+
tests_general_ir_test_SOURCES =\
$(top_srcdir)/src/mesa/main/imports.c \
$(top_srcdir)/src/mesa/program/prog_hash_table.c\
diff --git a/src/glsl/tests/.gitignore b/src/glsl/tests/.gitignore
index 15ce248..13dcdc4 100644
--- a/src/glsl/tests/.gitignore
+++ b/src/glsl/tests/.gitignore
@@ -1,3 +1,4 @@
+blob-test
ralloc-test
uniform-initializer-test
sampler-types-test
diff --git a/src/glsl/tests/blob_test.c b/src/glsl/tests/blob_test.c
new file mode 100644
index 000..4806029
--- /dev/null
+++ b/src/glsl/tests/blob_test.c
@@ -0,0 +1,320 @@
+/*
+ * Copyright © 2014 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the Software),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+/* A collection of unit tests for blob.c */
+
+#include stdio.h
+#include stdlib.h
+#include stdbool.h
+#include string.h
+
+#include util/ralloc.h
+#include blob.h
+
+#define bytes_test_str bytes_test
+#define reserve_test_str reserve_test
+
+/* This placeholder must be the same length as the next overwrite_test_str */
+#define placeholder_strXX
+#define overwrite_test_str overwrite_test
+#define uint32_test0x12345678
+#define uint32_placeholder 0xDEADBEEF
+#define uint32_overwrite 0xA1B2C3D4
+#define uint64_test0x1234567890ABCDEF
+#define string_test_strstring_test
+
+bool error = false;
+
+static void
+expect_equal(uint64_t expected, uint64_t actual, const char *test)
+{
+ if (actual != expected) {
+ fprintf (stderr, Error: Test '%s' failed: Expected=%ld, Actual=%ld\n,
+ test, expected, actual);
+ error = true;
+ }
+}
+
+static void
+expect_unequal(uint64_t expected, uint64_t actual, const char *test)
+{
+ if (actual == expected) {
+ fprintf (stderr, Error: Test '%s' failed: Result=%ld, but expected
something different.\n,
+ test, actual);
+ error = true;
+ }
+}
+
+static void
+expect_equal_str(const char *expected, const char *actual, const char *test)
+{
+ if (strcmp(expected, actual)) {
+ fprintf (stderr, Error: Test '%s' failed:\n\t
+ Expected=\%s\, Actual=\%s\\n,
+ test, expected, actual);
+ error = true;
+ }
+}
+
+static void
+expect_equal_bytes(uint8_t *expected, uint8_t *actual,
+
[Mesa-dev] [PATCH v2] glsl: Add blob.c---a simple interface for serializing data
This new interface allows for writing a series of objects to a chunk
of memory (a blob).. The allocated memory is maintained within the
blob itself, (and re-allocated by doubling when necessary).
There are also functions for reading objects from a blob as well. If
code attempts to read beyond the available memory, the read functions
return 0 values (or its moral equivalent) without reading past the
allocated memory. Once the caller is done with the reads, it can check
blob-overrun to ensure whether any invalid values were previously
returned due to attempts to read too far.
---
Jason,
This second revision of my patch is in response to your review.
I went to move the align_blob_reader function down by all the other
blob_reader functions, but I decided not to. You had a concern about the
alignment code, and it it's wrong then both align_blob() and
align_blob_reader() will need to be fixed. So it seems useful to me to keep
them close to each other.
-Carl
src/glsl/Makefile.sources | 3 +-
src/glsl/blob.c | 295 ++
src/glsl/blob.h | 245 ++
3 files changed, 542 insertions(+), 1 deletion(-)
create mode 100644 src/glsl/blob.c
create mode 100644 src/glsl/blob.h
diff --git a/src/glsl/Makefile.sources b/src/glsl/Makefile.sources
index 676fa0d..875e4bf 100644
--- a/src/glsl/Makefile.sources
+++ b/src/glsl/Makefile.sources
@@ -105,7 +105,8 @@ LIBGLSL_FILES = \
$(GLSL_SRCDIR)/opt_swizzle_swizzle.cpp \
$(GLSL_SRCDIR)/opt_tree_grafting.cpp \
$(GLSL_SRCDIR)/opt_vectorize.cpp \
- $(GLSL_SRCDIR)/s_expression.cpp
+ $(GLSL_SRCDIR)/s_expression.cpp \
+ $(GLSL_SRCDIR)/blob.c
# glsl_compiler
diff --git a/src/glsl/blob.c b/src/glsl/blob.c
new file mode 100644
index 000..75d3cda
--- /dev/null
+++ b/src/glsl/blob.c
@@ -0,0 +1,295 @@
+/*
+ * Copyright © 2014 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the Software),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+#include string.h
+
+#include main/macros.h
+#include util/ralloc.h
+#include blob.h
+
+#define BLOB_INITIAL_SIZE 4096
+
+/* Ensure that \blob will be able to fit an additional object of size
+ * \additional. The growing (if any) will occur by doubling the existing
+ * allocation.
+ */
+static bool
+grow_to_fit (struct blob *blob, size_t additional)
+{
+ size_t to_allocate;
+ uint8_t *new_data;
+
+ if (blob-size + additional = blob-allocated)
+ return true;
+
+ if (blob-allocated == 0)
+ to_allocate = BLOB_INITIAL_SIZE;
+ else
+ to_allocate = MAX2(blob-allocated * 2, additional);
+
+ new_data = reralloc_size(blob, blob-data, to_allocate);
+ if (new_data == NULL)
+ return false;
+
+ blob-data = new_data;
+ blob-allocated = to_allocate;
+
+ return true;
+}
+
+/* Align the blob-size so that reading or writing a value at (blob-data +
+ * blob-size) will result in an access aligned to a granularity of \alignment
+ * bytes.
+ *
+ * \return True unless allocation fails
+ */
+static bool
+align_blob (struct blob *blob, size_t alignment)
+{
+ size_t new_size;
+
+ new_size = ALIGN(blob-size, alignment);
+
+ if (! grow_to_fit (blob, new_size - blob-size))
+ return false;
+
+ blob-size = new_size;
+
+ return true;
+}
+
+static void
+align_blob_reader (struct blob_reader *blob, size_t alignment)
+{
+ blob-current = blob-data + ALIGN(blob-current - blob-data, alignment);
+}
+
+struct blob *
+blob_create (void *mem_ctx)
+{
+ struct blob *blob;
+
+ blob = ralloc(mem_ctx, struct blob);
+ if (blob == NULL)
+ return NULL;
+
+ blob-data = NULL;
+ blob-allocated = 0;
+ blob-size = 0;
+
+ return blob;
+}
+
+bool
+blob_write_bytes (struct blob *blob, const void *bytes, size_t to_write)
+{
+ if (! grow_to_fit (blob, to_write))
+ return false;
+
+ memcpy (blob-data + blob-size, bytes, to_write);
+ blob-size += to_write;
+
+
Re: [Mesa-dev] [PATCH v2] glsl: Add blob.c---a simple interface for serializing data
On Fri, Dec 12 2014, Jason Ekstrand wrote: Should be MAX2(blob-allocated * 2, blob-size + additional) Yikes! Yes. That's really embarrassing. Something to tuck in the back of your brain in case you have a need to grow this datastructure: The above write functions could be generated with three invocations of a macro that takes a function name and a type. That would prevent copy-and-paste typos. For only 3 functions, this probably isn't worth it. An excellent point. And before I sent this out the first time, I actually sat down to re-write it exactly that way. And just as you mention here, when I saw that it's only the three read functions that benefit, (read_string() is special), I decided it's more readable/maintainable in its current form. But yes, if it grows any, we can definitely do that. So I can at least capture that idea in a comment. Other than the couple comments here and the alignment thing (different e-mail). Reviewed-by: Jason Ekstrand jason.ekstr...@intel.com Thanks. I'll catch the missing mention of blob.h in Makefile.sources as Matt pointed out as well. And like I said, I think I will just hold onto this patch until landing some code that uses it. (It's not like there's going to be any difficulty rebasing it across future changes to Mesa.) -Carl PS. Aside from Gmail's whacky quoting/wrapping, would it be easy for you to omit from code-review emails the parts of the code that you're not replying to? I'm nervous that I might miss some useful comment when I'm cutting those pieces out of my replies. Thanks. pgphYaqwkse5v.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] glsl: Add blob.c---a simple interface for serializing data
On Fri, Dec 12 2014, Ian Romanick wrote: In addition to the (mostly) nits below, this seems ripe for some unit tests. Especially for the overrun cases and the data alignment cases. Thanks for the review, Ian. Testing makes sense. I'll cook something up. And thanks for the nit-picking. I've cleaned those up, and I'll just comment on one or two here: +grow_to_fit (struct blob *blob, size_t additional) ^ No space here or other similar places. I'm happy to conform to Mesa style here, (and I've already changed the patch). But in passing, let me say that I've learned to really love having a space between a function name and its left parenthesis, (in spite of it looking totally bizarre at first). Give it a try in some small project some time and see what you think. For me, it improves readability just as much as the space we use between if and its parenthsis. + if (blob-allocated == 0) + to_allocate = BLOB_INITIAL_SIZE; Can additional ever be BLOB_INITIAL_SIZE? Good point. In the current code, no, but there's no reason to have this fragility in place. So I've now got the MAX2 in place like this: if (blob-allocated == 0) to_allocate = BLOB_INITIAL_SIZE; else to_allocate = blob-allocated * 2; to_allocate = MAX2(to_allocate, blob-allocated + additional); (That's the only real change in the functionality of the code, so I won't bother re-sending a patch with the whitespace and comment fixes) +/* When done reading, the caller can ensure that everything was consumed by + * checking the following: + * + * 1. blob-data should be equal to blob-end, (if not, too little was + * read). blob-data or blob-current? Yes, blob-current. A good fix. -Carl pgp4zu4RDVdSe.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2] glsl: Prefer unreachable(condition) over assert(!condition)
The unreachable macro has the advantage (for modern compilers) of
hinting to the compiler that this code is actually unreachable. This
allows us to drop things like return statements or assignments that
existed only to quiet compiler warnings.
Also, this version is a bit easier to type correctly and understand
when reading without that seemingly out-of-place logical negation.
---
Thanks for the review, Ian. This second revision of this patch adopts all of
your recommendations: dropping unreachable return and assignments; and putting
two of the unreachable() calls back into assert().
I didn't change any of the strings being passed to unreachable(). If someone
really wants more descriptive strings there, then someone more familiar with
the code than I am should come up with something. But the current strings are
all the same as what was there before, so that shouldn't block this patch I
think.
-Carl
src/glsl/ast_function.cpp| 5 +-
src/glsl/ast_to_hir.cpp | 8 +--
src/glsl/glcpp/glcpp-parse.y | 2 +-
src/glsl/glsl_parser_extras.cpp | 5 +-
src/glsl/glsl_symbol_table.cpp | 6 +--
src/glsl/glsl_types.cpp | 19 +++
src/glsl/ir.cpp | 66 +++-
src/glsl/ir.h| 2 +-
src/glsl/ir_clone.cpp| 2 +-
src/glsl/ir_constant_expression.cpp | 8 +--
src/glsl/ir_equals.cpp | 2 +-
src/glsl/ir_validate.cpp | 2 +-
src/glsl/ir_visitor.h| 2 +-
src/glsl/link_interface_blocks.cpp | 2 +-
src/glsl/link_uniform_block_active_visitor.cpp | 3 +-
src/glsl/link_uniform_blocks.cpp | 2 +-
src/glsl/link_uniform_initializers.cpp | 4 +-
src/glsl/link_uniforms.cpp | 2 +-
src/glsl/link_varyings.cpp | 3 +-
src/glsl/loop_analysis.cpp | 2 +-
src/glsl/loop_controls.cpp | 3 +-
src/glsl/lower_packed_varyings.cpp | 4 +-
src/glsl/lower_packing_builtins.cpp | 2 +-
src/glsl/lower_ubo_reference.cpp | 8 ++-
src/glsl/lower_variable_index_to_cond_assign.cpp | 3 +-
src/glsl/lower_vector.cpp| 2 +-
src/glsl/opt_constant_propagation.cpp| 4 +-
src/glsl/opt_minmax.cpp | 2 +-
28 files changed, 65 insertions(+), 110 deletions(-)
diff --git a/src/glsl/ast_function.cpp b/src/glsl/ast_function.cpp
index cbff9d8..8c80a0d 100644
--- a/src/glsl/ast_function.cpp
+++ b/src/glsl/ast_function.cpp
@@ -661,8 +661,7 @@ dereference_component(ir_rvalue *src, unsigned component)
return dereference_component(col, r);
}
- assert(!Should not get here.);
- return NULL;
+ unreachable(Should not get here.);
}
@@ -1016,7 +1015,7 @@ emit_inline_vector_constructor(const glsl_type *type,
data.b[i + base_component] = c-get_bool_component(i);
break;
default:
- assert(!Should not get here.);
+ unreachable(Should not get here.);
break;
}
}
diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
index 811a955..ae68142 100644
--- a/src/glsl/ast_to_hir.cpp
+++ b/src/glsl/ast_to_hir.cpp
@@ -1193,7 +1193,7 @@ ast_expression::do_hir(exec_list *instructions,
switch (this-oper) {
case ast_aggregate:
- assert(!ast_aggregate: Should never get here.);
+ unreachable(ast_aggregate: Should never get here.);
break;
case ast_assign: {
@@ -2314,7 +2314,7 @@ validate_explicit_location(const struct
ast_type_qualifier *qual,
: (qual-location + VARYING_SLOT_VAR0);
break;
case MESA_SHADER_COMPUTE:
-assert(!Unexpected shader type);
+unreachable(Unexpected shader type);
break;
}
} else {
@@ -5412,7 +5412,7 @@ ast_interface_block::hir(exec_list *instructions,
} else {
var_mode = ir_var_auto;
iface_type_name = UNKNOWN;
- assert(!interface block layout qualifier not found!);
+ unreachable(interface block layout qualifier not found!);
}
enum glsl_matrix_layout matrix_layout = GLSL_MATRIX_LAYOUT_INHERITED;
@@ -6008,7 +6008,7 @@ remove_per_vertex_blocks(exec_list *instructions,
}
break;
default:
- assert(!Unexpected mode);
+ unreachable(Unexpected mode);
break;
}
diff --git a/src/glsl/glcpp/glcpp-parse.y b/src/glsl/glcpp/glcpp-parse.y
index f1119eb..f1c006e 100644
--- a/src/glsl/glcpp/glcpp-parse.y
+++ b/src/glsl/glcpp/glcpp-parse.y
@@ -1169,7 +1169,7 @@ _token_print (char **out, size_t *len, token_t *token)
/* Nothing to print. */
[Mesa-dev] [PATCH 1/2] glsl: Add blob.c---a simple interface for serializing data
This new interface allows for writing a series of objects to a chunk
of memory (a blob).. The allocated memory is maintained within the
blob itself, (and re-allocated by doubling when necessary).
There are also functions for reading objects from a blob as well. If
code attempts to read beyond the available memory, the read functions
return 0 values (or its moral equivalent) without reading past the
allocated memory. Once the caller is done with the reads, it can check
blob-overrun to ensure whether any invalid values were previously
returned due to attempts to read too far.
---
src/glsl/Makefile.sources | 3 +-
src/glsl/blob.c | 287 ++
src/glsl/blob.h | 263 ++
3 files changed, 552 insertions(+), 1 deletion(-)
create mode 100644 src/glsl/blob.c
create mode 100644 src/glsl/blob.h
diff --git a/src/glsl/Makefile.sources b/src/glsl/Makefile.sources
index 676fa0d..875e4bf 100644
--- a/src/glsl/Makefile.sources
+++ b/src/glsl/Makefile.sources
@@ -105,7 +105,8 @@ LIBGLSL_FILES = \
$(GLSL_SRCDIR)/opt_swizzle_swizzle.cpp \
$(GLSL_SRCDIR)/opt_tree_grafting.cpp \
$(GLSL_SRCDIR)/opt_vectorize.cpp \
- $(GLSL_SRCDIR)/s_expression.cpp
+ $(GLSL_SRCDIR)/s_expression.cpp \
+ $(GLSL_SRCDIR)/blob.c
# glsl_compiler
diff --git a/src/glsl/blob.c b/src/glsl/blob.c
new file mode 100644
index 000..0af71fc
--- /dev/null
+++ b/src/glsl/blob.c
@@ -0,0 +1,287 @@
+/*
+ * Copyright © 2014 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the Software),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+#include string.h
+
+#include main/macros.h
+#include util/ralloc.h
+#include blob.h
+
+#define BLOB_INITIAL_SIZE 4096
+
+/* Ensure that \blob will be able to fit an additional object of size
+ * \additional. The growing (if any) will occur by doubling the existing
+ * allocation.
+ */
+static bool
+grow_to_fit (struct blob *blob, size_t additional)
+{
+ size_t to_allocate;
+ uint8_t *new_data;
+
+ if (blob-size + additional = blob-allocated)
+ return true;
+
+ if (blob-allocated == 0)
+ to_allocate = BLOB_INITIAL_SIZE;
+ else
+ to_allocate = blob-allocated * 2;
+
+ new_data = reralloc_size(blob, blob-data, to_allocate);
+ if (new_data == NULL)
+ return false;
+
+ blob-data = new_data;
+ blob-allocated = to_allocate;
+
+ return true;
+}
+
+/* Align the blob-size so that reading or writing a value at (blob-data +
+ * blob-size) will result in an access aligned to a granularity of \alignment
+ * bytes.
+ *
+ * \return True unless allocation fails
+ */
+static bool
+align_blob (struct blob *blob, size_t alignment)
+{
+ size_t new_size;
+
+ new_size = ALIGN(blob-size, alignment);
+
+ if (! grow_to_fit (blob, new_size - blob-size))
+ return false;
+
+ blob-size = new_size;
+
+ return true;
+}
+
+static void
+align_blob_reader (struct blob_reader *blob, size_t alignment)
+{
+ blob-current = blob-data + ALIGN(blob-current - blob-data, alignment);
+}
+
+struct blob *
+blob_create (void *mem_ctx)
+{
+ struct blob *blob;
+
+ blob = ralloc(mem_ctx, struct blob);
+ if (blob == NULL)
+ return NULL;
+
+ blob-data = NULL;
+ blob-allocated = 0;
+ blob-size = 0;
+
+ return blob;
+}
+
+bool
+blob_write_bytes (struct blob *blob, const void *bytes, size_t to_write)
+{
+ if (! grow_to_fit (blob, to_write))
+ return false;
+
+ memcpy (blob-data + blob-size, bytes, to_write);
+ blob-size += to_write;
+
+ return true;
+}
+
+uint8_t *
+blob_reserve_bytes (struct blob *blob, size_t to_write)
+{
+ uint8_t *ret;
+
+ if (! grow_to_fit (blob, to_write))
+ return NULL;
+
+ ret = blob-data + blob-size;
+ blob-size += to_write;
+
+ return ret;
+}
+
+bool
+blob_write_uint32 (struct blob *blob, uint32_t value)
+{
+ align_blob(blob, sizeof(value));
+
+ return blob_write_bytes(blob, value,
[Mesa-dev] glsl: New blob.c (in preparation for shader cache)
Here are two patches that add some functions for serializing and deserializing simple data structures, (integers of various sizes, pointers, and strings). This code will be used by my upcoming shader-cache work. I'm submitting it here now because it is self-contained and documented, so it should be ready to be reviewed now. But it probably makes sense to wait to push this until I actually have some reviewed code that uses it. I want to thank Tapani for some similar code he wrote for his IR serialization work. As attributed in the second patch below, I shared some ideas directly from one of his patches. -Carl ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/2] glsl: Add blob_overwrite_bytes and blob_overwrite_uint32
From: Tapani Pälli tapani.pa...@intel.com
These functions are useful when serializing an unknown number of items
to a blob. The caller can first save the current offset, write a
placeholder uint32, write out (and count) the items, then use
blob_overwrite_uint32 with the saved offset to replace the placeholder
value.
Then, when deserializing, the reader will first read the count and
know how many subsequent items to expect.
(I wrote this code after reading a very similar patch written by
Tapani when he wrote serialization code for IR. Since I re-used the
idea of his code so directly, I've credited him as the author of this
code. --Carl)
---
src/glsl/blob.c | 23 +++
src/glsl/blob.h | 25 +
2 files changed, 48 insertions(+)
diff --git a/src/glsl/blob.c b/src/glsl/blob.c
index 0af71fc..dbd698a 100644
--- a/src/glsl/blob.c
+++ b/src/glsl/blob.c
@@ -101,6 +101,21 @@ blob_create (void *mem_ctx)
}
bool
+blob_overwrite_bytes (struct blob *blob,
+ size_t offset,
+ const void *bytes,
+ size_t to_write)
+{
+ /* Detect an attempt to overwrite data out of bounds. */
+ if (offset 0 || blob-size - offset to_write)
+ return false;
+
+ memcpy (blob-data + offset, bytes, to_write);
+
+ return true;
+}
+
+bool
blob_write_bytes (struct blob *blob, const void *bytes, size_t to_write)
{
if (! grow_to_fit (blob, to_write))
@@ -135,6 +150,14 @@ blob_write_uint32 (struct blob *blob, uint32_t value)
}
bool
+blob_overwrite_uint32 (struct blob *blob,
+ size_t offset,
+ uint32_t value)
+{
+ return blob_overwrite_bytes(blob, offset, value, sizeof(value));
+}
+
+bool
blob_write_uint64 (struct blob *blob, uint64_t value)
{
align_blob(blob, sizeof(value));
diff --git a/src/glsl/blob.h b/src/glsl/blob.h
index 374b21e..165de13 100644
--- a/src/glsl/blob.h
+++ b/src/glsl/blob.h
@@ -138,6 +138,31 @@ bool
blob_write_uint32 (struct blob *blob, uint32_t value);
/**
+ * Overwrite a uint32_t previously written to the blob.
+ *
+ * Writes a uint32_t value to an existing portion of the blob at an offset of
+ * \offset. This data range must have previously been written to the blob by
+ * one of the blob_write_* calls.
+ *
+ *
+ * The expected usage is something like the following pattern:
+ *
+ * size_t offset;
+ *
+ * offset = blob-size;
+ * blob_write_uint32 (blob, 0); // placeholder
+ * ... various blob write calls, writing N items ...
+ * blob_overwrite_uint32 (blob, offset, N);
+ *
+ * \return True unless the requested position or position+to_write lie outside
+ * the current blob's size.
+ */
+bool
+blob_overwrite_uint32 (struct blob *blob,
+ size_t offset,
+ uint32_t value);
+
+/**
* Add a uint64_t to a blob.
*
* Note: This function will only write to a uint64_t-aligned offset from the
--
2.1.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] mesa: Add mesa SHA-1 functions
From: Kristian Høgsberg k...@bitplanet.net
The upcoming shader cache uses the SHA-1 algorithm for cryptographic
naming. These new mesa_sha1 functions are implemented with the nettle
library.
---
This patch is another in support of my upcoming shader-cache work. Thanks to
Kritian for coding this piece.
As currently written, this patch introduces a new dependency of Mesa on the
Nettle library to implement SHA-1. I'm open to recommendations if people would
prefer some other option.
For example, the xserver can be configured to get a SHA-1 implementation from
libmd, libc, CommonCrypto, CryptoAPI, libnettle, libgcrypt, libsha1, or
openssl.
I don't know if it's important to offer as many options as that, which is why
I'm asking for opinions here.
Thanks,
-Carl
configure.ac | 7
src/util/Makefile.am | 2 ++
src/util/Makefile.sources | 2 ++
src/util/sha1.c | 81 +++
src/util/sha1.h | 22 +
5 files changed, 114 insertions(+)
create mode 100644 src/util/sha1.c
create mode 100644 src/util/sha1.h
diff --git a/configure.ac b/configure.ac
index 1d9d015..bbefd21 100644
--- a/configure.ac
+++ b/configure.ac
@@ -876,6 +876,13 @@ fi
AC_SUBST([MESA_LLVM])
+# Require libnettle for SHA-1 calculations
+PKG_CHECK_MODULES([NETTLE], [nettle],
+ [have_nettle=yes], [have_nettle=no])
+if test x$have_nettle = xno; then
+ AC_MSG_ERROR([Cannot find required nettle library (or nettle-dev package)])
+fi
+
# Check for libdrm
PKG_CHECK_MODULES([LIBDRM], [libdrm = $LIBDRM_REQUIRED],
[have_libdrm=yes], [have_libdrm=no])
diff --git a/src/util/Makefile.am b/src/util/Makefile.am
index 8d5f90e..30899de 100644
--- a/src/util/Makefile.am
+++ b/src/util/Makefile.am
@@ -37,6 +37,8 @@ libmesautil_la_SOURCES = \
$(MESA_UTIL_FILES) \
$(MESA_UTIL_GENERATED_FILES)
+libmesautil_la_LIBADD = -lnettle
+
BUILT_SOURCES = $(MESA_UTIL_GENERATED_FILES)
CLEANFILES = $(BUILT_SOURCES)
diff --git a/src/util/Makefile.sources b/src/util/Makefile.sources
index 9e27424..22c6a22 100644
--- a/src/util/Makefile.sources
+++ b/src/util/Makefile.sources
@@ -4,6 +4,8 @@ MESA_UTIL_FILES := \
register_allocate.c \
register_allocate.h \
rgtc.c \
+ sha1.c \
+ sha1.h \
strtod.cpp
MESA_UTIL_GENERATED_FILES = \
diff --git a/src/util/sha1.c b/src/util/sha1.c
new file mode 100644
index 000..3025df6
--- /dev/null
+++ b/src/util/sha1.c
@@ -0,0 +1,81 @@
+/*
+ * Copyright © 2014 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the Software),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ */
+
+#include stdlib.h
+#include nettle/sha.h
+
+#include sha1.h
+
+struct mesa_sha1 *
+_mesa_sha1_init(void)
+{
+struct sha1_ctx *ctx = malloc(sizeof(*ctx));
+
+if (!ctx)
+return NULL;
+sha1_init(ctx);
+
+return (struct mesa_sha1 *) ctx;
+}
+
+int
+_mesa_sha1_update(struct mesa_sha1 *ctx, const void *data, int size)
+{
+ sha1_update((struct sha1_ctx *) ctx, size, data);
+
+ return 1;
+}
+
+int
+_mesa_sha1_final(struct mesa_sha1 *ctx, unsigned char result[20])
+{
+sha1_digest((struct sha1_ctx *) ctx, 20, result);
+free(ctx);
+
+return 1;
+}
+
+void
+_mesa_sha1_compute(const void *data, size_t size, unsigned char result[20])
+{
+ struct sha1_ctx ctx;
+
+ sha1_init(ctx);
+ sha1_update(ctx, size, data);
+ sha1_digest(ctx, 20, result);
+}
+
+char *
+_mesa_sha1_format(char *buf, const unsigned char *sha1)
+{
+ static const char hex_digits[] = 0123456789abcdef;
+ int i;
+
+ for (i = 0; i 40; i += 2) {
+ buf[i] = hex_digits[sha1[i 1] 4];
+ buf[i + 1] = hex_digits[sha1[i 1] 0x0f];
+ }
+ buf[i] = '\0';
+
+ return buf;
+}
diff --git a/src/util/sha1.h b/src/util/sha1.h
new file mode 100644
index 000..1616134
--- /dev/null
+++ b/src/util/sha1.h
@@ -0,0 +1,22 @@
+#ifndef SHA1_H
+#define SHA1_H
Re: [Mesa-dev] [PATCH] mesa: Add mesa SHA-1 functions
On Thu, Dec 11 2014, Brian Paul wrote: We'll need a solution for Windows too. I don't have time right now to do any research into that. The code from xserver seems to cover that. I'll follow up with a (largely untested) patch that I ported over from xserver. If people can give some high-level feedback on that, that will be great. I'll then actually test all the parts I can on my Linux box, (but won't be able to test the Windows pieces directly). -Carl pgpQ4HeTmTYWy.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/2] mesa: Add mesa SHA-1 functions
; then
+ AC_MSG_ERROR([OpenSSL libcrypto requested but not found])
+ fi
+fi
+if test x$with_sha1 = xlibcrypto; then
+ if test x$HAVE_LIBCRYPTO = xyes; then
+ SHA1_LIBS=-lcrypto
+ else
+ SHA1_LIBS=$OPENSSL_LIBS
+ SHA1_CFLAGS=$OPENSSL_CFLAGS
+ fi
+fi
+AC_MSG_CHECKING([for SHA1 implementation])
+if test x$with_sha1 = x; then
+ AC_MSG_ERROR([No suitable SHA1 implementation found])
+fi
+AC_MSG_RESULT([$with_sha1])
+AC_SUBST(SHA1_LIBS)
+AC_SUBST(SHA1_CFLAGS)
+
# Check for libdrm
PKG_CHECK_MODULES([LIBDRM], [libdrm = $LIBDRM_REQUIRED],
[have_libdrm=yes], [have_libdrm=no])
diff --git a/src/util/Makefile.am b/src/util/Makefile.am
index 8d5f90e..ee3c57f 100644
--- a/src/util/Makefile.am
+++ b/src/util/Makefile.am
@@ -31,12 +31,15 @@ libmesautil_la_CPPFLAGS = \
-I$(top_srcdir)/src \
-I$(top_srcdir)/src/mapi \
-I$(top_srcdir)/src/mesa \
+ $(SHA1_CFLAGS) \
$(VISIBILITY_CFLAGS)
libmesautil_la_SOURCES = \
$(MESA_UTIL_FILES) \
$(MESA_UTIL_GENERATED_FILES)
+libmesautil_la_LIBADD = $(SHA1_LIBS)
+
BUILT_SOURCES = $(MESA_UTIL_GENERATED_FILES)
CLEANFILES = $(BUILT_SOURCES)
diff --git a/src/util/Makefile.sources b/src/util/Makefile.sources
index 9e27424..22c6a22 100644
--- a/src/util/Makefile.sources
+++ b/src/util/Makefile.sources
@@ -4,6 +4,8 @@ MESA_UTIL_FILES := \
register_allocate.c \
register_allocate.h \
rgtc.c \
+ sha1.c \
+ sha1.h \
strtod.cpp
MESA_UTIL_GENERATED_FILES = \
diff --git a/src/util/sha1.c b/src/util/sha1.c
new file mode 100644
index 000..d6c2c98
--- /dev/null
+++ b/src/util/sha1.c
@@ -0,0 +1,291 @@
+/* Copyright © 2007 Carl Worth
+ * Copyright © 2009 Jeremy Huddleston, Julien Cristau, and Matthieu Herrb
+ * Copyright © 2009-2010 Mikhail Gusarov
+ * Copyright © 2012 Yaakov Selkowitz and Keith Packard
+ * Copyright © 2014 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the Software),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ */
+
+#include sha1.h
+
+#if defined(HAVE_SHA1_IN_LIBMD) /* Use libmd for SHA1 */ \
+ || defined(HAVE_SHA1_IN_LIBC) /* Use libc for SHA1 */
+
+#include sha1.h
+
+struct mesa_sha1 *
+_mesa_sha1_init(void)
+{
+ SHA1_CTX *ctx = malloc(sizeof(*ctx));
+
+ if (!ctx)
+ return NULL;
+
+ SHA1Init(ctx);
+ return (struct mesa_sha1 *) ctx;
+}
+
+int
+_mesa_sha1_update(struct mesa_sha1 *ctx, const void *data, int size)
+{
+ SHA1_CTX *sha1_ctx = (SHA1_CTX *) ctx;
+
+ SHA1Update(sha1_ctx, data, size);
+ return 1;
+}
+
+int
+_mesa_sha1_final(struct mesa_sha1 *ctx, unsigned char result[20])
+{
+ SHA1_CTX *sha1_ctx = (SHA1_CTX *) ctx;
+
+ SHA1Final(result, sha1_ctx);
+ free(sha1_ctx);
+ return 1;
+}
+
+#elif defined(HAVE_SHA1_IN_COMMONCRYPTO)/* Use CommonCrypto for SHA1 */
+
+#include CommonCrypto/CommonDigest.h
+
+struct mesa_sha1 *
+_mesa_sha1_init(void)
+{
+ CC_SHA1_CTX *ctx = malloc(sizeof(*ctx));
+
+ if (!ctx)
+ return NULL;
+
+ CC_SHA1_Init(ctx);
+ return (struct mesa_sha1 *) ctx;
+}
+
+int
+_mesa_sha1_update(struct mesa_sha1 *ctx, const void *data, int size)
+{
+ CC_SHA1_CTX *sha1_ctx = (CC_SHA1_CTX *) ctx;
+
+ CC_SHA1_Update(sha1_ctx, data, size);
+ return 1;
+}
+
+int
+_mesa_sha1_final(struct mesa_sha1 *ctx, unsigned char result[20])
+{
+ CC_SHA1_CTX *sha1_ctx = (CC_SHA1_CTX *) ctx;
+
+ CC_SHA1_Final(result, sha1_ctx);
+ free(sha1_ctx);
+ return 1;
+}
+
+#elif defined(HAVE_SHA1_IN_CRYPTOAPI)/* Use CryptoAPI for SHA1 */
+
+#define WIN32_LEAN_AND_MEAN
+#include X11/Xwindows.h
+#include wincrypt.h
+
+static HCRYPTPROV hProv;
+
+struct mesa_sha1 *
+_mesa_sha1_init(void)
+{
+ HCRYPTHASH *ctx = malloc(sizeof(*ctx));
+
+ if (!ctx)
+ return NULL;
+
+ CryptAcquireContext(hProv, NULL, MS_DEF_PROV, PROV_RSA_FULL,
CRYPT_VERIFYCONTEXT);
+ CryptCreateHash(hProv, CALG_SHA1, 0, 0, ctx);
+ return
[Mesa-dev] [PATCH 1/2] configure: Add copyright and license block to configure.ac
Prior to copying in code from the xserver configure.ac file, it makes sense to have the license of this file clearly marked, (to show that it's licensed identically to the configure.ac file from the xserver repository). And since the text of the license refers to the above copyright notice it also makes sense to have an actual copyright attribution in place. I generated this list of names by looking at the output of: git shortlog -n --format=%aD -- configure.ac (and arbitrarily stopping for contributors with fewer than 15 commits). Then for each name, I looked for existing Copyright attributions in the mesa source tree with the same name, (and using Intel Corporation as the copyright holder where I knew that was appropriate). --- configure.ac | 31 +++ 1 file changed, 31 insertions(+) diff --git a/configure.ac b/configure.ac index 1d9d015..a11190c 100644 --- a/configure.ac +++ b/configure.ac @@ -1,3 +1,34 @@ +dnl Copyright © 2011-2014 Intel Corporation +dnl Copyright © 2011-2014 Emil Velikov emil.l.veli...@gmail.com +dnl Copyright © 2007-2010 Dan Nicholson +dnl Copyright © 2010-2014 Marek Olšák mar...@gmail.com +dnl Copyright © 2010-2014 Christian König +dnl Copyright © 2012-2014 Tom Stellard tstel...@gmail.com +dnl Copyright © 2009-2012 Jakob Bornecrantz +dnl Copyright © 2009-2014 Jon TURNEY +dnl Copyright © 2011-2012 Benjamin Franzke +dnl Copyright © 2008-2014 David Airlie +dnl Copyright © 2009-2013 Brian Paul +dnl +dnl Permission is hereby granted, free of charge, to any person obtaining a +dnl copy of this software and associated documentation files (the Software), +dnl to deal in the Software without restriction, including without limitation +dnl the rights to use, copy, modify, merge, publish, distribute, sublicense, +dnl and/or sell copies of the Software, and to permit persons to whom the +dnl Software is furnished to do so, subject to the following conditions: +dnl +dnl The above copyright notice and this permission notice (including the next +dnl paragraph) shall be included in all copies or substantial portions of the +dnl Software. +dnl +dnl THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +dnl IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +dnl FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL +dnl THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +dnl LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING +dnl FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +dnl DEALINGS IN THE SOFTWARE. +dnl dnl Process this file with autoconf to create configure. AC_PREREQ([2.60]) -- 2.1.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/2] util: Make unreachable at least be an assert
Previously, if __builtin_unreachable() was unavailable, the
unreachable macro was defined to do nothing. We do better here, by at
least still making it an assert.
---
src/util/macros.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/util/macros.h b/src/util/macros.h
index 5fc6729..eec8b93 100644
--- a/src/util/macros.h
+++ b/src/util/macros.h
@@ -82,7 +82,7 @@ do {\
#endif
#ifndef unreachable
-#define unreachable(str)
+#define unreachable(str) assert(!str)
#endif
/**
--
2.1.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/2] glsl: Prefer unreachable(condition) over assert(!condition)
The unreachable macro has the advantage (for modern compilers) of
hinting to the compiler that this code is actually unreachable, which
can help reduce spurious warnings, etc.
Also, this version is a bit easier to type correctly and understand
when reading without that seemingly out-of-place logical negation.
These were all found with the following:
git grep 'assert(! *' -- src/glsl
and then replaced automatically.
---
I did this only for for the glsl directory for now. There are still many more
of these throughout mesa's source tree, but it felt like it would be far too
invasive to make a change like this globally.
So most of the changes here are in the glsl_types.cpp file where I was already
working, and then there are just small numbers in many of the files in the
same directory.
src/glsl/ast_function.cpp| 4 +--
src/glsl/ast_to_hir.cpp | 8 ++---
src/glsl/glcpp/glcpp-parse.y | 2 +-
src/glsl/glsl_parser_extras.cpp | 4 +--
src/glsl/glsl_symbol_table.cpp | 4 +--
src/glsl/glsl_types.cpp | 14 -
src/glsl/ir.cpp | 38
src/glsl/ir.h| 2 +-
src/glsl/ir_clone.cpp| 2 +-
src/glsl/ir_constant_expression.cpp | 8 ++---
src/glsl/ir_equals.cpp | 2 +-
src/glsl/ir_set_program_inouts.cpp | 2 +-
src/glsl/ir_validate.cpp | 2 +-
src/glsl/ir_visitor.h| 2 +-
src/glsl/link_interface_blocks.cpp | 2 +-
src/glsl/link_uniform_block_active_visitor.cpp | 2 +-
src/glsl/link_uniform_blocks.cpp | 2 +-
src/glsl/link_uniform_initializers.cpp | 4 +--
src/glsl/link_uniforms.cpp | 2 +-
src/glsl/link_varyings.cpp | 4 +--
src/glsl/loop_analysis.cpp | 2 +-
src/glsl/loop_controls.cpp | 2 +-
src/glsl/lower_packed_varyings.cpp | 4 +--
src/glsl/lower_packing_builtins.cpp | 2 +-
src/glsl/lower_ubo_reference.cpp | 6 ++--
src/glsl/lower_variable_index_to_cond_assign.cpp | 2 +-
src/glsl/lower_vector.cpp| 2 +-
src/glsl/opt_constant_propagation.cpp| 4 +--
src/glsl/opt_minmax.cpp | 2 +-
29 files changed, 68 insertions(+), 68 deletions(-)
diff --git a/src/glsl/ast_function.cpp b/src/glsl/ast_function.cpp
index cbff9d8..60a5414 100644
--- a/src/glsl/ast_function.cpp
+++ b/src/glsl/ast_function.cpp
@@ -661,7 +661,7 @@ dereference_component(ir_rvalue *src, unsigned component)
return dereference_component(col, r);
}
- assert(!Should not get here.);
+ unreachable(Should not get here.);
return NULL;
}
@@ -1016,7 +1016,7 @@ emit_inline_vector_constructor(const glsl_type *type,
data.b[i + base_component] = c-get_bool_component(i);
break;
default:
- assert(!Should not get here.);
+ unreachable(Should not get here.);
break;
}
}
diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
index 811a955..ae68142 100644
--- a/src/glsl/ast_to_hir.cpp
+++ b/src/glsl/ast_to_hir.cpp
@@ -1193,7 +1193,7 @@ ast_expression::do_hir(exec_list *instructions,
switch (this-oper) {
case ast_aggregate:
- assert(!ast_aggregate: Should never get here.);
+ unreachable(ast_aggregate: Should never get here.);
break;
case ast_assign: {
@@ -2314,7 +2314,7 @@ validate_explicit_location(const struct
ast_type_qualifier *qual,
: (qual-location + VARYING_SLOT_VAR0);
break;
case MESA_SHADER_COMPUTE:
-assert(!Unexpected shader type);
+unreachable(Unexpected shader type);
break;
}
} else {
@@ -5412,7 +5412,7 @@ ast_interface_block::hir(exec_list *instructions,
} else {
var_mode = ir_var_auto;
iface_type_name = UNKNOWN;
- assert(!interface block layout qualifier not found!);
+ unreachable(interface block layout qualifier not found!);
}
enum glsl_matrix_layout matrix_layout = GLSL_MATRIX_LAYOUT_INHERITED;
@@ -6008,7 +6008,7 @@ remove_per_vertex_blocks(exec_list *instructions,
}
break;
default:
- assert(!Unexpected mode);
+ unreachable(Unexpected mode);
break;
}
diff --git a/src/glsl/glcpp/glcpp-parse.y b/src/glsl/glcpp/glcpp-parse.y
index f1119eb..f1c006e 100644
--- a/src/glsl/glcpp/glcpp-parse.y
+++ b/src/glsl/glcpp/glcpp-parse.y
@@ -1169,7 +1169,7 @@ _token_print (char **out, size_t *len, token_t *token)
/* Nothing to print. */
break;
default:
-
[Mesa-dev] [PATCH 1/3] glsl: Add convenience function get_sampler_instance
This is similar to the existing functions get_instance, get_array_instance,
etc. for getting a type singleton. The new get_sampler_instance() function
will be used by the upcoming shader cache.
---
src/glsl/glsl_types.cpp | 111
src/glsl/glsl_types.h | 9
2 files changed, 120 insertions(+)
diff --git a/src/glsl/glsl_types.cpp b/src/glsl/glsl_types.cpp
index 5f99193..7b28fe4 100644
--- a/src/glsl/glsl_types.cpp
+++ b/src/glsl/glsl_types.cpp
@@ -475,6 +475,117 @@ glsl_type::get_instance(unsigned base_type, unsigned
rows, unsigned columns)
return error_type;
}
+const glsl_type *
+glsl_type::get_sampler_instance(enum glsl_sampler_dim dim,
+bool shadow,
+bool array,
+unsigned type)
+{
+ switch (type) {
+ case GLSL_TYPE_FLOAT:
+ switch (dim) {
+ case GLSL_SAMPLER_DIM_1D:
+ if (shadow)
+return (array ? sampler1DArrayShadow_type : sampler1DShadow_type);
+ else
+return (array ? sampler1DArray_type : sampler1D_type);
+ case GLSL_SAMPLER_DIM_2D:
+ if (shadow)
+return (array ? sampler2DArrayShadow_type : sampler2DShadow_type
+);
+ else
+return (array ? sampler2DArray_type : sampler2D_type);
+ case GLSL_SAMPLER_DIM_3D:
+ if (shadow || array)
+return error_type;
+ else
+return sampler3D_type;
+ case GLSL_SAMPLER_DIM_CUBE:
+ if (shadow)
+return (array ? samplerCubeArrayShadow_type :
samplerCubeShadow_type);
+ else
+return (array ? samplerCubeArray_type : samplerCube_type);
+ case GLSL_SAMPLER_DIM_RECT:
+ if (array)
+return error_type;
+ if (shadow)
+return sampler2DRectShadow_type;
+ else
+return sampler2DRect_type;
+ case GLSL_SAMPLER_DIM_BUF:
+ if (shadow || array)
+return error_type;
+ else
+return samplerBuffer_type;
+ case GLSL_SAMPLER_DIM_MS:
+ if (shadow)
+return error_type;
+ return (array ? sampler2DMSArray_type : sampler2DMS_type);
+ case GLSL_SAMPLER_DIM_EXTERNAL:
+ if (shadow || array)
+return error_type;
+ else
+return samplerExternalOES_type;
+ }
+ case GLSL_TYPE_INT:
+ if (shadow)
+ return error_type;
+ switch (dim) {
+ case GLSL_SAMPLER_DIM_1D:
+ return (array ? isampler1DArray_type : isampler1D_type);
+ case GLSL_SAMPLER_DIM_2D:
+ return (array ? isampler2DArray_type : isampler2D_type);
+ case GLSL_SAMPLER_DIM_3D:
+ if (array)
+return error_type;
+ return isampler3D_type;
+ case GLSL_SAMPLER_DIM_CUBE:
+ return (array ? isamplerCubeArray_type : isamplerCube_type);
+ case GLSL_SAMPLER_DIM_RECT:
+ if (array)
+return error_type;
+ return isampler2DRect_type;
+ case GLSL_SAMPLER_DIM_BUF:
+ if (array)
+return error_type;
+ return isamplerBuffer_type;
+ case GLSL_SAMPLER_DIM_MS:
+ return (array ? isampler2DMSArray_type : isampler2DMS_type);
+ case GLSL_SAMPLER_DIM_EXTERNAL:
+ return error_type;
+ }
+ case GLSL_TYPE_UINT:
+ if (shadow)
+ return error_type;
+ switch (dim) {
+ case GLSL_SAMPLER_DIM_1D:
+ return (array ? usampler1DArray_type : usampler1D_type);
+ case GLSL_SAMPLER_DIM_2D:
+ return (array ? usampler2DArray_type : usampler2D_type);
+ case GLSL_SAMPLER_DIM_3D:
+ if (array)
+return error_type;
+ return usampler3D_type;
+ case GLSL_SAMPLER_DIM_CUBE:
+ return (array ? usamplerCubeArray_type : usamplerCube_type);
+ case GLSL_SAMPLER_DIM_RECT:
+ if (array)
+return error_type;
+ return usampler2DRect_type;
+ case GLSL_SAMPLER_DIM_BUF:
+ if (array)
+return error_type;
+ return usamplerBuffer_type;
+ case GLSL_SAMPLER_DIM_MS:
+ return (array ? usampler2DMSArray_type : usampler2DMS_type);
+ case GLSL_SAMPLER_DIM_EXTERNAL:
+ return error_type;
+ }
+ }
+
+ assert(!Should not get here.);
+ return error_type;
+}
const glsl_type *
glsl_type::get_array_instance(const glsl_type *base, unsigned array_size)
diff --git a/src/glsl/glsl_types.h b/src/glsl/glsl_types.h
index 474b129..d7f740c 100644
--- a/src/glsl/glsl_types.h
+++ b/src/glsl/glsl_types.h
@@ -244,6 +244,15 @@ struct glsl_type {
unsigned columns);
/**
+* Get the instance of a sampler type
+*/
+ static const glsl_type *get_sampler_instance(enum glsl_sampler_dim dim,
+bool shadow,
+bool array,
[Mesa-dev] [PATCH 2/3] mesa: iterate method for string_to_uint_map
From: Tapani Pälli tapani.pa...@intel.com
Shader binary cache requires this to be able to cache hash data from
the gl_shader_program structure.
Signed-off-by: Tapani Pälli tapani.pa...@intel.com
Reviewed-by: Paul Berry stereotype...@gmail.com
Reviewed-by: Carl Worth cwo...@cworth.org
---
src/mesa/program/hash_table.h | 8
1 file changed, 8 insertions(+)
diff --git a/src/mesa/program/hash_table.h b/src/mesa/program/hash_table.h
index e95fc49..ece43a1 100644
--- a/src/mesa/program/hash_table.h
+++ b/src/mesa/program/hash_table.h
@@ -229,6 +229,14 @@ public:
}
/**
+* Runs a passed callback for the hash
+*/
+ void iterate(void (*func)(const void *, void *, void *), void *closure)
+ {
+ hash_table_call_foreach(this-ht, func, closure);
+ }
+
+ /**
* Get the value associated with a particular key
*
* \return
--
2.1.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 3/3] mesa: Fix string_to_uint_map-iterate to return same numbers from put()
There is an internal implementation detail that the hash table
underlying the struct string_to_uint_map stores each value internally
as (value+1). The user needn't be very concerned with this (other than
knowing that a value of UINT_MAX cannot be stored) since put() adds 1
and get() subtracts 1.
However, the recently added iterate() method was written to directly
leak the internal, off-by-one values, which could be quite
confusing. Fix this with a little wrapper around the user's callback
function to perform the subtraction.
And now that we have a wrapper in place, we give a better signature to
the callback function being passed to iterate(), so that this callback
function can actually expect a char* and an unsigned argument, (rather
than a couple of void* ). So the iterate() method should be much more
convenient to use now.
---
src/mesa/program/hash_table.h | 29 +++--
1 file changed, 27 insertions(+), 2 deletions(-)
diff --git a/src/mesa/program/hash_table.h b/src/mesa/program/hash_table.h
index ece43a1..c7089b2 100644
--- a/src/mesa/program/hash_table.h
+++ b/src/mesa/program/hash_table.h
@@ -198,6 +198,11 @@ string_to_uint_map_dtor(struct string_to_uint_map *);
#ifdef __cplusplus
}
+struct string_map_iterate_wrapper_closure {
+ void (*callback)(const char *key, unsigned value, void *closure);
+ void *closure;
+};
+
/**
* Map from a string (name) to an unsigned integer value
*
@@ -231,9 +236,19 @@ public:
/**
* Runs a passed callback for the hash
*/
- void iterate(void (*func)(const void *, void *, void *), void *closure)
+ void iterate(void (*func)(const char *, unsigned, void *), void *closure)
{
- hash_table_call_foreach(this-ht, func, closure);
+ struct string_map_iterate_wrapper_closure *wrapper;
+
+ wrapper = (struct string_map_iterate_wrapper_closure *)
+ malloc(sizeof(struct string_map_iterate_wrapper_closure));
+ if (wrapper == NULL)
+ return;
+
+ wrapper-callback = func;
+ wrapper-closure = closure;
+
+ hash_table_call_foreach(this-ht, subtract_one_wrapper, wrapper);
}
/**
@@ -289,6 +304,16 @@ private:
free((char *)key);
}
+ static void subtract_one_wrapper(const void *key, void *data, void *closure)
+ {
+ struct string_map_iterate_wrapper_closure *wrapper =
+ (struct string_map_iterate_wrapper_closure *) closure;
+ unsigned value = (intptr_t) data;
+
+ value -= 1;
+
+ wrapper-callback((const char *) key, value, wrapper-closure);
+ }
struct hash_table *ht;
};
--
2.1.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/3] mesa: Fix string_to_uint_map-iterate to return same numbers from put()
On Thu, Dec 04 2014, Ian Romanick wrote: Should this just get squashed with the previous commit? Yes. I only hesitated because I wrote the second and not the first, (so I didn't want to lose authorship in the history). But it's not useful to have an iterate in the history that actually does the wrong thing. Either way, this patch and the previous patch are Reviewed-by: Ian Romanick ian.d.roman...@intel.com Thanks. One tiny nit below. ... Blank line here. Fixed. -Carl -- carl.d.wo...@intel.com pgpV1fk2C3Gsp.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/3] glsl: Add convenience function get_sampler_instance
On Thu, Dec 04 2014, Ian Romanick wrote: Shouldn't type be glsl_base_type instead of unsigned? Yes, thanks for the catch. Fixed. + if (shadow) +return (array ? sampler2DArrayShadow_type : sampler2DShadow_type +); It's weird that the closing paren ended up here. I don't think the parens are necessary, so maybe drop them? This was just a stray newline, not wrapping or anything. Thanks for catching it. I agree that the parentheses aren't required, but they are idiomatic with all of the surrounding code. So I'll leave them for now. -Carl pgpDZKI2unkAL.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/3] glsl: Add convenience function get_sampler_instance
On Thu, Dec 04 2014, Matt Turner wrote: On Thu, Dec 4, 2014 at 2:00 PM, Carl Worth cwo...@cworth.org wrote: + assert(!Should not get here.); + return error_type; Let's just use unreachable(not reached) here. That's a good idea. I copied this line from the surrounding code in the same file, (there are about 6 or 7 other instances in this file). So I think I'll follow up with a separate patch to clean these up. Should I pass over the whole source tree? Or would a change like that be too annoying for outstanding patches, etc.? -Carl -- carl.d.wo...@intel.com pgpowNyfgQzJM.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Please clean your old patches out of Patchwork
Emil Velikov emil.l.veli...@gmail.com writes: Would appreciate admin access to clear up some of the patches that I've committed on behalf of other people :) I just went to add this, but it looks like someone beat me to it already. Have fun! -Carl -- carl.d.wo...@intel.com pgp0S6c9mNLZn.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [Mesa-stable] [PATCH 1/3 v2] mesa: Handle uninitialized textures like other textures in get_tex_level_parameter_image
Emil Velikov emil.l.veli...@gmail.com writes: There is a recent bug in piglit (+ it's fix [1]) that causes the env vars used not to be printed in the HTML test summary. That would be a good bug fix. But the problem I had was that piglit wasn't running this test at all, (so I wasn't even getting any HTML test summary). The command line I came up with for the test was just from reading gleantest.py. I'll go try to figure out what's broken about my piglit. -Carl pgplRm8xD67SS.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [Mesa-stable] [PATCH 1/3 v2] mesa: Handle uninitialized textures like other textures in get_tex_level_parameter_image
Carl Worth cwo...@cworth.org writes: Emil Velikov emil.l.veli...@gmail.com writes: I will not have the chance to run piglit on a i965 system until next week so I would love if someone can take a look at this. I should be able to take a look at this today or tomorrow. I didn't see any failures on i965. Oddly, piglit doesn't seem to be running this test for me, (maybe I'm missing all glean testing from piglit?!---I'll debug that later.). For now, I'm running this test manually with: ./bin/glean -o -v -v -v -t +fragProg1 As a former stable-release manager would you have any suggestions - should I pull it out from the upcoming 10.2.7 ? I would not be too fussed it only swrast regresses, yet I'm suspecting that other classic drivers could be affected. Right. If piglit shows a regression, (on anything), I would recommend not including the patch. I still stand by that policy. But, while I did find a failure of this test on swrast, I wasn't able to identify this patch as a regression. I see this same testing failing similarly on both mesa-10.2.6 and 10.2-branchpoint. Let me know if I can be of any further assistance. -Carl -- carl.d.wo...@intel.com pgpcOQA4iWcM_.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/2] Eliminate several cases of multiplication in arguments to calloc
Matt Turner matts...@gmail.com writes: - configs = calloc(1, (num_modes + 1) * sizeof *configs); + configs = calloc((num_modes + 1), sizeof *configs); I'd drop the extra parentheses. Thanks for reading carefully, Matt! With that, both are Reviewed-by: Matt Turner matts...@gmail.com OK. These are both pushed now. -Carl -- carl.d.wo...@intel.com pgps5LnULwbMf.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] mesa/program_cache: calloc the correct size for the cache.
Matt Turner matts...@gmail.com writes: git blaming that turns up a sloppy search and replace commit that replaced _mesa_calloc(x) (taking only one argument) with calloc(1, x), even when x was a multiplication expression. Thanks for chasing that down. If someone wants to fix these up: git grep 'calloc.* \* ' Most of what that grep actually shows is harmless invocations along the lines of: pdp = calloc(1, sizeof *pdp); But there are a number of instances with actual multiplication occurring in the second argument to calloc. Here are the instances I found after culling through the grep results: src/gallium/drivers/freedreno/a2xx/ir-a2xx.c: ptr = dwords = calloc(1, 4 * info-sizedwords); src/gallium/drivers/freedreno/ir3/ir3.c:ptr = dwords = calloc(1, 4 * info-sizedwords); src/gallium/drivers/r600/r600_asm.c:bc-bytecode = calloc(1, bc-ndw * 4); src/mapi/glapi/gen/gl_gentable.py:struct _glapi_table *disp = calloc(1, _glapi_get_dispatch_table_size() * sizeof(_glapi_proc)); src/mesa/drivers/dri/common/utils.c: configs = calloc(1, (num_modes + 1) * sizeof *configs); src/mesa/drivers/dri/i965/brw_state_cache.c: calloc(1, cache-size * sizeof(struct brw_cache_item *)); src/mesa/main/atifragshader.c: calloc(1, sizeof(struct atifs_instruction) * src/mesa/main/atifragshader.c: calloc(1, sizeof(struct atifs_setupinst) * src/mesa/program/prog_cache.c: calloc(1, cache-size * sizeof(struct cache_item)); src/mesa/program/prog_instruction.c: calloc(1, numInst * sizeof(struct prog_instruction)); src/mesa/program/prog_optimize.c: calloc(1, prog-NumInstructions * sizeof(GLboolean)); src/mesa/program/prog_optimize.c: calloc(1, prog-NumInstructions * sizeof(GLboolean)); src/mesa/program/prog_optimize.c: calloc(1, prog-NumInstructions * sizeof(GLboolean)); src/mesa/program/prog_parameter.c: calloc(1, size * sizeof(struct gl_program_parameter)); src/mesa/vbo/vbo_exec_array.c: prim = calloc(1, primcount * sizeof(*prim)); And here's another that isn't the same pattern, (no 1, so not part of the same search/replace issue), but potentially still worth looking at. Here, there are three things being multiplied. I haven't looked at the ranges of the actual values to know if there is an issue with the way these things are split across the arguments: src/mesa/tnl/t_vertex.c: vtx-vertex_buf = _mesa_align_calloc(vb_size * max_vertex_size, 32 ); Since the first list above is small enough, I'm happy to put together a patch for this issue and trust that with peer review we can avoid introducing any new bugs with the patch. -Carl pgp1HoE5IdVNu.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] mesa/program_cache: calloc the correct size for the cache.
Carl Worth cwo...@cworth.org writes: And here's another that isn't the same pattern, (no 1, so not part of the same search/replace issue), but potentially still worth looking at. Here, there are three things being multiplied. I haven't looked at ... src/mesa/tnl/t_vertex.c: vtx-vertex_buf = _mesa_align_calloc(vb_size * max_vertex_size, 32 ); Sorry. I misread that one. The second argument is an alignment, not a term being multiplied. There is still a potential integer overflow here in computing a size to be allocated. And that same potential issue still exists in all calls to _mesa_align_malloc, _mesa_align_calloc, _mesa_align_realloc, malloc, and realloc where there is multiplication involved in computing the buffer size to be allocated. So the patch I just put together only fixes the tip of the iceberg, (the piece that's easy to fix since calloc already accepts two arguments with an implicit multiplication and catches overflow). For this issue in cairo, the Mozilla folks cooked up a little macro to provide a two-argument malloc that explicitly checks for integer overflow and evaluates to NULL in that case, (with no call to malloc at all). For mesa, a slightly more general solution would be useful, since a three-argument multiplication is quite common, (primitive_size * stride * height) in allocations. Since the first list above is small enough, I'm happy to put together a patch for this issue and trust that with peer review we can avoid introducing any new bugs with the patch. I've got this done now and I'll send it to the list separately. -Carl pgpsPVIITdMvk.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/2] egl: Restrict multiplication in calloc arguments to use compile-time constants
As explained in the previous commit, we want to avoid the possibility of
integer-multiplication overflow while allocating buffers.
In these two cases, the final allocation size is the product of three values:
one variable and two that are fixed constants at compile time.
In this commit, we move the explicit multiplication to involve only the
compile-time constants, preventing any overflow from that multiplication, (and
allowing calloc to catch any potential overflow from the remainining implicit
multiplication).
---
src/egl/drivers/dri2/platform_drm.c | 2 +-
src/egl/drivers/dri2/platform_wayland.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/egl/drivers/dri2/platform_drm.c
b/src/egl/drivers/dri2/platform_drm.c
index e272beb..70bd7d4 100644
--- a/src/egl/drivers/dri2/platform_drm.c
+++ b/src/egl/drivers/dri2/platform_drm.c
@@ -352,7 +352,7 @@ dri2_drm_get_buffers(__DRIdrawable * driDrawable,
const unsigned int format = 32;
int i;
- attachments_with_format = calloc(count * 2, sizeof(unsigned int));
+ attachments_with_format = calloc(count, 2 * sizeof(unsigned int));
if (!attachments_with_format) {
*out_count = 0;
return NULL;
diff --git a/src/egl/drivers/dri2/platform_wayland.c
b/src/egl/drivers/dri2/platform_wayland.c
index 537d26e..59b2792 100644
--- a/src/egl/drivers/dri2/platform_wayland.c
+++ b/src/egl/drivers/dri2/platform_wayland.c
@@ -468,7 +468,7 @@ dri2_wl_get_buffers(__DRIdrawable * driDrawable,
const unsigned int format = 32;
int i;
- attachments_with_format = calloc(count * 2, sizeof(unsigned int));
+ attachments_with_format = calloc(count, 2 * sizeof(unsigned int));
if (!attachments_with_format) {
*out_count = 0;
return NULL;
--
2.1.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/2] Eliminate several cases of multiplication in arguments to calloc
In commit 32f2fd1c5d6088692551c80352b7d6fa35b0cd09, several calls to
_mesa_calloc(x) were replaced with calls to calloc(1, x). This is strictly
equivalent to what the code was doing previously.
But for cases where x involves multiplication, now that we are explicitly
using the two-argument calloc, we can do one step better and replace:
calloc(1, A * B);
with:
calloc(A, B);
The advantage of the latter is that calloc will detect any overflow that would
have resulted from the multiplication and will fail the allocation, (whereas
the former would return a small allocation). So this fix can change
potentially exploitable buffer overruns into segmentation faults.
---
src/gallium/drivers/freedreno/a2xx/ir-a2xx.c | 2 +-
src/gallium/drivers/freedreno/ir3/ir3.c | 2 +-
src/gallium/drivers/r600/r600_asm.c | 2 +-
src/mapi/glapi/gen/gl_gentable.py| 2 +-
src/mesa/drivers/dri/common/utils.c | 2 +-
src/mesa/drivers/dri/i965/brw_state_cache.c | 4 ++--
src/mesa/main/atifragshader.c| 8
src/mesa/program/prog_instruction.c | 2 +-
src/mesa/program/prog_optimize.c | 6 +++---
src/mesa/program/prog_parameter.c| 2 +-
src/mesa/vbo/vbo_exec_array.c| 2 +-
11 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/src/gallium/drivers/freedreno/a2xx/ir-a2xx.c
b/src/gallium/drivers/freedreno/a2xx/ir-a2xx.c
index 18afba8..cff5a27 100644
--- a/src/gallium/drivers/freedreno/a2xx/ir-a2xx.c
+++ b/src/gallium/drivers/freedreno/a2xx/ir-a2xx.c
@@ -146,7 +146,7 @@ void * ir2_shader_assemble(struct ir2_shader *shader,
struct ir2_shader_info *in
goto fail;
}
- ptr = dwords = calloc(1, 4 * info-sizedwords);
+ ptr = dwords = calloc(4, info-sizedwords);
/* second pass, emit CF program in pairs: */
for (i = 0; i shader-cfs_count; i += 2) {
diff --git a/src/gallium/drivers/freedreno/ir3/ir3.c
b/src/gallium/drivers/freedreno/ir3/ir3.c
index ea2a925..3da10fb 100644
--- a/src/gallium/drivers/freedreno/ir3/ir3.c
+++ b/src/gallium/drivers/freedreno/ir3/ir3.c
@@ -554,7 +554,7 @@ void * ir3_assemble(struct ir3 *shader, struct ir3_info
*info)
*/
info-sizedwords = 2 * align(shader-instrs_count, 4);
- ptr = dwords = calloc(1, 4 * info-sizedwords);
+ ptr = dwords = calloc(4, info-sizedwords);
for (i = 0; i shader-instrs_count; i++) {
struct ir3_instruction *instr = shader-instrs[i];
diff --git a/src/gallium/drivers/r600/r600_asm.c
b/src/gallium/drivers/r600/r600_asm.c
index 4da918c..8aa69b5 100644
--- a/src/gallium/drivers/r600/r600_asm.c
+++ b/src/gallium/drivers/r600/r600_asm.c
@@ -1590,7 +1590,7 @@ int r600_bytecode_build(struct r600_bytecode *bc)
bc-ndw = cf-addr + cf-ndw;
}
free(bc-bytecode);
- bc-bytecode = calloc(1, bc-ndw * 4);
+ bc-bytecode = calloc(4, bc-ndw);
if (bc-bytecode == NULL)
return -ENOMEM;
LIST_FOR_EACH_ENTRY(cf, bc-cf, list) {
diff --git a/src/mapi/glapi/gen/gl_gentable.py
b/src/mapi/glapi/gen/gl_gentable.py
index 7577b66..ce9af99 100644
--- a/src/mapi/glapi/gen/gl_gentable.py
+++ b/src/mapi/glapi/gen/gl_gentable.py
@@ -113,7 +113,7 @@ __glapi_gentable_set_remaining_noop(struct _glapi_table
*disp) {
struct _glapi_table *
_glapi_create_table_from_handle(void *handle, const char *symbol_prefix) {
-struct _glapi_table *disp = calloc(1, _glapi_get_dispatch_table_size() *
sizeof(_glapi_proc));
+struct _glapi_table *disp = calloc(_glapi_get_dispatch_table_size(),
sizeof(_glapi_proc));
char symboln[512];
if(!disp)
diff --git a/src/mesa/drivers/dri/common/utils.c
b/src/mesa/drivers/dri/common/utils.c
index e0b3db8..1f30966 100644
--- a/src/mesa/drivers/dri/common/utils.c
+++ b/src/mesa/drivers/dri/common/utils.c
@@ -238,7 +238,7 @@ driCreateConfigs(mesa_format format,
is_srgb = _mesa_get_format_color_encoding(format) == GL_SRGB;
num_modes = num_depth_stencil_bits * num_db_modes * num_accum_bits *
num_msaa_modes;
- configs = calloc(1, (num_modes + 1) * sizeof *configs);
+ configs = calloc((num_modes + 1), sizeof *configs);
if (configs == NULL)
return NULL;
diff --git a/src/mesa/drivers/dri/i965/brw_state_cache.c
b/src/mesa/drivers/dri/i965/brw_state_cache.c
index 19079c8..bb5047e 100644
--- a/src/mesa/drivers/dri/i965/brw_state_cache.c
+++ b/src/mesa/drivers/dri/i965/brw_state_cache.c
@@ -115,7 +115,7 @@ rehash(struct brw_cache *cache)
GLuint size, i;
size = cache-size * 3;
- items = calloc(1, size * sizeof(*items));
+ items = calloc(size, sizeof(*items));
for (i = 0; i cache-size; i++)
for (c = cache-items[i]; c; c = next) {
@@ -334,7 +334,7 @@ brw_init_caches(struct brw_context *brw)
cache-size = 7;
cache-n_items = 0;
cache-items =
- calloc(1, cache-size * sizeof(struct brw_cache_item *));
+
Re: [Mesa-dev] [Mesa-stable] [PATCH 1/3 v2] mesa: Handle uninitialized textures like other textures in get_tex_level_parameter_image
Emil Velikov emil.l.veli...@gmail.com writes: This patch seems to regress two tests when running classic swrast. The gallium swrast is OK. glean: fragProg1-CMP glsl1-Preprocessor test 11 (#elif) I will not have the chance to run piglit on a i965 system until next week so I would love if someone can take a look at this. I should be able to take a look at this today or tomorrow. As a former stable-release manager would you have any suggestions - should I pull it out from the upcoming 10.2.7 ? I would not be too fussed it only swrast regresses, yet I'm suspecting that other classic drivers could be affected. Right. If piglit shows a regression, (on anything), I would recommend not including the patch. -Carl pgp3VBDX03ETA.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glcpp: Don't use alternation in the lookahead for empty pragmas.
Kenneth Graunke kenn...@whitecape.org writes:
Erm. Shouldn't there be a star here? ^^^
Yikes! I don't know which is more embarrassing. The regression that I
committed that showed I didn't take enough care in running piglit, or
this patch which I submitted to fix it which showed I took even less
care in compiling!
I can't imagine this compiles...or worse, it does, and continues the comment
to the */ in the rule below...
+HASHpragma{HSPACE}*/[\r\n] {
BEGIN INITIAL;
}
That would have been really bad. It looks like the extended comment
could have formed a legal rule.
Fortunately, the patch as sent didn't compile at all. I had actually
compiled this patch earlier and run piglit. I must have introduced the
bug after that, but before sending the patch off.
With that fixed, and piglit tested, this would get my Reviewed-by.
Thanks. I fixed this, added your Reviewed-by and a CC to stable, and
pushed this out.
-Carl
PS. Oddly, in a complete run of piglit, I saw that piglit's
17000-consecutive-chars-identifier test was fixed by my patch, but that
the 16385-consecutive-chars.frag test was passing before the patch, (but
that's the test that was originally cited in the bugzilla report).
--
carl.d.wo...@intel.com
pgp4DhzPOQbsz.pgp
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] Mesa 10.3 release candidate 1
Mesa 10.3 release candidate 1 is now available for testing. The current plan is to have an additional release candidate each Friday until the eventual 10.3 release, (Ian can follow up to state what the planned date is for that). The tag in the git repository for Mesa 10.3-rc1 is 'mesa-10.3-rc1'. I have also pushed a tag '10.3-branchpoint' to mark the point where master and 10.3 diverge. This should make git-describe a bit more useful. As a reminder, with the 10.3 branch now created, patches nominated with: CC: mesa-sta...@lists.freedesktop.org will now be candidates only for the new 10.3 branch. To nominate patches for the older 10.2 branch as well, please use: CC: 10.2 10.3 mesa-sta...@lists.freedesktop.org The expectation is that the 10.2 branch will remain alive with bi-weekly releases until after 10.3.1 release. Mesa 10.3 release candidate 1 is available for download from ftp://freedesktop.org/pub/mesa/10.3 sha256sums: db7b12e65db2443335dceec9c6076b6643ea0ebe93edbb89b0cfa22b2812d5e0 MesaLib-10.3.0-rc1.tar.bz2 8c5fe067942298f623aa4ffa0a542273d02c9051619fc3b4c268e119d07e8a38 MesaLib-10.3.0-rc1.tar.gz 5104228927595f88619cb26e54593e8ba7337f80f6dcd432b915a29bb29e1fdd MesaLib-10.3.0-rc1.zip I have verified building from the .tar.bz2 file by doing the following on a Debian (unstable) system: tar xjf MesaLib-10.3.0-rc1.tar.bz2 cd Mesa-10.3.0-rc1 ./configure --enable-gallium-llvm make -j6 make install -Carl pgphi54nBOUKc.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] Mesa 10.2.6
Mesa 10.2.6 has been released. Mesa 10.2.6 is a bug fix release fixing bugs since the 10.2.5 release, (see below for a list of changes). The tag in the git repository for Mesa 10.2.6 is 'mesa-10.2.6'. Mesa 10.2.6 is available for download at ftp://freedesktop.org/pub/mesa/10.2.6/ SHA-256 checksums (can be verified with the sha256sum program): 193314d2adba98e43697d726739ac46b4299aae324fa1821aa226890c28ac806 MesaLib-10.2.6.tar.bz2 f7a45a5977b485eb95ac024205c584a0c112fe3951c2313c797579bb16a7a448 MesaLib-10.2.6.tar.gz 6d086d6fcda8f317adfaaae40011decf2f2e2dc80819c4a7a77c76f73512e8d8 MesaLib-10.2.6.zip I have verified building from the .tar.bz2 file by doing: tar xjf MesaLib-10.2.6.tar.bz2 cd Mesa-10.2.6 ./configure --enable-gallium-llvm make -j6 make install I have also verified that I pushed the tag. -Carl -- carl.d.wo...@intel.com Changes from 10.2.5 to 10.2.6: Anuj Phogat (15): mesa: Fix error condition for valid texture targets in glTexStorage* functions mesa: Turn target_can_be_compressed() in to a utility function mesa: Add error condition for using compressed internalformat in glTexStorage3D() mesa: Fix condition for using compressed internalformat in glCompressedTexImage3D() mesa: Add utility function _mesa_is_enum_format_snorm() mesa: Don't allow snorm internal formats in glCopyTexImage*() in GLES3 mesa: Add a helper function _mesa_is_enum_format_unsized() mesa: Add a gles3 error condition for sized internalformat in glCopyTexImage*() mesa: Add gles3 error condition for GL_RGBA10_A2 buffer format in glCopyTexImage*() mesa: Add utility function _mesa_is_enum_format_unorm() mesa: Add gles3 condition for normalized internal formats in glCopyTexImage*() mesa: Allow GL_TEXTURE_CUBE_MAP target with compressed internal formats meta: Use _mesa_get_format_bits() to get the GL_RED_BITS egl: Fix OpenGL ES version checks in _eglParseContextAttribList() meta: Fix datatype computation in get_temp_image_type() Brian Paul (1): mesa: fix assertion in _mesa_drawbuffers() Carl Worth (3): docs: Add sha256 sums to the 10.2.5 release notes Update VERSION to 10.2.6 Add release notes for the 10.2.6 release Ilia Mirkin (1): mesa/st: only convert AND(a, NOT(b)) into MAD when not using native integers Jordan Justen (1): i965/miptree: Layout 1D Array as 2D Array with height of 1 Maarten Lankhorst (1): configure.ac: Do not require llvm on x32 Marek Olšák (4): st/mesa: fix blit-based partial TexSubImage for 1D arrays radeon,r200: fix buffer validation after CS flush radeonsi: fix a hang with instancing in Unigine Heaven/Valley on Hawaii radeonsi: fix CMASK and HTILE allocation on Tahiti Pali Rohár (1): configure: check for dladdr via AC_CHECK_FUNC/AC_CHECK_LIB Roland Scheidegger (1): gallivm: fix up out-of-bounds level when using conformant out-of-bound behavior pgpRK4RIxLP4f.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/3 v2] mesa: Handle uninitialized textures like other textures in get_tex_level_parameter_image
Anuj Phogat anuj.pho...@gmail.com writes: On Wed, Jul 30, 2014 at 4:09 PM, Carl Worth cwo...@cworth.org wrote: Ian Romanick i...@freedesktop.org writes: Anuj: Can you verify this does not regress proxy_textures_invalid_size? ... Cc: 10.2 mesa-sta...@lists.freedesktop.org Ian (or Anuj), is there an outstanding question of a regression here that should still be answered before this patch is picked over to the stable branch? Sorry I missed this question from you and Ian. I verified that the patch does not regress proxy_textures_invalid_size. Thanks, Anuj. With that confirmation I've just cherry-picked this patch and pushed it out to the 10.2 branch, (just after the 10.2.6 release). -Carl -- carl.d.wo...@intel.com pgpHgSwZRPZgo.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] glcpp: Don't use alternation in the lookahead for empty pragmas.
We've found that there's a buffer overrun bug in flex that's triggered by
using alternation in a lookahead pattern.
Fortunately, we don't need to match the exact {NEWLINE} expression to detect
an empty pragma. It suffices to verify that there are no non-space characters
before any newline character. So we can use a simple [\r\n] to get the desired
behavior while avoiding the flex bug.
Fixes Piglit's 16385-consecutive-chars and
17000-consecutive-chars-identifier tests.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82472
Signed-off-by: Carl Worth cwo...@cworth.org
Approach-suggested-by: Kenneth Graunke kenn...@whitecape.org
CC: Kenneth Graunke kenn...@whitecape.org
---
Thanks for chasing down the fix for this regression of mine, Ken.
I am embarrassed that I clearly didn't run piglit enough while testing my
original branch.
With your fix above, there is some state that's not updated as it should
be when returning a NEWLINE token, (such as incrementing yylineno, etc.).
I tried to improve things to update all that state, but it proved
problematic, (putting the state updates in a common function doesn't
work because only the outer lexing function has access to local variables
like yylineno).
The alternate approach here was your recommendation, of course.
src/glsl/glcpp/glcpp-lex.l | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/src/glsl/glcpp/glcpp-lex.l b/src/glsl/glcpp/glcpp-lex.l
index 98d500e..aaef7b8 100644
--- a/src/glsl/glcpp/glcpp-lex.l
+++ b/src/glsl/glcpp/glcpp-lex.l
@@ -289,8 +289,14 @@ HEXADECIMAL_INTEGER0[xX][0-9a-fA-F]+[uU]?
}
/* Swallow empty #pragma directives, (to avoid confusing the
-* downstream compiler). */
-HASHpragma{HSPACE}*/{NEWLINE} {
+* downstream compiler).
+*
+* Note: We use a simple regular expression for the lookahead
+* here. Specifically, we cannot use the complete {NEWLINE} expression
+* since it uses alternation and we've found that there's a flex bug
+* where using alternation in the lookahead portion of a pattern
+* triggers a buffer overrun. /
+HASHpragma{HSPACE}*/[\r\n] {
BEGIN INITIAL;
}
--
2.0.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/2] glcpp: Use printf instead of echo -n in glcpp-test
I noticed that with /bin/sh on Mac OS X, echo -n does not work as desired, (it actually prints -n rather than suppressing the final newline). There is a /bin/echo that could be used (it actually works) instead of the builtin echo. But I decided it's more robust to just use printf rather than hardcoding /bin/echo into the script. --- src/glsl/glcpp/tests/glcpp-test | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/glsl/glcpp/tests/glcpp-test b/src/glsl/glcpp/tests/glcpp-test index 640f576..ea69edf 100755 --- a/src/glsl/glcpp/tests/glcpp-test +++ b/src/glsl/glcpp/tests/glcpp-test @@ -59,7 +59,7 @@ clean=0 echo == Testing for correctness == for test in $testdir/*.c; do -echo -n Testing $test... +printf Testing $test... $glcpp $(test_specific_args $test) $test $test.out 21 total=$((total+1)) if cmp $test.expected $test.out /dev/null 21; then @@ -78,7 +78,7 @@ echo if [ $do_valgrind = yes ]; then echo == Testing for valgrind cleanliness == for test in $testdir/*.c; do - echo -n Testing $test with valgrind... + printf Testing $test with valgrind... valgrind --error-exitcode=31 --log-file=$test.valgrind-errors $glcpp $(test_specific_args $test) $test /dev/null 21 if [ $? = 31 ]; then echo ERRORS -- 2.0.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/2] glcpp: Fix glcpp-test-cr-lf make check test for Mac OS X
There were two problems with the way this script used sed on OS X: 1. The OS X sed doesn't interpret \r in a replacement list as a carriage-return character, (instead it was inserting a literal 'r' character). We fix this by putting an actual ^M character into the source of the script, (rather than a two-character escape sequence hoping for sed to do the right thing). 2. When generating the test files with LF-CR (\n\r) newlines, the OS X sed was adding an undesired final newline (\n) at the end of the file. We avoid this by first using sed to add the ^M before the newlines, then using tr to swap the \r and \n characters. This way, sed never sees any lines ending with anything but \n, so it doesn't get confused and doesn't add any bogus extra newlines. --- src/glsl/glcpp/tests/glcpp-test-cr-lf | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/glsl/glcpp/tests/glcpp-test-cr-lf b/src/glsl/glcpp/tests/glcpp-test-cr-lf index edaa29d..7988c05 100755 --- a/src/glsl/glcpp/tests/glcpp-test-cr-lf +++ b/src/glsl/glcpp/tests/glcpp-test-cr-lf @@ -111,7 +111,7 @@ rm -rf ./subtest-cr-lf mkdir subtest-cr-lf for file in $testdir/*.c; do base=$(basename $file) -sed -e 's/$/\r/' $file subtest-cr-lf/$base +sed -e 's/$//' $file subtest-cr-lf/$base cp subtest-lf/$base.out subtest-cr-lf/$base.expected done @@ -124,7 +124,7 @@ rm -rf ./subtest-lf-cr mkdir subtest-lf-cr for file in $testdir/*.c; do base=$(basename $file) -tr \n \r $file | sed -e 's/\r/\n\r/g' subtest-lf-cr/$base +sed -e 's/$//' $file | tr \n\r \r\n subtest-lf-cr/$base cp subtest-lf/$base.out subtest-lf-cr/$base.expected done -- 2.0.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] New stable-branch 10.2 candidate pushed
Carl Worth cwo...@cworth.org writes: Normally, I would have completed my own testing of the branch prior to pushing the candidate. This time, I'm a little late, but I plan to follow up tomorrow with my own testing results, (which hopefully will not lead to any changes in the branch). I've completed that testing now. With piglit, I compared Mesa 10.2.5 to the current 10.2 branch in three configurations: i965_dri.so with Haswell Gallium swrast_dri.so Non-Gallium swrast_dri.so And there were no regressions in any of the configurations. Unless I hear any negative testing reports from users of any other hardware, I will plan to release 10.2.6 late this upcoming Friday (in my timezone at least). -Carl pgpUw5ZRiyGJ1.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] New stable-branch 10.2 candidate pushed
I've just made my pass over the stable queue and pushed out a new candidate branch for the upcoming 10.2.6 release. You can see the 25 patches included (and the 42 still awaiting review) here: http://cworth.org/~cworth/mesa-stable-queue/ As usual, I plan to make the next stable release on Friday. So I will appreciate any testing reports (or general approval of the state of the branch. Here are the commits where I manually merged conflicts, (so these might merit additional review): commit 22ec7df0d7687eb7db72ac2a004b21d4cb35ec7b Author: Marek Olšák marek.ol...@amd.com radeonsi: fix a hang with instancing in Unigine Heaven/Valley on Hawaii (cherry picked from commit 515269b3a73cd64ac9c017e8b3c698be9a5383f6) commit 3e541f0ab8740efc662860f17a1263af28b30b2a Author: Anuj Phogat anuj.pho...@gmail.com meta: Fix datatype computation in get_temp_image_type() (cherry picked from commit 338fef61f86bb121e47b096428dce2a9109d3a3e) commit 29e2f11f5dda5833327bf2225f6ca4e8ca7f9c42 Author: Anuj Phogat anuj.pho...@gmail.com meta: Use _mesa_get_format_bits() to get the GL_RED_BITS (cherry picked from commit 7de90890c64d019c56e5a06e427b207220729997) Normally, I would have completed my own testing of the branch prior to pushing the candidate. This time, I'm a little late, but I plan to follow up tomorrow with my own testing results, (which hopefully will not lead to any changes in the branch). -Carl -- carl.d.wo...@intel.com pgpc_PaT36ghZ.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] RFC: glsl/glcpp: Allow for '#' characters to appear in shader body
Erik Faye-Lund kusmab...@gmail.com writes: It seems to me like this is there to support non-ASCII identifiers and strings, but GLSL doesn't support either. I'm not able to come up with a conclusion here. That's almost always the case with GLSL. The GLSL specification does have its own section for character set, etc. but weasels out of carefully specifying how #define works and just vaguely points to standard C++ behavior, (without saying what version of what C++ standard it might be referring to). So for nit-picky details like this, there's no possibility to come up with anything very definitive about how GLSL should work from the specification alone. -Carl -- carl.d.wo...@intel.com pgp6QuntftmuA.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 7/6] glsl/glcpp: Add testing of illegal characters in macro replacement lists
The desired behavior here is that there is no error for an illegal character appearing in the replacement list of a macro definition. However, any expansion of such a macro, causing the illegal character to appear in the preprocessed source, should emit an error. These two tests exercise both of the cases, ensuring that the error is not emitted until the macro is actually expanded. --- .../142-illegal-characters-in-macro-definition.c | 17 ...legal-characters-in-macro-definition.c.expected | 17 .../tests/143-illegal-characters-in-macro-use.c| 24 + .../143-illegal-characters-in-macro-use.c.expected | 31 ++ 4 files changed, 89 insertions(+) create mode 100644 src/glsl/glcpp/tests/142-illegal-characters-in-macro-definition.c create mode 100644 src/glsl/glcpp/tests/142-illegal-characters-in-macro-definition.c.expected create mode 100644 src/glsl/glcpp/tests/143-illegal-characters-in-macro-use.c create mode 100644 src/glsl/glcpp/tests/143-illegal-characters-in-macro-use.c.expected diff --git a/src/glsl/glcpp/tests/142-illegal-characters-in-macro-definition.c b/src/glsl/glcpp/tests/142-illegal-characters-in-macro-definition.c new file mode 100644 index 000..a58cdcc --- /dev/null +++ b/src/glsl/glcpp/tests/142-illegal-characters-in-macro-definition.c @@ -0,0 +1,17 @@ +/* Our reading of the C++ specification, (as deferred to by the GLSL + * specification), suggests that all characters are legal as part of a macro + * replacement list, but actually using such a macro with an illegal character + * should emit an error. + */ + +/* So no errors here for just defining things with illegal characters. */ +#define DOUBLE_QUOTE +#define DOLLAR $ +#define APOSTROPHE ' +#define AT_SIGN @ +#define BACK_TICK ` +#define HASH # +#define BACK_SLASH_NOT_AT_END \... + +/* See the following test which ensures that using any of the above macros + * will generate an error. */ diff --git a/src/glsl/glcpp/tests/142-illegal-characters-in-macro-definition.c.expected b/src/glsl/glcpp/tests/142-illegal-characters-in-macro-definition.c.expected new file mode 100644 index 000..46c7ab9 --- /dev/null +++ b/src/glsl/glcpp/tests/142-illegal-characters-in-macro-definition.c.expected @@ -0,0 +1,17 @@ + + + + + + + + + + + + + + + + + diff --git a/src/glsl/glcpp/tests/143-illegal-characters-in-macro-use.c b/src/glsl/glcpp/tests/143-illegal-characters-in-macro-use.c new file mode 100644 index 000..b3b1d37 --- /dev/null +++ b/src/glsl/glcpp/tests/143-illegal-characters-in-macro-use.c @@ -0,0 +1,24 @@ +/* Our reading of the C++ specification, (as deferred to by the GLSL + * specification), suggests that all characters are legal as part of a macro + * replacement list, but actually using such a macro with an illegal character + * should emit an error. + */ + +/* So no errors here for just defining things with illegal characters. + * (See the previous test which ensures there are no errors here.) */ +#define DOUBLE_QUOTE +#define DOLLAR $ +#define APOSTROPHE ' +#define AT_SIGN @ +#define BACK_TICK ` +#define HASH # +#define BACK_SLASH_NOT_AT_END \... + +/* But each of these should cause an error. */ +DOUBLE_QUOTE +DOLLAR +APOSTROPHE +AT_SIGN +BACK_TICK +HASH +BACK_SLASH_NOT_AT_END diff --git a/src/glsl/glcpp/tests/143-illegal-characters-in-macro-use.c.expected b/src/glsl/glcpp/tests/143-illegal-characters-in-macro-use.c.expected new file mode 100644 index 000..6b948d6 --- /dev/null +++ b/src/glsl/glcpp/tests/143-illegal-characters-in-macro-use.c.expected @@ -0,0 +1,31 @@ +0:9(22): preprocessor error: Illegal character '' +0:10(16): preprocessor error: Illegal character '$' +0:11(20): preprocessor error: Illegal character ''' +0:12(17): preprocessor error: Illegal character '@' +0:13(19): preprocessor error: Illegal character '`' +0:14(14): preprocessor error: Illegal character '#' +0:15(31): preprocessor error: Illegal character '\' + + + + + + + + + + + + + + + + + + + + + + + +... -- 2.0.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 5/6 v2.] glsl/glcpp: Emit an error for any illegal GLSL character.
The GLSL Language Specification (version 4.30.6) is quite clear about the GLSL
character set and the expected behavior for other characters:
Section 3.1 Character Set
The source character set used for the OpenGL shading languages, outside of
comments, is a subset of UTF-8. It includes the following characters:
The letters a-z, A-Z, and the underscore ( _ ).
The numbers 0-9.
The symbols period (.), plus (+), dash (-), slash (/), asterisk (*),
percent (%), angled brackets ( and ), square brackets ( [ and ] ),
parentheses ( ( and ) ), braces ( { and } ), caret (^), vertical bar
(|), ampersand (), tilde (~), equals (=), exclamation point (!),
colon (:), semicolon (;), comma (,), and question mark (?).
The number sign (#) for preprocessor use.
The backslash (\) as the line-continuation character when used as the
last character of a line, just before a new line.
White space: the space character, horizontal tab, vertical tab, form
feed, carriage-return, and line-feed.
A compile-time error will be given if any other character is used outside
a comment.
By taking the set of all possible 8-bit characters, and subtracting the above,
we have the set of illegal characters:
0x00 - 0x08 (^A - ^H)
0x0E - 0x1F (^N - ^Z, ^[, ^\, ^], ^^, ^_)
0x22 ()
0x24 ($)
0x27 (')
0x40 (@)
0x60 (')
0x7F (DEL or ^?)
0x80 - 0xFF (non-ASCII)
As well as (#) outside of uses defined by the preprocessor (not starting a
directive, nor as part of a legal paste operator in a replacement list), and
(\) appearing anywhere but at the end of a line.
So instead of the previous whitelist we had for OTHER characters, we now
add a blacklist for ILLEGAL characters based on the above, and then use a
simple regular expression of . to catch any characters that get past the
blacklist.
This approach also means the internal-error rule with . can no longer be
matched, so it goes away now.
v2: Instead of emitting the error as soon as the illegal character is lexed,
we instead emit an ILLEGAL token to the parser. This allows the parser to
allow the character as part of the replacement list of a macro, (since these
are specified to allow any character). However, if such a macro is actually
instantiated, the parser will emit an error when it goes to print the illegal
character as part of the preprocessed output.
---
src/glsl/glcpp/glcpp-lex.l | 32 +++-
src/glsl/glcpp/glcpp-parse.y | 25 ++---
2 files changed, 29 insertions(+), 28 deletions(-)
diff --git a/src/glsl/glcpp/glcpp-lex.l b/src/glsl/glcpp/glcpp-lex.l
index 0dbdab0..0482c4e 100644
--- a/src/glsl/glcpp/glcpp-lex.l
+++ b/src/glsl/glcpp/glcpp-lex.l
@@ -175,15 +175,7 @@ HASH #
IDENTIFIER [_a-zA-Z][_a-zA-Z0-9]*
PP_NUMBER [.]?[0-9]([._a-zA-Z0-9]|[eEpP][-+])*
PUNCTUATION[][(){}.*~!/%^|;,=+-]
-
-/* The OTHER class is simply a catch-all for things that the CPP
-parser just doesn't care about. Since flex regular expressions that
-match longer strings take priority over those matching shorter
-strings, we have to be careful to avoid OTHER matching and hiding
-something that CPP does care about. So we simply exclude all
-characters that appear in any other expressions. */
-
-OTHER [^][_#[:space:]#a-zA-Z0-9(){}.*~!/%^|;,=+-]
+ILLEGAL[\x00-\x08\x0E-\x1F$'@`\x7F\x80-\xFF\\]
DIGITS [0-9][0-9]*
DECIMAL_INTEGER[1-9][0-9]*[uU]?
@@ -276,9 +268,10 @@ HEXADECIMAL_INTEGER0[xX][0-9a-fA-F]+[uU]?
* token. */
if (parser-first_non_space_token_this_line) {
BEGIN HASH;
+ RETURN_TOKEN_NEVER_SKIP (HASH_TOKEN);
+ } else {
+ RETURN_STRING_TOKEN (ILLEGAL);
}
-
- RETURN_TOKEN_NEVER_SKIP (HASH_TOKEN);
}
HASHversion{HSPACE}+ {
@@ -505,8 +498,8 @@ HEXADECIMAL_INTEGER 0[xX][0-9a-fA-F]+[uU]?
RETURN_TOKEN (yytext[0]);
}
-{OTHER}+ {
- RETURN_STRING_TOKEN (OTHER);
+{ILLEGAL} {
+ RETURN_STRING_TOKEN (ILLEGAL);
}
{HSPACE} {
@@ -539,14 +532,7 @@ HEXADECIMAL_INTEGER0[xX][0-9a-fA-F]+[uU]?
RETURN_TOKEN (NEWLINE);
}
- /* This is a catch-all to avoid the annoying default flex action which
-* matches any character and prints it. If any input ever matches this
-* rule, then we have made a mistake above and need to fix one or more
-* of the preceding patterns to match that input. */
-
-*. {
- glcpp_error(yylloc, yyextra, Internal compiler error: Unexpected
character: %s, yytext);
-
+UNREACHABLE. {
/* We don't actually use the UNREACHABLE start condition. We
only have this block here so that we can pretend to call some
generated functions, (to avoid defined but not used
@@ -557,6 +543,10 @@ HEXADECIMAL_INTEGER0[xX][0-9a-fA-F]+[uU]?
}
}
Re: [Mesa-dev] [PATCH] RFC: glsl/glcpp: Allow for '#' characters to appear in shader body
Erik Faye-Lund kusmab...@gmail.com writes: On Tue, Aug 5, 2014 at 11:22 PM, Carl Worth cwo...@cworth.org wrote: Now, what we could do if we were so inclined, would be to defer the errors for illegal characters until they actually appeared in the pre-processor output. What you describe here seems to actually be what the standard requires: The relevant bit of the parse rule for a define statement of this type is like so (Section 16 [cpp] of the C++ spec): ... each non-white-space character that cannot be one of the above Yes, that does seem clear. Thanks for chasing that down. Note that '$' is a bit different, as it's not a part of the preprocessor's character set, so using it might be interpreted as undefined behavior. Right. That could easily go either way. Is the phrase each non-white-space character restricted to characters of the original character set? Regardless, I've just sent a second version of the relevant patch from my subsequent series that now implements exactly what is described above. (And I sent another patch (7/6) in that series to test things.) I'm pretty happy with the result, but would of course appreciate review From anyone. -Carl -- carl.d.wo...@intel.com pgp22eE6dRzzK.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Preprocessor patches
Ian Romanick i...@freedesktop.org writes: Are the preprocessor patches on the gles3conform-v4 branch of my fd.o repo the most up to date? Yes. The state of those patches matches what I have on my branch, (at least as far as GLES3 conformance is concerned---I have some subsequent preprocessor patches which shouldn't affect conformance). I've put R-b on almost all of them. Thanks! I want to look a little closer at glsl/glcpp: Fix for macros that expand to include defined operators. That makes sense. It's one of the meatiest patches in the series. So I'll appreciate any additional review here. Fortunately, there is a fairly complete commit message to guide the review. (Though, while you're looking you might fix the mixed space-vs.-tab indentation in the commit message.) I'm not sure about glsl/glcpp: Fix line-continuation code to handle multiple newline flavors. Well, it certainly shouldn't be any worse than what we had before. I imagine what looks scary is the comment that says: In this code, we explicitly do not support a shader that mixes different kinds of newlines. Prior to this commit, the corresponding comment that should be in the code is: In this code, we explicitly do not support a shader that uses anything except for a single linefeed as the line terminator. And the testing in the patch series verifies that things do only improve with this patch. A lot of shaders that we encounter are a combination of hand-written and run-time, machine generated. If the hand-writen parts use cr-lf and the machine generated parts use sprintf with \n, we'll get a mix of line endings. Any idea what GCC does? Yes, I can see how a file might reasonably end up with mixed line terminators. And this is something that's not currently handled (as far as the line continuation is concerned---I think the rest of glcpp does do better with mixed line terminators by the end of the series). I just checked, and gcc does do a good job here. It currently handles line continuations with any of LF, CR, or CR+LF all in the same file, (in my test the first line encountered is terminated with LF). GCC does botch the LF+CR line continuation. Obviously, that's not that interesting a case since no current, mainstream OS uses this line terminator, (but as worded, the GLSL specification does allow it). Meanwhile, with the same test file, glcpp gets the LF line continuation right, leaves the CR and CR+LF line continuations untouched, and for the LF+CR continuation it removes the '\' but doesn't correctly fold the lines. I've attached the source file I used for testing as well as the output From gcc and glcpp. It probably wouldn't be too hard to fix this code to be more general. I might take a whack at that now that I have this test in hand. -Carl mixed-newlines Description: Binary data mixed-newlines-gcc Description: Binary data mixed-newlines-glcpp Description: Binary data pgpo5P2yIO2AT.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 4/5] glsl/glcpp: Fix line-continuation code to handle multiple newline flavors
Sometimes the newline separator is a single character, and sometimes it is two
characters. Before we can fold away and line-continuation backslashes, we
identify the flavor of line separator that is in use.
With this identified, we then correctly search for backslashes followed
immediately by the first character of the line separator.
Also, when re-inserting newlines to replace collapsed newlines, we carefully
insert newlines of the same flavor.
With this commit, almost all remaining test are fixed as tested by
glcpp-test-cr-lf:
\r: 142/143 tests pass
\r\n: 142/143 tests pass
\n\r: 143/143 tests pass
(The only remaining failures have nothing to do with the actual pre-processor
code, but are due to a bug in the way the test suite uses grep to try to
extract test-specific command-line options from the source files.)
v2. Add explicit support for a file that has a mixture of different newline
terminators within it.
---
src/glsl/glcpp/pp.c | 96 -
1 file changed, 87 insertions(+), 9 deletions(-)
diff --git a/src/glsl/glcpp/pp.c b/src/glsl/glcpp/pp.c
index 4a623f8..a54bcbe 100644
--- a/src/glsl/glcpp/pp.c
+++ b/src/glsl/glcpp/pp.c
@@ -70,6 +70,42 @@ glcpp_warning (YYLTYPE *locp, glcpp_parser_t *parser, const
char *fmt, ...)
parser-info_log_length, \n);
}
+/* Given str, (that's expected to start with a newline terminator of some
+ * sort), return a pointer to the first character in str after the newline.
+ *
+ * A newline terminator can be any of the following sequences:
+ *
+ * \r\n
+ * \n\r
+ * \n
+ * \r
+ *
+ * And the longest such sequence will be skipped.
+ */
+static const char *
+skip_newline (const char *str)
+{
+ const char *ret = str;
+
+ if (ret == NULL)
+ return ret;
+
+ if (*ret == '\0')
+ return ret;
+
+ if (*ret == '\r') {
+ ret++;
+ if (*ret *ret == '\n')
+ ret++;
+ } else if (*ret == '\n') {
+ ret++;
+ if (*ret *ret == '\r')
+ ret++;
+ }
+
+ return ret;
+}
+
/* Remove any line continuation characters in the shader, (whether in
* preprocessing directives or in GLSL code).
*/
@@ -78,10 +114,49 @@ remove_line_continuations(glcpp_parser_t *ctx, const char
*shader)
{
char *clean = ralloc_strdup(ctx, );
const char *backslash, *newline, *search_start;
+const char *cr, *lf;
+char newline_separator[3];
int collapsed_newlines = 0;
search_start = shader;
+ /* Determine what flavor of newlines this shader is using. GLSL
+* provides for 4 different possible ways to separate lines, (using
+* one or two characters):
+*
+* \n (line-feed, like Linux, Unix, and new Mac OS)
+* \r (carriage-return, like old Mac files)
+* \r\n (carriage-return + line-feed, like DOS files)
+* \n\r (line-feed + carriage-return, like nothing, really)
+*
+* This code explicitly supports a shader that uses a mixture of
+* newline terminators and will properly handle line continuation
+* backslashes followed by any of the above.
+*
+* But, since we must also insert additional newlines in the output
+* (for any collapsed lines) we attempt to maintain consistency by
+* examining the first encountered newline terminator, and using the
+* same terminator for any newlines we insert.
+*/
+ cr = strchr(search_start, '\r');
+ lf = strchr(search_start, '\n');
+
+ newline_separator[0] = '\n';
+ newline_separator[1] = '\0';
+ newline_separator[2] = '\0';
+
+ if (cr == NULL) {
+ /* Nothing to do. */
+ } else if (lf == NULL) {
+ newline_separator[0] = '\r';
+ } else if (lf == cr + 1) {
+ newline_separator[0] = '\r';
+ newline_separator[1] = '\n';
+ } else if (cr == lf + 1) {
+ newline_separator[0] = '\n';
+ newline_separator[1] = '\r';
+ }
+
while (true) {
backslash = strchr(search_start, '\\');
@@ -91,17 +166,24 @@ remove_line_continuations(glcpp_parser_t *ctx, const char
*shader)
* line numbers.
*/
if (collapsed_newlines) {
- newline = strchr(search_start, '\n');
+ cr = strchr (search_start, '\r');
+ lf = strchr (search_start, '\n');
+ if (cr lf)
+ newline = cr lf ? cr : lf;
+ else if (cr)
+ newline = cr;
+ else
+ newline = lf;
if (newline
Re: [Mesa-dev] Preprocessor patches
Carl Worth cwo...@cworth.org writes: It probably wouldn't be too hard to fix this code to be more general. I might take a whack at that now that I have this test in hand. It wasn't too hard. I just sent a v2 patch in reply to the original (and also force-pushed a new glcpp-fixup branch to my repository). I've attached the result of glcpp on the mixed-newlines test file from my previous email. It looks quite lovely, (if I do say so myself). I haven't yet added testing for this to the test suite, (I think having a source file with a mixture of newlines in is likely to confuse the infrastructure I have for munging the test suite files to have various flavors of newlines.) But I'll look into adding testing for that soon. Thanks for nudging me to make this code more robust. -Carl mixed-newlines-glcpp-v2 Description: Binary data pgpYQPWFHzvaz.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] RFC: glsl/glcpp: Allow for '#' characters to appear in shader body
Kenneth Graunke kenn...@whitecape.org writes:
I agree that this is pretty bogus.
I'm coming around to thinking it's totally bogus.
How about emitting a warning in the RETURN_TOKEN ('#') case?
Thanks for the review, and thanks for suggesting the warning. I added
the warning, then decided it needed some additional testing. So I
whipped up a test that uses every possible non-identifier character
to separate a macro name from its definition, like so:
#define S* splat
(defining a macro S to expand to * splat). This is first patch
attached below.
With this test, there's the question of whether it's even legal to have
no space between a macro name and the replacement list in a macro
definition. As usual, the GLSL specification is mute on the question,
(deferring just to as is standard for C++). If we choose GCC as the
standard, it does accept this case, (and emits a missing space
warning). By accident, glcpp has always silently accepted this, until
recently barfing, but only when '#' was the separating character.
My next patch expanded this new test so that '#' is tested as well, (and
ensuring the warning about the known-bogus '#' character was
emitted). This is the second patch attached below.
The expanded test looks really odd to me. Even though there are
something like two-dozen pieces of punctuation used a separators, only
the case with '#' emits a warning.
The insight is that the warning really has nothing to do with #define
FOO* but everything to do with the '#' character appearing out of
proper context. So what we really want is better testing of illegal
characters in various contexts.
So I wrote two additional new tests. The first runs every possible legal
character through glcpp, ensuring they are all passed through correctly.
It turns out this test caught a long-standing bug, (glcpp was tripping
up on vertical tab and form-feed for which the GLSL specification is
explicit should be allowed). The second test runs every possible illegal
character through glcpp, ensuring that an error is generated for each.
Instead of attaching those two tests, I'll send patches for them to the
list. They're independent of the current discussion.
And what was _really_ striking when looking at the final test (as I
first wrote it, but not as I will submit it), is that every illegal
character was generating an error message except for '#' which was just
generating a warning. So that sealed it for me that this treatment is
bogus.
So I'm inclined to leave Mesa breaking Reaction Quake, (but I'll
go file a bug against that application).
Now, what we could do if we were so inclined, would be to defer the
errors for illegal characters until they actually appeared in the
pre-processor output. That, is, a line such as:
$
would be an immediate error, (Illegal character '$'), but a line such
as:
#define DOLLAR $
would be accepted with no error in and of itself, (the illegal character
has appeared in a replacement list, but may not ever appear in the
output to be seen by the compiler). Then, a subsequent line such as:
DOLLAR
would then emit the Illegal character '$' error.
If we had all of that in place, that would un-break Reaction Quake in a
way that wouldn't feel creepy to me, (and none of the tests would have
odd-looking exceptional behavior).
-Carl
From b57b0a5a88c318b71ac58a7c1569f07d416e6486 Mon Sep 17 00:00:00 2001
From: Carl Worth cwo...@cworth.org
Date: Tue, 5 Aug 2014 11:31:56 -0700
Subject: [PATCH] glsl/glcpp: Add testing for no space between macro name and
replacement list
GCC's preprocessor accepts a macro definition where there is no space between
the macro's identifier name and the replacementlist. (GCC does emit a missing
space warning that we don't, but that's fine.)
This is an exhaustive test that verifies that all legal GLSL characters that
could possibly be interpreted as separating the macro name from the
replacement list are interpreted as such. So the testing here includes all
valid GLSL symbols except for:
* Characters that can be part of an identifier (a-z, A-Z, 0-9, _)
* Backslash, (allowed only as line continuation)
* Hash, (allowed only to introduce pre-processor directive, or as part of a
paste operator in a replacement list---but not as first token of
replacement list)
* Space characters (since the point of the testing is to have missing space)
* Left parenthesis (which would indicate a function-like macro)
---
src/glsl/glcpp/tests/139-define-macro-no-space.c | 52 ++
.../tests/139-define-macro-no-space.c.expected | 52 ++
2 files changed, 104 insertions(+)
create mode 100644 src/glsl/glcpp/tests/139-define-macro-no-space.c
create mode 100644 src/glsl/glcpp/tests/139-define-macro-no-space.c.expected
diff --git a/src/glsl/glcpp/tests/139-define-macro-no-space.c b/src/glsl/glcpp/tests/139-define-macro-no-space.c
new file mode 100644
index 000..4e997a0
[Mesa-dev] [PATCH 2/6] glsl/glcpp: Allow vertical tab and form feed characters in GLSL
Of course, these aren't really useful for anything, but the GLSL language
specification does allow them:
The source character set used for the OpenGL shading languages,
outside of comments, is a subset of UTF-8. It includes the following
characters:
...
White space: the space character, horizontal tab, vertical tab, form
feed, carriage-return, and line- feed.
[GLSL Language Specification 4.30.6, section 3.1]
So treat vertical tab ('\v' or ^K) and form-feed ('\f' or ^L) as horizontal
space characters.
---
src/glsl/glcpp/glcpp-lex.l | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/glsl/glcpp/glcpp-lex.l b/src/glsl/glcpp/glcpp-lex.l
index c126850..0dbdab0 100644
--- a/src/glsl/glcpp/glcpp-lex.l
+++ b/src/glsl/glcpp/glcpp-lex.l
@@ -170,7 +170,7 @@ glcpp_lex_update_state_per_token (glcpp_parser_t *parser,
int token)
SPACE [[:space:]]
NONSPACE [^[:space:]]
-HSPACE [ \t]
+HSPACE [ \t\v\f]
HASH #
IDENTIFIER [_a-zA-Z][_a-zA-Z0-9]*
PP_NUMBER [.]?[0-9]([._a-zA-Z0-9]|[eEpP][-+])*
--
2.0.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
