Re: [Mesa-dev] [PATCH 15/15] radeonsi/nir: port some bindless and sampler code from TGSI
On 08/09/18 07:37, Marek Olšák wrote: Ping I had additional patches for bindless somewhere, tbh it's hard to know if this is all thats needed since we can't test but if you think this is correct feel free to push. It's not like it's going to break anything and we can always fix it later when core NIR finally gets bindless support. On Wed, Aug 29, 2018 at 4:13 PM, Marek Olšák wrote: From: Marek Olšák These might be all missing changes for bindless textures. --- src/gallium/drivers/radeonsi/si_shader_nir.c | 80 1 file changed, 50 insertions(+), 30 deletions(-) diff --git a/src/gallium/drivers/radeonsi/si_shader_nir.c b/src/gallium/drivers/radeonsi/si_shader_nir.c index 5d6280b80f7..87ca0161b45 100644 --- a/src/gallium/drivers/radeonsi/si_shader_nir.c +++ b/src/gallium/drivers/radeonsi/si_shader_nir.c @@ -890,64 +890,84 @@ si_nir_lookup_interp_param(struct ac_shader_abi *abi, } static LLVMValueRef si_nir_load_sampler_desc(struct ac_shader_abi *abi, unsigned descriptor_set, unsigned base_index, unsigned constant_index, LLVMValueRef dynamic_index, enum ac_descriptor_type desc_type, bool image, bool write, bool bindless) { struct si_shader_context *ctx = si_shader_context_from_abi(abi); + const struct tgsi_shader_info *info = >shader->selector->info; LLVMBuilderRef builder = ctx->ac.builder; - LLVMValueRef list = LLVMGetParam(ctx->main_fn, ctx->param_samplers_and_images); - LLVMValueRef index; + unsigned const_index = base_index + constant_index; + bool dcc_off = write; + + /* TODO: images_store and images_atomic are not set */ + if (!dynamic_index && image && + (info->images_store | info->images_atomic) & (1 << const_index)) + dcc_off = true; assert(!descriptor_set); + assert(!image || desc_type == AC_DESC_IMAGE || desc_type == AC_DESC_BUFFER); - dynamic_index = dynamic_index ? dynamic_index : ctx->ac.i32_0; - index = LLVMBuildAdd(builder, dynamic_index, -LLVMConstInt(ctx->ac.i32, base_index + constant_index, false), -""); + if (bindless) { + LLVMValueRef list = + LLVMGetParam(ctx->main_fn, ctx->param_bindless_samplers_and_images); - if (image) { - assert(desc_type == AC_DESC_IMAGE || desc_type == AC_DESC_BUFFER); - assert(base_index + constant_index < ctx->num_images); + /* dynamic_index is the bindless handle */ + if (image) { + return si_load_image_desc(ctx, list, dynamic_index, desc_type, + dcc_off, true); + } + + /* Since bindless handle arithmetic can contain an unsigned integer +* wraparound and si_load_sampler_desc assumes there isn't any, +* use GEP without "inbounds" (inside ac_build_pointer_add) +* to prevent incorrect code generation and hangs. +*/ + dynamic_index = LLVMBuildMul(ctx->ac.builder, dynamic_index, +LLVMConstInt(ctx->i32, 2, 0), ""); + list = ac_build_pointer_add(>ac, list, dynamic_index); + return si_load_sampler_desc(ctx, list, ctx->i32_0, desc_type); + } + + unsigned num_slots = image ? ctx->num_images : ctx->num_samplers; + assert(const_index < num_slots); - if (dynamic_index) - index = si_llvm_bound_index(ctx, index, ctx->num_images); + LLVMValueRef list = LLVMGetParam(ctx->main_fn, ctx->param_samplers_and_images); + LLVMValueRef index = LLVMConstInt(ctx->ac.i32, const_index, false); + + if (dynamic_index) { + index = LLVMBuildAdd(builder, index, dynamic_index, ""); + + /* From the GL_ARB_shader_image_load_store extension spec: +* +*If a shader performs an image load, store, or atomic +*operation using an image variable declared as an array, +*and if the index used to select an individual element is +*negative or greater than or equal to the size of the +*array, the results of the operation are undefined but may +*not lead to termination. +*/ + index = si_llvm_bound_index(ctx, index, num_slots); + } + if (image) { index = LLVMBuildSub(ctx->ac.builder, LLVMConstInt(ctx->i32, SI_NUM_IMAGES - 1, 0), index, ""); - - /* TODO: be smarter about when we use dcc_off */ - return si_load_image_desc(ctx, list, index,
Re: [Mesa-dev] [PATCH 15/15] radeonsi/nir: port some bindless and sampler code from TGSI
Ping On Wed, Aug 29, 2018 at 4:13 PM, Marek Olšák wrote: > From: Marek Olšák > > These might be all missing changes for bindless textures. > --- > src/gallium/drivers/radeonsi/si_shader_nir.c | 80 > 1 file changed, 50 insertions(+), 30 deletions(-) > > diff --git a/src/gallium/drivers/radeonsi/si_shader_nir.c > b/src/gallium/drivers/radeonsi/si_shader_nir.c > index 5d6280b80f7..87ca0161b45 100644 > --- a/src/gallium/drivers/radeonsi/si_shader_nir.c > +++ b/src/gallium/drivers/radeonsi/si_shader_nir.c > @@ -890,64 +890,84 @@ si_nir_lookup_interp_param(struct ac_shader_abi *abi, > } > > static LLVMValueRef > si_nir_load_sampler_desc(struct ac_shader_abi *abi, > unsigned descriptor_set, unsigned base_index, > unsigned constant_index, LLVMValueRef dynamic_index, > enum ac_descriptor_type desc_type, bool image, > bool write, bool bindless) > { > struct si_shader_context *ctx = si_shader_context_from_abi(abi); > + const struct tgsi_shader_info *info = >shader->selector->info; > LLVMBuilderRef builder = ctx->ac.builder; > - LLVMValueRef list = LLVMGetParam(ctx->main_fn, > ctx->param_samplers_and_images); > - LLVMValueRef index; > + unsigned const_index = base_index + constant_index; > + bool dcc_off = write; > + > + /* TODO: images_store and images_atomic are not set */ > + if (!dynamic_index && image && > + (info->images_store | info->images_atomic) & (1 << const_index)) > + dcc_off = true; > > assert(!descriptor_set); > + assert(!image || desc_type == AC_DESC_IMAGE || desc_type == > AC_DESC_BUFFER); > > - dynamic_index = dynamic_index ? dynamic_index : ctx->ac.i32_0; > - index = LLVMBuildAdd(builder, dynamic_index, > -LLVMConstInt(ctx->ac.i32, base_index + > constant_index, false), > -""); > + if (bindless) { > + LLVMValueRef list = > + LLVMGetParam(ctx->main_fn, > ctx->param_bindless_samplers_and_images); > > - if (image) { > - assert(desc_type == AC_DESC_IMAGE || desc_type == > AC_DESC_BUFFER); > - assert(base_index + constant_index < ctx->num_images); > + /* dynamic_index is the bindless handle */ > + if (image) { > + return si_load_image_desc(ctx, list, dynamic_index, > desc_type, > + dcc_off, true); > + } > + > + /* Since bindless handle arithmetic can contain an unsigned > integer > +* wraparound and si_load_sampler_desc assumes there isn't > any, > +* use GEP without "inbounds" (inside ac_build_pointer_add) > +* to prevent incorrect code generation and hangs. > +*/ > + dynamic_index = LLVMBuildMul(ctx->ac.builder, dynamic_index, > +LLVMConstInt(ctx->i32, 2, 0), > ""); > + list = ac_build_pointer_add(>ac, list, dynamic_index); > + return si_load_sampler_desc(ctx, list, ctx->i32_0, desc_type); > + } > + > + unsigned num_slots = image ? ctx->num_images : ctx->num_samplers; > + assert(const_index < num_slots); > > - if (dynamic_index) > - index = si_llvm_bound_index(ctx, index, > ctx->num_images); > + LLVMValueRef list = LLVMGetParam(ctx->main_fn, > ctx->param_samplers_and_images); > + LLVMValueRef index = LLVMConstInt(ctx->ac.i32, const_index, false); > + > + if (dynamic_index) { > + index = LLVMBuildAdd(builder, index, dynamic_index, ""); > + > + /* From the GL_ARB_shader_image_load_store extension spec: > +* > +*If a shader performs an image load, store, or atomic > +*operation using an image variable declared as an array, > +*and if the index used to select an individual element is > +*negative or greater than or equal to the size of the > +*array, the results of the operation are undefined but > may > +*not lead to termination. > +*/ > + index = si_llvm_bound_index(ctx, index, num_slots); > + } > > + if (image) { > index = LLVMBuildSub(ctx->ac.builder, > LLVMConstInt(ctx->i32, SI_NUM_IMAGES - > 1, 0), > index, ""); > - > - /* TODO: be smarter about when we use dcc_off */ > - return si_load_image_desc(ctx, list, index, desc_type, write, > bindless); > + return si_load_image_desc(ctx, list, index, desc_type, > dcc_off, false); > } > > -
[Mesa-dev] [PATCH 15/15] radeonsi/nir: port some bindless and sampler code from TGSI
From: Marek Olšák These might be all missing changes for bindless textures. --- src/gallium/drivers/radeonsi/si_shader_nir.c | 80 1 file changed, 50 insertions(+), 30 deletions(-) diff --git a/src/gallium/drivers/radeonsi/si_shader_nir.c b/src/gallium/drivers/radeonsi/si_shader_nir.c index 5d6280b80f7..87ca0161b45 100644 --- a/src/gallium/drivers/radeonsi/si_shader_nir.c +++ b/src/gallium/drivers/radeonsi/si_shader_nir.c @@ -890,64 +890,84 @@ si_nir_lookup_interp_param(struct ac_shader_abi *abi, } static LLVMValueRef si_nir_load_sampler_desc(struct ac_shader_abi *abi, unsigned descriptor_set, unsigned base_index, unsigned constant_index, LLVMValueRef dynamic_index, enum ac_descriptor_type desc_type, bool image, bool write, bool bindless) { struct si_shader_context *ctx = si_shader_context_from_abi(abi); + const struct tgsi_shader_info *info = >shader->selector->info; LLVMBuilderRef builder = ctx->ac.builder; - LLVMValueRef list = LLVMGetParam(ctx->main_fn, ctx->param_samplers_and_images); - LLVMValueRef index; + unsigned const_index = base_index + constant_index; + bool dcc_off = write; + + /* TODO: images_store and images_atomic are not set */ + if (!dynamic_index && image && + (info->images_store | info->images_atomic) & (1 << const_index)) + dcc_off = true; assert(!descriptor_set); + assert(!image || desc_type == AC_DESC_IMAGE || desc_type == AC_DESC_BUFFER); - dynamic_index = dynamic_index ? dynamic_index : ctx->ac.i32_0; - index = LLVMBuildAdd(builder, dynamic_index, -LLVMConstInt(ctx->ac.i32, base_index + constant_index, false), -""); + if (bindless) { + LLVMValueRef list = + LLVMGetParam(ctx->main_fn, ctx->param_bindless_samplers_and_images); - if (image) { - assert(desc_type == AC_DESC_IMAGE || desc_type == AC_DESC_BUFFER); - assert(base_index + constant_index < ctx->num_images); + /* dynamic_index is the bindless handle */ + if (image) { + return si_load_image_desc(ctx, list, dynamic_index, desc_type, + dcc_off, true); + } + + /* Since bindless handle arithmetic can contain an unsigned integer +* wraparound and si_load_sampler_desc assumes there isn't any, +* use GEP without "inbounds" (inside ac_build_pointer_add) +* to prevent incorrect code generation and hangs. +*/ + dynamic_index = LLVMBuildMul(ctx->ac.builder, dynamic_index, +LLVMConstInt(ctx->i32, 2, 0), ""); + list = ac_build_pointer_add(>ac, list, dynamic_index); + return si_load_sampler_desc(ctx, list, ctx->i32_0, desc_type); + } + + unsigned num_slots = image ? ctx->num_images : ctx->num_samplers; + assert(const_index < num_slots); - if (dynamic_index) - index = si_llvm_bound_index(ctx, index, ctx->num_images); + LLVMValueRef list = LLVMGetParam(ctx->main_fn, ctx->param_samplers_and_images); + LLVMValueRef index = LLVMConstInt(ctx->ac.i32, const_index, false); + + if (dynamic_index) { + index = LLVMBuildAdd(builder, index, dynamic_index, ""); + + /* From the GL_ARB_shader_image_load_store extension spec: +* +*If a shader performs an image load, store, or atomic +*operation using an image variable declared as an array, +*and if the index used to select an individual element is +*negative or greater than or equal to the size of the +*array, the results of the operation are undefined but may +*not lead to termination. +*/ + index = si_llvm_bound_index(ctx, index, num_slots); + } + if (image) { index = LLVMBuildSub(ctx->ac.builder, LLVMConstInt(ctx->i32, SI_NUM_IMAGES - 1, 0), index, ""); - - /* TODO: be smarter about when we use dcc_off */ - return si_load_image_desc(ctx, list, index, desc_type, write, bindless); + return si_load_image_desc(ctx, list, index, desc_type, dcc_off, false); } - assert(base_index + constant_index < ctx->num_samplers); - - if (dynamic_index) - index = si_llvm_bound_index(ctx, index, ctx->num_samplers); - index = LLVMBuildAdd(ctx->ac.builder, index, LLVMConstInt(ctx->i32,