Skip to content

Conversation

manuelcandales
Copy link
Contributor

Includes:

  • Shader library management
  • Kernel function handling
  • Command buffer execution
  • Metal memory operations

[ghstack-poisoned]
Copy link

pytorch-bot bot commented Oct 10, 2025

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/15022

Note: Links to docs will display an error until the docs builds have been completed.

❗ 2 Active SEVs

There are 2 currently active SEVs. If your PR is affected, please view them below:

❌ 3 New Failures

As of commit 5dfcd4f with merge base 896178e (image):

NEW FAILURES - The following jobs have failed:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

[ghstack-poisoned]
Comment on lines +453 to +464
auto src_mtl_buffer = (id<MTLBuffer>)src_buffer;
auto dst_mtl_buffer = (id<MTLBuffer>)dst_buffer;

uint8_t* src_contents = static_cast<uint8_t*>([src_mtl_buffer contents]);
uint8_t* dst_contents = static_cast<uint8_t*>([dst_mtl_buffer contents]);

if (!src_contents || !dst_contents) {
ET_LOG(Error, "aoti_torch_mps_copy_buffer: Failed to get buffer contents");
return Error::Internal;
}

memcpy(dst_contents + dst_offset, src_contents + src_offset, data_size);
Copy link
Contributor

Choose a reason for hiding this comment

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

aoti_torch_mps_free and aoti_torch_mps_memcpy expect contents ptr, but aoti_torch_mps_copy_buffer expects MTLBuffer objects. Is this intentional?

Isn't it something like this?

              auto src_it = ptr_to_mtl_buffer.find(src_buffer);
              auto dst_it = ptr_to_mtl_buffer.find(dst_buffer);

              if (src_it == ptr_to_mtl_buffer.end()) {
                  ET_LOG(Error, "aoti_torch_mps_copy_buffer: src_buffer %p not found", src_buffer);
                  return Error::InvalidArgument;
              }

              if (dst_it == ptr_to_mtl_buffer.end()) {
                  ET_LOG(Error, "aoti_torch_mps_copy_buffer: dst_buffer %p not found", dst_buffer);
                  return Error::InvalidArgument;
              }

              id<MTLBuffer> src_mtl_buffer = src_it->second;
              id<MTLBuffer> dst_mtl_buffer = dst_it->second;

              ETMetalStream* stream = getCurrentMetalStream();
              stream->copy(src_mtl_buffer, dst_mtl_buffer, data_size, src_offset, dst_offset, SyncType::NONE);

Comment on lines +416 to +423
id<MTLBuffer> subBuffer = [device newBufferWithBytesNoCopy:buffer_pointer + constant_offset
length:data_size
options:MTLResourceCPUCacheModeWriteCombined | MTLResourceStorageModeShared
deallocator:nil];

if (constant_offset != 0) {
ptr_to_mtl_buffer[buffer_pointer + constant_offset] = subBuffer; // Map contents to buffer
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you need this at all? subBuffer doesn't seem to be used anywhere

}
}

AOTITorchError aoti_torch_mps_get_kernel_function(
Copy link
Contributor

Choose a reason for hiding this comment

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

}
}

AOTITorchError aoti_torch_mps_start_encoding(
Copy link
Contributor

Choose a reason for hiding this comment

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

// Pure C dispatch functions - array versions
AOTITorchError aoti_torch_mps_dispatch_array(
AOTIMetalKernelFunctionHandle func,
const uint64_t* length,
Copy link
Contributor

Choose a reason for hiding this comment

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

Validate length != nullptr

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

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants