Skip to content

Commit e9ad10e

Browse files
jaladreipsigcbot
authored andcommitted
Fix copying initialized members in EmitInsertValueToStruct
Description: The function getAllDefinedMembers has a bug - it will not return any indices if the insertvalue instruction uses a constant struct instead of undef as a starting point. For example: %29 = insertvalue %__StructSOALayout_ <{ i32 194816, i32 undef, i32 undef, <1 x float> undef }>, i32 %18, 1 ; visa id: 45 %30 = insertvalue %__StructSOALayout_ %29, i32 0, 2 ; visa id: 47 In this case, when emitting instructions for %30, we should see that the indices 0 and 1 are initialized (with i32 194816 and i32 %18). However, we don't and we end up losing those values. The fix makes getAllDefinedMembers return all initialized indices by recursively traversing the constant struct (recursively in case the struct has structs inside it)
1 parent c344e49 commit e9ad10e

File tree

3 files changed

+37
-10
lines changed

3 files changed

+37
-10
lines changed

IGC/Compiler/CISACodeGen/EmitVISAPass.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2961,7 +2961,7 @@ void EmitPass::EmitInsertValueToStruct(InsertValueInst* inst)
29612961
// %13 = insertvalue %9, i32 %scalar40, 2
29622962
// %14 = insertvalue %13, i32 %scalar41, 3
29632963
//
2964-
std::list<ArrayRef<unsigned>> toBeCopied;
2964+
SmallVector<std::vector<unsigned>> toBeCopied;
29652965
getAllDefinedMembers(src0, toBeCopied);
29662966
for (const auto& II : toBeCopied)
29672967
{
@@ -3128,7 +3128,7 @@ void EmitPass::EmitInsertValueToLayoutStruct(InsertValueInst* IVI)
31283128
// Most often, SrcV has just one defined value and calling
31293129
// emitCopyToOrFromLayoutStruct() would copy all, thus special
31303130
// handling here to avoid copy undefined values.
3131-
std::list<ArrayRef<unsigned>> toBeCopied;
3131+
SmallVector<std::vector<unsigned>> toBeCopied;
31323132
getAllDefinedMembers(src0, toBeCopied);
31333133
for (const auto& II : toBeCopied)
31343134
{

IGC/Compiler/CISACodeGen/MemOpt.cpp

Lines changed: 34 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5234,23 +5234,50 @@ void getStructMemberOffsetAndType_2(const DataLayout* DL,
52345234
return;
52355235
}
52365236

5237+
static void searchForDefinedMembers(const ConstantAggregate* S, const std::vector<unsigned>& currentIndices, SmallVectorImpl<std::vector<unsigned>>& fieldsTBC)
5238+
{
5239+
for (unsigned i = 0; i < S->getNumOperands(); i++)
5240+
{
5241+
auto indices = currentIndices;
5242+
indices.push_back(i);
5243+
auto* E = S->getAggregateElement(i);
5244+
if (isa<UndefValue>(E))
5245+
continue;
5246+
5247+
if (auto* SE = dyn_cast<ConstantAggregate>(E))
5248+
{
5249+
searchForDefinedMembers(SE, indices, fieldsTBC);
5250+
}
5251+
else
5252+
{
5253+
fieldsTBC.push_back(indices);
5254+
}
5255+
5256+
}
5257+
}
5258+
52375259
void getAllDefinedMembers (const Value* IVI,
5238-
std::list<ArrayRef<unsigned>>& fieldsTBC)
5260+
SmallVectorImpl<std::vector<unsigned>>& fieldsTBC)
52395261
{
52405262
IGC_ASSERT(IVI != nullptr);
52415263
const Value* V = IVI;
52425264
while (isa<InsertValueInst>(V))
52435265
{
52445266
const InsertValueInst* I = cast<const InsertValueInst>(V);
5245-
fieldsTBC.push_front(I->getIndices());
5267+
fieldsTBC.push_back(I->getIndices().vec());
52465268
V = I->getOperand(0);
52475269
}
5248-
if (!isa<UndefValue>(V))
5270+
5271+
// at the end we may have a constant struct like this:
5272+
// % 28 = insertvalue % __StructSOALayout_ < { i32 194816, i32 undef, i32 undef, <1 x float> undef } > , i32 % 17, 1
5273+
// we should traverse it and find the indices pointing to the constant values
5274+
if (auto* S = dyn_cast<ConstantAggregate>(V))
52495275
{
5250-
// Don't know for sure, assume all have been defined.
5251-
fieldsTBC.clear();
5252-
StructType* stTy = cast<StructType>(IVI->getType());
5253-
fieldsTBC.insert(fieldsTBC.end(), 0, stTy->getNumElements() - 1);
5276+
std::vector<unsigned> indices = {};
5277+
searchForDefinedMembers(S, indices, fieldsTBC);
52545278
}
5279+
5280+
// reverse the vector to get the ascending order of indices
5281+
std::reverse(fieldsTBC.begin(), fieldsTBC.end());
52555282
}
52565283
}

IGC/Compiler/CISACodeGen/MemOpt.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ namespace IGC {
9191
//
9292
// The value %9 will return {0, 1}, and %10 will return {0, 1, 2}.
9393
void getAllDefinedMembers(const Value* IVI,
94-
std::list<ArrayRef<unsigned>>& fieldsTBC);
94+
SmallVectorImpl<std::vector<unsigned>>& fieldsTBC);
9595
}
9696

9797
#endif // _CISA_MEMOPT_H_

0 commit comments

Comments
 (0)