[Mesa-dev] [Bug 103909] anv_allocator.c:113:1: error: static declaration of ‘memfd_create’ follows non-static declaration
https://bugs.freedesktop.org/show_bug.cgi?id=103909 Bug ID: 103909 Summary: anv_allocator.c:113:1: error: static declaration of ‘memfd_create’ follows non-static declaration Product: Mesa Version: git Hardware: x86-64 (AMD64) OS: Linux (All) Status: NEW Severity: normal Priority: medium Component: Mesa core Assignee: mesa-dev@lists.freedesktop.org Reporter: v...@freedesktop.org QA Contact: mesa-dev@lists.freedesktop.org CC: k...@bitplanet.net mesa: 0bd83d04612520ff97e21d41bcc3ad2e68e160df (master 17.4.0-devel) CC vulkan/vulkan_libvulkan_common_la-anv_allocator.lo vulkan/anv_allocator.c:113:1: error: static declaration of ‘memfd_create’ follows non-static declaration memfd_create(const char *name, unsigned int flags) ^~~~ In file included from /usr/include/bits/mman.h:45:0, from /usr/include/sys/mman.h:41, from vulkan/anv_allocator.c:29: /usr/include/bits/mman-linux.h:125:5: note: previous declaration of ‘memfd_create’ was here int memfd_create (const char *__name, unsigned int __flags) __THROW; ^~~~ -- You are receiving this mail because: You are the QA Contact for the bug. You are the assignee for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] i965: Fix Smooth Point Enables.
We want to program the 3DSTATE_RASTER field to the gl_context value, not the other way around. Fixes: 13ac46557ab1 (i965: Port Gen8+ 3DSTATE_RASTER state to genxml.) --- src/mesa/drivers/dri/i965/genX_state_upload.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mesa/drivers/dri/i965/genX_state_upload.c b/src/mesa/drivers/dri/i965/genX_state_upload.c index 112f48181b0..2808363e3ad 100644 --- a/src/mesa/drivers/dri/i965/genX_state_upload.c +++ b/src/mesa/drivers/dri/i965/genX_state_upload.c @@ -4322,7 +4322,7 @@ genX(upload_raster)(struct brw_context *brw) raster.CullMode = CULLMODE_NONE; } - point->SmoothFlag = raster.SmoothPointEnable; + raster.SmoothPointEnable = point->SmoothFlag; raster.DXMultisampleRasterizationEnable = _mesa_is_multisample_enabled(ctx); -- 2.15.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] i965: Use C99 struct initializers in brw_bufmgr.c.
This is cleaner than using a non-standard memclear macro (which does a memset to 0) and then initializing fields after the fact. We move the declarations to where we initialized the fields. While we're at it, we move the declaration of 'ret' that goes with the ioctl, eliminating the declaration section altogether. --- src/mesa/drivers/dri/i965/brw_bufmgr.c | 140 - 1 file changed, 49 insertions(+), 91 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_bufmgr.c b/src/mesa/drivers/dri/i965/brw_bufmgr.c index 469895ed6b0..bd46035c37c 100644 --- a/src/mesa/drivers/dri/i965/brw_bufmgr.c +++ b/src/mesa/drivers/dri/i965/brw_bufmgr.c @@ -84,8 +84,6 @@ #define VG_DEFINED(ptr, size) VG(VALGRIND_MAKE_MEM_DEFINED(ptr, size)) #define VG_NOACCESS(ptr, size) VG(VALGRIND_MAKE_MEM_NOACCESS(ptr, size)) -#define memclear(s) memset(&s, 0, sizeof(s)) - #define PAGE_SIZE 4096 #define FILE_DEBUG_FLAG DEBUG_BUFMGR @@ -226,13 +224,9 @@ int brw_bo_busy(struct brw_bo *bo) { struct brw_bufmgr *bufmgr = bo->bufmgr; - struct drm_i915_gem_busy busy; - int ret; + struct drm_i915_gem_busy busy = { .handle = bo->gem_handle }; - memclear(busy); - busy.handle = bo->gem_handle; - - ret = drmIoctl(bufmgr->fd, DRM_IOCTL_I915_GEM_BUSY, &busy); + int ret = drmIoctl(bufmgr->fd, DRM_IOCTL_I915_GEM_BUSY, &busy); if (ret == 0) { bo->idle = !busy.busy; return busy.busy; @@ -243,12 +237,12 @@ brw_bo_busy(struct brw_bo *bo) int brw_bo_madvise(struct brw_bo *bo, int state) { - struct drm_i915_gem_madvise madv; + struct drm_i915_gem_madvise madv = { + .handle = bo->gem_handle, + .madv = state, + .retained = 1, + }; - memclear(madv); - madv.handle = bo->gem_handle; - madv.madv = state; - madv.retained = 1; drmIoctl(bo->bufmgr->fd, DRM_IOCTL_I915_GEM_MADVISE, &madv); return madv.retained; @@ -366,8 +360,6 @@ retry: } if (!alloc_from_cache) { - struct drm_i915_gem_create create; - bo = calloc(1, sizeof(*bo)); if (!bo) goto err; @@ -375,8 +367,7 @@ retry: bo->size = bo_size; bo->idle = true; - memclear(create); - create.size = bo_size; + struct drm_i915_gem_create create = { .size = bo_size }; /* All new BOs we get from the kernel are zeroed, so we don't need to * worry about that here. @@ -500,9 +491,6 @@ brw_bo_gem_create_from_name(struct brw_bufmgr *bufmgr, const char *name, unsigned int handle) { struct brw_bo *bo; - int ret; - struct drm_gem_open open_arg; - struct drm_i915_gem_get_tiling get_tiling; /* At the moment most applications only have a few named bo. * For instance, in a DRI client only the render buffers passed @@ -517,9 +505,8 @@ brw_bo_gem_create_from_name(struct brw_bufmgr *bufmgr, goto out; } - memclear(open_arg); - open_arg.name = handle; - ret = drmIoctl(bufmgr->fd, DRM_IOCTL_GEM_OPEN, &open_arg); + struct drm_gem_open open_arg = { .name = handle }; + int ret = drmIoctl(bufmgr->fd, DRM_IOCTL_GEM_OPEN, &open_arg); if (ret != 0) { DBG("Couldn't reference %s handle 0x%08x: %s\n", name, handle, strerror(errno)); @@ -554,8 +541,7 @@ brw_bo_gem_create_from_name(struct brw_bufmgr *bufmgr, _mesa_hash_table_insert(bufmgr->handle_table, &bo->gem_handle, bo); _mesa_hash_table_insert(bufmgr->name_table, &bo->global_name, bo); - memclear(get_tiling); - get_tiling.handle = bo->gem_handle; + struct drm_i915_gem_get_tiling get_tiling = { .handle = bo->gem_handle }; ret = drmIoctl(bufmgr->fd, DRM_IOCTL_I915_GEM_GET_TILING, &get_tiling); if (ret != 0) goto err_unref; @@ -579,8 +565,6 @@ static void bo_free(struct brw_bo *bo) { struct brw_bufmgr *bufmgr = bo->bufmgr; - struct drm_gem_close close; - int ret; if (bo->map_cpu) { VG_NOACCESS(bo->map_cpu, bo->size); @@ -608,9 +592,8 @@ bo_free(struct brw_bo *bo) } /* Close this object */ - memclear(close); - close.handle = bo->gem_handle; - ret = drmIoctl(bufmgr->fd, DRM_IOCTL_GEM_CLOSE, &close); + struct drm_gem_close close = { .handle = bo->gem_handle }; + int ret = drmIoctl(bufmgr->fd, DRM_IOCTL_GEM_CLOSE, &close); if (ret != 0) { DBG("DRM_IOCTL_GEM_CLOSE %d failed (%s): %s\n", bo->gem_handle, bo->name, strerror(errno)); @@ -739,14 +722,12 @@ brw_bo_map_cpu(struct brw_context *brw, struct brw_bo *bo, unsigned flags) assert(bo->cache_coherent || !(flags & MAP_WRITE)); if (!bo->map_cpu) { - struct drm_i915_gem_mmap mmap_arg; - void *map; - DBG("brw_bo_map_cpu: %d (%s)\n", bo->gem_handle, bo->name); - memclear(mmap_arg); - mmap_arg.handle = bo->gem_handle; - mmap_arg.size = bo->size; + struct drm_i915_gem_mmap mmap_arg = { + .handle = bo->gem_handle, + .size = bo->size, + }; int ret = drmIoctl(bufmgr->fd, DRM_IOCTL_I915_GEM_MMAP,
[Mesa-dev] [PATCH] i965: Move perf_debug and WARN_ONCE back to brw_context.h.
These were moved to src/intel/common/gen_debug.h, but they are not common code. They assume that brw_context or gl_context variables exist, named brw or ctx. That isn't remotely true outside of i965. --- src/intel/common/gen_debug.h| 29 - src/mesa/drivers/dri/i965/brw_context.h | 29 + 2 files changed, 29 insertions(+), 29 deletions(-) diff --git a/src/intel/common/gen_debug.h b/src/intel/common/gen_debug.h index e418e3fb166..a069d6b238a 100644 --- a/src/intel/common/gen_debug.h +++ b/src/intel/common/gen_debug.h @@ -100,35 +100,6 @@ extern uint64_t INTEL_DEBUG; dbg_printf(__VA_ARGS__);\ } while(0) -#define perf_debug(...) do { \ - static GLuint msg_id = 0;\ - if (unlikely(INTEL_DEBUG & DEBUG_PERF)) \ - dbg_printf(__VA_ARGS__); \ - if (brw->perf_debug) \ - _mesa_gl_debug(&brw->ctx, &msg_id,\ - MESA_DEBUG_SOURCE_API, \ - MESA_DEBUG_TYPE_PERFORMANCE, \ - MESA_DEBUG_SEVERITY_MEDIUM,\ - __VA_ARGS__); \ -} while(0) - -#define WARN_ONCE(cond, fmt...) do {\ - if (unlikely(cond)) {\ - static bool _warned = false; \ - static GLuint msg_id = 0; \ - if (!_warned) { \ - fprintf(stderr, "WARNING: "); \ - fprintf(stderr, fmt); \ - _warned = true;\ -\ - _mesa_gl_debug(ctx, &msg_id, \ -MESA_DEBUG_SOURCE_API, \ -MESA_DEBUG_TYPE_OTHER, \ -MESA_DEBUG_SEVERITY_HIGH, fmt); \ - } \ - }\ -} while (0) - extern uint64_t intel_debug_flag_for_shader_stage(gl_shader_stage stage); extern void brw_process_intel_debug_variable(void); diff --git a/src/mesa/drivers/dri/i965/brw_context.h b/src/mesa/drivers/dri/i965/brw_context.h index b3d7c6baf8a..c68e2aff348 100644 --- a/src/mesa/drivers/dri/i965/brw_context.h +++ b/src/mesa/drivers/dri/i965/brw_context.h @@ -375,6 +375,35 @@ struct brw_cache { uint32_t next_offset; }; +#define perf_debug(...) do { \ + static GLuint msg_id = 0;\ + if (unlikely(INTEL_DEBUG & DEBUG_PERF)) \ + dbg_printf(__VA_ARGS__); \ + if (brw->perf_debug) \ + _mesa_gl_debug(&brw->ctx, &msg_id,\ + MESA_DEBUG_SOURCE_API, \ + MESA_DEBUG_TYPE_PERFORMANCE, \ + MESA_DEBUG_SEVERITY_MEDIUM,\ + __VA_ARGS__); \ +} while(0) + +#define WARN_ONCE(cond, fmt...) do {\ + if (unlikely(cond)) {\ + static bool _warned = false; \ + static GLuint msg_id = 0; \ + if (!_warned) { \ + fprintf(stderr, "WARNING: "); \ + fprintf(stderr, fmt); \ + _warned = true;\ +\ + _mesa_gl_debug(ctx, &msg_id, \ +MESA_DEBUG_SOURCE_API, \ +MESA_DEBUG_TYPE_OTHER, \ +MESA_DEBUG_SEVERITY_HIGH, fmt); \ + } \ + }\ +} while (0) + /* Considered adding a member to this struct to document which flags * an update might raise so that ordering of the state atoms can be * checked or derived at runtime. Dropped the idea in favor of having -- 2.15.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965: Use C99 struct initializers in brw_bufmgr.c.
Quoting Kenneth Graunke (2017-11-26 10:08:49) > This is cleaner than using a non-standard memclear macro (which does a > memset to 0) and then initializing fields after the fact. We move the > declarations to where we initialized the fields. While we're at it, we > move the declaration of 'ret' that goes with the ioctl, eliminating the > declaration section altogether. > --- > @@ -990,15 +965,15 @@ brw_bo_subdata(struct brw_bo *bo, uint64_t offset, > uint64_t size, const void *data) > { > struct brw_bufmgr *bufmgr = bo->bufmgr; > - struct drm_i915_gem_pwrite pwrite; > - int ret; > > - memclear(pwrite); > - pwrite.handle = bo->gem_handle; > - pwrite.offset = offset; > - pwrite.size = size; > - pwrite.data_ptr = (uint64_t) (uintptr_t) data; > - ret = drmIoctl(bufmgr->fd, DRM_IOCTL_I915_GEM_PWRITE, &pwrite); > + struct drm_i915_gem_pwrite pwrite = { > + .handle = bo->gem_handle, > + .offset = offset, > + .size = size, > + .data_ptr = (uint64_t) (uintptr_t) data, The (uint64_t) here is implicit in the assignment, just (uintptr_t) is required to keep gcc quiet. > + }; > + > + int ret = drmIoctl(bufmgr->fd, DRM_IOCTL_I915_GEM_PWRITE, &pwrite); > if (ret != 0) { >ret = -errno; >DBG("%s:%d: Error writing data to buffer %d: " > @@ -1050,17 +1025,16 @@ int > brw_bo_wait(struct brw_bo *bo, int64_t timeout_ns) > { > struct brw_bufmgr *bufmgr = bo->bufmgr; > - struct drm_i915_gem_wait wait; > - int ret; > > /* If we know it's idle, don't bother with the kernel round trip */ > if (bo->idle && !bo->external) >return 0; > > - memclear(wait); > - wait.bo_handle = bo->gem_handle; > - wait.timeout_ns = timeout_ns; > - ret = drmIoctl(bufmgr->fd, DRM_IOCTL_I915_GEM_WAIT, &wait); > + struct drm_i915_gem_wait wait = { > + .bo_handle = bo->gem_handle, > + .timeout_ns = timeout_ns, > + }; > + int ret = drmIoctl(bufmgr->fd, DRM_IOCTL_I915_GEM_WAIT, &wait); > if (ret == -1) >return -errno; if (ret != 0) should be tidier for gcc, but more about being consistent. > @@ -1330,11 +1299,8 @@ init_cache_buckets(struct brw_bufmgr *bufmgr) > uint32_t > brw_create_hw_context(struct brw_bufmgr *bufmgr) > { > - struct drm_i915_gem_context_create create; > - int ret; > - > - memclear(create); > - ret = drmIoctl(bufmgr->fd, DRM_IOCTL_I915_GEM_CONTEXT_CREATE, &create); > + struct drm_i915_gem_context_create create = { }; = { /* keep valgrind happy */ } ? > + int ret = drmIoctl(bufmgr->fd, DRM_IOCTL_I915_GEM_CONTEXT_CREATE, > &create); > if (ret != 0) { >DBG("DRM_IOCTL_I915_GEM_CONTEXT_CREATE failed: %s\n", strerror(errno)); >return 0; Reviewed-by: Chris Wilson -Chris ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 103814] incorrect dust rendering in hl2 without sisched
https://bugs.freedesktop.org/show_bug.cgi?id=103814 almos changed: What|Removed |Added CC||aaalmo...@gmail.com --- Comment #3 from almos --- I have the same problem on R9 270x with Mesa 17.2.2 -- You are receiving this mail because: You are the assignee for the bug. You are the QA Contact for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 103668] mesa-17.3.0_rc3: FAIL: glsl/tests/cache-test
https://bugs.freedesktop.org/show_bug.cgi?id=103668 --- Comment #7 from erhar...@mailbox.org --- I can confirm that the test passes now. Thanks! -- You are receiving this mail because: You are the QA Contact for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965: Fix Smooth Point Enables.
Wow... Rb On November 26, 2017 01:27:33 Kenneth Graunke wrote: We want to program the 3DSTATE_RASTER field to the gl_context value, not the other way around. Fixes: 13ac46557ab1 (i965: Port Gen8+ 3DSTATE_RASTER state to genxml.) --- src/mesa/drivers/dri/i965/genX_state_upload.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mesa/drivers/dri/i965/genX_state_upload.c b/src/mesa/drivers/dri/i965/genX_state_upload.c index 112f48181b0..2808363e3ad 100644 --- a/src/mesa/drivers/dri/i965/genX_state_upload.c +++ b/src/mesa/drivers/dri/i965/genX_state_upload.c @@ -4322,7 +4322,7 @@ genX(upload_raster)(struct brw_context *brw) raster.CullMode = CULLMODE_NONE; } - point->SmoothFlag = raster.SmoothPointEnable; + raster.SmoothPointEnable = point->SmoothFlag; raster.DXMultisampleRasterizationEnable = _mesa_is_multisample_enabled(ctx); -- 2.15.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] freedreno/ir3: avoid using shr.b for immediate offset inputs
Since this is all happening as a post-optimization fixup, and offsets are generally immediates, we can just do the calculation directly. Signed-off-by: Ilia Mirkin --- Only very mildly tested. Noticed it when looking closely at our shaders, thinking why it tries to shift 0 by a constant. This is why. src/gallium/drivers/freedreno/ir3/ir3_compiler_nir.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/gallium/drivers/freedreno/ir3/ir3_compiler_nir.c b/src/gallium/drivers/freedreno/ir3/ir3_compiler_nir.c index c97df4f1d63..ab326c24aa7 100644 --- a/src/gallium/drivers/freedreno/ir3/ir3_compiler_nir.c +++ b/src/gallium/drivers/freedreno/ir3/ir3_compiler_nir.c @@ -1351,6 +1351,7 @@ emit_intrinsic_atomic_ssbo(struct ir3_context *ctx, nir_intrinsic_instr *intr) ssbo = create_immed(b, const_offset->u32[0]); offset = get_src(ctx, &intr->src[1])[0]; + const_offset = nir_src_as_const_value(intr->src[1]); /* src0 is data (or uvec2(data, compare)) * src1 is offset @@ -1359,7 +1360,10 @@ emit_intrinsic_atomic_ssbo(struct ir3_context *ctx, nir_intrinsic_instr *intr) * Note that nir already multiplies the offset by four */ src0 = get_src(ctx, &intr->src[2])[0]; - src1 = ir3_SHR_B(b, offset, 0, create_immed(b, 2), 0); + if (const_offset) + src1 = create_immed(b, const_offset->u32[0] >> 2); + else + src1 = ir3_SHR_B(b, offset, 0, create_immed(b, 2), 0); src2 = create_collect(b, (struct ir3_instruction*[]){ offset, create_immed(b, 0), -- 2.13.6 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [Freedreno] [PATCH] freedreno/ir3: avoid using shr.b for immediate offset inputs
On Sun, Nov 26, 2017 at 12:08 PM, Ilia Mirkin wrote: > Since this is all happening as a post-optimization fixup, and offsets > are generally immediates, we can just do the calculation directly. > > Signed-off-by: Ilia Mirkin > --- > > Only very mildly tested. Noticed it when looking closely at our shaders, > thinking > why it tries to shift 0 by a constant. This is why. not strictly against this, but a few thoughts: 1) I'm not sure how common in real life it is to access ssbo at hard-coded offsets.. I've noticed the funny shaders like shifting an immed zero by constant too, but figured it wasn't too likely to happen in real life. Although undoing nir's shl w/ our shr might be useful. 2) if it is common, maybe support in ir3_cp to recognize the handful of instructions that are added when lowering nir instructions to ir3 would be more beneficial (ie. ssbo load/store isn't the only one to add shl/shr/etc.. although the instructions added are a small subset of possible instructions so might be sane to make cp a bit more clever.. 3) or, perhaps an even better idea is nir->nir pass that lowers things into ir3 specific nir instructions and then run nir's opt passes again.. that has been kinda on my todo list for a while BR, -R > src/gallium/drivers/freedreno/ir3/ir3_compiler_nir.c | 6 +- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/src/gallium/drivers/freedreno/ir3/ir3_compiler_nir.c > b/src/gallium/drivers/freedreno/ir3/ir3_compiler_nir.c > index c97df4f1d63..ab326c24aa7 100644 > --- a/src/gallium/drivers/freedreno/ir3/ir3_compiler_nir.c > +++ b/src/gallium/drivers/freedreno/ir3/ir3_compiler_nir.c > @@ -1351,6 +1351,7 @@ emit_intrinsic_atomic_ssbo(struct ir3_context *ctx, > nir_intrinsic_instr *intr) > ssbo = create_immed(b, const_offset->u32[0]); > > offset = get_src(ctx, &intr->src[1])[0]; > + const_offset = nir_src_as_const_value(intr->src[1]); > > /* src0 is data (or uvec2(data, compare)) > * src1 is offset > @@ -1359,7 +1360,10 @@ emit_intrinsic_atomic_ssbo(struct ir3_context *ctx, > nir_intrinsic_instr *intr) > * Note that nir already multiplies the offset by four > */ > src0 = get_src(ctx, &intr->src[2])[0]; > - src1 = ir3_SHR_B(b, offset, 0, create_immed(b, 2), 0); > + if (const_offset) > + src1 = create_immed(b, const_offset->u32[0] >> 2); > + else > + src1 = ir3_SHR_B(b, offset, 0, create_immed(b, 2), 0); > src2 = create_collect(b, (struct ir3_instruction*[]){ > offset, > create_immed(b, 0), > -- > 2.13.6 > > ___ > Freedreno mailing list > freedr...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/freedreno ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [Freedreno] [PATCH] freedreno/ir3: avoid using shr.b for immediate offset inputs
On Sun, Nov 26, 2017 at 1:29 PM, Rob Clark wrote: > On Sun, Nov 26, 2017 at 12:08 PM, Ilia Mirkin wrote: >> Since this is all happening as a post-optimization fixup, and offsets >> are generally immediates, we can just do the calculation directly. >> >> Signed-off-by: Ilia Mirkin >> --- >> >> Only very mildly tested. Noticed it when looking closely at our shaders, >> thinking >> why it tries to shift 0 by a constant. This is why. > > not strictly against this, but a few thoughts: > > 1) I'm not sure how common in real life it is to access ssbo at > hard-coded offsets.. I've noticed the funny shaders like shifting an > immed zero by constant too, but figured it wasn't too likely to happen > in real life. Although undoing nir's shl w/ our shr might be useful. I suspect it's moderately common. Any time you don't have a variably-indexed array, that will happen. > > 2) if it is common, maybe support in ir3_cp to recognize the handful > of instructions that are added when lowering nir instructions to ir3 > would be more beneficial (ie. ssbo load/store isn't the only one to > add shl/shr/etc.. although the instructions added are a small subset > of possible instructions so might be sane to make cp a bit more > clever.. > > 3) or, perhaps an even better idea is nir->nir pass that lowers things > into ir3 specific nir instructions and then run nir's opt passes > again.. that has been kinda on my todo list for a while Yeah, that's clearly the right way to go. Having new instructions added after opt is ... not a good idea. (This is why I've never warmed up to the "frontend" vs "backend" concept -- the backend needs the opts just as much.) Happy to drop this until that happens. I just hated seeing shr.b r0.x, 0, c0.x (Where c0.x == 2, of course.) -ilia > > BR, > -R > >> src/gallium/drivers/freedreno/ir3/ir3_compiler_nir.c | 6 +- >> 1 file changed, 5 insertions(+), 1 deletion(-) >> >> diff --git a/src/gallium/drivers/freedreno/ir3/ir3_compiler_nir.c >> b/src/gallium/drivers/freedreno/ir3/ir3_compiler_nir.c >> index c97df4f1d63..ab326c24aa7 100644 >> --- a/src/gallium/drivers/freedreno/ir3/ir3_compiler_nir.c >> +++ b/src/gallium/drivers/freedreno/ir3/ir3_compiler_nir.c >> @@ -1351,6 +1351,7 @@ emit_intrinsic_atomic_ssbo(struct ir3_context *ctx, >> nir_intrinsic_instr *intr) >> ssbo = create_immed(b, const_offset->u32[0]); >> >> offset = get_src(ctx, &intr->src[1])[0]; >> + const_offset = nir_src_as_const_value(intr->src[1]); >> >> /* src0 is data (or uvec2(data, compare)) >> * src1 is offset >> @@ -1359,7 +1360,10 @@ emit_intrinsic_atomic_ssbo(struct ir3_context *ctx, >> nir_intrinsic_instr *intr) >> * Note that nir already multiplies the offset by four >> */ >> src0 = get_src(ctx, &intr->src[2])[0]; >> - src1 = ir3_SHR_B(b, offset, 0, create_immed(b, 2), 0); >> + if (const_offset) >> + src1 = create_immed(b, const_offset->u32[0] >> 2); >> + else >> + src1 = ir3_SHR_B(b, offset, 0, create_immed(b, 2), 0); >> src2 = create_collect(b, (struct ir3_instruction*[]){ >> offset, >> create_immed(b, 0), >> -- >> 2.13.6 >> >> ___ >> Freedreno mailing list >> freedr...@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/freedreno ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 103921] llvmpipe consume all resources on GPU Vega RX (amdgpu driver) with mesa 17.2.4 if disabled IGPU
https://bugs.freedesktop.org/show_bug.cgi?id=103921 Bug ID: 103921 Summary: llvmpipe consume all resources on GPU Vega RX (amdgpu driver) with mesa 17.2.4 if disabled IGPU Product: Mesa Version: 17.2 Hardware: Other OS: All Status: NEW Severity: normal Priority: medium Component: Mesa core Assignee: mesa-dev@lists.freedesktop.org Reporter: mikhail.v.gavri...@gmail.com QA Contact: mesa-dev@lists.freedesktop.org How reproduce: 1. On Fedora 27 compile and install latest amdgpu driver from branch amd-staging-drm-next by this instruction https://fedoraproject.org/wiki/Building_a_custom_kernel Or install mainline vanilla kernel from this repo https://fedoraproject.org/wiki/Kernel_Vanilla_Repositories If you don't make this step Vega RX would be runned under VESA driver which don't have this issue. 2. By default Fedora 27 shipped with mesa 17.2.4, don't update it to 17.4 from CORP repo. 3. Restart computer. 4. Goto BIOS and disable IGPU if it was enabled. Expected result: llvmpipe should work with amdgpu same as with VESA driver without consuming all CPU resources. Interesting note: If go in BIOS and enable Intel GPU graphics without without reconnecting monitor to mother board (means monitor still connected to Radeon VEGA RX card) then this issue is gone. Why? In this case still used llvmpipe and amdgpu driver. Demonstration: https://youtu.be/rFDeR8-_yMo -- You are receiving this mail because: You are the QA Contact for the bug. You are the assignee for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965: Fix Smooth Point Enables.
Reviewed-by: Lionel Landwerlin On 26/11/17 09:27, Kenneth Graunke wrote: We want to program the 3DSTATE_RASTER field to the gl_context value, not the other way around. Fixes: 13ac46557ab1 (i965: Port Gen8+ 3DSTATE_RASTER state to genxml.) --- src/mesa/drivers/dri/i965/genX_state_upload.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mesa/drivers/dri/i965/genX_state_upload.c b/src/mesa/drivers/dri/i965/genX_state_upload.c index 112f48181b0..2808363e3ad 100644 --- a/src/mesa/drivers/dri/i965/genX_state_upload.c +++ b/src/mesa/drivers/dri/i965/genX_state_upload.c @@ -4322,7 +4322,7 @@ genX(upload_raster)(struct brw_context *brw) raster.CullMode = CULLMODE_NONE; } - point->SmoothFlag = raster.SmoothPointEnable; + raster.SmoothPointEnable = point->SmoothFlag; raster.DXMultisampleRasterizationEnable = _mesa_is_multisample_enabled(ctx); ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965: Move perf_debug and WARN_ONCE back to brw_context.h.
Reviewed-by: Lionel Landwerlin On 26/11/17 10:09, Kenneth Graunke wrote: These were moved to src/intel/common/gen_debug.h, but they are not common code. They assume that brw_context or gl_context variables exist, named brw or ctx. That isn't remotely true outside of i965. --- src/intel/common/gen_debug.h| 29 - src/mesa/drivers/dri/i965/brw_context.h | 29 + 2 files changed, 29 insertions(+), 29 deletions(-) diff --git a/src/intel/common/gen_debug.h b/src/intel/common/gen_debug.h index e418e3fb166..a069d6b238a 100644 --- a/src/intel/common/gen_debug.h +++ b/src/intel/common/gen_debug.h @@ -100,35 +100,6 @@ extern uint64_t INTEL_DEBUG; dbg_printf(__VA_ARGS__);\ } while(0) -#define perf_debug(...) do { \ - static GLuint msg_id = 0;\ - if (unlikely(INTEL_DEBUG & DEBUG_PERF)) \ - dbg_printf(__VA_ARGS__); \ - if (brw->perf_debug) \ - _mesa_gl_debug(&brw->ctx, &msg_id,\ - MESA_DEBUG_SOURCE_API, \ - MESA_DEBUG_TYPE_PERFORMANCE, \ - MESA_DEBUG_SEVERITY_MEDIUM,\ - __VA_ARGS__); \ -} while(0) - -#define WARN_ONCE(cond, fmt...) do {\ - if (unlikely(cond)) {\ - static bool _warned = false; \ - static GLuint msg_id = 0; \ - if (!_warned) { \ - fprintf(stderr, "WARNING: "); \ - fprintf(stderr, fmt); \ - _warned = true;\ -\ - _mesa_gl_debug(ctx, &msg_id, \ -MESA_DEBUG_SOURCE_API, \ -MESA_DEBUG_TYPE_OTHER, \ -MESA_DEBUG_SEVERITY_HIGH, fmt); \ - } \ - }\ -} while (0) - extern uint64_t intel_debug_flag_for_shader_stage(gl_shader_stage stage); extern void brw_process_intel_debug_variable(void); diff --git a/src/mesa/drivers/dri/i965/brw_context.h b/src/mesa/drivers/dri/i965/brw_context.h index b3d7c6baf8a..c68e2aff348 100644 --- a/src/mesa/drivers/dri/i965/brw_context.h +++ b/src/mesa/drivers/dri/i965/brw_context.h @@ -375,6 +375,35 @@ struct brw_cache { uint32_t next_offset; }; +#define perf_debug(...) do { \ + static GLuint msg_id = 0;\ + if (unlikely(INTEL_DEBUG & DEBUG_PERF)) \ + dbg_printf(__VA_ARGS__); \ + if (brw->perf_debug) \ + _mesa_gl_debug(&brw->ctx, &msg_id,\ + MESA_DEBUG_SOURCE_API, \ + MESA_DEBUG_TYPE_PERFORMANCE, \ + MESA_DEBUG_SEVERITY_MEDIUM,\ + __VA_ARGS__); \ +} while(0) + +#define WARN_ONCE(cond, fmt...) do {\ + if (unlikely(cond)) {\ + static bool _warned = false; \ + static GLuint msg_id = 0; \ + if (!_warned) { \ + fprintf(stderr, "WARNING: "); \ + fprintf(stderr, fmt); \ + _warned = true;\ +\ + _mesa_gl_debug(ctx, &msg_id, \ +MESA_DEBUG_SOURCE_API, \ +MESA_DEBUG_TYPE_OTHER, \ +MESA_DEBUG_SEVERITY_HIGH, fmt); \ + } \ + }\ +} while (0) + /* Considered adding a member to this struct to document which flags * an update might raise so that ordering of the state atoms can be * checked or derived at runtime. Dropped the idea in favor of having ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] r600/sb: handle jump after target to end of program.
From: Dave Airlie This fixes hangs on cayman with tests/spec/arb_tessellation_shader/execution/trivial-tess-gs_no-gs-inputs.shader_test This has a single if/else in it, and when this peephole activated, it would set the jump target to NULL if there was no instruction after the final POP. This adds a NOP if we get a jump in this case, and seems to fix the hangs, so we have a valid target for the ELSE instruction to go to, instead of 0 (which causes infinite loops). Cc: Signed-off-by: Dave Airlie --- src/gallium/drivers/r600/sb/sb_bc_finalize.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/gallium/drivers/r600/sb/sb_bc_finalize.cpp b/src/gallium/drivers/r600/sb/sb_bc_finalize.cpp index 82826a9092..20ec0dbc2b 100644 --- a/src/gallium/drivers/r600/sb/sb_bc_finalize.cpp +++ b/src/gallium/drivers/r600/sb/sb_bc_finalize.cpp @@ -933,6 +933,9 @@ void bc_finalizer::cf_peephole() { cf_node *c = static_cast(*I); if (c->jump_after_target) { + if (c->jump_target->next == NULL) { + c->jump_target->insert_after(sh.create_cf(CF_OP_NOP)); + } c->jump_target = static_cast(c->jump_target->next); c->jump_after_target = false; } -- 2.14.3 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] r600/sb: handle jump after target to end of program. (v2)
From: Dave Airlie This fixes hangs on cayman with tests/spec/arb_tessellation_shader/execution/trivial-tess-gs_no-gs-inputs.shader_test This has a single if/else in it, and when this peephole activated, it would set the jump target to NULL if there was no instruction after the final POP. This adds a NOP if we get a jump in this case, and seems to fix the hangs, so we have a valid target for the ELSE instruction to go to, instead of 0 (which causes infinite loops). v2: update last_cf correctly. (I had some other patches hide this) Cc: Signed-off-by: Dave Airlie --- src/gallium/drivers/r600/sb/sb_bc_finalize.cpp | 5 + 1 file changed, 5 insertions(+) diff --git a/src/gallium/drivers/r600/sb/sb_bc_finalize.cpp b/src/gallium/drivers/r600/sb/sb_bc_finalize.cpp index 82826a9092..7f1dd0a7a0 100644 --- a/src/gallium/drivers/r600/sb/sb_bc_finalize.cpp +++ b/src/gallium/drivers/r600/sb/sb_bc_finalize.cpp @@ -933,6 +933,11 @@ void bc_finalizer::cf_peephole() { cf_node *c = static_cast(*I); if (c->jump_after_target) { + if (c->jump_target->next == NULL) { + c->jump_target->insert_after(sh.create_cf(CF_OP_NOP)); + if (last_cf == c->jump_target) + last_cf = static_cast(c->jump_target->next); + } c->jump_target = static_cast(c->jump_target->next); c->jump_after_target = false; } -- 2.14.3 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 01/24] mesa: add GL_ARB_gl_spirv boilerplate
On 16/11/17 00:50, Eric Engestrom wrote: On Wednesday, 2017-11-15 14:22:04 +0100, Eduardo Lima Mitev wrote: From: Nicolai Hähnle --- src/mapi/glapi/gen/ARB_gl_spirv.xml | 21 ++ src/mapi/glapi/gen/Makefile.am | 1 + src/mapi/glapi/gen/gl_API.xml | 4 +++ src/mapi/glapi/gen/gl_genexec.py| 1 + src/mesa/Makefile.sources | 2 ++ src/mesa/main/extensions_table.h| 1 + src/mesa/main/glspirv.c | 33 + src/mesa/main/glspirv.h | 51 + src/mesa/main/mtypes.h | 1 + src/mesa/main/tests/dispatch_sanity.cpp | 3 ++ 10 files changed, 118 insertions(+) create mode 100644 src/mapi/glapi/gen/ARB_gl_spirv.xml create mode 100644 src/mesa/main/glspirv.c create mode 100644 src/mesa/main/glspirv.h diff --git a/src/mapi/glapi/gen/ARB_gl_spirv.xml b/src/mapi/glapi/gen/ARB_gl_spirv.xml new file mode 100644 index 000..0dd615480f7 --- /dev/null +++ b/src/mapi/glapi/gen/ARB_gl_spirv.xml @@ -0,0 +1,21 @@ + + + + + + + + + + + + + + + + + + + + + diff --git a/src/mapi/glapi/gen/Makefile.am b/src/mapi/glapi/gen/Makefile.am index 87d8517b7ba..35e37e95a9f 100644 --- a/src/mapi/glapi/gen/Makefile.am +++ b/src/mapi/glapi/gen/Makefile.am @@ -144,6 +144,7 @@ API_XML = \ ARB_framebuffer_object.xml \ ARB_get_program_binary.xml \ ARB_get_texture_sub_image.xml \ + ARB_gl_spirv.xml \ ARB_gpu_shader_fp64.xml \ ARB_gpu_shader_int64.xml \ ARB_gpu_shader5.xml \ For this and the other autotools hunk further down, please add to this patch: 8< diff --git a/src/mapi/glapi/gen/meson.build b/src/mapi/glapi/gen/meson.build index 69ef57bc1460e8f92411..ddd0e856e3e5f8e89b01 100644 --- a/src/mapi/glapi/gen/meson.build +++ b/src/mapi/glapi/gen/meson.build @@ -52,6 +52,7 @@ api_xml_files = files( 'ARB_framebuffer_object.xml', 'ARB_get_program_binary.xml', 'ARB_get_texture_sub_image.xml', + 'ARB_gl_spirv.xml', 'ARB_gpu_shader_fp64.xml', 'ARB_gpu_shader_int64.xml', 'ARB_gpu_shader5.xml', diff --git a/src/mesa/meson.build b/src/mesa/meson.build index b839fd0298189263d632..32135446999b5812a984 100644 --- a/src/mesa/meson.build +++ b/src/mesa/meson.build @@ -162,6 +162,8 @@ files_libmesa_common = files( 'main/getstring.c', 'main/glformats.c', 'main/glformats.h', + 'main/glspirv.c', + 'main/glspirv.h', 'main/glthread.c', 'main/glthread.h', 'main/glheader.h', >8 With these meson fixes included this is: Reviewed-by: Timothy Arceri diff --git a/src/mapi/glapi/gen/gl_API.xml b/src/mapi/glapi/gen/gl_API.xml index eb1d9b83b27..d3594cfe195 100644 --- a/src/mapi/glapi/gen/gl_API.xml +++ b/src/mapi/glapi/gen/gl_API.xml @@ -8400,6 +8400,10 @@ http://www.w3.org/2001/XInclude"/> + + +http://www.w3.org/2001/XInclude"/> + diff --git a/src/mapi/glapi/gen/gl_genexec.py b/src/mapi/glapi/gen/gl_genexec.py index b7b22328ff8..aaff9f230b3 100644 --- a/src/mapi/glapi/gen/gl_genexec.py +++ b/src/mapi/glapi/gen/gl_genexec.py @@ -77,6 +77,7 @@ header = """/** #include "main/eval.h" #include "main/externalobjects.h" #include "main/get.h" +#include "main/glspirv.h" #include "main/feedback.h" #include "main/fog.h" #include "main/fbobject.h" diff --git a/src/mesa/Makefile.sources b/src/mesa/Makefile.sources index 6da1e3fef9d..e9680bf004c 100644 --- a/src/mesa/Makefile.sources +++ b/src/mesa/Makefile.sources @@ -118,6 +118,8 @@ MAIN_FILES = \ main/getstring.c \ main/glformats.c \ main/glformats.h \ + main/glspirv.c \ + main/glspirv.h \ main/glthread.c \ main/glthread.h \ main/glheader.h \ diff --git a/src/mesa/main/extensions_table.h b/src/mesa/main/extensions_table.h index 5b66e7d30df..ab15ceb9414 100644 --- a/src/mesa/main/extensions_table.h +++ b/src/mesa/main/extensions_table.h @@ -72,6 +72,7 @@ EXT(ARB_framebuffer_object , ARB_framebuffer_object EXT(ARB_framebuffer_sRGB, EXT_framebuffer_sRGB , GLL, GLC, x , x , 1998) EXT(ARB_get_program_binary , dummy_true , GLL, GLC, x , x , 2010) EXT(ARB_get_texture_sub_image , dummy_true , GLL, GLC, x , x , 2014) +EXT(ARB_gl_spirv, ARB_gl_spirv , x, GLC, x , x , 2016) EXT(ARB_gpu_shader5 , ARB_gpu_shader5 , x , GLC, x , x , 2010) EXT(ARB_gpu_shader_fp64 , ARB_gpu_shader_fp64 , x , GLC, x , x , 2010) EXT(ARB_gpu_shader_int64, ARB_gpu_shader_int64 , x , GLC, x , x , 2015) diff --git a/src/mesa/main/glspirv.c b/src/mesa/main/glspir
Re: [Mesa-dev] [PATCH mesa 0/7] remove upstreamed specs
Eric Engestrom writes: > On Wednesday, 2017-11-22 12:28:17 -0800, Eric Anholt wrote: >> Jordan Justen writes: >> >> > On 2017-11-22 09:59:34, Eric Engestrom wrote: >> >> A recent thread [1] made me check our local specs to see which ones were >> >> upstream. This series removes the ones that are identical upstream >> >> (modulo "TBD" extension numbers in some cases). >> > >> > While I don't have too strong of an opinion on it, I think we should >> > keep a copy of Mesa specs that are in the upstream registry. >> > >> > I think it makes sense to send a patch to mesa-dev for new Mesa specs >> > or changes to Mesa specs. Having a copy in docs/specs works well for >> > that. >> >> The downside is that that process means that we'll inevitably keep stale >> or divergent copies in Mesa, when the canonical location for GL specs is >> Khronos. We do have a reasonable process for modifying Khronos's specs >> now, which we didn't before. > > Exactly, our local copies are not the authority, Khronos is. > > Changes to specs should be sent to Khronos, on the relevant repo, by > creating a pull request like I've now done for the specs I mentioned > in the cover letter: > https://github.com/KhronosGroup/EGL-Registry/pull/36 > https://github.com/KhronosGroup/OpenGL-Registry/pull/132 > https://github.com/KhronosGroup/OpenGL-Registry/pull/133 > https://github.com/KhronosGroup/OpenGL-Registry/pull/134 > https://github.com/KhronosGroup/OpenGL-Registry/pull/135 > https://github.com/KhronosGroup/OpenGL-Registry/pull/136 > https://github.com/KhronosGroup/OpenGL-Registry/pull/137 > >> >> I think we should get all our specs out and into the Khronos. > > Ack; should I let the specs authors do this themselves, or push them for > them? If you have the time and energy to upstream them, I think that would be quite welcome. I'm sure a lot of these are basically forgotten and people's original motivation for the extensions has faded away. signature.asc Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH mesa 11/16] broadcom: use NDEBUG to guard asserts
Eric Engestrom writes: > Signed-off-by: Eric Engestrom This and the previous patch are: Reviewed-by: Eric Anholt signature.asc Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] mesa/st: glsl_to_tgsi: Dissolve arrays who's elements are only accessed directly
Gert Wollny writes: > Array who's elements are only accessed directly are replaced by the > according number of temporary registers. By doing so the otherwise > reserved register range becomes subject to further optimizations like > copy propagation and register merging. > > Thanks to the resulting reduced register pressure this patch makes > the piglits > > spec/glsl-1.50/execution - > variable-indexing/vs-output-array-vec3-index-wr-before-gs > geometry/max-input-components > > pass on r600 (barts) where they would fail before with a "GPR limit exceeded" > error. > > Signed-off-by: Gert Wollny > --- > No further changes are observed with the piglit shader subset. > > PS: Submitter has no write access to mesa-git This looks like something that should be done with (at most) a small change to opt_array_splitting.cpp, rather than reimplementing it here. signature.asc Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 16/24] mesa: Implement glSpecializeShaderARB
I suspect this patch doesn't compile. I think pEntryPoint is meant to be passed as an argument to this function. On 16/11/17 00:22, Eduardo Lima Mitev wrote: From: Nicolai Hähnle v2: use gl_spirv_validation instead of spirv_to_nir. This method just validates the shader. The conversion to NIR will happen later, during linking. (Alejandro Piñeiro) v3: Use gl_shader_spirv_data struct to store the SPIR-V data. (Eduardo Lima) --- src/mesa/main/glspirv.c | 107 1 file changed, 107 insertions(+) diff --git a/src/mesa/main/glspirv.c b/src/mesa/main/glspirv.c index 390fa9e6984..fe954b05c7b 100644 --- a/src/mesa/main/glspirv.c +++ b/src/mesa/main/glspirv.c @@ -24,6 +24,10 @@ #include "glspirv.h" #include "errors.h" +#include "shaderobj.h" + +#include "compiler/nir/nir.h" +#include "compiler/spirv/nir_spirv.h" #include "util/u_atomic.h" @@ -120,4 +124,107 @@ _mesa_SpecializeShaderARB(GLuint shader, const GLuint *pConstantIndex, const GLuint *pConstantValue) { + GET_CURRENT_CONTEXT(ctx); + struct gl_shader *sh; + bool has_entry_point; + struct nir_spirv_specialization *spec_entries = NULL; + + if (!ctx->Extensions.ARB_gl_spirv) { + _mesa_error(ctx, GL_INVALID_OPERATION, "glSpecializeShaderARB"); + return; + } + + sh = _mesa_lookup_shader_err(ctx, shader, "glSpecializeShaderARB"); + if (!sh) + return; + + if (!sh->SpirVBinary) { + _mesa_error(ctx, GL_INVALID_OPERATION, + "glSpecializeShaderARB(not SPIR-V)"); + return; + } + + if (sh->CompileStatus) { + _mesa_error(ctx, GL_INVALID_OPERATION, + "glSpecializeShaderARB(already specialized)"); + return; + } + + struct gl_shader_spirv_data *spirv_data = sh->spirv_data; + assert(spirv_data); + + /* From the GL_ARB_gl_spirv spec: +* +*"The OpenGL API expects the SPIR-V module to have already been +* validated, and can return an error if it discovers anything invalid +* in the module. An invalid SPIR-V module is allowed to result in +* undefined behavior." +* +* However, the following errors still need to be detected (from the same +* spec): +* +*"INVALID_VALUE is generated if does not name a valid +* entry point for . +* +* INVALID_VALUE is generated if any element of +* refers to a specialization constant that does not exist in the +* shader module contained in ." +* +* We cannot flag those errors a-priori because detecting them requires +* parsing the module. However, flagging them during specialization is okay, +* since it makes no difference in terms of application-visible state. +*/ + spec_entries = calloc(sizeof(*spec_entries), numSpecializationConstants); + + for (unsigned i = 0; i < numSpecializationConstants; ++i) { + spec_entries[i].id = pConstantIndex[i]; + spec_entries[i].data32 = pConstantValue[i]; + spec_entries[i].defined_on_module = false; + } + + has_entry_point = + gl_spirv_validation((uint32_t *)&spirv_data->SpirVModule->Binary[0], + spirv_data->SpirVModule->Length / 4, + spec_entries, numSpecializationConstants, + sh->Stage, pEntryPoint); + + /* See previous spec comment */ + if (!has_entry_point) { + _mesa_error(ctx, GL_INVALID_VALUE, + "glSpecializeShaderARB(\"%s\" is not a valid entry point" + " for shader)", pEntryPoint); + goto end; + } + + for (unsigned i = 0; i < numSpecializationConstants; ++i) { + if (spec_entries[i].defined_on_module == false) { + _mesa_error(ctx, GL_INVALID_VALUE, + "glSpecializeShaderARB(constant \"%i\" does not exist " + "in shader)", spec_entries[i].id); + goto end; + } + } + + spirv_data->SpirVEntryPoint = ralloc_strdup(spirv_data, pEntryPoint); + + /* Note that we didn't make a real compilation of the module (spirv_to_nir), +* but just checked some error conditions. Real "compilation" will be done +* later, upon linking. +*/ + sh->CompileStatus = compile_success; + + spirv_data->NumSpecializationConstants = numSpecializationConstants; + spirv_data->SpecializationConstantsIndex = + rzalloc_array_size(spirv_data, sizeof(GLuint), + numSpecializationConstants); + spirv_data->SpecializationConstantsValue = + rzalloc_array_size(spirv_data, sizeof(GLuint), + numSpecializationConstants); + for (unsigned i = 0; i < numSpecializationConstants; ++i) { + spirv_data->SpecializationConstantsIndex[i] = pConstantIndex[i]; + spirv_data->SpecializationConstantsValue[i] = pConstantValue[i]; + } + + end: + free(spec_entries); } _
Re: [Mesa-dev] [PATCH] mesa/st: glsl_to_tgsi: Dissolve arrays who's elements are only accessed directly
On 27/11/17 13:01, Eric Anholt wrote: Gert Wollny writes: Array who's elements are only accessed directly are replaced by the according number of temporary registers. By doing so the otherwise reserved register range becomes subject to further optimizations like copy propagation and register merging. Thanks to the resulting reduced register pressure this patch makes the piglits spec/glsl-1.50/execution - variable-indexing/vs-output-array-vec3-index-wr-before-gs geometry/max-input-components pass on r600 (barts) where they would fail before with a "GPR limit exceeded" error. Signed-off-by: Gert Wollny --- No further changes are observed with the piglit shader subset. PS: Submitter has no write access to mesa-git This looks like something that should be done with (at most) a small change to opt_array_splitting.cpp, rather than reimplementing it here. Not sure how much it matters since I also have a NIR version of this waiting review on the list, so a GLSL IR version is probably not going to help with much. Either way there will be duplication. Ultimately it would be nice to see the following, although I feel its probably unlikely: 1. The ARB_gl_spriv work result partial support for a NIR based GLSL linker. 2. Remaining (non spriv) NIR GLSL linker support added. 3. Gallium drivers switch to nir->tgsi (probably solves the soft fp64 r600 support issues). 4. i915 gets a NIR backend. 5. Drop any unrequired GLSL IR linker support and now obsolete GLSL IR opts. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 21/24] mesa/glspirv: Create gl_linked_shader objects for a SPIR-V program
On 16/11/17 00:22, Eduardo Lima Mitev wrote: --- src/mesa/main/glspirv.c | 51 ++--- 1 file changed, 48 insertions(+), 3 deletions(-) diff --git a/src/mesa/main/glspirv.c b/src/mesa/main/glspirv.c index 86f1d221cc9..9332764f152 100644 --- a/src/mesa/main/glspirv.c +++ b/src/mesa/main/glspirv.c @@ -29,6 +29,8 @@ #include "compiler/nir/nir.h" #include "compiler/spirv/nir_spirv.h" +#include "program/program.h" + #include "util/u_atomic.h" void @@ -117,14 +119,57 @@ _mesa_spirv_shader_binary(struct gl_context *ctx, } } +/** + * This is the equivalent to compiler/glsl/linker.cpp::link_shaders() + * but for SPIR-V programs. + * + * This method just creates the gl_linked_shader structs with a reference to + * the SPIR-V data collected during previous steps. + * + * The real linking happens later in the driver-specifc call LinkShader(). + * This is so backends can implement different linking strategies for + * SPIR-V programs. + */ void _mesa_spirv_link_shaders(struct gl_context *ctx, struct gl_shader_program *prog) { - /* @TODO: This is a placeholder for the equivalent of -* compiler/glsl/linker.cpp::link_shaders() but for SPIR-V. -*/ prog->data->LinkStatus = linking_success; prog->data->Validated = false; + + for (unsigned i = 0; i < prog->NumShaders; i++) { The GL api allows multiple shaders to be attached to the same stage of a program. This code assumes a 1-to-1 mapping of shaders to stages. I don't see anything in the spec that says its an error to attach more than one shader to a single stage, but this looks like a probable and might require a spec bug to be filed. With this existing code we will leak gl_linked_shader and never unref spirv_data should multiple shaders be attached to the same stage. + struct gl_shader *shader = prog->Shaders[i]; + gl_shader_stage shader_type = shader->Stage; + + assert(shader->spirv_data); + + struct gl_linked_shader *linked = rzalloc(NULL, struct gl_linked_shader); + linked->Stage = shader_type; + + /* Create program and attach it to the linked shader */ + struct gl_program *gl_prog = + ctx->Driver.NewProgram(ctx, +_mesa_shader_stage_to_program(shader_type), +prog->Name, false); + if (!gl_prog) { + prog->data->LinkStatus = linking_failure; + _mesa_delete_linked_shader(ctx, linked); + return; + } + + _mesa_reference_shader_program_data(ctx, + &gl_prog->sh.data, + prog->data); + + /* Don't use _mesa_reference_program() just take ownership */ + linked->Program = gl_prog; + + /* Reference the SPIR-V data from shader to the linked shader */ + _mesa_shader_spirv_data_reference(&linked->spirv_data, +shader->spirv_data); + + prog->_LinkedShaders[shader_type] = linked; + prog->data->linked_stages |= 1 << shader_type; + } } void GLAPIENTRY ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] st/glsl_to_mesa: make use of driver_cache_blob with the disk cache
driver_cache_blob was introduced with the i965 disk cache, it allows us to simplify the cache a little and possibly offers some minor speed improvements since we load the GLSL metadata and TGSI from disk in one pass. Using driver_cache_blob should also make it straight forward to implement binary support for ARB_get_program_binary in gallium. --- src/compiler/glsl/shader_cache.cpp | 2 +- src/mesa/main/mtypes.h | 7 - src/mesa/state_tracker/st_program.h | 3 - src/mesa/state_tracker/st_shader_cache.c | 329 ++- 4 files changed, 110 insertions(+), 231 deletions(-) diff --git a/src/compiler/glsl/shader_cache.cpp b/src/compiler/glsl/shader_cache.cpp index 1034e2b196b..5e1682b351c 100644 --- a/src/compiler/glsl/shader_cache.cpp +++ b/src/compiler/glsl/shader_cache.cpp @@ -141,21 +141,21 @@ bool shader_cache_read_program_metadata(struct gl_context *ctx, struct gl_shader_program *prog) { /* Fixed function programs generated by Mesa are not cached. So don't * try to read metadata for them from the cache. */ if (prog->Name == 0) return false; struct disk_cache *cache = ctx->Cache; - if (!cache || prog->data->skip_cache) + if (!cache) return false; /* Include bindings when creating sha1. These bindings change the resulting * binary so they are just as important as the shader source. */ char *buf = ralloc_strdup(NULL, "vb: "); prog->AttributeBindings->iterate(create_binding_str, &buf); ralloc_strcat(&buf, "fb: "); prog->FragDataBindings->iterate(create_binding_str, &buf); ralloc_strcat(&buf, "fbi: "); diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h index 79d5fcad3e0..2babe4229be 100644 --- a/src/mesa/main/mtypes.h +++ b/src/mesa/main/mtypes.h @@ -2881,27 +2881,20 @@ struct gl_shader_program_data * * From the ARB_get_program_binary spec: * *"A successful call to ProgramBinary will reset all uniform *variables to their initial values. The initial value is either *the value of the variable's initializer as specified in the *original shader source, or 0 if no initializer was present. */ union gl_constant_value *UniformDataDefaults; - /* TODO: This used by Gallium drivers to skip the cache on tgsi fallback. -* All structures (gl_program, uniform storage, etc) will get recreated -* even though we have already loaded them from cache. We should instead -* switch to storing the GLSL metadata and TGSI IR in a single cache item -* like the i965 driver does with NIR. -*/ - bool skip_cache; GLboolean Validated; /** List of all active resources after linking. */ struct gl_program_resource *ProgramResourceList; unsigned NumProgramResourceList; enum gl_link_status LinkStatus; /**< GL_LINK_STATUS */ GLchar *InfoLog; unsigned Version; /**< GLSL version used for linking */ diff --git a/src/mesa/state_tracker/st_program.h b/src/mesa/state_tracker/st_program.h index 6049fba517b..0e6c8e00c6e 100644 --- a/src/mesa/state_tracker/st_program.h +++ b/src/mesa/state_tracker/st_program.h @@ -143,23 +143,20 @@ struct st_fragment_program struct gl_program Base; struct pipe_shader_state tgsi; struct glsl_to_tgsi_visitor* glsl_to_tgsi; struct ati_fragment_shader *ati_fs; uint64_t affected_states; /**< ST_NEW_* flags to mark dirty when binding */ /* used when bypassing glsl_to_tgsi: */ struct gl_shader_program *shader_program; struct st_fp_variant *variants; - - /** SHA1 hash of linked tgsi shader program, used for on-disk cache */ - unsigned char sha1[20]; }; /** Vertex program variant key */ struct st_vp_variant_key { struct st_context *st; /**< variants are per-context */ boolean passthrough_edgeflags; diff --git a/src/mesa/state_tracker/st_shader_cache.c b/src/mesa/state_tracker/st_shader_cache.c index 2e9b7cc6708..a5e33133b3c 100644 --- a/src/mesa/state_tracker/st_shader_cache.c +++ b/src/mesa/state_tracker/st_shader_cache.c @@ -33,100 +33,90 @@ static void write_stream_out_to_cache(struct blob *blob, struct pipe_shader_state *tgsi) { blob_write_bytes(blob, &tgsi->stream_output, sizeof(tgsi->stream_output)); } static void write_tgsi_to_cache(struct blob *blob, struct pipe_shader_state *tgsi, -struct st_context *st, unsigned char *sha1, -unsigned num_tokens) +struct gl_program *prog, unsigned num_tokens) { blob_write_uint32(blob, num_tokens); blob_write_bytes(blob, tgsi->tokens, num_tokens * sizeof(struct tgsi_token)); - disk_cache_put(st->ctx->Cache, sha1, blob->data, blob->size, NULL); + prog->driver_cache_blob = ralloc_size(NULL, blob->size); + memcpy(prog->driver_cache_blob, blob->data, blob->size);
Re: [Mesa-dev] [PATCH 4/5] Android: add Wundef to the build flags
Acked-by: Tapani Pälli On 11/24/2017 04:25 PM, Emil Velikov wrote: From: Emil Velikov The compiler will warns us when we're misusing undefined macros. Note: this will trigger a bunch of warnings, which will be resolved ASAP. Cc: Tapani Pälli Cc: Rob Herring Signed-off-by: Emil Velikov --- Android.common.mk | 1 + 1 file changed, 1 insertion(+) diff --git a/Android.common.mk b/Android.common.mk index 5671c1c1a59..544d813c02d 100644 --- a/Android.common.mk +++ b/Android.common.mk @@ -66,6 +66,7 @@ LOCAL_CFLAGS += \ -DHAVE_DLADDR \ -DHAVE_DL_ITERATE_PHDR \ -DMAJOR_IN_SYSMACROS \ + -Wundef \ -fvisibility=hidden \ -Wno-sign-compare ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 5/5] Android: copy -fno*math* options from the autotools build
Acked-by: Tapani Pälli On 11/24/2017 04:25 PM, Emil Velikov wrote: From: Emil Velikov Add -fno-math-errno and -fno-trapping-math to the build. Mesa does not depend on the functionality provided, thus this should result in slightly faster code and smaller binaries. Cc: Tapani Pälli Cc: Rob Herring Signed-off-by: Emil Velikov --- Gents, please do some basic runtime checks. This should work fine OOTB although one can never be too sure considering the different compiler/C runtime used on Android. --- Android.common.mk | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Android.common.mk b/Android.common.mk index 544d813c02d..d97974b1c84 100644 --- a/Android.common.mk +++ b/Android.common.mk @@ -68,6 +68,8 @@ LOCAL_CFLAGS += \ -DMAJOR_IN_SYSMACROS \ -Wundef \ -fvisibility=hidden \ + -fno-math-errno \ + -fno-trapping-math \ -Wno-sign-compare LOCAL_CPPFLAGS += \ ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] st/glsl_to_mesa: make use of driver_cache_blob with the disk cache
Whoops subject should be st/glsl_to_tgsi fixed locally. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 12/17] etnaviv: GC7000: State changes for HALTI3..5
Hello Christian, On Sat, Nov 25, 2017 at 05:10:36PM +0100, Christian Gmeiner wrote: > 2017-11-18 10:44 GMT+01:00 Wladimir J. van der Laan : > > Update state objects to add new state, and emit function to emit new > > state. > > > > Signed-off-by: Wladimir J. van der Laan > > Reviewed-by: Christian Gmeiner > > --- > > src/gallium/drivers/etnaviv/etnaviv_emit.c | 246 > > +++-- > > src/gallium/drivers/etnaviv/etnaviv_internal.h | 4 + > > src/gallium/drivers/etnaviv/etnaviv_state.c| 35 +++- > > src/gallium/drivers/etnaviv/etnaviv_zsa.c | 3 +- > > 4 files changed, 216 insertions(+), 72 deletions(-) > > > > Conceptually unchanged since v1, but needed serious rebase. > > > > I had to fix a conflict during git am in etnaviv_emit.c reagrding > ETNA_DIRTY_VERTEX_BUFFERS. I assume the conflict is with "etnaviv: Emit vertex buffers consecutively" - which makes a change to vertex buffer emission, while this moves the surrounding code around. Need to preserve changes to 0064C/00650 and 00680/006A0: --- a/src/gallium/drivers/etnaviv/etnaviv_emit.c +++ b/src/gallium/drivers/etnaviv/etnaviv_emit.c @@ -391,18 +391,18 @@ etna_emit_state(struct etna_context *ctx) /*00644*/ EMIT_STATE_RELOC(FE_INDEX_STREAM_BASE_ADDR, &ctx->index_buffer.FE_INDEX_STREAM_BASE_ADDR); /*00648*/ EMIT_STATE(FE_INDEX_STREAM_CONTROL, ctx->index_buffer.FE_INDEX_STREAM_CONTROL); } - if (likely(dirty & (ETNA_DIRTY_VERTEX_BUFFERS))) { + if (likely((dirty & (ETNA_DIRTY_VERTEX_BUFFERS) && ctx->specs.stream_count == 1))) { /*0064C*/ EMIT_STATE_RELOC(FE_VERTEX_STREAM_BASE_ADDR, &ctx->vertex_buffer.cvb[0].FE_VERTEX_STREAM_BASE_ADDR); /*00650*/ EMIT_STATE(FE_VERTEX_STREAM_CONTROL, ctx->vertex_buffer.cvb[0].FE_VERTEX_STREAM_CONTROL); } if (likely(dirty & (ETNA_DIRTY_INDEX_BUFFER))) { /*00674*/ EMIT_STATE(FE_PRIMITIVE_RESTART_INDEX, ctx->index_buffer.FE_PRIMITIVE_RESTART_INDEX); } - if (likely(dirty & (ETNA_DIRTY_VERTEX_BUFFERS))) { - for (int x = 1; x < ctx->vertex_buffer.count; ++x) { + if (likely((dirty & (ETNA_DIRTY_VERTEX_BUFFERS)) && ctx->specs.stream_count > 1)) { + for (int x = 0; x < ctx->vertex_buffer.count; ++x) { /*00680*/ EMIT_STATE_RELOC(FE_VERTEX_STREAMS_BASE_ADDR(x), &ctx->vertex_buffer.cvb[x].FE_VERTEX_STREAM_BASE_ADDR); } - for (int x = 1; x < ctx->vertex_buffer.count; ++x) { + for (int x = 0; x < ctx->vertex_buffer.count; ++x) { if (ctx->vertex_buffer.cvb[x].FE_VERTEX_STREAM_BASE_ADDR.bo) { /*006A0*/ EMIT_STATE(FE_VERTEX_STREAMS_CONTROL(x), ctx->vertex_buffer.cvb[x].FE_VERTEX_STREAM_CONTROL); } If it helps I can send a rebased patch. Regards, Wladimir ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] r600/cayman: looks like cmpxchg moved to Z
From: Dave Airlie On cayman it appears the cmp component is now in Z. Fixes: arb_shader_image_load_store-dead-fragments on cayman. Signed-off-by: Dave Airlie --- src/gallium/drivers/r600/r600_shader.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/gallium/drivers/r600/r600_shader.c b/src/gallium/drivers/r600/r600_shader.c index 66638f29399..b688aeebdb5 100644 --- a/src/gallium/drivers/r600/r600_shader.c +++ b/src/gallium/drivers/r600/r600_shader.c @@ -8185,9 +8185,12 @@ static int tgsi_atomic_op_rat(struct r600_shader_ctx *ctx) return r; memset(&alu, 0, sizeof(struct r600_bytecode_alu)); - alu.op = ALU_OP1_MOV; + alu.op = ALU_OP1_MOV; alu.dst.sel = ctx->thread_id_gpr; - alu.dst.chan = 3; + if (ctx->bc->chip_class == CAYMAN) + alu.dst.chan = 2; + else + alu.dst.chan = 3; alu.dst.write = 1; r600_bytecode_src(&alu.src[0], &ctx->src[2], 0); alu.last = 1; -- 2.14.3 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/2] i965/tcs: support gl_PatchVerticesIn as a system value from SPIR-V
This patch is still missing a review, any takers? This is required to pass one of the SPIR-V CTS tests. Iago On Thu, 2017-11-16 at 08:53 +0100, Iago Toral Quiroga wrote: > We currently handle this by lowering it to a uniform for gen8+ but > the SPIR-V path generates this as a system value, so handle that > case as well. > --- > src/mesa/drivers/dri/i965/brw_tcs.c | 9 - > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/src/mesa/drivers/dri/i965/brw_tcs.c > b/src/mesa/drivers/dri/i965/brw_tcs.c > index 4424efea4f0..b07b11f485d 100644 > --- a/src/mesa/drivers/dri/i965/brw_tcs.c > +++ b/src/mesa/drivers/dri/i965/brw_tcs.c > @@ -296,7 +296,14 @@ brw_tcs_populate_key(struct brw_context *brw, > per_patch_slots |= prog->info.patch_outputs_written; > } > > - if (devinfo->gen < 8 || !tcp) > + /* For GLSL, gen8+ lowers gl_PatchVerticesIn to a uniform, > however > +* the SPIR-V path always lowers it to a system value. > +*/ > + bool reads_patch_vertices_as_system_value = > + tcp && (tcp->program.nir->info.system_values_read & > + BITFIELD64_BIT(SYSTEM_VALUE_VERTICES_IN)); > + > + if (devinfo->gen < 8 || !tcp || > reads_patch_vertices_as_system_value) > key->input_vertices = brw->ctx.TessCtrlProgram.patch_vertices; > key->outputs_written = per_vertex_slots; > key->patch_outputs_written = per_patch_slots; ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev