From a1aed20842ed3e62015b870ee87441190f0a5350 Mon Sep 17 00:00:00 2001 From: Karol Herbst Date: Thu, 7 Aug 2025 13:46:15 +0200 Subject: [PATCH] rusticl: implement cl_ext_immutable_memory_objects MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reviewed-by: Seán de Búrca Part-of: --- docs/features.txt | 2 +- docs/relnotes/new_features.txt | 1 + src/gallium/frontends/rusticl/api/kernel.rs | 15 ++- src/gallium/frontends/rusticl/api/memory.rs | 127 +++++++++++++++++-- src/gallium/frontends/rusticl/core/device.rs | 3 +- src/gallium/frontends/rusticl/core/memory.rs | 4 + 6 files changed, 138 insertions(+), 14 deletions(-) diff --git a/docs/features.txt b/docs/features.txt index 591a20f6993..57647ce6c1c 100644 --- a/docs/features.txt +++ b/docs/features.txt @@ -885,7 +885,7 @@ Rusticl extensions: cl_ext_image_raw10_raw12 not started cl_ext_image_requirements_info not started cl_ext_image_unorm_int_2_101010 DONE - cl_ext_immutable_memory_objects not started + cl_ext_immutable_memory_objects DONE cl_ext_migrate_memobject not started cl_arm_non_uniform_work_group_size not started cl_arm_shared_virtual_memory in progress (nvc0) diff --git a/docs/relnotes/new_features.txt b/docs/relnotes/new_features.txt index 95d46ebe36d..4fd61ef9e13 100644 --- a/docs/relnotes/new_features.txt +++ b/docs/relnotes/new_features.txt @@ -4,3 +4,4 @@ VK_EXT_mutable_descriptor_type on panvk/v9+ GL_KHR_robustness on v3d VK_ARM_shader_core_builtins on panvk VK_KHR_shader_untyped_pointers on anv +cl_ext_immutable_memory_objects diff --git a/src/gallium/frontends/rusticl/api/kernel.rs b/src/gallium/frontends/rusticl/api/kernel.rs index 0cc04f76254..21904399615 100644 --- a/src/gallium/frontends/rusticl/api/kernel.rs +++ b/src/gallium/frontends/rusticl/api/kernel.rs @@ -432,6 +432,14 @@ fn set_kernel_arg( } else { // SAFETY: as above let buffer = Buffer::arc_from_raw(unsafe { *ptr })?; + // We are required to prevent mutable access to immutable memory objects, + // however no explicit error code has been specified yet. + if arg.kind == KernelArgType::MemGlobal + && bit_check(buffer.flags, CL_MEM_IMMUTABLE_EXT) + { + return Err(CL_INVALID_ARG_VALUE); + } + KernelArgValue::Buffer(Arc::downgrade(&buffer)) } } @@ -446,8 +454,13 @@ fn set_kernel_arg( // of CL_MEM_WRITE_ONLY or if the image argument is declared with the write_only // qualifier and arg_value refers to an image object created with cl_mem_flags // of CL_MEM_READ_ONLY. + // We are required to prevent mutable access to immutable memory objects, however no + // explicit error code has been specified yet. if arg.kind == KernelArgType::Texture && bit_check(img.flags, CL_MEM_WRITE_ONLY) - || arg.kind == KernelArgType::Image && bit_check(img.flags, CL_MEM_READ_ONLY) + || arg.kind == KernelArgType::Image + && bit_check(img.flags, CL_MEM_READ_ONLY | CL_MEM_IMMUTABLE_EXT) + || arg.kind == KernelArgType::RWImage + && bit_check(img.flags, CL_MEM_IMMUTABLE_EXT) { return Err(CL_INVALID_ARG_VALUE); } diff --git a/src/gallium/frontends/rusticl/api/memory.rs b/src/gallium/frontends/rusticl/api/memory.rs index 6ba9411c5c4..596ef023e55 100644 --- a/src/gallium/frontends/rusticl/api/memory.rs +++ b/src/gallium/frontends/rusticl/api/memory.rs @@ -33,11 +33,18 @@ enum MemFlagValidationType { /// For when the memory object is imported. Imported, + + /// For when he memory object is sub-allocated from an existing memory object. + SubAlloc, } fn validate_mem_flags(flags: cl_mem_flags, validation: MemFlagValidationType) -> CLResult<()> { let mut valid_flags = cl_bitfield::from( - CL_MEM_READ_WRITE | CL_MEM_WRITE_ONLY | CL_MEM_READ_ONLY | CL_MEM_KERNEL_READ_AND_WRITE, + CL_MEM_READ_WRITE + | CL_MEM_WRITE_ONLY + | CL_MEM_READ_ONLY + | CL_MEM_KERNEL_READ_AND_WRITE + | CL_MEM_IMMUTABLE_EXT, ); if validation != MemFlagValidationType::Imported { @@ -72,6 +79,9 @@ fn validate_mem_flags_create( let host_read_write_group = cl_bitfield::from(CL_MEM_HOST_WRITE_ONLY | CL_MEM_HOST_READ_ONLY | CL_MEM_HOST_NO_ACCESS); + let write_flags_group = + cl_bitfield::from(CL_MEM_READ_WRITE | CL_MEM_WRITE_ONLY | CL_MEM_HOST_WRITE_ONLY); + if (flags & read_write_group).count_ones() > 1 || (flags & alloc_host_group).count_ones() > 1 || (flags & copy_host_group).count_ones() > 1 @@ -80,6 +90,23 @@ fn validate_mem_flags_create( return Err(CL_INVALID_VALUE); } + // CL_INVALID_VALUE if CL_MEM_IMMUTABLE_EXT is set in flags and CL_MEM_READ_WRITE, + // CL_MEM_WRITE_ONLY, or CL_MEM_HOST_WRITE_ONLY is set in flags + if flags & cl_mem_flags::from(CL_MEM_IMMUTABLE_EXT) != 0 { + if flags & write_flags_group != 0 { + return Err(CL_INVALID_VALUE); + } + + // CL_INVALID_VALUE .. if CL_MEM_IMMUTABLE_EXT is set in flags and none of the following + // conditions are met: + // CL_MEM_COPY_HOST_PTR or CL_MEM_USE_HOST_PTR is set in flags + // the image is being created from another memory object (buffer or image) + // properties includes an external memory handle + if validation == MemFlagValidationType::Plain && flags & copy_host_group == 0 { + return Err(CL_INVALID_VALUE); + } + } + validate_mem_flags(flags, validation) } @@ -108,7 +135,9 @@ fn validate_map_flags(m: &MemBase, map_flags: cl_mem_flags) -> CLResult<()> { bit_check(map_flags, CL_MAP_READ) || // or if buffer has been created with CL_MEM_HOST_READ_ONLY or CL_MEM_HOST_NO_ACCESS and // CL_MAP_WRITE or CL_MAP_WRITE_INVALIDATE_REGION is set in map_flags. - bit_check(m.flags, CL_MEM_HOST_READ_ONLY | CL_MEM_HOST_NO_ACCESS) && + // CL_INVALID_OPERATION if buffer was created with CL_MEM_IMMUTABLE_EXT in flags and + // CL_MAP_WRITE or CL_MAP_WRITE_INVALIDATE_REGION is set in map_flags. + bit_check(m.flags, CL_MEM_HOST_READ_ONLY | CL_MEM_HOST_NO_ACCESS | CL_MEM_IMMUTABLE_EXT) && bit_check(map_flags, CL_MAP_WRITE | CL_MAP_WRITE_INVALIDATE_REGION) { return Err(CL_INVALID_OPERATION); @@ -119,8 +148,11 @@ fn validate_map_flags(m: &MemBase, map_flags: cl_mem_flags) -> CLResult<()> { fn filter_image_access_flags(flags: cl_mem_flags) -> cl_mem_flags { flags - & (CL_MEM_READ_WRITE | CL_MEM_WRITE_ONLY | CL_MEM_READ_ONLY | CL_MEM_KERNEL_READ_AND_WRITE) - as cl_mem_flags + & (CL_MEM_READ_WRITE + | CL_MEM_WRITE_ONLY + | CL_MEM_READ_ONLY + | CL_MEM_KERNEL_READ_AND_WRITE + | CL_MEM_IMMUTABLE_EXT) as cl_mem_flags } fn inherit_mem_flags(mut flags: cl_mem_flags, mem: &MemBase) -> cl_mem_flags { @@ -222,7 +254,9 @@ fn validate_matching_buffer_flags(mem: &MemBase, flags: cl_mem_flags) -> CLResul // or if mem_object was created with CL_MEM_HOST_READ_ONLY and flags specifies CL_MEM_HOST_WRITE_ONLY bit_check(mem.flags, CL_MEM_HOST_READ_ONLY) && bit_check(flags, CL_MEM_HOST_WRITE_ONLY) || // or if mem_object was created with CL_MEM_HOST_NO_ACCESS and_flags_ specifies CL_MEM_HOST_READ_ONLY or CL_MEM_HOST_WRITE_ONLY. - bit_check(mem.flags, CL_MEM_HOST_NO_ACCESS) && bit_check(flags, CL_MEM_HOST_READ_ONLY | CL_MEM_HOST_WRITE_ONLY) + bit_check(mem.flags, CL_MEM_HOST_NO_ACCESS) && bit_check(flags, CL_MEM_HOST_READ_ONLY | CL_MEM_HOST_WRITE_ONLY) || + // CL_INVALID_VALUE if buffer was created with CL_MEM_IMMUTABLE_EXT and flags specifies CL_MEM_READ_WRITE, CL_MEM_WRITE_ONLY, or CL_MEM_HOST_WRITE_ONLY. + bit_check(mem.flags, CL_MEM_IMMUTABLE_EXT) && bit_check(flags, CL_MEM_READ_WRITE | CL_MEM_WRITE_ONLY | CL_MEM_HOST_WRITE_ONLY) { return Err(CL_INVALID_VALUE); } @@ -377,7 +411,7 @@ fn create_sub_buffer( validate_matching_buffer_flags(&b, flags)?; flags = inherit_mem_flags(flags, &b); - validate_mem_flags_create(flags, MemFlagValidationType::Plain)?; + validate_mem_flags_create(flags, MemFlagValidationType::SubAlloc)?; let (offset, size) = match buffer_create_type { CL_BUFFER_CREATE_TYPE_REGION => { @@ -462,6 +496,7 @@ fn validate_image_desc( image_desc: *const cl_image_desc, host_ptr: *mut ::std::os::raw::c_void, elem_size: usize, + flags: cl_mem_flags, devs: &[&Device], ) -> CLResult<(cl_image_desc, Option)> { // CL_INVALID_IMAGE_DESCRIPTOR if values specified in image_desc are not valid @@ -544,6 +579,13 @@ fn validate_image_desc( } { return Err(CL_INVALID_OPERATION); } + + // CL_INVALID_VALUE if CL_MEM_IMMUTABLE_EXT is not set in flags and mem_object was created + // with CL_MEM_IMMUTABLE_EXT + if bit_check(p.flags, CL_MEM_IMMUTABLE_EXT) && !bit_check(flags, CL_MEM_IMMUTABLE_EXT) { + return Err(CL_INVALID_VALUE); + } + Some(p) } else { None @@ -817,7 +859,8 @@ fn create_image_with_properties( .ok_or(CL_INVALID_OPERATION)?; let (format, elem_size) = validate_image_format(image_format)?; - let (desc, parent) = validate_image_desc(image_desc, host_ptr, elem_size.into(), &c.devs)?; + let (desc, parent) = + validate_image_desc(image_desc, host_ptr, elem_size.into(), flags, &c.devs)?; // validate host_ptr before merging flags validate_host_ptr(host_ptr, flags)?; @@ -830,7 +873,14 @@ fn create_image_with_properties( flags = CL_MEM_READ_WRITE.into(); } - validate_mem_flags_create(flags, MemFlagValidationType::Plain)?; + validate_mem_flags_create( + flags, + if parent.is_some() { + MemFlagValidationType::SubAlloc + } else { + MemFlagValidationType::Plain + }, + )?; let filtered_flags = filter_image_access_flags(flags); // CL_IMAGE_FORMAT_NOT_SUPPORTED if there are no devices in context that support image_format. @@ -1176,7 +1226,12 @@ fn enqueue_write_buffer( // CL_INVALID_OPERATION if clEnqueueWriteBuffer is called on buffer which has been created with // CL_MEM_HOST_READ_ONLY or CL_MEM_HOST_NO_ACCESS. - if bit_check(b.flags, CL_MEM_HOST_READ_ONLY | CL_MEM_HOST_NO_ACCESS) { + // CL_INVALID_OPERATION if clEnqueueWriteBuffer is called on buffer which has been created with + // CL_MEM_IMMUTABLE_EXT. + if bit_check( + b.flags, + CL_MEM_HOST_READ_ONLY | CL_MEM_HOST_NO_ACCESS | CL_MEM_IMMUTABLE_EXT, + ) { return Err(CL_INVALID_OPERATION); } @@ -1218,6 +1273,11 @@ fn enqueue_copy_buffer( return Err(CL_INVALID_CONTEXT); } + // CL_INVALID_OPERATION if dst_buffer was created with CL_MEM_IMMUTABLE_EXT. + if bit_check(dst.flags, CL_MEM_IMMUTABLE_EXT) { + return Err(CL_INVALID_OPERATION); + } + // CL_INVALID_VALUE if src_offset, dst_offset, size, src_offset + size or dst_offset + size // require accessing elements outside the src_buffer and dst_buffer buffer objects respectively. if src_offset + size > src.size || dst_offset + size > dst.size { @@ -1396,7 +1456,12 @@ fn enqueue_write_buffer_rect( // CL_INVALID_OPERATION if clEnqueueWriteBufferRect is called on buffer which has been created // with CL_MEM_HOST_READ_ONLY or CL_MEM_HOST_NO_ACCESS. - if bit_check(buf.flags, CL_MEM_HOST_READ_ONLY | CL_MEM_HOST_NO_ACCESS) { + // CL_INVALID_OPERATION if clEnqueueWriteBufferRect is called on buffer which has been created + // with CL_MEM_IMMUTABLE_EXT. + if bit_check( + buf.flags, + CL_MEM_HOST_READ_ONLY | CL_MEM_HOST_NO_ACCESS | CL_MEM_IMMUTABLE_EXT, + ) { return Err(CL_INVALID_OPERATION); } @@ -1516,6 +1581,11 @@ fn enqueue_copy_buffer_rect( return Err(CL_INVALID_VALUE); } + // CL_INVALID_OPERATION if dst_buffer was created with CL_MEM_IMMUTABLE_EXT. + if bit_check(dst.flags, CL_MEM_IMMUTABLE_EXT) { + return Err(CL_INVALID_OPERATION); + } + let r = unsafe { CLVec::from_raw(region) }; let src_ori = unsafe { CLVec::from_raw(src_origin) }; let dst_ori = unsafe { CLVec::from_raw(dst_origin) }; @@ -1644,6 +1714,11 @@ fn enqueue_fill_buffer( let b = Buffer::arc_from_raw(buffer)?; let evs = event_list_from_cl(&q, num_events_in_wait_list, event_wait_list)?; + // CL_INVALID_OPERATION if buffer was created with CL_MEM_IMMUTABLE_EXT. + if bit_check(b.flags, CL_MEM_IMMUTABLE_EXT) { + return Err(CL_INVALID_OPERATION); + } + // CL_INVALID_VALUE if offset or offset + size require accessing elements outside the buffer // buffer object respectively. if offset + size > b.size { @@ -1851,7 +1926,12 @@ fn enqueue_write_image( // CL_INVALID_OPERATION if clEnqueueWriteImage is called on image which has been created with // CL_MEM_HOST_READ_ONLY or CL_MEM_HOST_NO_ACCESS. - if bit_check(i.flags, CL_MEM_HOST_READ_ONLY | CL_MEM_HOST_NO_ACCESS) { + // CL_INVALID_OPERATION if clEnqueueWriteImage is called on image which has been created with + // CL_MEM_IMMUTABLE_EXT. + if bit_check( + i.flags, + CL_MEM_HOST_READ_ONLY | CL_MEM_HOST_NO_ACCESS | CL_MEM_IMMUTABLE_EXT, + ) { return Err(CL_INVALID_OPERATION); } @@ -1930,6 +2010,11 @@ fn enqueue_copy_image( return Err(CL_INVALID_CONTEXT); } + // CL_INVALID_OPERATION if dst_image was created with CL_MEM_IMMUTABLE_EXT. + if bit_check(dst_image.flags, CL_MEM_IMMUTABLE_EXT) { + return Err(CL_INVALID_OPERATION); + } + // CL_IMAGE_FORMAT_MISMATCH if src_image and dst_image do not use the same image format. if src_image.image_format != dst_image.image_format { return Err(CL_IMAGE_FORMAT_MISMATCH); @@ -1998,6 +2083,11 @@ fn enqueue_fill_image( return Err(CL_INVALID_CONTEXT); } + // CL_INVALID_OPERATION if image was created with CL_MEM_IMMUTABLE_EXT. + if bit_check(i.flags, CL_MEM_IMMUTABLE_EXT) { + return Err(CL_INVALID_OPERATION); + } + // Not supported with depth stencil or msaa images. if i.image_format.image_channel_order == CL_DEPTH_STENCIL || i.image_desc.num_samples > 0 { return Err(CL_INVALID_OPERATION); @@ -2067,6 +2157,11 @@ fn enqueue_copy_buffer_to_image( return Err(CL_INVALID_CONTEXT); } + // CL_INVALID_OPERATION if dst_image was created with CL_MEM_IMMUTABLE_EXT. + if bit_check(dst.flags, CL_MEM_IMMUTABLE_EXT) { + return Err(CL_INVALID_OPERATION); + } + // Not supported with depth stencil or msaa images. if dst.image_format.image_channel_order == CL_DEPTH_STENCIL || dst.image_desc.num_samples > 0 { return Err(CL_INVALID_OPERATION); @@ -2127,6 +2222,11 @@ fn enqueue_copy_image_to_buffer( return Err(CL_INVALID_CONTEXT); } + // CL_INVALID_OPERATION if dst_buffer was created with CL_MEM_IMMUTABLE_EXT. + if bit_check(dst.flags, CL_MEM_IMMUTABLE_EXT) { + return Err(CL_INVALID_OPERATION); + } + // Not supported with depth stencil or msaa images. if src.image_format.image_channel_order == CL_DEPTH_STENCIL || src.image_desc.num_samples > 0 { return Err(CL_INVALID_OPERATION); @@ -2395,6 +2495,11 @@ pub fn svm_alloc( return Err(CL_INVALID_VALUE); } + // flags contains CL_MEM_IMMUTABLE_EXT. + if bit_check(flags, CL_MEM_IMMUTABLE_EXT) { + return Err(CL_INVALID_VALUE); + } + // When alignment is 0, the size of the largest supported type is used. // In the case of the full profile, that's `long16`. let alignment = NonZeroU64::new(alignment.into()) diff --git a/src/gallium/frontends/rusticl/core/device.rs b/src/gallium/frontends/rusticl/core/device.rs index 4d5d1a6bd16..94de6360b6c 100644 --- a/src/gallium/frontends/rusticl/core/device.rs +++ b/src/gallium/frontends/rusticl/core/device.rs @@ -286,7 +286,7 @@ impl DeviceBase { cl_mem_type_to_texture_target(t), PIPE_BIND_SAMPLER_VIEW, ) { - flags |= CL_MEM_READ_ONLY; + flags |= CL_MEM_READ_ONLY | CL_MEM_IMMUTABLE_EXT; } // TODO: cl_khr_srgb_image_writes @@ -623,6 +623,7 @@ impl DeviceBase { add_ext(1, 0, 0, "cl_khr_spirv_no_integer_wrap_decoration"); add_ext(1, 0, 0, "cl_khr_spirv_queries"); add_ext(1, 0, 0, "cl_khr_suggested_local_work_size"); + add_ext(1, 0, 0, "cl_ext_immutable_memory_objects"); add_feat(2, 0, 0, "__opencl_c_integer_dot_product_input_4x8bit"); add_feat( diff --git a/src/gallium/frontends/rusticl/core/memory.rs b/src/gallium/frontends/rusticl/core/memory.rs index 945635790fc..ab1527a8e43 100644 --- a/src/gallium/frontends/rusticl/core/memory.rs +++ b/src/gallium/frontends/rusticl/core/memory.rs @@ -837,6 +837,8 @@ impl MemBase { } else { let res_type = if bit_check(flags, CL_MEM_ALLOC_HOST_PTR) { ResourceType::Staging + } else if bit_check(flags, CL_MEM_IMMUTABLE_EXT) { + ResourceType::Immutable } else { ResourceType::Normal }; @@ -960,6 +962,8 @@ impl MemBase { let res_type = if bit_check(flags, CL_MEM_ALLOC_HOST_PTR) { ResourceType::Staging + } else if bit_check(flags, CL_MEM_IMMUTABLE_EXT) { + ResourceType::Immutable } else { ResourceType::Normal };