已合并
soc整改math仓common同步到非dev仓 #802
chenfeng创建于 1月20日
soc整改math仓common同步到非dev仓 #802
已合并
chenfeng创建于 1月20日
4 个文件变更+27-19
@@ -26,6 +26,7 @@ target_include_directories(${COMMON_NAME}_obj
26 ${PLATFORM_INC_DIRS}26 ${PLATFORM_INC_DIRS}
27 ${METADEF_INCLUDE_DIRS}27 ${METADEF_INCLUDE_DIRS}
28 ${OPS_MATH_DIR}/common/inc28 ${OPS_MATH_DIR}/common/inc
29+ ${ASCEND_DIR}/pkg_inc
29)30)
30 31 
31target_link_libraries(${COMMON_NAME}_obj32target_link_libraries(${COMMON_NAME}_obj
@@ -39,4 +40,4 @@ foreach(SUB_DIR ${CURRENT_DIRS})
39 if(EXISTS "${CMAKE_CURRENT_SOURCE_DIR}/${SUB_DIR}/framework/CMakeLists.txt")40 if(EXISTS "${CMAKE_CURRENT_SOURCE_DIR}/${SUB_DIR}/framework/CMakeLists.txt")
40 add_subdirectory(${SUB_DIR}/framework)41 add_subdirectory(${SUB_DIR}/framework)
41 endif()42 endif()
42-endforeach()43+endforeach()
@@ -137,5 +137,12 @@ static inline bool IsRegBase(NpuArch arch)
137 return regbaseArch.find(arch) != regbaseArch.end();137 return regbaseArch.find(arch) != regbaseArch.end();
138}138}
139 139 
140+static inline bool IsRegBase()
CANN-robot
CANN-robotCANN-robot1月20日

代码重复: 新增的IsRegBase()函数与已有的IsRegBase(NpuArch arch)函数存在代码重复。两个函数都定义了相同的静态集合regbaseArch = {NpuArch::DAV_3510},这违反了DRY(Don't Repeat Yourself)原则。当需要修改支持的架构列表时,需要在两个地方进行修改,增加了维护成本和出错风险。

问题类型: 代码重复 文件路径: common/inc/op_api/aclnn_check.h 行号: 140 问题代码:

static inline bool IsRegBase()
{
    const static std::set<NpuArch> regbaseArch = {NpuArch::DAV_3510};
    auto curArch = GetCurrentPlatformInfo().GetCurNpuArch();
    return regbaseArch.find(curArch) != regbaseArch.end();
}

修改建议:

建议重构代码,消除重复。可以有以下几种方案:
1. 让无参数的IsRegBase()函数调用有参数的版本:
   static inline bool IsRegBase() {
       return IsRegBase(GetCurrentPlatformInfo().GetCurNpuArch());
   }
2. 将支持的架构集合定义为一个公共的静态常量,供两个函数共享。
3. 如果无参数的版本使用频率更高,可以考虑只保留一个版本,并在需要时通过GetCurrentPlatformInfo().GetCurNpuArch()获取当前架构。

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

likedislike
141+{
142+ const static std::set<NpuArch> regbaseArch = {NpuArch::DAV_3510};
CANN-robot
CANN-robotCANN-robot1月20日

代码结构与可维护性: 代码中存在重复的静态常量定义。第136行和第142行都定义了相同的静态set容器regbaseArch,包含相同的值{NpuArch::DAV_3510}。这种重复违反了DRY(Don't Repeat Yourself)原则,增加了维护成本,如果未来需要修改支持的架构列表,需要在两个地方同时修改,容易遗漏。

问题类型: 代码结构与可维护性 文件路径: common/inc/op_api/aclnn_check.h 行号: 142 问题代码:

const static std::set<NpuArch> regbaseArch = {NpuArch::DAV_3510};

修改建议:

1. 将regbaseArch的定义提取为命名空间级的静态常量,在函数外部定义一次:
   static const std::set<NpuArch> kRegBaseArchs = {NpuArch::DAV_3510};
2. 两个函数都引用这个公共常量,避免重复定义。
3. 如果未来需要支持更多架构,只需在一个地方修改。

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

likedislike
143+ auto curArch = GetCurrentPlatformInfo().GetCurNpuArch();
144+ return regbaseArch.find(curArch) != regbaseArch.end();
145+}
146+ 
140} // namespace op147} // namespace op
141#endif148#endif
@@ -11,7 +11,9 @@
11#ifndef LEVEL2_BASE_H_MATH11#ifndef LEVEL2_BASE_H_MATH
12#define LEVEL2_BASE_H_MATH12#define LEVEL2_BASE_H_MATH
13 13 
14+#include <stdio.h>
14#include "op_api/op_api_def.h"15#include "op_api/op_api_def.h"
16+#include "op_api/aclnn_check.h"
15#include "aclnn/aclnn_base.h"17#include "aclnn/aclnn_base.h"
16 18 
17#ifdef __cplusplus19#ifdef __cplusplus
@@ -99,8 +101,7 @@ namespace op {
99[[maybe_unused]] static const std::initializer_list<DataType>& GetDtypeSupportListV1(101[[maybe_unused]] static const std::initializer_list<DataType>& GetDtypeSupportListV1(
100 const std::initializer_list<op::DataType>& l1, const std::initializer_list<op::DataType>& l2)102 const std::initializer_list<op::DataType>& l1, const std::initializer_list<op::DataType>& l2)
101{103{
102- if (GetCurrentPlatformInfo().GetSocVersion() == SocVersion::ASCEND910B ||104+ if (GetCurrentPlatformInfo().GetCurNpuArch() == NpuArch::DAV_2201) {
103- GetCurrentPlatformInfo().GetSocVersion() == SocVersion::ASCEND910_93) {
104 return l1;105 return l1;
105 } else {106 } else {
106 return l2;107 return l2;
@@ -114,8 +115,8 @@ namespace op {
114[[maybe_unused]] static const std::initializer_list<DataType>& GetDtypeSupportListV2(115[[maybe_unused]] static const std::initializer_list<DataType>& GetDtypeSupportListV2(
115 const std::initializer_list<op::DataType>& l1, const std::initializer_list<op::DataType>& l2)116 const std::initializer_list<op::DataType>& l1, const std::initializer_list<op::DataType>& l2)
116{117{
117- if (GetCurrentPlatformInfo().GetSocVersion() >= SocVersion::ASCEND910B &&118+ auto curArch = GetCurrentPlatformInfo().GetCurNpuArch();
118- GetCurrentPlatformInfo().GetSocVersion() <= SocVersion::ASCEND910E) {119+ if(curArch == NpuArch::DAV_2201 || IsRegBase(curArch)) {
119 return l1;120 return l1;
120 } else {121 } else {
121 return l2;122 return l2;
@@ -125,14 +126,13 @@ namespace op {
125[[maybe_unused]] static const std::initializer_list<op::DataType> GetDtypeSupportListV3(126[[maybe_unused]] static const std::initializer_list<op::DataType> GetDtypeSupportListV3(
126 const std::initializer_list<op::DataType>& l1, const std::initializer_list<op::DataType>& l2)127 const std::initializer_list<op::DataType>& l1, const std::initializer_list<op::DataType>& l2)
127{128{
128- auto socVersion = GetCurrentPlatformInfo().GetSocVersion();129+ auto curArch = GetCurrentPlatformInfo().GetCurNpuArch();
129- switch (socVersion) {130+ switch (curArch) {
130- case SocVersion::ASCEND910_93:131+ case NpuArch::DAV_2201:
131- case SocVersion::ASCEND910_95:132+ case NpuArch::DAV_3510: {
132- case SocVersion::ASCEND910B: {
133 return l1;133 return l1;
134 }134 }
135- case SocVersion::ASCEND910: {135+ case NpuArch::DAV_1001: {
136 return l2;136 return l2;
137 }137 }
138 default: {138 default: {
@@ -22,26 +22,26 @@ namespace Math {
22namespace OpTiling {22namespace OpTiling {
23static const gert::Shape g_vec_1_shape = {1};23static const gert::Shape g_vec_1_shape = {1};
24 24 
25-static bool IsRegbaseSocVersion(platform_ascendc::SocVersion version)25+static bool IsRegbaseSocVersion(NpuArch npuArch)
26{26{
27- const static std::set<platform_ascendc::SocVersion> regbaseSocVersions = {27+ const static std::set<NpuArch> regbaseArch = {
28- platform_ascendc::SocVersion::ASCEND910_95};28+ NpuArch::DAV_3510};
29 29 
30- return regbaseSocVersions.find(version) != regbaseSocVersions.end();30+ return regbaseArch.find(npuArch) != regbaseArch.end();
31}31}
32 32 
33bool IsRegbaseSocVersion(const gert::TilingParseContext* context)33bool IsRegbaseSocVersion(const gert::TilingParseContext* context)
34{34{
35 auto ascendcPlatform = platform_ascendc::PlatformAscendC(context->GetPlatformInfo());35 auto ascendcPlatform = platform_ascendc::PlatformAscendC(context->GetPlatformInfo());
36- auto socVersion = ascendcPlatform.GetSocVersion();36+ auto arch = ascendcPlatform.GetCurNpuArch();
37- return IsRegbaseSocVersion(socVersion);37+ return IsRegbaseSocVersion(arch);
38}38}
39 39 
40bool IsRegbaseSocVersion(const gert::TilingContext* context)40bool IsRegbaseSocVersion(const gert::TilingContext* context)
41{41{
42 auto ascendcPlatform = platform_ascendc::PlatformAscendC(context->GetPlatformInfo());42 auto ascendcPlatform = platform_ascendc::PlatformAscendC(context->GetPlatformInfo());
43- auto socVersion = ascendcPlatform.GetSocVersion();43+ auto arch = ascendcPlatform.GetCurNpuArch();
44- return IsRegbaseSocVersion(socVersion);44+ return IsRegbaseSocVersion(arch);
45}45}
46 46 
47const gert::Shape &EnsureNotScalar(const gert::Shape &inShape) {47const gert::Shape &EnsureNotScalar(const gert::Shape &inShape) {