Skip to content

Commit d0c8f80

Browse files
BruceGliffigcbot
authored andcommitted
Determines order of structure generation.
Determining order of structure generation in GenXStructureSplitter. Order of elements in new structure defines by order of elements in original structure. This is to prevent such cases: A { int, float, int, float } C { A, float } Ai { int, int } Af { float, float } 1 - C_BS { Ai, Af, float} and can be 2 - C_BS { Af, Ai, float } Now only the first case.
1 parent 9efd6cb commit d0c8f80

File tree

1 file changed

+54
-23
lines changed

1 file changed

+54
-23
lines changed

IGC/VectorCompiler/lib/GenXCodeGen/GenXStructSplitter.cpp

Lines changed: 54 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -718,8 +718,9 @@ void DependencyGraph::processNode(Node &SNode) {
718718
// new splitted structs.
719719
VecOfNewIndiciesDefinition IndicesMap(OldSTy->getNumElements());
720720

721-
std::vector<Type *> GeneratedTypes;
722-
GeneratedTypes.reserve(Types.size());
721+
// First initialization with zeros as pos in vector will be Index of
722+
// Element.
723+
std::vector<Type *> GeneratedTypesInOrder(OldSTy->getNumElements());
723724

724725
StringRef OldSTyName = OldSTy->getName();
725726

@@ -730,7 +731,7 @@ void DependencyGraph::processNode(Node &SNode) {
730731
// It means that structure contains only one element that can be used
731732
// directly without structure.
732733
NewPlainType = Elements.getTyAt(0);
733-
IndicesMap[Elements.getIdxAt(0)].emplace_front(NewPlainType);
734+
IndicesMap[Elements.getIdxAt(0)].emplace_back(NewPlainType);
734735
} else if (!isPlain(*OldSTy)) {
735736
StructType *NewPlainStruct = StructType::create(
736737
Ctx, Elements.getTypesArray(),
@@ -740,19 +741,31 @@ void DependencyGraph::processNode(Node &SNode) {
740741
// Update AllStructs info.
741742
setInfoAboutStructure(*NewPlainStruct);
742743
// Match old elements with new elements.
743-
// for (auto ElmIndex : enumerate(Elements))
744744
for (auto ElmIndex : enumerate(Elements.indices()))
745-
IndicesMap[ElmIndex.value()].emplace_front(NewPlainStruct,
746-
ElmIndex.index());
745+
IndicesMap[ElmIndex.value()].emplace_back(NewPlainStruct,
746+
ElmIndex.index());
747747
} else
748748
// Plain structs with more than 1 elements -> skip as there is nothing
749749
// to do.
750750
continue;
751-
GeneratedTypes.push_back(NewPlainType);
751+
// Way to implement ordering in Types placing.
752+
// Affects on ordering in List and ordering of elements in structures.
753+
// Prevents from cases like this:
754+
//
755+
// A { int, float, int, float };
756+
// C_BS { Af, Ai, float } and at another time: C_BS { Ai, Af, float };
757+
//
758+
// Order defined by elements in original structure.
759+
GeneratedTypesInOrder[Elements.getIdxAt(0)] = NewPlainType;
752760
}
753761

762+
// Cleans an array from nullptr elements.
763+
GeneratedTypesInOrder.erase(
764+
llvm::remove_if(GeneratedTypesInOrder, [](Type *Ty) { return !Ty; }),
765+
GeneratedTypesInOrder.end());
766+
754767
// Do if there is splitting or simplification.
755-
if (GeneratedTypes.size()) {
768+
if (GeneratedTypesInOrder.size()) {
756769
// Update SplittedStructs
757770
SplittedStructs.emplace_back(OldSTy, std::move(IndicesMap));
758771

@@ -774,7 +787,7 @@ void DependencyGraph::processNode(Node &SNode) {
774787
// F { float } C { F, float } C_BS -> { float , float }
775788
std::for_each(SNode.parent_begin(), SNode.parent_end(),
776789
[&](Node *ParentNode) {
777-
remakeParent(*ParentNode, SNode, GeneratedTypes);
790+
remakeParent(*ParentNode, SNode, GeneratedTypesInOrder);
778791
});
779792
}
780793
}
@@ -817,16 +830,16 @@ void DependencyGraph::remakeParent(Node &SNode, Node &SNodeToChange,
817830
// this element with new.
818831
for (auto &&NewSTy : NewReplaceTypes) {
819832
NewElements.emplace_back(NewSTy);
820-
NewIndices[Index].emplace_front(BeforeSplitingS,
821-
Index + ExpandIndicies++);
833+
NewIndices[Index].emplace_back(BeforeSplitingS,
834+
Index + ExpandIndicies++);
822835
}
823836
// The Index will be inc, so there is no need of extra offset
824837
--ExpandIndicies;
825838
} else {
826839
// If element of structure is not changed, then just copies info about it
827840
// and places right indices.
828841
NewElements.emplace_back(Elm);
829-
NewIndices[Index].emplace_front(BeforeSplitingS, Index + ExpandIndicies);
842+
NewIndices[Index].emplace_back(BeforeSplitingS, Index + ExpandIndicies);
830843
}
831844
++Index;
832845
}
@@ -892,14 +905,19 @@ void DependencyGraph::mergeStructGenerationInfo() {
892905
LLVM_DEBUG(dbgs() << "Merging structs.\n");
893906
for (auto It = SplittedStructs.rbegin(), End = SplittedStructs.rend();
894907
It != End; ++It) {
895-
if (StructType *SToMerge = checkAbilityToMerge(It->second)) {
896-
LLVM_DEBUG(dbgs() << "Able to merge: " << *It->first << "\n\tWith "
897-
<< *SToMerge << "\n");
908+
StructType &SToMerge = *It->first;
909+
VecOfNewIndiciesDefinition &InfoAboutS = It->second;
898910

899-
VecOfNewIndiciesDefinition const &InfoAboutTemporaryS =
900-
getVecOfStructIdxMapping(*SToMerge);
911+
if (StructType *SToMergeWith = checkAbilityToMerge(InfoAboutS)) {
912+
LLVM_DEBUG(dbgs() << "Able to merge: " << SToMerge << "\n\tWith "
913+
<< *SToMergeWith << "\n");
901914

902-
for (ListOfSplittedElements &ElementsList : It->second) {
915+
VecOfNewIndiciesDefinition const &InfoAboutTemporaryS =
916+
getVecOfStructIdxMapping(*SToMergeWith);
917+
// Every element of the structure SToMerge will be substituted with
918+
// element from the structure SToMergeWith and/or new elements from
919+
// SToMergeWith will be placed in SToMerge.
920+
for (ListOfSplittedElements &ElementsList : InfoAboutS) {
903921
for (SElement &Element : ElementsList) {
904922
IGC_ASSERT_MESSAGE(!Element.isUnwrapped(),
905923
"Attempt to merge unwrapped type.");
@@ -908,12 +926,25 @@ void DependencyGraph::mergeStructGenerationInfo() {
908926
ListOfSplittedElements const &NewElement =
909927
InfoAboutTemporaryS.at(Element.getIndex());
910928

911-
ListOfSplittedElements::const_iterator EIt = NewElement.begin();
912-
// Changes current element and if on this 'Element.Index' lots of new
913-
// elements are to be placed, then extend list from begining not to
914-
// invalidate iterations.
929+
auto EIt = NewElement.rbegin();
930+
// Changes current element and, if on this 'Element.Index' lots of new
931+
// elements are to be placed, extends list with new elements.
932+
// Pushes front new element not to invalidate iterations.
933+
// Iterates from end to begin (rbegin to rend) to keep order of
934+
// elements.
935+
// FE, merges information
936+
// G : (G_BS, 0) (...)
937+
// (...)
938+
// G_BS : (SomeS, 5) (Gf, 0) (Gi, 0)
939+
//
940+
// Will become
941+
// G : (SomeS, 5) (Gf, 0) (Gi, 0) (...)
942+
943+
// Element (G_BS, 0) will become (Gi, 0).
915944
Element = *EIt;
916-
while (++EIt != NewElement.end())
945+
// Elements (SomeS, 5) (Gf, 0) will be placed before (G_BS, 0) in the
946+
// same order as in G_BS.
947+
while (++EIt != NewElement.rend())
917948
ElementsList.push_front(*EIt);
918949
}
919950
}

0 commit comments

Comments
 (0)