Skip to content

Commit 2fe85dd

Browse files
committed
[Attributor] Don't access pointer elem type in constructPointer (NFC)
Splitting this out as the change is non-trivial: The way this code handled pointer types doesn't really make sense, as GEPs can only apply an offset to the outermost pointer, but can't drill down into interior pointer types (which would require dereferencing memory). Instead give special treatment to the first (pointer) index. I've hardcoded it to zero as that's the only way the function is used right now, but handling non-zero indexes would be straightforward. The original goal here was to have an element type for CreateGEP.
1 parent 87fd09b commit 2fe85dd

File tree

1 file changed

+41
-43
lines changed

1 file changed

+41
-43
lines changed

llvm/lib/Transforms/IPO/AttributorAttributes.cpp

Lines changed: 41 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -190,58 +190,55 @@ static const Value *getPointerOperand(const Instruction *I,
190190
///
191191
/// TODO: This could probably live somewhere more prominantly if it doesn't
192192
/// already exist.
193-
static Value *constructPointer(Type *ResTy, Value *Ptr, int64_t Offset,
194-
IRBuilder<NoFolder> &IRB, const DataLayout &DL) {
193+
static Value *constructPointer(Type *ResTy, Type *PtrElemTy, Value *Ptr,
194+
int64_t Offset, IRBuilder<NoFolder> &IRB,
195+
const DataLayout &DL) {
195196
assert(Offset >= 0 && "Negative offset not supported yet!");
196197
LLVM_DEBUG(dbgs() << "Construct pointer: " << *Ptr << " + " << Offset
197198
<< "-bytes as " << *ResTy << "\n");
198199

199-
// The initial type we are trying to traverse to get nice GEPs.
200-
Type *Ty = Ptr->getType();
200+
if (Offset) {
201+
SmallVector<Value *, 4> Indices;
202+
std::string GEPName = Ptr->getName().str() + ".0";
203+
204+
// Add 0 index to look through the pointer.
205+
assert((uint64_t)Offset < DL.getTypeAllocSize(PtrElemTy) &&
206+
"Offset out of bounds");
207+
Indices.push_back(Constant::getNullValue(IRB.getInt32Ty()));
201208

202-
SmallVector<Value *, 4> Indices;
203-
std::string GEPName = Ptr->getName().str();
204-
while (Offset) {
205-
uint64_t Idx, Rem;
209+
Type *Ty = PtrElemTy;
210+
do {
211+
auto *STy = dyn_cast<StructType>(Ty);
212+
if (!STy)
213+
// Non-aggregate type, we cast and make byte-wise progress now.
214+
break;
206215

207-
if (auto *STy = dyn_cast<StructType>(Ty)) {
208216
const StructLayout *SL = DL.getStructLayout(STy);
209217
if (int64_t(SL->getSizeInBytes()) < Offset)
210218
break;
211-
Idx = SL->getElementContainingOffset(Offset);
219+
220+
uint64_t Idx = SL->getElementContainingOffset(Offset);
212221
assert(Idx < STy->getNumElements() && "Offset calculation error!");
213-
Rem = Offset - SL->getElementOffset(Idx);
222+
uint64_t Rem = Offset - SL->getElementOffset(Idx);
214223
Ty = STy->getElementType(Idx);
215-
} else if (auto *PTy = dyn_cast<PointerType>(Ty)) {
216-
Ty = PTy->getElementType();
217-
if (!Ty->isSized())
218-
break;
219-
uint64_t ElementSize = DL.getTypeAllocSize(Ty);
220-
assert(ElementSize && "Expected type with size!");
221-
Idx = Offset / ElementSize;
222-
Rem = Offset % ElementSize;
223-
} else {
224-
// Non-aggregate type, we cast and make byte-wise progress now.
225-
break;
226-
}
227224

228-
LLVM_DEBUG(errs() << "Ty: " << *Ty << " Offset: " << Offset
229-
<< " Idx: " << Idx << " Rem: " << Rem << "\n");
225+
LLVM_DEBUG(errs() << "Ty: " << *Ty << " Offset: " << Offset
226+
<< " Idx: " << Idx << " Rem: " << Rem << "\n");
230227

231-
GEPName += "." + std::to_string(Idx);
232-
Indices.push_back(ConstantInt::get(IRB.getInt32Ty(), Idx));
233-
Offset = Rem;
234-
}
228+
GEPName += "." + std::to_string(Idx);
229+
Indices.push_back(ConstantInt::get(IRB.getInt32Ty(), Idx));
230+
Offset = Rem;
231+
} while (Offset);
235232

236-
// Create a GEP if we collected indices above.
237-
if (Indices.size())
238-
Ptr = IRB.CreateGEP(Ptr, Indices, GEPName);
233+
// Create a GEP for the indices collected above.
234+
Ptr = IRB.CreateGEP(PtrElemTy, Ptr, Indices, GEPName);
239235

240-
// If an offset is left we use byte-wise adjustment.
241-
if (Offset) {
242-
Ptr = IRB.CreateBitCast(Ptr, IRB.getInt8PtrTy());
243-
Ptr = IRB.CreateGEP(Ptr, IRB.getInt32(Offset),
244-
GEPName + ".b" + Twine(Offset));
236+
// If an offset is left we use byte-wise adjustment.
237+
if (Offset) {
238+
Ptr = IRB.CreateBitCast(Ptr, IRB.getInt8PtrTy());
239+
Ptr = IRB.CreateGEP(IRB.getInt8Ty(), Ptr, IRB.getInt32(Offset),
240+
GEPName + ".b" + Twine(Offset));
241+
}
245242
}
246243

247244
// Ensure the result has the requested type.
@@ -5607,16 +5604,17 @@ struct AAPrivatizablePtrArgument final : public AAPrivatizablePtrImpl {
56075604
for (unsigned u = 0, e = PrivStructType->getNumElements(); u < e; u++) {
56085605
Type *PointeeTy = PrivStructType->getElementType(u)->getPointerTo();
56095606
Value *Ptr = constructPointer(
5610-
PointeeTy, &Base, PrivStructLayout->getElementOffset(u), IRB, DL);
5607+
PointeeTy, PrivType, &Base, PrivStructLayout->getElementOffset(u),
5608+
IRB, DL);
56115609
new StoreInst(F.getArg(ArgNo + u), Ptr, &IP);
56125610
}
56135611
} else if (auto *PrivArrayType = dyn_cast<ArrayType>(PrivType)) {
56145612
Type *PointeeTy = PrivArrayType->getElementType();
56155613
Type *PointeePtrTy = PointeeTy->getPointerTo();
56165614
uint64_t PointeeTySize = DL.getTypeStoreSize(PointeeTy);
56175615
for (unsigned u = 0, e = PrivArrayType->getNumElements(); u < e; u++) {
5618-
Value *Ptr =
5619-
constructPointer(PointeePtrTy, &Base, u * PointeeTySize, IRB, DL);
5616+
Value *Ptr = constructPointer(
5617+
PointeePtrTy, PrivType, &Base, u * PointeeTySize, IRB, DL);
56205618
new StoreInst(F.getArg(ArgNo + u), Ptr, &IP);
56215619
}
56225620
} else {
@@ -5646,7 +5644,7 @@ struct AAPrivatizablePtrArgument final : public AAPrivatizablePtrImpl {
56465644
for (unsigned u = 0, e = PrivStructType->getNumElements(); u < e; u++) {
56475645
Type *PointeeTy = PrivStructType->getElementType(u);
56485646
Value *Ptr =
5649-
constructPointer(PointeeTy->getPointerTo(), Base,
5647+
constructPointer(PointeeTy->getPointerTo(), PrivType, Base,
56505648
PrivStructLayout->getElementOffset(u), IRB, DL);
56515649
LoadInst *L = new LoadInst(PointeeTy, Ptr, "", IP);
56525650
L->setAlignment(Alignment);
@@ -5657,8 +5655,8 @@ struct AAPrivatizablePtrArgument final : public AAPrivatizablePtrImpl {
56575655
uint64_t PointeeTySize = DL.getTypeStoreSize(PointeeTy);
56585656
Type *PointeePtrTy = PointeeTy->getPointerTo();
56595657
for (unsigned u = 0, e = PrivArrayType->getNumElements(); u < e; u++) {
5660-
Value *Ptr =
5661-
constructPointer(PointeePtrTy, Base, u * PointeeTySize, IRB, DL);
5658+
Value *Ptr = constructPointer(
5659+
PointeePtrTy, PrivType, Base, u * PointeeTySize, IRB, DL);
56625660
LoadInst *L = new LoadInst(PointeeTy, Ptr, "", IP);
56635661
L->setAlignment(Alignment);
56645662
ReplacementValues.push_back(L);

0 commit comments

Comments
 (0)