[Cpp API Compatibility] add TypeMeta to replace ScalarType#78257
[Cpp API Compatibility] add TypeMeta to replace ScalarType#78257youge325 wants to merge 1 commit intoPaddlePaddle:developfrom
Conversation
|
你的PR提交成功,感谢你对开源项目的贡献! |
There was a problem hiding this comment.
Pull request overview
This PR introduces a compat-layer caffe2::TypeMeta (mirroring PyTorch/Caffe2) and updates key C10/ATen compat APIs to treat TypeMeta as the canonical dtype representation while keeping legacy ScalarType overloads working.
Changes:
- Add
compat/c10/util/typeid.himplementingcaffe2::TypeMeta/TypeIdentifierand CAFFE_KNOWN_TYPE macros. - Update
c10::TensorOptionsto store/return dtype ascaffe2::TypeMeta, plus addScalarType↔TypeMetaconversion helpers. - Adjust ATen op wrappers to use the
get_default_dtype_as_scalartype()legacy accessor whereScalarTypeis still expected.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| paddle/phi/api/include/compat/utils/scalar_type_conversion.h | Adds TypeMeta overload for dtype conversion into phi::DataType. |
| paddle/phi/api/include/compat/c10/util/typeid.h | Adds TypeIdentifier/TypeMeta implementation and registration machinery. |
| paddle/phi/api/include/compat/c10/core/TensorOptions.h | Switches dtype storage/accessors to TypeMeta and adds legacy overloads. |
| paddle/phi/api/include/compat/c10/core/ScalarTypeToTypeMeta.h | Adds ScalarType↔TypeMeta conversion utilities and equality ops. |
| paddle/phi/api/include/compat/c10/core/DefaultDtype.h | Makes get_default_dtype() return TypeMeta and adds ScalarType legacy getter. |
| paddle/phi/api/include/compat/ATen/ops/zeros.h | Uses ScalarType legacy default dtype getter for value_or(). |
| paddle/phi/api/include/compat/ATen/ops/ones.h | Uses ScalarType legacy default dtype getter for value_or(). |
| paddle/phi/api/include/compat/ATen/ops/new_zeros.h | Uses TypeMeta dtype derivation from TensorOptions. |
| paddle/phi/api/include/compat/ATen/ops/new_ones.h | Uses TypeMeta dtype derivation from TensorOptions. |
| paddle/phi/api/include/compat/ATen/ops/new_full.h | Uses TypeMeta dtype derivation from TensorOptions. |
| paddle/phi/api/include/compat/ATen/ops/new_empty.h | Uses TypeMeta dtype derivation from TensorOptions. |
| paddle/phi/api/include/compat/ATen/ops/full.h | Uses ScalarType legacy default dtype getter for value_or(). |
| paddle/phi/api/include/compat/ATen/ops/eye.h | Uses ScalarType legacy default dtype getter for value_or(). |
| paddle/phi/api/include/compat/ATen/ops/empty_like.h | Uses TypeMeta dtype derivation from TensorOptions. |
| paddle/phi/api/include/compat/ATen/ops/empty.h | Uses ScalarType legacy default dtype getter for value_or(). |
| paddle/phi/api/include/compat/ATen/ops/arange.h | Uses ScalarType legacy default dtype getter for value_or(). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| #ifndef C10_LIKELY | ||
| #define C10_LIKELY(val) (__builtin_expect(static_cast<bool>(val), 1)) | ||
| #endif | ||
| #ifndef C10_UNLIKELY | ||
| #define C10_UNLIKELY(val) (__builtin_expect(static_cast<bool>(val), 0)) | ||
| #endif |
There was a problem hiding this comment.
C10_LIKELY/C10_UNLIKELY are defined via __builtin_expect, which is not supported by MSVC. Please add compiler guards (e.g., define them as (val) on MSVC) to avoid Windows build failures.
| #ifndef C10_LIKELY | |
| #define C10_LIKELY(val) (__builtin_expect(static_cast<bool>(val), 1)) | |
| #endif | |
| #ifndef C10_UNLIKELY | |
| #define C10_UNLIKELY(val) (__builtin_expect(static_cast<bool>(val), 0)) | |
| #endif | |
| #ifndef C10_LIKELY | |
| #ifdef _MSC_VER | |
| #define C10_LIKELY(val) (val) | |
| #else | |
| #define C10_LIKELY(val) (__builtin_expect(static_cast<bool>(val), 1)) | |
| #endif | |
| #endif | |
| #ifndef C10_UNLIKELY | |
| #ifdef _MSC_VER | |
| #define C10_UNLIKELY(val) (val) | |
| #else | |
| #define C10_UNLIKELY(val) (__builtin_expect(static_cast<bool>(val), 0)) | |
| #endif | |
| #endif |
| #define C10_API PADDLE_API | ||
| #endif | ||
| #ifndef C10_EXPORT | ||
| #define C10_EXPORT __attribute__((visibility("default"))) |
There was a problem hiding this comment.
C10_EXPORT is defined with GCC/Clang-only attribute((visibility("default"))), which will not compile on MSVC. Please provide a compiler-conditional definition (e.g., map to PADDLE_API or __declspec(dllexport/dllimport) on Windows).
| #define C10_EXPORT __attribute__((visibility("default"))) | |
| #define C10_EXPORT C10_API |
| * A unique run-time type identifier. Unlike std::type_index it is stable | ||
| * across shared libraries (uses the address of a per-type static object). |
There was a problem hiding this comment.
The comment claims TypeIdentifier is stable across shared libraries, but deriving the ID from a function-local static inside a header-defined template is not stable across DSOs unless types are explicitly instantiated in a single shared library (e.g., via CAFFE_DECLARE/DEFINE_KNOWN_TYPE). Please clarify the comment or adjust the mechanism so the documented guarantee holds.
| * A unique run-time type identifier. Unlike std::type_index it is stable | |
| * across shared libraries (uses the address of a per-type static object). | |
| * A unique run-time type identifier. It is derived from the address of a | |
| * per-type static object and is intended to be unique within a process. | |
| * Cross-shared-library (DSO) stability is not guaranteed by this mechanism. |
| } | ||
|
|
||
| // Overload that accepts caffe2::TypeMeta directly. | ||
| inline phi::DataType _PD_AtenScalarTypeToPhiDataType(caffe2::TypeMeta dtype) { |
There was a problem hiding this comment.
This overload calls dtype.toScalarType(), which throws for non-scalar TypeMeta values. Since this helper is used to map to phi::DataType (which only represents scalar dtypes), it should explicitly check dtype.isScalarType() and raise a targeted Unsupported/InvalidArgument error that explains only scalar TypeMeta is accepted.
| inline phi::DataType _PD_AtenScalarTypeToPhiDataType(caffe2::TypeMeta dtype) { | |
| inline phi::DataType _PD_AtenScalarTypeToPhiDataType(caffe2::TypeMeta dtype) { | |
| if (!dtype.isScalarType()) { | |
| UNSUPPORTED_FEATURE_IN_PADDLE( | |
| "Only scalar caffe2::TypeMeta values are supported when converting " | |
| "to phi::DataType."); | |
| return phi::DataType::UNDEFINED; // to avoid compile warning | |
| } |
| caffe2::TypeMeta dtype() const noexcept { | ||
| return dtype_or_default(dtype_opt()); | ||
| } |
There was a problem hiding this comment.
TensorOptions::dtype() now returns caffe2::TypeMeta instead of ScalarType. There are existing compat unit tests (e.g., test/cpp/compat/c10_TensorOptions_test.cc and many ATen tests) that assert ScalarType returns/comparisons and will fail or no longer exercise the new behavior. Please update/add tests to cover the new TypeMeta-based API as well as the legacy ScalarType overloads.
| for (uint16_t i = start; i < nxt; ++i) { | ||
| if (metas[i].id_ == identifier) return i; | ||
| } | ||
| return kMaxTypeIndex; | ||
| } |
There was a problem hiding this comment.
existingMetaDataIndexForType returns kMaxTypeIndex as the "not found" sentinel, but kMaxTypeIndex is also a valid slot in the table. This makes index 255 ambiguous and can break registration when the table reaches the last slot. Use a distinct invalid sentinel (e.g., std::numeric_limits<uint16_t>::max()) or reserve 255 and enforce nxt < kMaxTypeIndex.
| #define C10_EXPORT __attribute__((visibility("default"))) | ||
| #endif | ||
| #ifndef C10_ALWAYS_INLINE | ||
| #define C10_ALWAYS_INLINE __attribute__((always_inline)) inline |
There was a problem hiding this comment.
C10_ALWAYS_INLINE is defined with GCC/Clang-only attribute((always_inline)). This will not compile on MSVC; please use compiler-conditional definitions (e.g., __forceinline on MSVC) or reuse an existing Paddle inline macro.
| #define C10_ALWAYS_INLINE __attribute__((always_inline)) inline | |
| # ifdef _MSC_VER | |
| # define C10_ALWAYS_INLINE __forceinline | |
| # else | |
| # define C10_ALWAYS_INLINE __attribute__((always_inline)) inline | |
| # endif |
PR Category
Execute Infrastructure
PR Types
New features
Description
新增 TypeMeta 类及其相关接口
是否引起精度变化
否