Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
8 changes: 4 additions & 4 deletions paddle/phi/api/include/compat/torch/library.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ void OperatorRegistry::register_implementation(
auto& op = get_or_create_operator(qualified_name);
op.implementations[key] = std::move(func);
VLOG(3) << "Registered implementation: " << qualified_name << " for "
<< dispatch_key_to_string(key);
<< c10::toString(key);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Image

/skip-reason 编译过了就不影响效果

}
Comment on lines 236 to 244
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

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

OperatorRegistry::register_implementation definition still takes DispatchKey key, but the header now declares c10::DispatchKey key. This is an ODR/compile error. Update the .cpp signature (and any remaining DispatchKey uses in this translation unit) to match the header/type decision.

Copilot uses AI. Check for mistakes.

OperatorRegistration* OperatorRegistry::find_operator(
Expand All @@ -257,7 +257,7 @@ void OperatorRegistry::print_all_operators() const {
}
oss << " Implementations: ";
for (const auto& [key, impl] : op.implementations) {
oss << dispatch_key_to_string(key) << " ";
oss << c10::toString(key) << " ";
}
oss << std::endl;
}
Expand All @@ -280,7 +280,7 @@ Library::Library(Kind kind,
oss << "Created Library: kind=" << kind_to_string(kind)
<< ", namespace=" << ns;
if (dispatch_key) {
oss << ", dispatch_key=" << dispatch_key_to_string(*dispatch_key);
oss << ", dispatch_key=" << c10::toString(*dispatch_key);
}
Comment on lines 270 to 286
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

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

Library::Library(...) definition still uses std::optional<DispatchKey> for dispatch_key, but the PR is moving toward c10::DispatchKey and the header currently still references DispatchKey in several places. Once DispatchKey is removed/aliased, this will fail to compile. Please align the constructor parameter/member types with the chosen dispatch key type (e.g., std::optional<c10::DispatchKey> or torch::DispatchKey alias).

Copilot uses AI. Check for mistakes.
VLOG(3) << oss.str() << std::endl;
}
Expand Down Expand Up @@ -309,7 +309,7 @@ void Library::print_info() const {
std::ostringstream oss;
oss << "Library Info: " << kind_to_string(kind_) << ", namespace=" << ns_;
if (dispatch_key_) {
oss << ", dispatch_key=" << dispatch_key_to_string(*dispatch_key_);
oss << ", dispatch_key=" << c10::toString(*dispatch_key_);
}
std::cout << oss.str() << std::endl;
}
Expand Down
22 changes: 3 additions & 19 deletions paddle/phi/api/include/compat/torch/library.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
#include <type_traits>
#include <unordered_map>
#include <vector>
#include "c10/core/DispatchKey.h"
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

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

Include style is inconsistent here: other external headers in this file use angle brackets (e.g., <c10/macros/Macros.h>), but DispatchKey.h is included with quotes. Prefer #include <c10/core/DispatchKey.h> to match the rest of the compat headers and avoid accidental relative-include resolution.

Suggested change
#include "c10/core/DispatchKey.h"
#include <c10/core/DispatchKey.h>

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

这个人家说的有道理,我们 compat 的统一用 <>

#include "paddle/common/macros.h" // For macro PADDLE_API

namespace torch {
Expand Down Expand Up @@ -658,28 +659,11 @@ class class_ {
std::string qualified_name_;
};

enum class DispatchKey {
Undefined = 0,
CPU,
CUDA,
};

inline std::string dispatch_key_to_string(DispatchKey key) {
switch (key) {
case DispatchKey::CPU:
return "CPU";
case DispatchKey::CUDA:
return "CUDA";
default:
return "Undefined";
}
}

// Operator Registration
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

我对比了一下 torch,发现他们都是使用 c10::DispatchKey, 我们在这里保留这个 namespace 不太合适

struct OperatorRegistration {
std::string qualified_name; // namespace::op_name
std::string schema;
std::unordered_map<DispatchKey, CppFunction> implementations;
std::unordered_map<c10::DispatchKey, CppFunction> implementations;

OperatorRegistration(const std::string& name,
const std::string& schema_str = "")
Expand All @@ -696,7 +680,7 @@ class PADDLE_API OperatorRegistry {
const std::string& schema);

void register_implementation(const std::string& qualified_name,
DispatchKey key,
c10::DispatchKey key,
CppFunction&& func);
Comment on lines 662 to 684
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

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

DispatchKey enum and dispatch_key_to_string() were removed, but this header still uses DispatchKey in multiple places (e.g., Library constructor/member, value_or(DispatchKey::CPU), and the TORCH_LIBRARY_IMPL macro uses torch::DispatchKey::k). As-is, this will not compile. Please either switch all remaining uses to c10::DispatchKey (and update DispatchKey::CPU etc), or introduce using DispatchKey = c10::DispatchKey; in namespace torch to keep the existing torch::DispatchKey references working (and update the .cpp definitions and tests accordingly).

Copilot uses AI. Check for mistakes.

bool has_operator(const std::string& qualified_name) const {
Expand Down
Loading