Skip to content

Commit dcc2ffc

Browse files
[SYCL] Fix ModuleSplitterBase::totalSplits usage (#7485)
`ModuleSplitterBase::totalSplits` method returns the amount of _remaining_ splits, i.e. every call to `ModuleSplitterBase::nextSplit` decrements that value. Fixed an incorrect initialization of `SplitByMode` and `SplitByOptionalFeatures` variables by re-arranging code so we ask for amount of splits _before_ we extract them. Previously, those two variables were always initialized with `false`: from what I see in a code this shouldn't have resulted in any major issues, but it caused a problem in a downstream. Additionally renamed `totalSplits` into `remainingSplits` to avoid possible confusion in the future.
1 parent ac2b203 commit dcc2ffc

File tree

2 files changed

+11
-8
lines changed

2 files changed

+11
-8
lines changed

llvm/tools/sycl-post-link/ModuleSplitter.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -236,10 +236,12 @@ class ModuleSplitterBase {
236236
// submodule containing these entry points and their dependencies.
237237
virtual ModuleDesc nextSplit() = 0;
238238

239-
size_t totalSplits() const { return Groups.size(); }
239+
// Returns a number of remaining modules, which can be split out using this
240+
// splitter. The value is reduced by 1 each time nextSplit is called.
241+
size_t remainingSplits() const { return Groups.size(); }
240242

241243
// Check that there are still submodules to split.
242-
bool hasMoreSplits() const { return totalSplits() > 0; }
244+
bool hasMoreSplits() const { return remainingSplits() > 0; }
243245
};
244246

245247
std::unique_ptr<ModuleSplitterBase>

llvm/tools/sycl-post-link/sycl-post-link.cpp

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -758,12 +758,14 @@ processInputModule(std::unique_ptr<Module> M) {
758758
EmitOnlyKernelsAsEntryPoints);
759759

760760
SmallVector<module_split::ModuleDesc, 8> TopLevelModules;
761-
bool SplitByOptionalFeatures = false;
762761

763762
// FIXME: this check should be performed on all split levels
764763
if (DeviceGlobals)
765764
ScopedSplitter->verifyNoCrossModuleDeviceGlobalUsage();
766765

766+
const bool SplitByScope = ScopedSplitter->remainingSplits() > 1;
767+
bool SplitByOptionalFeatures = false;
768+
767769
while (ScopedSplitter->hasMoreSplits()) {
768770
module_split::ModuleDesc MD = ScopedSplitter->nextSplit();
769771

@@ -782,14 +784,13 @@ processInputModule(std::unique_ptr<Module> M) {
782784
// This step is mandatory, because it is required for functional
783785
// correctness, i.e. to prevent speculative compilation of kernels that use
784786
// optional features on a HW which doesn't support them.
787+
SplitByOptionalFeatures |= OptionalFeaturesSplitter->remainingSplits() > 1;
788+
785789
while (OptionalFeaturesSplitter->hasMoreSplits()) {
786790
TopLevelModules.emplace_back(OptionalFeaturesSplitter->nextSplit());
787791
}
788-
789-
SplitByOptionalFeatures |= OptionalFeaturesSplitter->totalSplits() > 1;
790792
}
791793

792-
const bool SplitByScope = ScopedSplitter->totalSplits() > 1;
793794
Modified |= SplitByScope;
794795
Modified |= SplitByOptionalFeatures;
795796

@@ -809,7 +810,7 @@ processInputModule(std::unique_ptr<Module> M) {
809810
std::unique_ptr<module_split::ModuleSplitterBase> LargeGRFSplitter =
810811
module_split::getLargeGRFSplitter(std::move(MDesc),
811812
EmitOnlyKernelsAsEntryPoints);
812-
const bool SplitByLargeGRF = LargeGRFSplitter->totalSplits() > 1;
813+
const bool SplitByLargeGRF = LargeGRFSplitter->remainingSplits() > 1;
813814
Modified |= SplitByLargeGRF;
814815

815816
// Now split further by "large-grf" attribute.
@@ -827,7 +828,7 @@ processInputModule(std::unique_ptr<Module> M) {
827828
std::unique_ptr<module_split::ModuleSplitterBase> ESIMDSplitter =
828829
module_split::getSplitterByKernelType(std::move(MDesc1),
829830
EmitOnlyKernelsAsEntryPoints);
830-
const bool SplitByESIMD = ESIMDSplitter->totalSplits() > 1;
831+
const bool SplitByESIMD = ESIMDSplitter->remainingSplits() > 1;
831832
Modified |= SplitByESIMD;
832833

833834
if (SplitByESIMD && SplitByScope &&

0 commit comments

Comments
 (0)