Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
a7ce502
static: Introduce STATIC
kurtmcmillan Mar 18, 2026
6086baa
configuration: Allow MConfiguration to replace Configuration in Conte…
kurtmcmillan Mar 18, 2026
81a73c2
configuration: Allow fastpath enablement to be overridden
kurtmcmillan Mar 18, 2026
ad176c3
configuration: Allow fallback enablement to be overridden
kurtmcmillan Mar 18, 2026
cccbb9d
backend/fastpath: Check fastpath enablement when scoring IO
kurtmcmillan Mar 18, 2026
47c4db9
backend/fastpath: Check fastpath enablement when performing IO
kurtmcmillan Mar 18, 2026
3ae360d
backend/fallback: Test cleanup - Rework fallback scoring tests
kurtmcmillan Mar 18, 2026
9ce6fa7
backend/fallback: Test cleanup - Test names should use camel case
kurtmcmillan Mar 19, 2026
ec8c763
backend/fallback: Test cleanup - Test names should use camel case
kurtmcmillan Mar 19, 2026
770050c
backend/fallback: Test cleanup - Move msys, mhip, mlibmounthelper int…
kurtmcmillan Mar 19, 2026
bfaa660
backend/fallback: Test cleanup - Move nonnull_ptr into FallbackIo
kurtmcmillan Mar 19, 2026
f13fa3f
backend/fallback: Test cleanup - Move mhip, msys, mlibmounthelper int…
kurtmcmillan Mar 19, 2026
d966d1d
backend/fallback: Test cleanup - Use uniform initialization
kurtmcmillan Mar 19, 2026
b8d9396
backend/fallback: Check fallback enablement when scoring IO
kurtmcmillan Mar 18, 2026
a83d6a3
backend/fallback: Check fallback enablement when performing IO
kurtmcmillan Mar 18, 2026
1e3f70a
test/system: Test cleanup - Give parameterized tests better names
kurtmcmillan Mar 18, 2026
f4139a3
test/system: Use backend override functions to enable the desired bac…
kurtmcmillan Mar 18, 2026
69bb16f
state: Always instantiate all backends
kurtmcmillan Mar 18, 2026
3c912b9
test/system: Move io.cpp to amd/io.cpp
kurtmcmillan Mar 19, 2026
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
7 changes: 7 additions & 0 deletions src/amd_detail/backend.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,13 @@ namespace hipFile {
// write() system call. Mirrors kernel's MAX_RW_COUNT
static const size_t MAX_RW_COUNT = 0x7ffff000;

/// @brief Backend is not enabled
struct BackendDisabled : public std::runtime_error {
BackendDisabled() : std::runtime_error("Backend is disabled")
{
}
};

struct Backend {
virtual ~Backend() = default;

Expand Down
8 changes: 7 additions & 1 deletion src/amd_detail/backend/fallback.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include "buffer.h"
#include "backend/asyncop-fallback.h"
#include "backend/memcpy-kernel.h"
#include "configuration.h"
#include "context.h"
#include "fallback.h"
#include "file.h"
Expand Down Expand Up @@ -47,7 +48,8 @@ Fallback::score(std::shared_ptr<IFile> file, std::shared_ptr<IBuffer> buffer, si
(void)file;
(void)file_offset;
(void)size;
return buffer->getType() == hipMemoryTypeDevice ? 0 : -1;

return Context<Configuration>::get()->fallback() && buffer->getType() == hipMemoryTypeDevice ? 0 : -1;
}

ssize_t
Expand All @@ -61,6 +63,10 @@ ssize_t
Fallback::io(IoType io_type, shared_ptr<IFile> file, shared_ptr<IBuffer> buffer, size_t size,
hoff_t file_offset, hoff_t buffer_offset, size_t chunk_size)
{
if (!Context<Configuration>::get()->fallback()) {
throw BackendDisabled();
}

size = min(size, hipFile::MAX_RW_COUNT);

if (!paramsValid(buffer, size, file_offset, buffer_offset)) {
Expand Down
7 changes: 7 additions & 0 deletions src/amd_detail/backend/fastpath.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
*/

#include "buffer.h"
#include "configuration.h"
#include "context.h"
#include "fastpath.h"
#include "file.h"
Expand Down Expand Up @@ -128,6 +129,8 @@ Fastpath::score(shared_ptr<IFile> file, shared_ptr<IBuffer> buffer, size_t size,
{
bool accept_io{true};

accept_io &= Context<Configuration>::get()->fastpath();

accept_io &= file->getUnbufferedFd().has_value();

accept_io &= buffer->getType() == hipMemoryTypeDevice;
Comment on lines 130 to 136
Copy link

Copilot AI Mar 19, 2026

Choose a reason for hiding this comment

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

Fastpath::score() now checks Context<Configuration>::get()->fastpath(), but because it uses accept_io &= ... it will still evaluate all subsequent conditions (and call into file/buffer) even when fastpath is disabled. This adds unnecessary work and can trigger side effects in helpers/mocks. Consider early-returning -1 when fastpath is disabled, or rewriting the logic to use short-circuiting && checks.

Copilot uses AI. Check for mistakes.
Expand Down Expand Up @@ -158,6 +161,10 @@ ssize_t
Fastpath::io(IoType type, shared_ptr<IFile> file, shared_ptr<IBuffer> buffer, size_t size, hoff_t file_offset,
hoff_t buffer_offset)
{
if (!Context<Configuration>::get()->fastpath()) {
throw BackendDisabled();
}

void *devptr{reinterpret_cast<void *>(reinterpret_cast<intptr_t>(buffer->getBuffer()) + buffer_offset)};
hipAmdFileHandle_t handle{};
size_t nbytes{};
Expand Down
48 changes: 20 additions & 28 deletions src/amd_detail/configuration.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,51 +6,43 @@
#include "configuration.h"
#include "environment.h"
#include "hip.h"
#include "static.h"

#include <optional>

using namespace hipFile;

Configuration::Configuration() : m_fastpath(true), m_fallback(true), m_statsLevel(0)
bool
Configuration::fastpath() const noexcept
{
auto maybe_env_force_compat{Environment::force_compat_mode()};
if (maybe_env_force_compat && maybe_env_force_compat.value()) {
m_fastpath = false;
}

auto maybe_env_allow_compat{Environment::allow_compat_mode()};
if (maybe_env_allow_compat && !maybe_env_allow_compat.value()) {
m_fallback = false;
}

auto maybe_stats_level{Environment::stats_level()};
if (maybe_stats_level) {
m_statsLevel = maybe_stats_level.value();
}
STATIC bool fastpath_env{!Environment::force_compat_mode().value_or(false)};
STATIC bool readExists{!!getHipAmdFileReadPtr()};
STATIC bool writeExists{!!getHipAmdFileWritePtr()};
return readExists && writeExists && m_fastpath_override.value_or(fastpath_env);
}

bool
Configuration::fastpath() const noexcept
void
Configuration::fastpath(bool enabled) noexcept
{
#ifndef AIS_TESTING
static
#endif
bool readExists{!!getHipAmdFileReadPtr()};
#ifndef AIS_TESTING
static
#endif
bool writeExists{!!getHipAmdFileWritePtr()};
return readExists && writeExists && m_fastpath;
m_fastpath_override = enabled;
}

bool
Configuration::fallback() const noexcept
{
return m_fallback;
STATIC bool fallback_env{Environment::allow_compat_mode().value_or(true)};
return m_fallback_override.value_or(fallback_env);
}

void
Configuration::fallback(bool enabled) noexcept
{
m_fallback_override = enabled;
}

unsigned int
Configuration::statsLevel() const noexcept
{
return m_statsLevel;
STATIC unsigned int stats_level_env{Environment::stats_level().value_or(0)};
return stats_level_env;
}
40 changes: 20 additions & 20 deletions src/amd_detail/configuration.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,38 +5,38 @@

#pragma once

#include <optional>

namespace hipFile {

class IConfiguration {
class Configuration {

std::optional<bool> m_fastpath_override;
std::optional<bool> m_fallback_override;

public:
virtual ~IConfiguration()
{
}
virtual ~Configuration() = default;

/// @brief Checks if the fastpath backend is enabled
/// @return true if the fastpath backend is enabled, false otherwise
virtual bool fastpath() const noexcept = 0;
virtual bool fastpath() const noexcept;

/// @brief Override fastpath backend enablement.
///
/// If hipAmdFileRead/hipAmdFileWrite are not available fastpath() will
/// return false even if fastpath(true) is called.
virtual void fastpath(bool enabled) noexcept;

/// @brief Checks if the fallback backend is enabled
/// @return true if the fallback backend is enabled, false otherwise
virtual bool fallback() const noexcept = 0;
virtual bool fallback() const noexcept;

/// @brief Override fallback backend enablement
virtual void fallback(bool enabled) noexcept;

/// @brief Shows the level of detail for stats collection
/// @return 0 if stats collection disabled, higher levels of detail as value increases
virtual unsigned int statsLevel() const noexcept = 0;
};

class Configuration : public IConfiguration {
bool m_fastpath;
bool m_fallback;
unsigned int m_statsLevel;

public:
Configuration();

bool fastpath() const noexcept override;
bool fallback() const noexcept override;
unsigned int statsLevel() const noexcept override;
virtual unsigned int statsLevel() const noexcept;
};

}
15 changes: 5 additions & 10 deletions src/amd_detail/hip.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

#include "context.h"
#include "hip.h"
#include "static.h"

#include <cstdlib>
#include <system_error>
Expand All @@ -24,22 +25,16 @@ catch (...) {
hipAmdFileRead_t
getHipAmdFileReadPtr()
{
#ifndef AIS_TESTING
static
#endif
hipAmdFileRead_t hipAmdFileReadPtr{
reinterpret_cast<hipAmdFileRead_t>(hipGetProcAddressHelper("hipAmdFileRead"))};
STATIC hipAmdFileRead_t hipAmdFileReadPtr{
reinterpret_cast<hipAmdFileRead_t>(hipGetProcAddressHelper("hipAmdFileRead"))};
return hipAmdFileReadPtr;
}

hipAmdFileWrite_t
getHipAmdFileWritePtr()
{
#ifndef AIS_TESTING
static
#endif
hipAmdFileWrite_t hipAmdFileWritePtr{
reinterpret_cast<hipAmdFileWrite_t>(hipGetProcAddressHelper("hipAmdFileWrite"))};
STATIC hipAmdFileWrite_t hipAmdFileWritePtr{
reinterpret_cast<hipAmdFileWrite_t>(hipGetProcAddressHelper("hipAmdFileWrite"))};
return hipAmdFileWritePtr;
}

Expand Down
8 changes: 2 additions & 6 deletions src/amd_detail/state.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -275,12 +275,8 @@ std::vector<std::shared_ptr<Backend>>
DriverState::getBackends() const
{
static bool once = [&]() {
if (Context<Configuration>::get()->fastpath()) {
backends.emplace_back(new Fastpath{});
}
if (Context<Configuration>::get()->fallback()) {
backends.emplace_back(new Fallback{});
}
backends.emplace_back(new Fastpath{});
backends.emplace_back(new Fallback{});
return true;
}();
(void)once;
Expand Down
15 changes: 15 additions & 0 deletions src/amd_detail/static.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
/* Copyright (c) Advanced Micro Devices, Inc. All rights reserved.
*
* SPDX-License-Identifier: MIT
*/

#pragma once

// When testing it is sometimes inconvenient if a variable is statically initialized.
// Declaring a variable as STATIC will result in the variable being static only
// when tests are not being built.
#ifdef AIS_TESTING
#define STATIC
#else
#define STATIC static
Comment on lines +9 to +14
Copy link

Copilot AI Mar 19, 2026

Choose a reason for hiding this comment

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

The new macro name STATIC is very generic and can easily collide with other headers (or with identifiers named STATIC) when included. It also breaks the project’s existing macro naming convention (most macros are prefixed, e.g. HIPFILE_*). Consider renaming it to something namespaced like HIPFILE_STATIC/AIS_STATIC, and keeping the helper header’s macros as narrowly scoped as possible.

Suggested change
// Declaring a variable as STATIC will result in the variable being static only
// when tests are not being built.
#ifdef AIS_TESTING
#define STATIC
#else
#define STATIC static
// Declaring a variable as AIS_STATIC will result in the variable being static only
// when tests are not being built.
#ifdef AIS_TESTING
#define AIS_STATIC
#else
#define AIS_STATIC static

Copilot uses AI. Check for mistakes.
#endif
2 changes: 1 addition & 1 deletion test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ set(SYSTEM_TEST_SOURCE_FILES
system/buffer.cpp
system/config.cpp
system/driver.cpp
system/io.cpp
system/main.cpp
system/version.cpp
)
Expand All @@ -38,6 +37,7 @@ if(AIS_BUILD_NVIDIA_DETAIL)
list(APPEND UNIT_TEST_SOURCE_FILES nvidia_detail/cufile-api-compat.cpp)
list(APPEND TEST_SYSINCLS ${HIPFILE_NVIDIA_SOURCE_PATH})
else()
list(APPEND SYSTEM_TEST_SOURCE_FILES system/amd/io.cpp)
list(APPEND TEST_SYSINCLS ${HIPFILE_AMD_SOURCE_PATH})
endif()

Expand Down
Loading
Loading