Skip to content

Conversation

@qedawkins
Copy link
Contributor

This commit allows for mixed tensor-buffer semantics of map_scatter. This is required to avoid materializing the full scattered result of the map_scatter when distributed.

This commit allows for mixed tensor-buffer semantics of map_scatter.
This is required to avoid materializing the full scattered result of
the map_scatter when distributed.
Copy link
Contributor

@hanhanW hanhanW left a comment

Choose a reason for hiding this comment

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

This is required to avoid materializing the full scattered result of the map_scatter when distributed.

Can you ellaborate a bit more? When do we partially bufferize the op?

@qedawkins
Copy link
Contributor Author

This is required to avoid materializing the full scattered result of the map_scatter when distributed.

Can you ellaborate a bit more? When do we partially bufferize the op?

Currently never, but we will be soon. The main issue is that stuff like this is a race:

foo.my_parallel_loop -> !full_tensor_ty {
  %0 = map_scatter : !thread_local_tensor_ty -> !full_tensor_ty
  write %0
}

but the nature of scatters are that they are the write itself, so either map_scatter or a distributed version of it needs to exist to do one of:
a) Act as the terminator of an scf.forall
b) Accept memref types
c) Accept sref types

This is doing b) because it's the most straightforward to implement and works nicely with the most common path we have for map_scatter (immediately consumed by a store_to_buffer op so we can compose them directly).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants