[Cpp API Compatibility] Align device related APIs#78551
[Cpp API Compatibility] Align device related APIs#78551SigureMo merged 2 commits intoPaddlePaddle:developfrom
Conversation
|
你的PR提交成功,感谢你对开源项目的贡献! |
There was a problem hiding this comment.
Pull request overview
This PR continues the C++ API compatibility work by aligning the compat c10::DeviceType / c10::Device surface area and parsing behavior with upstream expectations (notably around PrivateUse1 / privateuseone), and adds tests to lock in the parity.
Changes:
- Add
DeviceType::PrivateUse1aliasingCUSTOM, pluskPrivateUse1and hashing support forDeviceType. - Expand
c10::Deviceinterface (parity helpers likeis_privateuseone(),set_index(),supports_as_strided()), add hashing forDevice, and rework device-string parsing (includingprivateuseone). - Adjust
TORCH_CHECK_OPerror text and extend C++ compat tests to cover the new device behaviors (includingunordered_maphashing).
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/cpp/compat/c10_Device_test.cc | Adds parity tests for new Device/DeviceType behaviors and hashing usage. |
| paddle/phi/api/include/compat/c10/util/Exception.h | Updates TORCH_CHECK_OP error message formatting. |
| paddle/phi/api/include/compat/c10/core/DeviceType.h | Introduces PrivateUse1 / kPrivateUse1 and std::hash<DeviceType>. |
| paddle/phi/api/include/compat/c10/core/Device.h | Adds device parity methods, validation calls, and std::hash<Device>. |
| paddle/phi/api/include/compat/c10/core/Device.cpp | Extends parsing to privateuseone and reimplements device-string parsing logic. |
| .gitignore | Ignores .codex. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| TORCH_CHECK(false, | ||
| "Expected one of cpu, cuda, ipu, xpu, privateuseone device type " | ||
| "at start of device string: ", | ||
| device_string); |
There was a problem hiding this comment.
parse_type() can reach the end of a non-void function without returning a DeviceType (after the TORCH_CHECK(false, ...)). Even if TORCH_CHECK throws at runtime, this is still UB in the language rules and may trigger -Wreturn-type/-Werror build failures. Add an explicit return (or an unreachable annotation) after the check to satisfy the compiler.
| device_string); | |
| device_string); | |
| return DeviceType::CPU; // Unreachable, added to satisfy compiler. |
| namespace std { | ||
| template <> | ||
| struct hash<c10::Device> { | ||
| size_t operator()(c10::Device d) const noexcept { | ||
| static_assert(sizeof(c10::DeviceType) == 1, "DeviceType is not 8-bit"); | ||
| static_assert(sizeof(c10::DeviceIndex) == 1, "DeviceIndex is not 8-bit"); | ||
| uint32_t bits = static_cast<uint32_t>(static_cast<uint8_t>(d.type())) | ||
| << 16 | | ||
| static_cast<uint32_t>(static_cast<uint8_t>(d.index())); | ||
| return std::hash<uint32_t>{}(bits); | ||
| } |
There was a problem hiding this comment.
std::hash<c10::Device>::operator() takes the key by value, which will copy c10::Device (including its std::string custom_device_type_) every time the hash is computed. This is avoidable overhead for unordered_map/unordered_set usage. Take the parameter as const c10::Device& instead, and consider incorporating custom_device_type_ into the hash to reduce collisions since operator== includes it.
|
/re-run all-failed |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #78551 +/- ##
==========================================
Coverage ? 94.91%
==========================================
Files ? 2
Lines ? 59
Branches ? 0
==========================================
Hits ? 56
Misses ? 3
Partials ? 0 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
ShigureNyako
left a comment
There was a problem hiding this comment.
整体看下来我这边倾向于 approve。
我重点核对了:
c10/core/Device.cpp的字符串解析状态机和privateuseone解析,行为与 upstream PyTorch 当前实现保持一致;c10/core/Device.h新增的operator!=/set_index/ 设备谓词 /supports_as_strided()/hash<Device>等接口,覆盖了这次 PR 声称补齐的 Device 主路径;c10/util/Exception.h中TORCH_CHECK_OP的报错前缀,已对齐为 PyTorch 的Check failed: ... (lhs vs. rhs).形式;test/cpp/compat/c10_Device_test.cc新增用例已经覆盖 strict parsing、PrivateUse1、谓词和unordered_map主路径。
关于 BREAKING CHANGE:这里最明显的行为变化是设备字符串解析变严格了,像 cuda:-1 / cuda:01 / cuda:1:2 现在会抛异常;但这正是向 upstream PyTorch 语义收敛,我认为属于预期的兼容性修正,不是额外引入的 breaking risk。
小建议:.gitignore 里的 .codex 变更和本 PR 主题关系不大,最好后续单独拆出去,保持拆分 PR 更聚焦。
PR Category
Execute Infrastructure
PR Types
Improvements
Description
拆分自 #78484
Align Device 相关接口,补齐
c10::Device和c10::DeviceType的 PyTorch C++ API 兼容层。变更详情
1.
c10/core/DeviceType.h扩展DeviceType::PrivateUse1/kPrivateUse1别名std::hash<c10::DeviceType>特化2.
c10/core/Device.h接口补齐新增以下方法:
3.
c10/core/Device.cpp字符串解析对齐privateuseone设备类型cuda:-1、cuda:01、cuda:1:2等Windows 兼容性处理:
为规避 Windows 头文件中的
ERROR宏污染,内部状态枚举使用kStart/kIndexStart/kIndexRest/kError命名(而非 PyTorch 上游的START/INDEX_START/INDEX_REST/ERROR),但对外行为完全一致。4.
c10/util/Exception.hTORCH_CHECK_OP 对齐对齐
TORCH_CHECK_EQ/NE/LT/LE/GT/GE等宏的报错前缀格式,使其与 PyTorch 的异常消息格式一致。相关文档
是否引起精度变化
否