Skip to content

Commit b8eeb30

Browse files
[rocm-libraries] ROCm/rocm-libraries#1561 (commit c28c222)
[MIOpen] Asm code fix and test update ## Motivation With last compiler version kernel build failed: ``` /tmp/comgr-xxx/input/conv1x1wrw.s:1606:21: error: redefinition of 'data_prefetch' MIOpen Error: /xxx/src/MIOpen/src/hipoc/hipoc_program.cpp:299: Code object build failed. Source: conv1x1wrw.s ``` In latest versions of clang , looks like assembler has become stricter about symbol definitions and treating that the data_prefetch is defined twice once with default and the other with the = assignment. ## Technical Details Added test with affected kernel configuration. Redefinition replaced by new variable. ## Test Plan mi2xx ./bin/miopen_gtest --gtest_filter=*GPU_Conv2d*AsmBwdWrw* <!-- Explain any relevant testing done to verify this PR. --> PASSED <!-- Briefly summarize test outcomes. --> ## Submission Checklist - [x] Look over the contributing guidelines at https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests.
1 parent b097e3d commit b8eeb30

4 files changed

Lines changed: 68 additions & 17 deletions

File tree

src/env_debug.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ MIOPEN_DECLARE_ENV_VAR_STR(MIOPEN_FIND_ENFORCE)
6565
MIOPEN_DECLARE_ENV_VAR_STR(MIOPEN_FIND_MODE)
6666
MIOPEN_DECLARE_ENV_VAR_STR(MIOPEN_USER_DB_PATH)
6767
MIOPEN_DECLARE_ENV_VAR_UINT64(MIOPEN_LOG_LEVEL)
68+
MIOPEN_DECLARE_ENV_VAR_STR(MIOPEN_DEBUG_CONV_DIRECT_ASM_WRW1X1_PERF_VALS)
6869

6970
namespace miopen {
7071
namespace debug {
@@ -200,6 +201,7 @@ const LibEnvVar& FindEnvVariable(std::string_view name)
200201
{MIOPEN_FIND_MODE.GetName(), MIOPEN_FIND_MODE},
201202
{MIOPEN_LOG_LEVEL.GetName(), MIOPEN_LOG_LEVEL},
202203
{MIOPEN_USER_DB_PATH.GetName(), MIOPEN_USER_DB_PATH},
204+
{MIOPEN_DEBUG_CONV_DIRECT_ASM_WRW1X1_PERF_VALS.GetName(), MIOPEN_DEBUG_CONV_DIRECT_ASM_WRW1X1_PERF_VALS}
203205
// clang-format on
204206
};
205207

src/kernels/conv1x1wrw.s

Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -811,14 +811,15 @@ miopenGcnAsmConv1x1WrW:
811811
.endr
812812
.endm
813813

814+
data_prefetch_limited = data_prefetch
814815
.if(full_reads > 0 && full_reads < data_prefetch + 1)
815-
data_prefetch = full_reads - 1
816+
data_prefetch_limited = full_reads - 1
816817
.endif
817818

818819
LD_PARTIAL_CHUNKS = 1
819820
LD_FULL_CHUNKS = 0
820821
LD_PART_A_ID = 0
821-
LD_PART_B_ID = data_prefetch
822+
LD_PART_B_ID = data_prefetch_limited
822823
last_free_ld_part = LD_PART_B_ID
823824

824825

@@ -843,14 +844,14 @@ last_free_ld_part = LD_PART_B_ID
843844

844845
.macro data_prefetch_init_q q_wait_cnt, singl_wait_cnt
845846
q_id = LD_PART_A_ID
846-
.rept data_prefetch
847+
.rept data_prefetch_limited
847848
ld_buffers_inc_pointers_rept_waitcnt LD_FULL_CHUNKS, q_id, \singl_wait_cnt
848849
\q_wait_cnt = \q_wait_cnt + \singl_wait_cnt
849850
q_id = (q_id + 1)
850851
.endr
851852
.endm
852853

853-
.macro data_ld_prefetch_active_loop q_wait_cnt, loop_cnt=data_prefetch+1
854+
.macro data_ld_prefetch_active_loop q_wait_cnt, loop_cnt=data_prefetch_limited+1
854855
q_id_conv = LD_PART_A_ID
855856
q_id_data_ld = LD_PART_B_ID
856857
.rept \loop_cnt
@@ -860,19 +861,19 @@ last_free_ld_part = LD_PART_B_ID
860861

861862
m_conv_accums elements_per_lane, q_id_conv
862863

863-
q_id_conv = ((q_id_conv + 1) % (data_prefetch + 1))
864-
q_id_data_ld = ((q_id_data_ld + 1) % (data_prefetch + 1))
864+
q_id_conv = ((q_id_conv + 1) % (data_prefetch_limited + 1))
865+
q_id_data_ld = ((q_id_data_ld + 1) % (data_prefetch_limited + 1))
865866
.endr
866867
.endm
867868

868869
.macro data_prefetch_conv_finalizing q_wait_cnt, singl_wait_cnt, q_id_conv_off=0
869-
q_id_conv = ((LD_PART_A_ID + \q_id_conv_off) % (data_prefetch + 1))
870+
q_id_conv = ((LD_PART_A_ID + \q_id_conv_off) % (data_prefetch_limited + 1))
870871

871-
.rept data_prefetch
872+
.rept data_prefetch_limited
872873
\q_wait_cnt = (\q_wait_cnt - \singl_wait_cnt)
873874
s_wait \q_wait_cnt
874875
m_conv_accums elements_per_lane, q_id_conv
875-
q_id_conv = ((q_id_conv + 1) % (data_prefetch + 1))
876+
q_id_conv = ((q_id_conv + 1) % (data_prefetch_limited + 1))
876877
.endr
877878
.endm
878879

@@ -889,21 +890,21 @@ loop_n_begin: // loop over batch (n)
889890

890891
data_prefetch_init_q q_wait_vec_load_full, single_wait_vec_load_full
891892

892-
.if(full_reads >= 2 * data_prefetch + 1)
893+
.if(full_reads >= 2 * data_prefetch_limited + 1)
893894
loop_hw_begin:
894895
data_ld_prefetch_active_loop q_wait_vec_load_full
895896

896897
loop_hw_end:
897-
s_addk_i32 s[loop_hw_cnt], 1 + data_prefetch
898-
s_cmpk_gt_u32 s[loop_hw_cnt], 0 + full_reads - (2 * data_prefetch + 1)
898+
s_addk_i32 s[loop_hw_cnt], 1 + data_prefetch_limited
899+
s_cmpk_gt_u32 s[loop_hw_cnt], 0 + full_reads - (2 * data_prefetch_limited + 1)
899900
s_cbranch_scc0 loop_hw_begin
900901
.endif
901-
.if( (full_reads - data_prefetch) % (data_prefetch + 1) != 0)
902-
loop_resi = ((full_reads - data_prefetch) % (data_prefetch + 1))
902+
.if( (full_reads - data_prefetch_limited) % (data_prefetch_limited + 1) != 0)
903+
loop_resi = ((full_reads - data_prefetch_limited) % (data_prefetch_limited + 1))
903904

904905
data_ld_prefetch_active_loop q_wait_vec_load_full, loop_resi
905906

906-
last_free_ld_part = ((LD_PART_B_ID + loop_resi) % (data_prefetch + 1))
907+
last_free_ld_part = ((LD_PART_B_ID + loop_resi) % (data_prefetch_limited + 1))
907908
.endif
908909
.endif
909910

@@ -919,7 +920,7 @@ loop_n_begin: // loop over batch (n)
919920
q_wait_vec_load_full = q_wait_vec_load_full + wait_vec_load_part
920921
.endif
921922

922-
.if(full_reads != 0 && data_prefetch != 0)
923+
.if(full_reads != 0 && data_prefetch_limited != 0)
923924
data_prefetch_conv_finalizing q_wait_vec_load_full, single_wait_vec_load_full, loop_resi
924925
.endif
925926

test/gtest/gtest_common.hpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,14 @@ inline void tuning_check(const std::string& err)
120120
default_check(err);
121121
}
122122

123+
inline void compiler_check(const std::string& err)
124+
{
125+
// the test should fail if kernel build failed.
126+
EXPECT_FALSE(err.find("Error") != std::string::npos ||
127+
err.find("Code object build failed") != std::string::npos);
128+
default_check(err);
129+
}
130+
123131
inline void db_check(const std::string& err)
124132
{
125133
EXPECT_FALSE(err.find("Perf Db: record not found") != std::string::npos);

test/gtest/smoke_solver_convasmbwdwrw.cpp

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,26 @@
3232

3333
namespace {
3434

35+
MIOPEN_LIB_ENV_VAR(MIOPEN_DEBUG_CONV_DIRECT_ASM_WRW1X1_PERF_VALS)
36+
37+
auto GetTestCompilerRegressionCases()
38+
{
39+
const auto env_regression_check =
40+
std::tuple{std::pair{MIOPEN_FIND_ENFORCE, "SEARCH"},
41+
std::pair{wa::MIOPEN_DEBUG_TUNING_ITERATIONS_MAX, 1},
42+
std::pair{MIOPEN_FIND_MODE, "normal"},
43+
std::pair{MIOPEN_DEBUG_CONV_DIRECT_ASM_WRW1X1_PERF_VALS, "2,8,4,2,4,2,2,4,0,2"},
44+
std::pair{MIOPEN_DEBUG_FIND_ONLY_SOLVER, "ConvAsmBwdWrW1x1"}};
45+
46+
const std::string vw = " --verbose --disable-forward --disable-backward-data";
47+
48+
return std::vector{
49+
// clang-format off
50+
std::pair{env_regression_check, vw + " --input 1 4 6 6 --weights 4 4 1 1 --pads_strides_dilations 0 0 1 1 1 1"}
51+
// clang-format on
52+
};
53+
}
54+
3555
auto GetTestCases()
3656
{
3757
const auto env_w1 = std::tuple{std::pair{MIOPEN_FIND_ENFORCE, "SEARCH_DB_UPDATE"},
@@ -48,7 +68,8 @@ auto GetTestCases()
4868
};
4969
}
5070

51-
using TestCase = decltype(GetTestCases())::value_type;
71+
using RegrTestCase = decltype(GetTestCompilerRegressionCases())::value_type;
72+
using TestCase = decltype(GetTestCases())::value_type;
5273

5374
bool SkipTest() { return get_handle_xnack(); }
5475

@@ -61,6 +82,10 @@ bool IsTestSupportedForDevice()
6182

6283
} // namespace
6384

85+
class GPU_Conv2dSingleAsmBwdWrw_FP32 : public FloatTestCase<std::vector<RegrTestCase>>
86+
{
87+
};
88+
6489
class GPU_Conv2dTuningAsmBwdWrw_FP32 : public FloatTestCase<std::vector<TestCase>>
6590
{
6691
};
@@ -85,6 +110,18 @@ TEST_P(GPU_Conv2dTuningAsmBwdWrw_FP32, FloatTest_smoke_solver_convasmbwdwrw)
85110
}
86111
};
87112

113+
TEST_P(GPU_Conv2dSingleAsmBwdWrw_FP32, FloatTest_smoke_solver_convasmbwdwrw)
114+
{
115+
if(IsTestSupportedForDevice() && !SkipTest())
116+
{
117+
invoke_with_params<conv2d_driver, GPU_Conv2dSingleAsmBwdWrw_FP32>(compiler_check);
118+
}
119+
else
120+
{
121+
GTEST_SKIP();
122+
}
123+
};
124+
88125
TEST_P(GPU_Conv2dTuningAsmBwdWrw_FP16, HalfTest_smoke_solver_convasmbwdwrw)
89126
{
90127
if(IsTestSupportedForDevice() && !SkipTest())
@@ -109,6 +146,9 @@ TEST_P(GPU_Conv2dTuningAsmBwdWrw_BFP16, Bf16Test_smoke_solver_convasmbwdwrw)
109146
}
110147
};
111148

149+
INSTANTIATE_TEST_SUITE_P(Smoke,
150+
GPU_Conv2dSingleAsmBwdWrw_FP32,
151+
testing::Values(GetTestCompilerRegressionCases()));
112152
INSTANTIATE_TEST_SUITE_P(Smoke, GPU_Conv2dTuningAsmBwdWrw_FP32, testing::Values(GetTestCases()));
113153
INSTANTIATE_TEST_SUITE_P(Smoke, GPU_Conv2dTuningAsmBwdWrw_FP16, testing::Values(GetTestCases()));
114154
INSTANTIATE_TEST_SUITE_P(Smoke, GPU_Conv2dTuningAsmBwdWrw_BFP16, testing::Values(GetTestCases()));

0 commit comments

Comments
 (0)