已合并
消减mat_mul_v3算子编译时间 #1453
zhang-junming21创建于 2月3日
消减mat_mul_v3算子编译时间 #1453
已合并
zhang-junming21创建于 2月3日
共 2 个文件变更+47-115
@@ -1382,50 +1382,27 @@ __aicore__ inline void MatMulKernelDeterministicSplitK(GM_ADDR aGM, GM_ADDR bGM,
1382 }1382 }
1383 GM_ADDR mmOffsetGM = reinterpret_cast<GM_ADDR>(mmGM + GetBlockIdx() * singleSize * NUM_TWO * sizeof(float));1383 GM_ADDR mmOffsetGM = reinterpret_cast<GM_ADDR>(mmGM + GetBlockIdx() * singleSize * NUM_TWO * sizeof(float));
1384 using cType = MatmulType<C_TYPE::pos, C_TYPE::format, float, C_TYPE::isTrans>;1384 using cType = MatmulType<C_TYPE::pos, C_TYPE::format, float, C_TYPE::isTrans>;
1385+ // 默认情况:处理isNzA和isNzB均为true的情况
1386+ using aType = MatmulType<A_TYPE::pos, CubeFormat::NZ, typename A_TYPE::T, A_TYPE::isTrans>;
1387+ using bType = MatmulType<B_TYPE::pos, CubeFormat::NZ, typename B_TYPE::T, B_TYPE::isTrans>;
1388+ if (!matmulTilingData.matmulRunInfo.isNzA && !matmulTilingData.matmulRunInfo.isNzB) {
CANN-robot
CANN-robotCANN-robot2月3日

逻辑运算与副作用: 条件判断逻辑中缺少对matmulTilingData.matmulRunInfo.isNzA和matmulTilingData.matmulRunInfo.isNzB同时为true的情况处理。虽然代码中通过if-else if结构覆盖了三种情况,但没有显式处理两者都为true的情况。在重构前的代码中,两者都为true的情况是通过最后的else分支处理的,但重构后的代码逻辑不够清晰。

问题类型: 逻辑运算与副作用 文件路径: matmul/mat_mul_v3/op_kernel/mat_mul_deterministic_splitk_kernel.h 行号: 1387 问题代码:

        if (!matmulTilingData.matmulRunInfo.isNzA && !matmulTilingData.matmulRunInfo.isNzB) {
            using aType = A_TYPE;
            using bType = B_TYPE;
        } else if (matmulTilingData.matmulRunInfo.isNzA && !matmulTilingData.matmulRunInfo.isNzB) {
            using aType = MatmulType<A_TYPE::pos, CubeFormat::NZ, typename A_TYPE::T, A_TYPE::isTrans>;
            using bType = B_TYPE;
        } else if (!matmulTilingData.matmulRunInfo.isNzA && matmulTilingData.matmulRunInfo.isNzB) {
            using aType = A_TYPE;
            using bType = MatmulType<B_TYPE::pos, CubeFormat::NZ, typename B_TYPE::T, B_TYPE::isTrans>;
        }

修改建议:

建议添加一个else分支或显式处理两者都为true的情况,以提高代码的可读性和健壮性。修改建议:
1. 添加else分支处理两者都为true的情况
2. 或者添加注释说明默认情况处理两者都为true的场景
3. 重构为更清晰的switch-like结构

此评论由代码审查工具自动生成

likedislike
zhang-junming21
zhang-junming21
2月26日 评论:
zhang-junming21
zhang-junming21
2月27日 评论:
1389+ using aType = A_TYPE;
1390+ using bType = B_TYPE;
1391+ } else if (matmulTilingData.matmulRunInfo.isNzA && !matmulTilingData.matmulRunInfo.isNzB) {
1392+ using aType = MatmulType<A_TYPE::pos, CubeFormat::NZ, typename A_TYPE::T, A_TYPE::isTrans>;
1393+ using bType = B_TYPE;
1394+ } else if (!matmulTilingData.matmulRunInfo.isNzA && matmulTilingData.matmulRunInfo.isNzB) {
1395+ using aType = A_TYPE;
1396+ using bType = MatmulType<B_TYPE::pos, CubeFormat::NZ, typename B_TYPE::T, B_TYPE::isTrans>;
1397+ }
1385 if (isL2cacheSplit) {1398 if (isL2cacheSplit) {
1386- if (!matmulTilingData.matmulRunInfo.isNzA && !matmulTilingData.matmulRunInfo.isNzB) {1399+ MatMulMultiCoreSplitKDivideL2cache<aType, bType, cType, BIAS_TYPE>(aGM, bGM, biasGM, mmOffsetGM, singleSize,
1387- MatMulMultiCoreSplitKDivideL2cache<A_TYPE, B_TYPE, cType, BIAS_TYPE>(aGM, bGM, biasGM, mmOffsetGM, singleSize,1400+ matmulTilingData.matmulRunInfo.isHf32,
1388- matmulTilingData.matmulRunInfo.isHf32,1401+ &que, tiling, tiling.isBias);
1389- &que, tiling, tiling.isBias);
1390- } else if (matmulTilingData.matmulRunInfo.isNzA && !matmulTilingData.matmulRunInfo.isNzB) {
1391- using aType = MatmulType<A_TYPE::pos, CubeFormat::NZ, typename A_TYPE::T, A_TYPE::isTrans>;
1392- MatMulMultiCoreSplitKDivideL2cache<aType, B_TYPE, cType, BIAS_TYPE>(aGM, bGM, biasGM, mmOffsetGM, singleSize,
1393- matmulTilingData.matmulRunInfo.isHf32,
1394- &que, tiling, tiling.isBias);
1395- } else if (!matmulTilingData.matmulRunInfo.isNzA && matmulTilingData.matmulRunInfo.isNzB) {
1396- using bType = MatmulType<B_TYPE::pos, CubeFormat::NZ, typename B_TYPE::T, B_TYPE::isTrans>;
1397- MatMulMultiCoreSplitKDivideL2cache<A_TYPE, bType, cType, BIAS_TYPE>(aGM, bGM, biasGM, mmOffsetGM, singleSize,
1398- matmulTilingData.matmulRunInfo.isHf32,
1399- &que, tiling, tiling.isBias);
1400- } else {
1401- using aType = MatmulType<A_TYPE::pos, CubeFormat::NZ, typename A_TYPE::T, A_TYPE::isTrans>;
1402- using bType = MatmulType<B_TYPE::pos, CubeFormat::NZ, typename B_TYPE::T, B_TYPE::isTrans>;
1403- MatMulMultiCoreSplitKDivideL2cache<aType, bType, cType, BIAS_TYPE>(aGM, bGM, biasGM, mmOffsetGM, singleSize,
1404- matmulTilingData.matmulRunInfo.isHf32,
1405- &que, tiling, tiling.isBias);
1406- }
1407 } else {1402 } else {
CANN-robot
CANN-robotCANN-robot2月3日

代码结构与可维护性: 在代码重构中,非L2cache切分路径(else分支)的aType和bType类型定义存在问题。重构后的代码在第1402-1403行重新定义了aType和bType,但这两个类型在之前的条件判断中(第1387-1396行)可能已经被重新定义过。这会导致在非L2cache切分路径中,aType和bType的类型定义与之前的条件判断结果不一致,可能使用了错误的矩阵格式类型。

问题类型: 代码结构与可维护性 文件路径: matmul/mat_mul_v3/op_kernel/mat_mul_deterministic_splitk_kernel.h 行号: 1401 问题代码:

        } else {
            using aType = MatmulType<A_TYPE::pos, CubeFormat::NZ, typename A_TYPE::T, A_TYPE::isTrans>;
            using bType = MatmulType<B_TYPE::pos, CubeFormat::NZ, typename B_TYPE::T, B_TYPE::isTrans>;
            MatMulMultiCoreSplitKDivide<aType, bType, cType, BIAS_TYPE>(aGM, bGM, biasGM, mmOffsetGM, singleSize,
                                                                matmulTilingData.matmulRunInfo.isHf32,
                                                                &que, tiling, tiling.isBias);
        }

修改建议:

建议将非L2cache切分路径的aType和bType类型定义与L2cache切分路径保持一致,使用相同的条件判断逻辑。或者将类型定义提取到条件判断之前,确保两种路径使用相同的类型定义逻辑。修改建议:
1. 将第1387-1396行的条件判断逻辑提取到公共位置
2. 确保两种路径都使用相同的aType和bType定义
3. 或者将类型定义移到条件判断之前,避免重复定义

此评论由代码审查工具自动生成

likedislike
zhang-junming21
zhang-junming21
2月26日 评论:
zhang-junming21
zhang-junming21
2月27日 评论:
1408- if (!matmulTilingData.matmulRunInfo.isNzA && !matmulTilingData.matmulRunInfo.isNzB) {1403+ MatMulMultiCoreSplitKDivide<aType, bType, cType, BIAS_TYPE>(aGM, bGM, biasGM, mmOffsetGM, singleSize,
1409- MatMulMultiCoreSplitKDivide<A_TYPE, B_TYPE, cType, BIAS_TYPE>(aGM, bGM, biasGM, mmOffsetGM, singleSize,1404+ matmulTilingData.matmulRunInfo.isHf32,
1410- matmulTilingData.matmulRunInfo.isHf32,1405+ &que, tiling, tiling.isBias);
1411- &que, tiling, tiling.isBias);
1412- } else if (matmulTilingData.matmulRunInfo.isNzA && !matmulTilingData.matmulRunInfo.isNzB) {
1413- using aType = MatmulType<A_TYPE::pos, CubeFormat::NZ, typename A_TYPE::T, A_TYPE::isTrans>;
1414- MatMulMultiCoreSplitKDivide<aType, B_TYPE, cType, BIAS_TYPE>(aGM, bGM, biasGM, mmOffsetGM, singleSize,
1415- matmulTilingData.matmulRunInfo.isHf32,
1416- &que, tiling, tiling.isBias);
1417- } else if (!matmulTilingData.matmulRunInfo.isNzA && matmulTilingData.matmulRunInfo.isNzB) {
1418- using bType = MatmulType<B_TYPE::pos, CubeFormat::NZ, typename B_TYPE::T, B_TYPE::isTrans>;
1419- MatMulMultiCoreSplitKDivide<A_TYPE, bType, cType, BIAS_TYPE>(aGM, bGM, biasGM, mmOffsetGM, singleSize,
1420- matmulTilingData.matmulRunInfo.isHf32,
1421- &que, tiling, tiling.isBias);
1422- } else {
1423- using aType = MatmulType<A_TYPE::pos, CubeFormat::NZ, typename A_TYPE::T, A_TYPE::isTrans>;
1424- using bType = MatmulType<B_TYPE::pos, CubeFormat::NZ, typename B_TYPE::T, B_TYPE::isTrans>;
1425- MatMulMultiCoreSplitKDivide<aType, bType, cType, BIAS_TYPE>(aGM, bGM, biasGM, mmOffsetGM, singleSize,
1426- matmulTilingData.matmulRunInfo.isHf32,
1427- &que, tiling, tiling.isBias);
1428- }
1429 }1406 }
1430 return;1407 return;
1431 }1408 }
@@ -153,83 +153,38 @@ __aicore__ inline void MatMulUnAlignedKernelDeterministicSplitK(GM_ADDR aGM, GM_
153 return;153 return;
154 }154 }
155 using cType = MatmulType<C_TYPE::pos, C_TYPE::format, float, C_TYPE::isTrans>;155 using cType = MatmulType<C_TYPE::pos, C_TYPE::format, float, C_TYPE::isTrans>;
156- if (isL2cacheSplit) {156+ // 默认情况:当A和B均不存在nd2nz的情况
157- if (matmulTilingData.matmulRunInfo.nd2nzA && !matmulTilingData.matmulRunInfo.nd2nzB) {157+ using aType = MatmulType<A_TYPE::pos, CubeFormat::NZ, typename A_TYPE::T, A_TYPE::isTrans>;
158- using aType = MatmulType<A_TYPE::pos, CubeFormat::NZ, typename A_TYPE::T, A_TYPE::isTrans>;158+ using bType = MatmulType<B_TYPE::pos, CubeFormat::NZ, typename B_TYPE::T, B_TYPE::isTrans>;
159- if (matmulTilingData.matmulRunInfo.isNzB) {159+ GM_ADDR aActualGM = aGM;
160- using bType = MatmulType<B_TYPE::pos, CubeFormat::NZ, typename B_TYPE::T, B_TYPE::isTrans>;160+ GM_ADDR bActualGM = bGM;
161- MatMulMultiCoreSplitKDivideL2cache<aType, bType, cType, BIAS_TYPE>(alignedworkspaceGM, bGM, biasGM, mmOffsetGM,161+ if (matmulTilingData.matmulRunInfo.nd2nzA && !matmulTilingData.matmulRunInfo.nd2nzB) {
162- singleSize,162+ using aType = MatmulType<A_TYPE::pos, CubeFormat::NZ, typename A_TYPE::T, A_TYPE::isTrans>;
163- matmulTilingData.matmulRunInfo.isHf32,163+ aActualGM = alignedworkspaceGM;
164- &que, tiling, tiling.isBias);164+ if (matmulTilingData.matmulRunInfo.isNzB) {
165- } else {
166- MatMulMultiCoreSplitKDivideL2cache<aType, B_TYPE, cType, BIAS_TYPE>(alignedworkspaceGM, bGM, biasGM, mmOffsetGM,
167- singleSize,
168- matmulTilingData.matmulRunInfo.isHf32,
169- &que, tiling, tiling.isBias);
170- }
171- } else if (!matmulTilingData.matmulRunInfo.nd2nzA && matmulTilingData.matmulRunInfo.nd2nzB) {
172 using bType = MatmulType<B_TYPE::pos, CubeFormat::NZ, typename B_TYPE::T, B_TYPE::isTrans>;165 using bType = MatmulType<B_TYPE::pos, CubeFormat::NZ, typename B_TYPE::T, B_TYPE::isTrans>;
173- if (matmulTilingData.matmulRunInfo.isNzA) {166+ }
174- using aType = MatmulType<A_TYPE::pos, CubeFormat::NZ, typename A_TYPE::T, A_TYPE::isTrans>;167+ } else if (!matmulTilingData.matmulRunInfo.nd2nzA && matmulTilingData.matmulRunInfo.nd2nzB) {
175- MatMulMultiCoreSplitKDivideL2cache<aType, bType, cType, BIAS_TYPE>(aGM, alignedworkspaceGM,168+ using bType = MatmulType<B_TYPE::pos, CubeFormat::NZ, typename B_TYPE::T, B_TYPE::isTrans>;
176- biasGM, mmOffsetGM, singleSize,169+ bActualGM = alignedworkspaceGM;
177- matmulTilingData.matmulRunInfo.isHf32,170+ if (matmulTilingData.matmulRunInfo.isNzA) {
178- &que, tiling, tiling.isBias);
179- } else {
180- MatMulMultiCoreSplitKDivideL2cache<A_TYPE, bType, cType, BIAS_TYPE>(aGM, alignedworkspaceGM,
181- biasGM, mmOffsetGM, singleSize,
182- matmulTilingData.matmulRunInfo.isHf32,
183- &que, tiling, tiling.isBias);
184- }
185- } else if (matmulTilingData.matmulRunInfo.nd2nzA && matmulTilingData.matmulRunInfo.nd2nzB) {
186 using aType = MatmulType<A_TYPE::pos, CubeFormat::NZ, typename A_TYPE::T, A_TYPE::isTrans>;171 using aType = MatmulType<A_TYPE::pos, CubeFormat::NZ, typename A_TYPE::T, A_TYPE::isTrans>;
187- using bType = MatmulType<B_TYPE::pos, CubeFormat::NZ, typename B_TYPE::T, B_TYPE::isTrans>;172+ }
173+ } else if (matmulTilingData.matmulRunInfo.nd2nzA && matmulTilingData.matmulRunInfo.nd2nzB) {
174+ using aType = MatmulType<A_TYPE::pos, CubeFormat::NZ, typename A_TYPE::T, A_TYPE::isTrans>;
175+ using bType = MatmulType<B_TYPE::pos, CubeFormat::NZ, typename B_TYPE::T, B_TYPE::isTrans>;
176+ aActualGM = alignedworkspaceGM;
177+ bActualGM = alignedworkspaceGM + alignedOriM * alignedKaSize * sizeof(A_T);
178+ }
188 179 
189- MatMulMultiCoreSplitKDivideL2cache<aType, bType, cType, BIAS_TYPE>(alignedworkspaceGM, alignedworkspaceGM +180+ if (isL2cacheSplit) {
CANN-robot
CANN-robotCANN-robot2月3日

代码结构与可维护性: 重构后的代码将原本分散的多个条件分支合并为统一的函数调用,这是一个好的改进。但是,第177-185行的函数调用中使用的aType和bType可能不是根据nd2nzA、nd2nzB、isNzA、isNzB等条件动态选择的类型。实际上,代码总是使用第156-157行定义的aType和bType,这可能不符合原始代码的逻辑意图。

问题类型: 代码结构与可维护性 文件路径: matmul/mat_mul_v3/op_kernel/mat_mul_unaligned_deterministic_splitk_kernel.h 行号: 177 问题代码:

        if (isL2cacheSplit) {
            MatMulMultiCoreSplitKDivideL2cache<aType, bType, cType, BIAS_TYPE>(aGM, bGM, biasGM, mmOffsetGM,
                                                                singleSize, matmulTilingData.matmulRunInfo.isHf32,
                                                                &que, tiling, tiling.isBias);
        } else {
            MatMulMultiCoreSplitKDivide<aType, bType, cType, BIAS_TYPE>(aGM, bGM, biasGM, mmOffsetGM, singleSize,
                                                                    matmulTilingData.matmulRunInfo.isHf32,
                                                                    &que, tiling, tiling.isBias);
        }

修改建议:

需要确认重构后的逻辑是否正确。如果原始代码中不同条件分支使用不同的类型模板参数,那么重构后的代码可能需要使用条件编译或运行时类型选择来确保类型正确性。建议添加注释说明类型选择逻辑,或者使用static_assert确保类型符合预期。

此评论由代码审查工具自动生成

likedislike
zhang-junming21
zhang-junming21
2月26日 评论:
zhang-junming21
zhang-junming21
2月27日 评论:
190- alignedOriM * alignedKaSize * sizeof(A_T),181+ MatMulMultiCoreSplitKDivideL2cache<aType, bType, cType, BIAS_TYPE>(aActualGM, bActualGM, biasGM, mmOffsetGM,
191- biasGM, mmOffsetGM, singleSize,182+ singleSize, matmulTilingData.matmulRunInfo.isHf32,
192- matmulTilingData.matmulRunInfo.isHf32,183+ &que, tiling, tiling.isBias);
193- &que, tiling, tiling.isBias);
194- }
195 } else {184 } else {
196- if (matmulTilingData.matmulRunInfo.nd2nzA && !matmulTilingData.matmulRunInfo.nd2nzB) {185+ MatMulMultiCoreSplitKDivide<aType, bType, cType, BIAS_TYPE>(aActualGM, bActualGM, biasGM, mmOffsetGM, singleSize,
197- using aType = MatmulType<A_TYPE::pos, CubeFormat::NZ, typename A_TYPE::T, A_TYPE::isTrans>;186+ matmulTilingData.matmulRunInfo.isHf32,
198- if (matmulTilingData.matmulRunInfo.isNzB) {187+ &que, tiling, tiling.isBias);
199- using bType = MatmulType<B_TYPE::pos, CubeFormat::NZ, typename B_TYPE::T, B_TYPE::isTrans>;
200- MatMulMultiCoreSplitKDivide<aType, bType, cType, BIAS_TYPE>(alignedworkspaceGM, bGM, biasGM, mmOffsetGM,
201- singleSize,
202- matmulTilingData.matmulRunInfo.isHf32,
203- &que, tiling, tiling.isBias);
204- } else {
205- MatMulMultiCoreSplitKDivide<aType, B_TYPE, cType, BIAS_TYPE>(alignedworkspaceGM, bGM, biasGM, mmOffsetGM,
206- singleSize,
207- matmulTilingData.matmulRunInfo.isHf32,
208- &que, tiling, tiling.isBias);
209- }
210- } else if (!matmulTilingData.matmulRunInfo.nd2nzA && matmulTilingData.matmulRunInfo.nd2nzB) {
211- using bType = MatmulType<B_TYPE::pos, CubeFormat::NZ, typename B_TYPE::T, B_TYPE::isTrans>;
212- if (matmulTilingData.matmulRunInfo.isNzA) {
213- using aType = MatmulType<A_TYPE::pos, CubeFormat::NZ, typename A_TYPE::T, A_TYPE::isTrans>;
214- MatMulMultiCoreSplitKDivide<aType, bType, cType, BIAS_TYPE>(aGM, alignedworkspaceGM,
215- biasGM, mmOffsetGM, singleSize,
216- matmulTilingData.matmulRunInfo.isHf32,
217- &que, tiling, tiling.isBias);
218- } else {
219- MatMulMultiCoreSplitKDivide<A_TYPE, bType, cType, BIAS_TYPE>(aGM, alignedworkspaceGM,
220- biasGM, mmOffsetGM, singleSize,
221- matmulTilingData.matmulRunInfo.isHf32,
222- &que, tiling, tiling.isBias);
223- }
224- } else if (matmulTilingData.matmulRunInfo.nd2nzA && matmulTilingData.matmulRunInfo.nd2nzB) {
225- using aType = MatmulType<A_TYPE::pos, CubeFormat::NZ, typename A_TYPE::T, A_TYPE::isTrans>;
226- using bType = MatmulType<B_TYPE::pos, CubeFormat::NZ, typename B_TYPE::T, B_TYPE::isTrans>;
227- MatMulMultiCoreSplitKDivide<aType, bType, cType, BIAS_TYPE>(alignedworkspaceGM, alignedworkspaceGM +
228- alignedOriM * alignedKaSize * sizeof(A_T),
229- biasGM, mmOffsetGM, singleSize,
230- matmulTilingData.matmulRunInfo.isHf32,
231- &que, tiling, tiling.isBias);
232- }
233 }188 }
234 return;189 return;
235 }190 }