rusticl: implement cl_ext_immutable_memory_objects

Reviewed-by: Seán de Búrca <leftmostcat@gmail.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/36630>

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)

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

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);
                 }

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<Mem>)> {
     // 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())

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(

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
         };