Prhub

#29508 [Bugfix] fix quickreduce acc error in cudagraph mode

原始 PR 作者 haoyangli0109 合并时间 2026-07-15 13:16 文件变更 2 提交数 3 评论 11 代码增减 +141 / -7

执行摘要

修复 CUDA Graph 模式下 QuickReduce 的 flag 残留错误

在 CUDA Graph 模式下,flag_color 被 graph 固定后每轮保持不变,写入的 flag 值重复,无法与上一轮的残留值区分,导致等待方被旧值过早满足并读取旧数据,造成精度错误。PR body 提供了复现代码和详细分析。

值得精读。该 PR 展示了 CUDA Graph 下常见但隐蔽的 flag 固化问题,以及通过在设备端维护动态状态来绕过的设计模式。对于需要支持 CUDA Graph 的通信库实现具有参考价值。

讨论亮点
  • 资源泄漏防护建议:Code review 机器人建议在 destroy() 中同等地释放 dbuffer 和 dbuffer_list 防止部分初始化失败时的泄漏,PR 仅添加了 d_flag_counters 的独立释放,未完整处理。(未采纳)
  • 测试覆盖要求:合并者 HaiShaw 要求添加测试覆盖本次变更,作者随后提交了测试文件。(已解决)

实现拆解

  1. 问题定位:发现 kernel allreduce_prototype_twoshot 接收的 flag_color 标量在 graph capture 时被固化。
  2. 参数改造:将参数类型从 uint32_t flag_color 改为 uint32_t* d_flag_counters,在 kernel 内部从设备内存读取并递增,写回。
  3. 宏同步更新:修改 TWOSHOT_DISPATCH 宏,将 d_flag_counters 传入 kernel launch。
  4. 生命周期管理:在 DeviceComms 中新增 d_flag_counters 成员,init() 中分配并初始化为 1,destroy() 中独立释放(绕过 initialized 标志)。
  5. 测试覆盖:在 test/manual/test_quick_allreduce.py 中新增 qr_graph_replay 函数和 TestQuickreduceGraphReplay 类,捕获单个 graph 并多次 replay 验证正确性。
文件 模块 状态 重要度
test/manual/test_quick_allreduce.py 快速规约 modified 7.02
sgl-kernel/csrc/allreduce/quick_all_reduce.h GPU 内核 modified 6.91

关键符号

allreduce_prototype_twoshot DeviceComms::init DeviceComms::destroy qr_graph_replay TestQuickreduceGraphReplay.test_quick_allreduce_graph_replay

关键源码片段

sgl-kernel/csrc/allreduce/quick_all_reduce.h core-logic

核心修改:将 flag_color 由 launch 标量改为设备端指针,在 kernel 内部动态递增

#include <hip/hip_runtime.h>
#include <vector>
#include "quick_all_reduce.cuh"namespace quickreduce {template <typename AllReduceKernel, typename T>
__global__ __quickreduce_launch_bounds_two_shot__ static void allreduce_prototype_twoshot(
    T const* A,
    T* B,
    uint32_t N,
    uint32_t num_blocks,
    int rank,
    uint8_t** dbuffer_list,
    uint32_t data_offset,
    uint32_t* d_flag_counters, // 每个 block 的 color 计数器(设备端)
    int64_t data_size_per_phase) {
  int block = blockIdx.x;
  int grid = gridDim.x;  // 从设备内存读取此 block 的 flag_color,使每次 graph replay 得到新值
  uint32_t flag_color = d_flag_counters[blockIdx.x];  while (block < num_blocks) {
    AllReduceKernel::run(A, B, N, block, rank, dbuffer_list, data_offset, flag_color, data_size_per_phase);
    block += grid;
    flag_color++;
  }
  // 整个 block 最终值一致,由 thread (0,0) 写回
  if (threadIdx.x == 0 && threadIdx.y == 0) {
    d_flag_counters[blockIdx.x] = flag_color;
  }
}#define TWOSHOT_DISPATCH(__codec) \
  if (world_size == 2) { \
    using LineCodec = __codec<T, 2>; \
    using AllReduceKernel = AllReduceTwoshot<T, LineCodec, cast_bf2half>; \
    hipLaunchKernelGGL( \
        (allreduce_prototype_twoshot<AllReduceKernel, T>), \
        dim3(grid), \
        dim3(kBlockTwoShot), \
        0, \
        stream, \
        A, B, N, num_blocks, rank, dbuffer_list, data_offset, \
        d_flag_counters, \
        this->kMaxProblemSize); \
  } else if (world_size == 4) { \
    // ... 类似 ( 省略 ) \
  } else if (world_size == 8) { \
    // ... 类似 ( 省略 ) \
  }struct DeviceComms {
  uint32_t* d_flag_counters = nullptr; // 新增:per-block color 指针  void init() {
    // ... 其他初始化
    // 为每个 block 分配 counter,初始值为 1(避免与零初始化的 flags 冲突)
    HIP_CHECK(hipMalloc(&d_flag_counters, kMaxNumBlocks * sizeof(uint32_t)));
    std::vector<uint32_t> init_color(kMaxNumBlocks, 1u);
    HIP_CHECK(hipMemcpy(d_flag_counters, init_color.data(),
                        kMaxNumBlocks * sizeof(uint32_t),
                        hipMemcpyHostToDevice));
  }  void destroy() {
    // 在 initialized 标志前释放 d_flag_counters,避免部分初始化泄漏
    if (d_flag_counters) {
      HIP_CHECK(hipFree(d_flag_counters));
      d_flag_counters = nullptr;
    }
    if (initialized) {
      // 释放其他资源 ...
    }
  }
};} // namespace quickreduce

评论区精华

资源泄漏防护建议 正确性

Code review 机器人建议在 destroy() 中同等地释放 dbuffer 和 dbuffer_list 以防止部分初始化失败时的资源泄漏。

结论:PR 仅添加了 d_flag_counters 的独立释放,未处理其他 buffer。合并者未明确提出要求,机器人建议未被采纳。 · acknowledged

测试覆盖要求 测试

合并者 HaiShaw 要求作者添加测试覆盖本次变更,因为 CI 未覆盖此路径。

结论:作者随后提交了测试文件 test/manual/test_quick_allreduce.py,包含 qr_graph_replay 测试。 · 已解决

风险与影响

  1. 修改了 kernel launch 参数,如果 d_flag_counters 指针无效或同步不当可能导致 GPU 错误。
  2. 新测试为手动测试,未集成到 CI 中,可能遗漏回归。
  3. d_flag_counters 在 destroy() 中释放,但 dbuffer 等仍依赖 initialized 标志,部分初始化失败时存在资源泄漏风险(机器人已指出)。
  4. 性能声明无影响。

影响范围:所有使用 AMD ROCm QuickReduce 的用户,特别是启用 CUDA Graph 加速的场景。影响程度:修复了导致计算结果错误的严重 bug,提升了正确性。不影响其他架构或非 QuickReduce 路径。团队增加了手动测试,需定期运行。

核心路径变更 缺少测试覆盖 平台依赖

关联 Issue

未识别关联 Issue

当前没有检测到明确关联的 Issue 链接,后续同步到相关引用后会出现在这里。

完整报告

参与讨论