[Feature]【Hackathon 10th Spring No.46】WIN32 ifdef compilation guards [cf]#7701
[Feature]【Hackathon 10th Spring No.46】WIN32 ifdef compilation guards [cf]#7701ghost wants to merge 1 commit intoPaddlePaddle:developfrom
Conversation
|
Thanks for your contribution! |
CI报告基于以下代码生成(30分钟更新一次): 1 任务总览所有已执行的 CI 任务均已通过,但有 7 个 Workflow 处于
2 任务状态汇总2.1 Required 任务 : 0/0 通过
2.2 可选任务 — 2/2 通过
3 失败详情(仅 required)无 required 失败任务。 |
PaddlePaddle-bot
left a comment
There was a problem hiding this comment.
🤖 Paddle-CI-Agent | pr_review |
2026-05-03 22:04:04
📋 Review 摘要
PR 概述:为 custom_ops/gpu_ops/ 下 28 个 CUDA/C++ 文件添加 #ifndef _WIN32 编译保护,将 POSIX 专属头文件(<fcntl.h>、<sys/mman.h>、<sys/ipc.h> 等)置于条件编译块内,并为依赖 POSIX API 的函数添加 Windows 下的 PD_THROW 存根。
变更范围:custom_ops/gpu_ops/(GPU 算子层)、custom_ops/gpu_ops/speculate_decoding/(投机解码算子)
影响面 Tag:[OP] [Speculative Decoding]
📝 PR 规范检查
PR 描述中 ## Motivation、## Modifications、## Usage or Command、## Accuracy Tests 四个段落均为 <!-- TODO: ... --> 占位符,内容未填写;Checklist 条目也与仓库模板不符。不满足 PR 描述规范,需补全。
标题建议(可直接复制):
[Feature] Add WIN32 compilation guards for POSIX headers in custom_ops/gpu_ops
PR 描述建议(可直接复制,必须复刻 checklist §D2 模板的完整结构):
## Motivation
`custom_ops/gpu_ops/` 下各算子文件无条件引入了 POSIX 专属头文件(`<fcntl.h>`、`<sys/mman.h>`、`<unistd.h>`、`<sys/ipc.h>`、`<sys/msg.h>` 等),导致在 Windows 平台编译时直接报错。本 PR 通过添加 `#ifndef _WIN32` 保护块,使代码库在 Windows 下可正常编译,并为依赖 POSIX 共享内存 / 消息队列的函数添加明确的 `PD_THROW` 不支持存根。
## Modifications
- 为 28 个 `custom_ops/gpu_ops/` 文件中的 POSIX 专属头文件引入添加 `#ifndef _WIN32 ... #endif` 保护块
- 在依赖 `mmap`/`shm_open`/`msgget`/`msgsnd`/`msgrcv` 等 POSIX API 的函数体内,添加 `#ifdef _WIN32 PD_THROW(...) #endif` 存根,明确声明 Windows 下不支持
- 在 `env.h` 中补充缺失的 `#include <cstdlib>` 和 `#include <string>`
- 在 `speculate_get_output_with_topk.cc` 和 `speculate_save_output_with_topk.cc` 中,移除对 `speculate_logprob_msg.h` 的依赖并内联定义相关 IPC 结构体
## Usage or Command
N/A
## Accuracy Tests
N/A
## Checklist
- [x] Add at least a tag in the PR title.
- Tag list: [`[FDConfig]`,`[APIServer]`,`[Engine]`, `[Scheduler]`, `[PD Disaggregation]`, `[Executor]`, `[Graph Optimization]`, `[Speculative Decoding]`, `[RL]`, `[Models]`, `[Quantization]`, `[Loader]`, `[OP]`, `[KVCache]`, `[DataProcessor]`, `[BugFix]`, `[Docs]`, `[CI]`, `[Optimization]`, `[Feature]`, `[Benchmark]`, `[Others]`, `[XPU]`, `[HPU]`, `[GCU]`, `[DCU]`, `[Iluvatar]`, `[Metax]`]
- You can add new tags based on the PR content, but the semantics must be clear.
- [ ] Format your code, run `pre-commit` before commit.
- [ ] Add unit tests. Please write the reason in this PR if no unit tests.
- [ ] Provide accuracy results.
- [ ] If the current PR is submitting to the `release` branch, make sure the PR has been submitted to the `develop` branch, then cherry-pick it to the `release` branch with the `[Cherry-Pick]` PR tag.问题
| 级别 | 文件 | 概述 |
|---|---|---|
| 🟡 建议 | custom_ops/gpu_ops/speculate_decoding/speculate_save_output_with_topk.cc:31 |
#define K 20 使用单字母宏名,存在与下游头文件宏冲突的风险 |
| 🟡 建议 | custom_ops/gpu_ops/speculate_decoding/speculate_get_output_with_topk.cc:31 |
同上,#define K 20 宏命名冲突风险 |
| 🟡 建议 | custom_ops/gpu_ops/speculate_decoding/speculate_save_output_with_topk.cc:34 |
移除共享头文件 speculate_logprob_msg.h,在 sender/receiver 两文件中各自内联定义 IPC 通信结构体,破坏单一数据源,后续维护时存在静默二进制不兼容风险 |
| 📝 PR 规范 | — | PR 描述四个正文段落均为 TODO 占位符,Checklist 与仓库模板不符 |
总体评价
整体方向正确,系统性地为 28 个文件补齐了 WIN32 编译保护,env.h 补充缺失 include 也是合理修复。主要隐患集中在投机解码相关的两个 topk 文件:移除共享头文件并重复内联 IPC 结构体定义破坏了通信协议的单一数据源,#define K 20 等单字母宏名也存在命名冲突风险,建议修复后合入。
| #endif | ||
|
|
||
| #define MAX_BSZ 512 | ||
| #define K 20 |
There was a problem hiding this comment.
🟡 建议 #define K 20 使用单字母宏名,极易与下游头文件(如数学库、Eigen、CUDA 工具库)中同名符号冲突,导致难以排查的编译错误或逻辑错误。原代码使用 SPEC_LOGPROB_K(语义明确、无污染),建议沿用该风格,改为 SPEC_TOPK_K 或 SPECULATE_TOPK_K。同理 MAX_BSZ、MAX_DRAFT_TOKEN_NUM 也建议加前缀以避免命名冲突。
| #endif | ||
|
|
||
| #define MAX_BSZ 512 | ||
| #define K 20 |
There was a problem hiding this comment.
🟡 建议 同 speculate_save_output_with_topk.cc,#define K 20 使用单字母宏名存在冲突风险。
| #define K 20 | ||
| #define MAX_DRAFT_TOKEN_NUM 6 | ||
|
|
||
| struct batch_msgdata { |
There was a problem hiding this comment.
🟡 建议 此 PR 删除了 #include "speculate_logprob_msg.h" 并在本文件与 speculate_get_output_with_topk.cc 中各自内联定义了完全相同的 batch_msgdata/msgdata 结构体。两文件通过 IPC 消息队列收发数据,二者结构体布局必须完全一致(二进制协议)。当前重复定义虽暂时一致,但后续只修改一侧时,编译器不会给出任何警告,将导致消息解析静默损坏。
建议:提取 struct 定义到一个不含 POSIX 依赖的纯头文件中(例如 speculate_topk_msg.h),两侧均 #include 该头文件,保持单一数据源(Single Source of Truth)。
Motivation
Modifications
Usage or Command
Accuracy Tests
Checklist