Fix too strict interleaved/unique layout check by stiepan · Pull Request #9788 · NVIDIA/cccl

stiepan · 2026-07-10T14:11:11Z

Description

closes

The __has_interleaved_stride_order utility blocks callers from writing into (used in copy(mdspan_dst, mdspan_src) or fill_bytes(mdspan)) mdspans that have multiple indices mapped to the same memory offset. However, the check is too strict, blocking valid cases that arise from slicing a tensor with step > 1.

For instance

a = np.zeros((2, 5), dtype=np.uint8)[:, ::2]
a.shape == (2, 3)
a.strides == (5, 2)

# the old check would return true, because 2 * 3 > 5 (smallest strides * extent > next stride)
# incorrectly concluding the layout is overlapping

The new check, is still a conservative approximation (never misses interleaved layout but may flag too much as interleaved),
but returns less false postives and should handle all layouts that are a result of permuting, slicing, reshaping or broadcasting dense layout.

Checklist

New or existing tests cover these changes.
The documentation is up to date with these changes.

Signed-off-by: Kamil Tokarski <ktokarski@nvidia.com>

copy-pr-bot · 2026-07-10T14:11:15Z

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

Jacobfaib · 2026-07-10T14:16:21Z

@mhoemmen could you take a look at this?

fbusato

The improvement makes sense at high-level, but I'm still not 100% sure about the implications of having situations like the example you provided. Namely, a row that "starts" before the previous one ends. It is not overlapping at index level, but still hard to predict potentially issues.

fbusato · 2026-07-10T21:53:03Z

 #  include <cuda/experimental/__copy_bytes/mdspan_to_raw_tensor.cuh>
 #  include <cuda/experimental/__copy_bytes/types.cuh>

+#  include <iostream>


please remove this header

fbusato · 2026-07-10T21:57:42Z

    {
-      const auto __extent = static_cast<__stride_t>(__extents[__i]);
-      if (__extent * cudax::__abs_integer(__strides[__i]) > cudax::__abs_integer(__strides[__i + 1]))
+      const __stride_t extent = __extents[__i];


Suggested change

const __stride_t extent = __extents[__i];

const auto __extent = static_cast<__stride_t>(__extents[__i]);

fbusato · 2026-07-10T21:58:22Z

+        {
+          return true;
+        }
+        // note that slicing a layout, while it may increase _abs_stride,


please use 120 columns width

fbusato · 2026-07-10T21:59:43Z

    const auto __tensor   = cudax::__to_raw_tensor(__mdspan);
    const auto __sorted   = cudax::__sort_by_stride(__tensor);
-    using __stride_t      = decltype(__sorted.__strides[0]);
+    using __stride_t      = ::cuda::std::remove_cvref_t<decltype(__sorted.__strides[0])>;


missing header

fbusato · 2026-07-10T22:00:29Z

+        }
+        // note that slicing a layout, while it may increase _abs_stride,
+        // cannot increase dimension's contribution to max_dist
+        __max_dist += (extent - 1) * __abs_stride;


Suggested change

__max_dist += (extent - 1) * __abs_stride;

__max_dist += (__extent - 1) * __abs_stride;

fbusato · 2026-07-10T22:04:25Z

+    // max_offset = sum((e_j - 1) * s_j for j in range(i) if s_j > 0)
+    // min_offset = sum((e_j - 1) * s_j for j in range(i) if s_j < 0)
+    // max_dist = max_offset - min_offset = sum((e_j - 1) * |s_j| for j in range(i))
+    __stride_t __max_dist = 0;


this should be the extent type because it needs to be representable by required_span_size()

Arbitrary layout mappings aren't necessarily strided, so I'm not sure if speaking of "strides" would generally make sense.

I think I understand now. Is the goal to test specifically layout_stride_relaxed::mapping to see whether it's unique? If so, then this test might make sense.

mhoemmen · 2026-07-12T22:21:11Z

+//! @brief Conservative check for non-unique/interleaved layout.
 //!
-//! Sorts modes by ascending absolute stride, then verifies two conditions:
-//! 1. No mode with extent > 1 has stride == 0 (broadcast)
-//! 2. No mode's span (extent * |stride|) exceeds the next mode's |stride|
+//! A layout is non-unique/interleaved if there exists two different indices
+//! that map to the same element (i.e. `dot(idx1, strides) == dot(idx2, strides)`).


I don't quite understand the purpose of this check. Is the goal to check the preconditions of the strides in the layout_stride::mapping(extents, array) constructor? Is the goal to check the strides of a general is_strided() == true mapping? Or is the goal to check whether an arbitrary mapping is unique?

If the goal is to check whether an arbitrary mapping is unique, then why not use the is_unique() query?

I think I understand now. Is the goal to test specifically layout_stride_relaxed::mapping to see whether it's unique? If so, then this test might make sense.

mhoemmen · 2026-07-12T22:26:45Z

+//! vectorizing) applied to a dense layout. In general case, it may
+//! incorrectly return true for non-interleaved/unique layouts.
 //!
 //! @param[in] __mdspan Mdspan view to inspect


If I have an mdspan with arbitrary layout, then I can just ask its layout mapping if is_unique() is true.

Layout mappings can implement that check conservatively by returning false, but It's up to the layout mapping to decide that. For the special case of layout_stride_relaxed, the right approach would be for layout_stride_relaxed::mapping to have this code.

mhoemmen · 2026-07-13T01:01:03Z

@Jacobfaib wrote:

@mhoemmen could you take a look at this?

Thanks for asking! : - )

I'm not quite sure what's going on here.

If I have an mdspan with arbitrary layout, then I can just ask its layout mapping if is_unique() is true. Layout mappings can implement that check conservatively by returning false, but It's up to the layout mapping to decide that.

For the special case of layout_stride_relaxed, the right approach would be for layout_stride_relaxed::mapping::is_unique to implement this check.

For user-defined strided layouts (i.e., layouts that aren't layout_stride_relaxed, layout_stride, layout_{left,right}, or layout_{left,right}_padded), again, the right approach would be to trust what the user-defined layout mapping's is_strided() function returns.

stiepan · 2026-07-13T12:22:22Z

@mhoemmen thanks for the overview! @fbusato I like the idea to move this (relaxed for step > 1) check to layout_stride_realaxed::is_unique and just use whatever is_unique we have in the mdspan/fill_bytes validation, do you see any reason not to?

Fix too strict interleaved/unique layout check

68c6675

Signed-off-by: Kamil Tokarski <ktokarski@nvidia.com>

stiepan requested a review from a team as a code owner July 10, 2026 14:11

stiepan requested a review from caugonnet July 10, 2026 14:11

github-project-automation Bot added this to CCCL Jul 10, 2026

github-project-automation Bot moved this to Todo in CCCL Jul 10, 2026

cccl-authenticator-app Bot moved this from Todo to In Review in CCCL Jul 10, 2026

fbusato self-requested a review July 10, 2026 16:35

fbusato reviewed Jul 10, 2026

View reviewed changes

mhoemmen reviewed Jul 12, 2026

View reviewed changes

	const __stride_t extent = __extents[__i];
	const auto __extent = static_cast<__stride_t>(__extents[__i]);

	__max_dist += (extent - 1) * __abs_stride;
	__max_dist += (__extent - 1) * __abs_stride;

Uh oh!

Conversation

stiepan commented Jul 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Uh oh!

copy-pr-bot Bot commented Jul 10, 2026

Uh oh!

Jacobfaib commented Jul 10, 2026

Uh oh!

fbusato left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mhoemmen commented Jul 13, 2026

Uh oh!

stiepan commented Jul 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

stiepan commented Jul 10, 2026 •

edited

Loading

stiepan commented Jul 13, 2026 •

edited

Loading