Skip to content

Commit 12bcdd3

Browse files
aparshin-inteligcbot
authored andcommitted
fixed loading of constant structs by ConstantLoader
constant returns are not allowed, and PostLegalization is expected to insert a proper constant loading sequence. However, if we return a constant structure, before this patch the generated sequence looked like this: ``` ret { <4 x i32> } { <4 x i32> <i32 1, i32 1, i32 1, i32 1> }) ``` -> ``` %const_val = call { <4 x i32> } @llvm.genx.constanti.s1v4i32( { <4 x i32> } { <4 x i32> <i32 1, i32 1, i32 1, i32 1> }) ret %const_val ``` Issue: *genx.constanti* is designed to handle proper vector types, not aggregates! This commit introduces the following changes: 1. All constant structures are treated as non-simple constants and always loaded by loadNonSimpleConstants 2. Undef values are left as is 3. ConstantLoader class must not be used with aggregate types (including structs). NOTE_1: In theory, restriction #3 can be relaxed by allowing ConstantLoader to operate on such types. 2 minor issues are prevent us from doing that: I. I haven't observed such cases in practice :). II. This would require us to handle *undef* values. The resulting code looks extremely quirky as the constant loader must produce instruction, not a value. NOTE_2: This change may not solve all the remaining issues with constant structures. For example - loadPhiConstants could potentially encounter a constant struct. If this happens in practice - then we must relax our restrictions on ConsantLoader (see NOTE_1).
1 parent 3a72cd2 commit 12bcdd3

File tree

2 files changed

+28
-8
lines changed

2 files changed

+28
-8
lines changed

IGC/VectorCompiler/lib/GenXCodeGen/GenXConstants.cpp

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -113,9 +113,10 @@ using namespace genx;
113113
/***********************************************************************
114114
* loadConstantStruct : insert instructions to load a constant struct
115115
*/
116-
static Value *loadConstantStruct(Constant *C, Instruction *InsertBefore,
117-
const GenXSubtarget &Subtarget,
118-
const DataLayout &DL) {
116+
static Value *loadConstantStruct(
117+
Constant *C, Instruction *InsertPt, const GenXSubtarget &Subtarget,
118+
const DataLayout &DL,
119+
SmallVectorImpl<Instruction *> *AddedInstructions = nullptr) {
119120
auto ST = cast<StructType>(C->getType());
120121
Value *Agg = UndefValue::get(ST);
121122
for (unsigned i = 0, e = ST->getNumElements(); i != e; ++i) {
@@ -124,10 +125,17 @@ static Value *loadConstantStruct(Constant *C, Instruction *InsertBefore,
124125
continue;
125126
Value *LoadedEl = nullptr;
126127
if (isa<StructType>(El->getType()))
127-
LoadedEl = loadConstantStruct(El, InsertBefore, Subtarget, DL);
128-
else
129-
LoadedEl = ConstantLoader(El, Subtarget, DL).load(InsertBefore);
130-
Agg = InsertValueInst::Create(Agg, LoadedEl, i, "loadstruct", InsertBefore);
128+
LoadedEl =
129+
loadConstantStruct(El, InsertPt, Subtarget, DL, AddedInstructions);
130+
else {
131+
LoadedEl = ConstantLoader(El, Subtarget, DL, nullptr, AddedInstructions)
132+
.loadBig(InsertPt);
133+
}
134+
auto *InsertInst =
135+
InsertValueInst::Create(Agg, LoadedEl, i, "loadstruct", InsertPt);
136+
Agg = InsertInst;
137+
if (AddedInstructions)
138+
AddedInstructions->push_back(InsertInst);
131139
}
132140
return Agg;
133141
}
@@ -170,6 +178,12 @@ bool genx::loadNonSimpleConstants(
170178
continue;
171179
if (opMustBeConstant(Inst, i))
172180
continue;
181+
if (C->getType()->isStructTy()) {
182+
*U = loadConstantStruct(C, Inst, Subtarget, DL, AddedInstructions);
183+
Modified = true;
184+
continue;
185+
}
186+
173187
ConstantLoader CL(C, Subtarget, DL, Inst, AddedInstructions);
174188
if (CL.needFixingSimple()) {
175189
Modified = true;
@@ -307,8 +321,10 @@ bool genx::loadConstants(Instruction *Inst, const GenXSubtarget &Subtarget,
307321
if (Ret->getNumOperands() && Ret->getParent()->getParent()->getLinkage()
308322
== GlobalValue::InternalLinkage) {
309323
if (auto C = dyn_cast<Constant>(Ret->getOperand(0))) {
310-
if (!C->getType()->isVoidTy())
324+
if (!C->getType()->isVoidTy() && !isa<UndefValue>(C)) {
311325
Ret->setOperand(0, ConstantLoader(C, Subtarget, DL).load(Ret));
326+
Modified = true;
327+
}
312328
}
313329
}
314330
return Modified;

IGC/VectorCompiler/lib/GenXCodeGen/GenXConstants.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ SPDX-License-Identifier: MIT
1515
#include "llvm/IR/Dominators.h"
1616
#include "llvm/IR/Instructions.h"
1717

18+
#include "Probe/Assertion.h"
19+
1820
namespace llvm {
1921
namespace genx {
2022

@@ -60,6 +62,8 @@ class ConstantLoader {
6062
SmallVectorImpl<Instruction *> *AddedInstructions = nullptr)
6163
: C(C), User(User), Subtarget(InSubtarget), DL(InDL),
6264
AddedInstructions(AddedInstructions) {
65+
IGC_ASSERT_MESSAGE(!C->getType()->isAggregateType(),
66+
"aggregate types are not supported by constant loader");
6367
analyze();
6468
}
6569

0 commit comments

Comments
 (0)