From 431e03381d9a8d73f16d902d596fc644d7fe6b73 Mon Sep 17 00:00:00 2001 From: Kyle Lucke Date: Tue, 11 Jun 2024 13:47:24 -0700 Subject: [PATCH] Remove unused options argument from Platform::Initialize. PiperOrigin-RevId: 642379707 --- tensorflow/dtensor/cc/dtensor_tpu_kernels.cc | 2 +- .../xla/xla/stream_executor/platform.cc | 11 +-------- .../xla/xla/stream_executor/platform.h | 23 ++++--------------- .../xla/stream_executor/platform_manager.cc | 17 +++++++------- .../xla/stream_executor/platform_manager.h | 8 +------ .../stream_executor/tpu/tpu_executor_c_api.h | 4 +--- .../xla/stream_executor/tpu/tpu_platform.cc | 23 ++----------------- .../xla/stream_executor/tpu/tpu_platform.h | 3 +-- 8 files changed, 19 insertions(+), 72 deletions(-) diff --git a/tensorflow/dtensor/cc/dtensor_tpu_kernels.cc b/tensorflow/dtensor/cc/dtensor_tpu_kernels.cc index c6d1b560d4f108..ed228eecd30eaa 100644 --- a/tensorflow/dtensor/cc/dtensor_tpu_kernels.cc +++ b/tensorflow/dtensor/cc/dtensor_tpu_kernels.cc @@ -158,7 +158,7 @@ class ConfigureAndInitializeGlobalTPUOpKernel : public OpKernel { while (!tpu_platform->Initialized() && (absl::Now() - start < retry_timeout)) { VLOG(1) << "Initializaing global TPU system."; - init_status = tpu_platform->Initialize({}); + init_status = tpu_platform->Initialize(); } if (!tpu_platform->Initialized()) { return errors::Unavailable("Unable to initialize TPU system."); diff --git a/third_party/xla/xla/stream_executor/platform.cc b/third_party/xla/xla/stream_executor/platform.cc index b1c2680868cc02..15ae4036199307 100644 --- a/third_party/xla/xla/stream_executor/platform.cc +++ b/third_party/xla/xla/stream_executor/platform.cc @@ -38,17 +38,8 @@ StreamExecutorConfig::StreamExecutorConfig() : ordinal(-1) {} StreamExecutorConfig::StreamExecutorConfig(int ordinal_in) : ordinal(ordinal_in) {} -Platform::~Platform() {} - bool Platform::Initialized() const { return true; } -absl::Status Platform::Initialize( - const std::map &platform_options) { - if (!platform_options.empty()) { - return absl::UnimplementedError( - "this platform does not support custom initialization"); - } - return absl::OkStatus(); -} +absl::Status Platform::Initialize() { return absl::OkStatus(); } } // namespace stream_executor diff --git a/third_party/xla/xla/stream_executor/platform.h b/third_party/xla/xla/stream_executor/platform.h index e9e27bba23598c..1763e4d5663527 100644 --- a/third_party/xla/xla/stream_executor/platform.h +++ b/third_party/xla/xla/stream_executor/platform.h @@ -59,7 +59,7 @@ struct StreamExecutorConfig { // Abstract base class for a platform registered with the PlatformManager. class Platform { public: - virtual ~Platform(); + virtual ~Platform() = default; // A platform ID is a unique identifier for each registered platform type - // each platform is required to expose an ID to ensure unique registration and @@ -93,14 +93,9 @@ class Platform { // Returns true iff the platform has been initialized. virtual bool Initialized() const; - // Initializes the platform with a custom set of options. The platform must be - // initialized before obtaining StreamExecutor objects. The interpretation of - // the platform_options argument is implementation specific. This method may - // return an error if unrecognized options are provided. If using - // PlatformManager, this method will be called automatically by - // InitializePlatformWithId/InitializePlatformWithName. - virtual absl::Status Initialize( - const std::map& platform_options); + // Initializes the platform. The platform must be initialized before obtaining + // StreamExecutor objects. + virtual absl::Status Initialize(); // Returns a populated DeviceDescription for the device at the given ordinal. // This should not require device initialization. Note that not all platforms @@ -130,16 +125,6 @@ class Platform { // Ownership IS transferred to the caller. virtual absl::StatusOr> GetUncachedExecutor( const StreamExecutorConfig& config) = 0; - - protected: - // SE_DISALLOW_COPY_AND_ASSIGN declares a constructor, which suppresses the - // presence of the default constructor. This statement re-enables it, which - // simplifies subclassing. - Platform() = default; - - private: - Platform(const Platform&) = delete; - void operator=(const Platform&) = delete; }; } // namespace stream_executor diff --git a/third_party/xla/xla/stream_executor/platform_manager.cc b/third_party/xla/xla/stream_executor/platform_manager.cc index 61da41ef0c5c5b..a53a56e91d112e 100644 --- a/third_party/xla/xla/stream_executor/platform_manager.cc +++ b/third_party/xla/xla/stream_executor/platform_manager.cc @@ -59,8 +59,7 @@ class PlatformManagerImpl { bool initialize_platform) ABSL_LOCKS_EXCLUDED(mu_); - absl::StatusOr InitializePlatformWithId( - const Platform::Id& id, const std::map& options) + absl::StatusOr InitializePlatformWithId(const Platform::Id& id) ABSL_LOCKS_EXCLUDED(mu_); absl::StatusOr> PlatformsWithFilter( @@ -126,7 +125,7 @@ absl::StatusOr PlatformManagerImpl::PlatformWithName( TF_ASSIGN_OR_RETURN(Platform * platform, LookupByNameLocked(target)); if (initialize_platform && !platform->Initialized()) { - TF_RETURN_IF_ERROR(platform->Initialize({})); + TF_RETURN_IF_ERROR(platform->Initialize()); } return platform; @@ -138,14 +137,14 @@ absl::StatusOr PlatformManagerImpl::PlatformWithId( TF_ASSIGN_OR_RETURN(Platform * platform, LookupByIdLocked(id)); if (initialize_platform && !platform->Initialized()) { - TF_RETURN_IF_ERROR(platform->Initialize({})); + TF_RETURN_IF_ERROR(platform->Initialize()); } return platform; } absl::StatusOr PlatformManagerImpl::InitializePlatformWithId( - const Platform::Id& id, const std::map& options) { + const Platform::Id& id) { absl::MutexLock lock(&mu_); TF_ASSIGN_OR_RETURN(Platform * platform, LookupByIdLocked(id)); @@ -154,7 +153,7 @@ absl::StatusOr PlatformManagerImpl::InitializePlatformWithId( absl::StrFormat("platform with id %p is already initialized", id)); } - TF_RETURN_IF_ERROR(platform->Initialize(options)); + TF_RETURN_IF_ERROR(platform->Initialize()); return platform; } @@ -170,7 +169,7 @@ absl::StatusOr> PlatformManagerImpl::PlatformsWithFilter( Platform* platform = entry.second; if (filter(platform)) { if (initialize_platform && !platform->Initialized()) { - TF_RETURN_IF_ERROR(platform->Initialize({})); + TF_RETURN_IF_ERROR(platform->Initialize()); } platforms.push_back(platform); } @@ -245,8 +244,8 @@ PlatformManagerImpl& Impl() { } /*static*/ absl::StatusOr PlatformManager::InitializePlatformWithId( - const Platform::Id& id, const std::map& options) { - return Impl().InitializePlatformWithId(id, options); + const Platform::Id& id) { + return Impl().InitializePlatformWithId(id); } /*static*/ absl::StatusOr> diff --git a/third_party/xla/xla/stream_executor/platform_manager.h b/third_party/xla/xla/stream_executor/platform_manager.h index 4f88d2a79f2d28..161c3d9bc6c233 100644 --- a/third_party/xla/xla/stream_executor/platform_manager.h +++ b/third_party/xla/xla/stream_executor/platform_manager.h @@ -62,9 +62,7 @@ limitations under the License. #define XLA_STREAM_EXECUTOR_PLATFORM_MANAGER_H_ #include -#include #include -#include #include #include "absl/status/status.h" @@ -104,15 +102,11 @@ class PlatformManager { // Retrieves the platform registered with the given platform id (an opaque, // comparable value provided by the Platform's Id() method). // - // The platform will be initialized with the given options. If the platform - // was already initialized, an error will be returned. - // // If the requested platform is not registered, an error status is returned. // Ownership of the platform is NOT transferred to the caller -- // the PlatformManager owns the platforms in a singleton-like fashion. static absl::StatusOr InitializePlatformWithId( - const Platform::Id& id, - const std::map& options); + const Platform::Id& id); // Retrieves the platforms satisfying the given filter, i.e. returns true. // Returned Platforms are always initialized. diff --git a/third_party/xla/xla/stream_executor/tpu/tpu_executor_c_api.h b/third_party/xla/xla/stream_executor/tpu/tpu_executor_c_api.h index a6e0e067fba453..3019f6f7a0bb3c 100644 --- a/third_party/xla/xla/stream_executor/tpu/tpu_executor_c_api.h +++ b/third_party/xla/xla/stream_executor/tpu/tpu_executor_c_api.h @@ -26,9 +26,7 @@ extern "C" { SE_Platform* TpuPlatform_New(); void TpuPlatform_Free(SE_Platform* platform); -void TpuPlatform_Initialize(SE_Platform* platform, size_t options_size, - const char** options_key, - const char** options_value, TF_Status* status); +void TpuPlatform_Initialize(SE_Platform* platform, TF_Status* status); bool TpuPlatform_Initialized(SE_Platform* platform); SE_StreamExecutor* TpuPlatform_GetExecutor(SE_Platform* platform, SE_StreamExecutorConfig* config, diff --git a/third_party/xla/xla/stream_executor/tpu/tpu_platform.cc b/third_party/xla/xla/stream_executor/tpu/tpu_platform.cc index 6fd5d05e1aa6fa..16efed10f42179 100644 --- a/third_party/xla/xla/stream_executor/tpu/tpu_platform.cc +++ b/third_party/xla/xla/stream_executor/tpu/tpu_platform.cc @@ -56,29 +56,10 @@ TpuPlatform* TpuPlatform::GetRegisteredPlatform() { return tpu_registered_platform; } -absl::Status TpuPlatform::Initialize( - const std::map& platform_options) { +absl::Status TpuPlatform::Initialize() { StatusHelper status; - - size_t options_size = platform_options.size(); - const char** options_key = - static_cast(malloc(sizeof(const char*) * options_size)); - const char** options_value = - static_cast(malloc(sizeof(const char*) * options_size)); - - size_t i = 0; - for (const auto& option : platform_options) { - options_key[i] = option.first.c_str(); - options_value[i] = option.second.c_str(); - i++; - } - stream_executor::tpu::ExecutorApiFn()->TpuPlatform_InitializeFn( - platform_, options_size, options_key, options_value, status.c_status); - - free(options_key); - free(options_value); - + platform_, status.c_status); return status.status(); } diff --git a/third_party/xla/xla/stream_executor/tpu/tpu_platform.h b/third_party/xla/xla/stream_executor/tpu/tpu_platform.h index 59b154bb289be9..f2ea4f9dba8b2e 100644 --- a/third_party/xla/xla/stream_executor/tpu/tpu_platform.h +++ b/third_party/xla/xla/stream_executor/tpu/tpu_platform.h @@ -70,8 +70,7 @@ class TpuPlatform : public ::tensorflow::tpu::TpuPlatformInterface { bool Initialized() const override; - absl::Status Initialize( - const std::map& platform_options) override; + absl::Status Initialize() override; absl::Status Reset(bool only_tear_down, absl::string_view reason) override { LOG(FATAL) << "Not yet implemented";