Skip to content

LDS access proposal #29

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
136 changes: 136 additions & 0 deletions rfcs/000-safe-lds-access.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@

# Summary

Provide safe access to a (limited) set of LDS memory without race conditions and without violating SPIR-V rules.

# Explanation


```rust
struct LdsWriter;

impl LdsWriter {
fn write_thread_idx(&mut self, value: T);
fn barrier(self) -> LdsReader;
}

struct LdsReader;

impl LdsReader {
fn read(&self, idx: usize) -> T;
fn barrier(self) -> LdsWriter;
}
```
## Barriers in non-uniform control flow

```rust
// sample 1:
let lds = LdsWriter::<u32>::new();

lds.write_thread_idx(0);

// barrier should be in uniform control flow
let value = if something {
lds.barrier().read(12)
} else {
lds.barrier().read(23)
};
```

## Multiple barriers with write in one branch
```rust
// sample 2:

let lds = LdsWriter::<u32>::new();

lds.write_thread_idx(0);

let value = if something {
lds.barrier().read(12)
} else {
let rdr = lds.barrier();
let value = rdr.read(23);

rdr.barrier().write_thread_idx(12);
value
};
```

## Multiple consecutive writes to same location
```rust
// sample 3:
let lds = LdsWriter::<u32>::new();

lds.write_thread_idx(0);
lds.write_thread_idx(666); // race?
```
Comment on lines +61 to +66

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fine. It's just consecutive writes to the same address, the later write wins due to program order.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah - this was mostly to discuss what the potential rules from the Rust point of view are. However, since this shouldn't ever lead to invalid data I think we're fine. We could probably elide the first store entirely in this case.


## Multiple writes to same memory location after a read
```rust
// sample 4:
let lds = LdsWriter::<u32>::new();

lds.write_thread_idx(0);

let (value, rdr) = if something {
let rdr = lds.barrier();
let value = rdr.read(12)
(value, rdr)
} else {
let rdr = lds.barrier();
let value = rdr.read(12)
(value, rdr)
};

let wrt = rdr.barrier();
wrt.write_thread_idx(12234);
```
Comment on lines +85 to +87

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part seems fine as long as rdr.barrier() does an OpControlBarrier. The part above with barriers in non-uniform control flow is a problem, but it's orthogonal.


# Drawbacks

* Limited amount of indexing for write operations

# Alternatives

@Tobski proposed making join operations explict and potentially passing in ranges in, to partition the data
```rust
struct BufferWriter;

impl BufferWriter {
fn write_thread_idx(&mut self, value: T);
}

struct BufferReader;


impl BufferReader {
fn read(&self, idx: usize) -> T;
fn partition_readers(self, ranges: [(usize, usize)]) -> [BufferReader];
fn partition_writers(self, ranges: [(usize, usize)]) -> [BufferWriter];
}

fn join_writers(a: BufferWriter, b: BufferWriter) -> BufferReader;
fn join_readers(a: BufferReader, b: BufferReader) -> BufferReader;
```

@Jasper-Bekkers proposed making thread index a type with limited safe operations to extend the writer types with some more flexibilty. One concern here is also that this should be done in uniform controlflow most likely.

```rust
struct ThreadIdx;

impl ThreadIdx {
/// XOR shuffle with a constant
fn butterfly_shuffle(&self, i: u32) -> ThreadIdx;

/// Shuffle all lanes up / down by a constant
fn up(&self, i: u32) -> ThreadIdx;
fn down(&self, i: u32) -> ThreadIdx;

/// Selecting arbitrary threads is unsafe
unsafe fn new(idx: u32) -> ThreadIdx;
}
```

# Prior art

* Regular shading languages only provide unsafe/unsound access