Skip to content

Commit e868a35

Browse files
authored
[hip-kernel-provider] Remove HIP_PLUGIN_BN_MAXN macro (ROCm#8095)
## Motivation In the hip-kernel-provider, the `HIP_PLUGIN_BN_MAXN` macro is never changed from the default value by plans, this is also the case in the original MIOpen code. Therefore to make it clear that this value is invariant, this PR folds the default value into the batchnorm configuration code as a constant expression. ## Technical Details * Replace macro with hardcoded literatal * Change related macro guarded code to constexpr * Add comment documenting relationship between `max_n` and kernel selection heuristics in BN plan. ## Test Plan Run `ninja check` in hip-kernel-provider build ## Test Result Passes on MI100
1 parent 60b7a10 commit e868a35

10 files changed

Lines changed: 23 additions & 23 deletions

File tree

dnn-providers/hip-kernel-provider/src/engines/hip_mlops_engine/kernels/batchnorm/BatchNormBwdSpatial.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -679,7 +679,7 @@ struct BatchNormBwdSpatialImpl<3, FpType, FpPrecType, FpAccumType>
679679
FpPrecType ds = 0;
680680
FpPrecType db = 0;
681681

682-
// maybe unused if HIP_PLUGIN_BN_N >= HIP_PLUGIN_BN_MAXN
682+
// Unused if hip_plugin_bn_config::n >= hip_plugin_bn_config::max_n
683683
FpPrecType batchvalues[hip_plugin_bn_config::n];
684684
FpPrecType dyvalues[hip_plugin_bn_config::n];
685685

dnn-providers/hip-kernel-provider/src/engines/hip_mlops_engine/kernels/batchnorm/BatchNormFwdTrainSpatial.cpp

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -526,9 +526,8 @@ struct BatchNormFwdTrainSpatialImpl<3, FpType, FpPrecType, FpAccumType>
526526
unsigned int grpid = blockIdx.x;
527527
unsigned int cidx = grpid * hip_plugin_bn_config::hw;
528528

529-
#if(HIP_PLUGIN_BN_N < HIP_PLUGIN_BN_MAXN)
529+
// Unused if hip_plugin_bn_config::n >= hip_plugin_bn_config::max_n
530530
FpType minibatch[HIP_PLUGIN_BN_N];
531-
#endif
532531

533532
if(lid == 0)
534533
{
@@ -545,9 +544,10 @@ struct BatchNormFwdTrainSpatialImpl<3, FpType, FpPrecType, FpAccumType>
545544
mean += xin;
546545
variance = fma(xin, xin, variance);
547546

548-
#if(HIP_PLUGIN_BN_N < HIP_PLUGIN_BN_MAXN)
549-
minibatch[n] = (*(in + index));
550-
#endif
547+
if constexpr(hip_plugin_bn_config::n < hip_plugin_bn_config::max_n)
548+
{
549+
minibatch[n] = (*(in + index));
550+
}
551551
}};
552552
}
553553
__syncthreads();
@@ -592,12 +592,15 @@ struct BatchNormFwdTrainSpatialImpl<3, FpType, FpPrecType, FpAccumType>
592592
static_unroll_count<unsigned int, 0, hip_plugin_bn_config::n, 1, 2>{
593593
[&](unsigned int n) { // apply normalization
594594
index = n * hip_plugin_bn_config::chw + cidx + lid;
595-
#if(HIP_PLUGIN_BN_N < HIP_PLUGIN_BN_MAXN)
596-
inhat = (cast<FpPrecType>(minibatch[n]) - mean)
597-
* invVariance; // (in[index] - mean) * invVariance;
598-
#else
599-
inhat = (cast<FpPrecType>(*(in + index)) - mean) * invVariance;
600-
#endif
595+
if constexpr(hip_plugin_bn_config::n < hip_plugin_bn_config::max_n)
596+
{
597+
inhat = (cast<FpPrecType>(minibatch[n]) - mean)
598+
* invVariance; // (in[index] - mean) * invVariance;
599+
}
600+
else
601+
{
602+
inhat = (cast<FpPrecType>(*(in + index)) - mean) * invVariance;
603+
}
601604
out[index] = cast<FpType>(
602605
hip_kernel_provider::applyActivation<FpPrecType,
603606
hip_kernel_provider::ActivationMode{

dnn-providers/hip-kernel-provider/src/engines/hip_mlops_engine/kernels/batchnorm/Configuration.hpp

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,6 @@ template <typename HipKernelConfig,
150150
typename Architecture,
151151
int Variant,
152152
int NCHW,
153-
int MaxN,
154153
int NElements,
155154
int N,
156155
int C,
@@ -170,7 +169,6 @@ struct proto_config
170169
static_assert(Vectorize == 0 || Vectorize == 1, "Vectorize must be 0 or 1");
171170
static_assert(UseNodpp == 0 || UseNodpp == 1, "UseNodpp must be 0 or 1");
172171
static_assert(NCHW >= 0, "HIP_PLUGIN_BN_NCHW should be always >= 0");
173-
static_assert(MaxN >= 0, "HIP_PLUGIN_BN_MAXN should be always >= 0");
174172
static_assert(C >= 0, "HIP_PLUGIN_BN_C should be always >= 0");
175173
static_assert(N >= 0, "HIP_PLUGIN_BN_N should be always >= 0");
176174
static_assert(HW >= 0, "HIP_PLUGIN_BN_HW should be always >= 0");
@@ -196,7 +194,14 @@ struct proto_config
196194
: 0); // TODO: not sure if 0 should be the default value of this.
197195
static constexpr auto launch_dim = LaunchDim{};
198196
static constexpr unsigned int nchw = static_cast<unsigned int>(NCHW);
199-
static constexpr unsigned int max_n = static_cast<unsigned int>(MaxN);
197+
198+
// `max_n` is a limit on the number of batch elements from the x tensor that can be cached in per-thread memory as part of
199+
// variant 3 kernels. If batchsize (n_elements) exceeds this limit then the kernel doesn't cache the global memory accesses.
200+
// The constant of 65 is related to the heuristic used in `defaultConfigSpatialSingle` where Batchnorm plans select the kernel
201+
// variant to use. In BN Fwd training, the heuristics only selects variant 3 when N <= 32, so the caching optimization is always
202+
// used. In BN Bwd training, the heuristics only selects variant 3 when N > 64, so the caching optimization is never used. If
203+
// there are future changes to how the heuristic selects variant 3 kernels, then it may be worth revisiting this caching limit.
204+
static constexpr unsigned int max_n = 65;
200205
static constexpr unsigned int n_elements = static_cast<unsigned int>(NElements);
201206
static constexpr unsigned int n = static_cast<unsigned int>(N);
202207
static constexpr unsigned int c = static_cast<unsigned int>(C);
@@ -267,7 +272,6 @@ using config = hip_kernel_provider::batchnorm::detail::proto_config<
267272
HIP_PLUGIN_GFX115X>,
268273
HIP_PLUGIN_BN_VARIANT,
269274
HIP_PLUGIN_BN_NCHW,
270-
HIP_PLUGIN_BN_MAXN,
271275
HIP_PLUGIN_BN_N_ELEMENTS,
272276
HIP_PLUGIN_BN_N,
273277
HIP_PLUGIN_BN_C,

dnn-providers/hip-kernel-provider/src/engines/hip_mlops_engine/plans/batchnorm/BatchnormBwdPlan.cpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -293,7 +293,6 @@ void BatchnormBwdPlan::compile(const IKernelCompiler& kernelCompiler,
293293
options.update("HIP_PLUGIN_BN_GRP1", ylocalsize);
294294
options.update("HIP_PLUGIN_BN_GRP2", zlocalsize);
295295
options.update("HIP_PLUGIN_BN_LDS_SIZE", ldsSize);
296-
options.update("HIP_PLUGIN_BN_MAXN", 65);
297296
options.update("HIP_PLUGIN_BN_VEC_SIZE", config.vectorsize);
298297

299298
_compiledProgram = kernelCompiler.compile("BatchNormBwdSpatial.cpp", options);

dnn-providers/hip-kernel-provider/src/engines/hip_mlops_engine/plans/batchnorm/BatchnormKernelCompileOptions.hpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,6 @@ class BatchnormKernelCompileOptions : public KernelCompileOptions
5454
add("HIP_PLUGIN_BN_VARIANT", 255);
5555
add("HIP_PLUGIN_BN_USESAVED", 0);
5656
add("HIP_PLUGIN_BN_NCHW", 1);
57-
add("HIP_PLUGIN_BN_MAXN", 65);
5857
add("HIP_PLUGIN_BN_VECTORIZE", 0);
5958
add("HIP_PLUGIN_BN_VEC_SIZE", 1);
6059
add("HIP_PLUGIN_BN_STASH_METHOD", 0);

dnn-providers/hip-kernel-provider/src/tests/engines/hip_mlops_engine/plans/Batchnorm/TestBatchnormBwdPlan.cpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,6 @@ TEST(TestBatchnormBwdPlan, CompileDefaultSetsCorrectDefines)
189189
EXPECT_TRUE(hasOption("-DHIP_PLUGIN_BN_GRP1=1"));
190190
EXPECT_TRUE(hasOption("-DHIP_PLUGIN_BN_GRP2=1"));
191191
EXPECT_TRUE(hasOption("-DHIP_PLUGIN_BN_N_ELEMENTS=HIP_PLUGIN_BN_N"));
192-
EXPECT_TRUE(hasOption("-DHIP_PLUGIN_BN_MAXN=65"));
193192
EXPECT_TRUE(hasOption("-DHIP_PLUGIN_USE_AMDGCN=0"));
194193
EXPECT_TRUE(hasOption("-DHIP_PLUGIN_BN_LOOP_UNROLL_MAXN=768"));
195194
EXPECT_TRUE(hasOption("-DHIP_PLUGIN_BN_LDS_SIZE=1024"));

dnn-providers/hip-kernel-provider/src/tests/engines/hip_mlops_engine/plans/Batchnorm/TestBatchnormFwdInferencePlan.cpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -310,7 +310,6 @@ TEST(TestBatchnormFwdInferencePlanFp32, CompileDefaultSetsCorrectDefines)
310310
EXPECT_TRUE(hasOption("-DHIP_PLUGIN_BN_GRP1=256"));
311311
EXPECT_TRUE(hasOption("-DHIP_PLUGIN_BN_GRP2=1"));
312312
EXPECT_TRUE(hasOption("-DHIP_PLUGIN_BN_N_ELEMENTS=HIP_PLUGIN_BN_N"));
313-
EXPECT_TRUE(hasOption("-DHIP_PLUGIN_BN_MAXN=65"));
314313
EXPECT_TRUE(hasOption("-DHIP_PLUGIN_USE_AMDGCN=0"));
315314
EXPECT_TRUE(hasOption("-DHIP_PLUGIN_BN_LOOP_UNROLL_MAXN=768"));
316315
EXPECT_TRUE(hasOption("-DHIP_PLUGIN_BN_LDS_SIZE=256"));

dnn-providers/hip-kernel-provider/src/tests/engines/hip_mlops_engine/plans/Batchnorm/TestBatchnormFwdInferenceWithVariancePlan.cpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -317,7 +317,6 @@ TEST(TestBatchnormFwdInferenceWithVariancePlanFp32, CompileDefaultSetsCorrectDef
317317
EXPECT_TRUE(hasOption("-DHIP_PLUGIN_BN_GRP1=256"));
318318
EXPECT_TRUE(hasOption("-DHIP_PLUGIN_BN_GRP2=1"));
319319
EXPECT_TRUE(hasOption("-DHIP_PLUGIN_BN_N_ELEMENTS=HIP_PLUGIN_BN_N"));
320-
EXPECT_TRUE(hasOption("-DHIP_PLUGIN_BN_MAXN=65"));
321320
EXPECT_TRUE(hasOption("-DHIP_PLUGIN_USE_AMDGCN=0"));
322321
EXPECT_TRUE(hasOption("-DHIP_PLUGIN_BN_LOOP_UNROLL_MAXN=768"));
323322
EXPECT_TRUE(hasOption("-DHIP_PLUGIN_BN_LDS_SIZE=256"));

dnn-providers/hip-kernel-provider/src/tests/engines/hip_mlops_engine/plans/Batchnorm/TestBatchnormFwdTrainingActivPlan.cpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -897,7 +897,6 @@ TEST(TestBatchnormFwdTrainingActivPlan, CompileDefaultSetsCorrectDefines)
897897
EXPECT_TRUE(hasOption("-DHIP_PLUGIN_BN_NGRPS=1"));
898898
EXPECT_TRUE(hasOption("-DHIP_PLUGIN_BN_NGRPS2=1"));
899899
EXPECT_TRUE(hasOption("-DHIP_PLUGIN_BN_N_ELEMENTS=1"));
900-
EXPECT_TRUE(hasOption("-DHIP_PLUGIN_BN_MAXN=65"));
901900
EXPECT_TRUE(hasOption("-DHIP_PLUGIN_USE_AMDGCN=0"));
902901
EXPECT_TRUE(hasOption("-DHIP_PLUGIN_BN_LOOP_UNROLL_MAXN=768"));
903902
EXPECT_TRUE(hasOption("-DHIP_PLUGIN_BN_LDS_SIZE=1024"));

dnn-providers/hip-kernel-provider/src/tests/engines/hip_mlops_engine/plans/Batchnorm/TestBatchnormFwdTrainingPlan.cpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -351,7 +351,6 @@ TEST(TestBatchnormFwdTrainingPlan, CompileDefaultSetsCorrectDefines)
351351
EXPECT_TRUE(hasOption("-DHIP_PLUGIN_BN_GRP1_FINAL=1"));
352352
EXPECT_TRUE(hasOption("-DHIP_PLUGIN_BN_GRP2_FINAL=1"));
353353
EXPECT_TRUE(hasOption("-DHIP_PLUGIN_BN_N_ELEMENTS=1"));
354-
EXPECT_TRUE(hasOption("-DHIP_PLUGIN_BN_MAXN=65"));
355354
EXPECT_TRUE(hasOption("-DHIP_PLUGIN_USE_AMDGCN=0"));
356355
EXPECT_TRUE(hasOption("-DHIP_PLUGIN_BN_LOOP_UNROLL_MAXN=768"));
357356
EXPECT_TRUE(hasOption("-DHIP_PLUGIN_BN_LDS_SIZE=1024"));

0 commit comments

Comments
 (0)