Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Mark most GEPs inbounds and add additional param attributes #1232

Merged
merged 2 commits into from
Dec 30, 2015
Merged

Mark most GEPs inbounds and add additional param attributes #1232

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
956061c
Revise DtoGEP() helpers and mark most GEPs `inbounds`
kinke Dec 13, 2015
7fc28d8
Use `nonnull` and `dereferenceable` attributes for function params
kinke Dec 13, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions gen/arrays.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ static void DtoArrayInit(Loc &loc, LLValue *ptr, LLValue *length,
LLValue *itr_val = DtoLoad(itr);
// assign array element value
DValue *arrayelem = new DVarValue(
dvalue->type->toBasetype(), DtoGEP1(ptr, itr_val, "arrayinit.arrayelem"));
dvalue->type->toBasetype(), DtoGEP1(ptr, itr_val, true, "arrayinit.arrayelem"));
DtoAssign(loc, arrayelem, dvalue, op);

// increment iterator
Expand Down Expand Up @@ -761,7 +761,7 @@ void DtoCatAssignElement(Loc &loc, Type *arrayType, DValue *array,
appendedArray = DtoAggrPaint(appendedArray, DtoType(arrayType));

LLValue *val = DtoArrayPtr(array);
val = DtoGEP1(val, oldLength, ".lastElem");
val = DtoGEP1(val, oldLength, true, ".lastElem");
DtoAssign(loc, new DVarValue(arrayType->nextOf(), val), expVal, TOKblit);
callPostblit(loc, exp, val);
}
Expand Down
5 changes: 5 additions & 0 deletions gen/attributes.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,11 @@ AttrBuilder &AttrBuilder::addByVal(unsigned alignment) {
return *this;
}

AttrBuilder &AttrBuilder::addDereferenceable(unsigned size) {
builder.addDereferenceableAttr(size);
return *this;
}


AttrSet::AttrSet(const AttrSet &base, unsigned index, LLAttribute attribute)
: set(base.set.addAttribute(gIR->context(), index, attribute)) {}
Expand Down
1 change: 1 addition & 0 deletions gen/attributes.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ class AttrBuilder {

AttrBuilder &addAlignment(unsigned alignment);
AttrBuilder &addByVal(unsigned alignment);
AttrBuilder &addDereferenceable(unsigned size);

operator llvm::AttrBuilder &() { return builder; }
operator const llvm::AttrBuilder &() const { return builder; }
Expand Down
4 changes: 2 additions & 2 deletions gen/complex.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,9 +139,9 @@ void DtoGetComplexParts(Loc &loc, Type *to, DValue *val, DValue *&re,
if (to->iscomplex()) {
if (v->isLVal()) {
LLValue *reVal =
DtoGEP(v->getLVal(), DtoConstInt(0), DtoConstInt(0), ".re_part");
DtoGEPi(v->getLVal(), 0, 0, ".re_part");
LLValue *imVal =
DtoGEP(v->getLVal(), DtoConstInt(0), DtoConstInt(1), ".im_part");
DtoGEPi(v->getLVal(), 0, 1, ".im_part");
re = new DVarValue(baserety, reVal);
im = new DVarValue(baseimty, imVal);
} else {
Expand Down
34 changes: 20 additions & 14 deletions gen/functions.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ llvm::FunctionType *DtoFunctionType(Type *type, IrFuncTy &irFty, Type *thistype,
} else {
Type *rt = f->next;
const bool byref = f->isref && rt->toBasetype()->ty != Tvoid;
AttrBuilder attrBuilder;
AttrBuilder attrs;

if (abi->returnInArg(f)) {
// sret return
Expand All @@ -87,24 +87,27 @@ llvm::FunctionType *DtoFunctionType(Type *type, IrFuncTy &irFty, Type *thistype,
++nextLLArgIdx;
} else {
// sext/zext return
attrBuilder.add(DtoShouldExtend(byref ? rt->pointerTo() : rt));
attrs.add(DtoShouldExtend(byref ? rt->pointerTo() : rt));
}
newIrFty.ret = new IrFuncTyArg(rt, byref, attrBuilder);
newIrFty.ret = new IrFuncTyArg(rt, byref, attrs);
}
++nextLLArgIdx;

if (thistype) {
// Add the this pointer for member functions
AttrBuilder attrBuilder;
AttrBuilder attrs;
attrs.add(LLAttribute::NonNull);
if (isCtor) {
attrBuilder.add(LLAttribute::Returned);
attrs.add(LLAttribute::Returned);
}
newIrFty.arg_this = new IrFuncTyArg(
thistype, thistype->toBasetype()->ty == Tstruct, attrBuilder);
newIrFty.arg_this =
new IrFuncTyArg(thistype, thistype->toBasetype()->ty == Tstruct, attrs);
++nextLLArgIdx;
} else if (nesttype) {
// Add the context pointer for nested functions
newIrFty.arg_nest = new IrFuncTyArg(nesttype, false);
AttrBuilder attrs;
attrs.add(LLAttribute::NonNull);
newIrFty.arg_nest = new IrFuncTyArg(nesttype, false, attrs);
++nextLLArgIdx;
}

Expand Down Expand Up @@ -141,27 +144,30 @@ llvm::FunctionType *DtoFunctionType(Type *type, IrFuncTy &irFty, Type *thistype,
bool passPointer = arg->storageClass & (STCref | STCout);

Type *loweredDType = arg->type;
AttrBuilder attrBuilder;
AttrBuilder attrs;
if (arg->storageClass & STClazy) {
// Lazy arguments are lowered to delegates.
Logger::println("lazy param");
auto ltf = new TypeFunction(nullptr, arg->type, 0, LINKd);
auto ltd = new TypeDelegate(ltf);
loweredDType = ltd;
} else if (!passPointer) {
} else if (passPointer) {
// ref/out
attrs.addDereferenceable(loweredDType->size());
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getTypeAllocSize(DtoType(loweredDType)) doesn't work here as some LL types seem opaque (forward-declared).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's strange, but I guess we'll roll with it for now.

} else {
if (abi->passByVal(loweredDType)) {
// LLVM ByVal parameters are pointers to a copy in the function
// parameters stack. The caller needs to provide a pointer to the
// original argument.
attrBuilder.addByVal(DtoAlignment(loweredDType));
attrs.addByVal(DtoAlignment(loweredDType));
passPointer = true;
} else {
// Add sext/zext as needed.
attrBuilder.add(DtoShouldExtend(loweredDType));
attrs.add(DtoShouldExtend(loweredDType));
}
}
newIrFty.args.push_back(
new IrFuncTyArg(loweredDType, passPointer, attrBuilder));

newIrFty.args.push_back(new IrFuncTyArg(loweredDType, passPointer, attrs));
newIrFty.args.back()->parametersIdx = i;
++nextLLArgIdx;
}
Expand Down
2 changes: 1 addition & 1 deletion gen/statements.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1218,7 +1218,7 @@ class ToIRVisitor : public Visitor {

// get value for this iteration
LLValue *loadedKey = irs->ir->CreateLoad(keyvar);
LLValue *gep = DtoGEP1(val, loadedKey);
LLValue *gep = DtoGEP1(val, loadedKey, true);

if (!stmt->value->isRef() && !stmt->value->isOut()) {
// Copy value to local variable, and use it as the value variable.
Expand Down
2 changes: 1 addition & 1 deletion gen/tocall.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -531,7 +531,7 @@ bool DtoLowerMagicIntrinsic(IRState *p, FuncDeclaration *fndecl, CallExp *e,
assert(bitmask == 31 || bitmask == 63);
// auto q = cast(size_t*)ptr + (bitnum >> (64bit ? 6 : 5));
LLValue *q = DtoBitCast(ptr, DtoSize_t()->getPointerTo());
q = DtoGEP1(q, p->ir->CreateLShr(bitnum, bitmask == 63 ? 6 : 5), "bitop.q");
q = DtoGEP1(q, p->ir->CreateLShr(bitnum, bitmask == 63 ? 6 : 5), true, "bitop.q");
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure about this being a legal inbounds.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The intended semantics for the intrinsics are not really defined in any detail in the upstream docs, but I guess it should be fine.


// auto mask = 1 << (bitnum & bitmask);
LLValue *mask =
Expand Down
38 changes: 13 additions & 25 deletions gen/toir.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -739,8 +739,8 @@ class ToElemVisitor : public Visitor {
if (negateOffset) {
noStrideInc = p->ir->CreateNeg(noStrideInc);
}
return new DImValue(
base->type, DtoGEP1(base->getRVal(), noStrideInc, "", p->scopebb()));
return new DImValue(base->type,
DtoGEP1(base->getRVal(), noStrideInc, false));
}

// This might not actually be generated by the frontend, just to be
Expand All @@ -750,7 +750,7 @@ class ToElemVisitor : public Visitor {
inc = p->ir->CreateNeg(inc);
}
llvm::Value *bytePtr = DtoBitCast(base->getRVal(), getVoidPtrType());
DValue *result = new DImValue(Type::tvoidptr, DtoGEP1(bytePtr, inc));
DValue *result = new DImValue(Type::tvoidptr, DtoGEP1(bytePtr, inc, false));
return DtoCast(loc, result, resultType);
}

Expand Down Expand Up @@ -1280,22 +1280,20 @@ class ToElemVisitor : public Visitor {
DValue *r = toElem(e->e2);
p->arrays.pop_back();

LLValue *zero = DtoConstUint(0);

LLValue *arrptr = nullptr;
if (e1type->ty == Tpointer) {
arrptr = DtoGEP1(l->getRVal(), r->getRVal());
arrptr = DtoGEP1(l->getRVal(), r->getRVal(), false);
} else if (e1type->ty == Tsarray) {
if (p->emitArrayBoundsChecks() && !e->indexIsInBounds) {
DtoIndexBoundsCheck(e->loc, l, r);
}
arrptr = DtoGEP(l->getRVal(), zero, r->getRVal());
arrptr = DtoGEP(l->getRVal(), DtoConstUint(0), r->getRVal(),
e->indexIsInBounds);
} else if (e1type->ty == Tarray) {
if (p->emitArrayBoundsChecks() && !e->indexIsInBounds) {
DtoIndexBoundsCheck(e->loc, l, r);
}
arrptr = DtoArrayPtr(l);
arrptr = DtoGEP1(arrptr, r->getRVal());
arrptr = DtoGEP1(DtoArrayPtr(l), r->getRVal(), e->indexIsInBounds);
} else if (e1type->ty == Taarray) {
result = DtoAAIndex(e->loc, e->type, l, r, e->modifiable);
return;
Expand Down Expand Up @@ -1382,7 +1380,7 @@ class ToElemVisitor : public Visitor {
}

// offset by lower
eptr = DtoGEP1(eptr, vlo, "lowerbound");
eptr = DtoGEP1(eptr, vlo, !needCheckLower, "lowerbound");

// adjust length
elen = p->ir->CreateSub(vup, vlo);
Expand Down Expand Up @@ -1637,19 +1635,9 @@ class ToElemVisitor : public Visitor {
}
} else if (e1type->ty == Tpointer) {
assert(e->e2->op == TOKint64);
LLConstant *offset;
if (e->op == TOKplusplus) {
offset =
LLConstantInt::get(DtoSize_t(), static_cast<uint64_t>(1), false);
} else {
offset =
LLConstantInt::get(DtoSize_t(), static_cast<uint64_t>(-1), true);
}
post = llvm::GetElementPtrInst::Create(
#if LDC_LLVM_VER >= 307
isaPointer(val)->getElementType(),
#endif
val, offset, "", p->scopebb());
LLConstant *offset =
e->op == TOKplusplus ? DtoConstUint(1) : DtoConstInt(-1);
post = DtoGEP1(val, offset, false, "", p->scopebb());
} else if (e1type->isfloating()) {
assert(e2type->isfloating());
LLValue *one = DtoConstFP(e1type, ldouble(1.0));
Expand Down Expand Up @@ -2668,8 +2656,8 @@ class ToElemVisitor : public Visitor {
Type *indexType = static_cast<TypeAArray *>(aatype)->index;
assert(indexType && vtype);

llvm::Function *func = getRuntimeFunction(
e->loc, gIR->module, "_d_assocarrayliteralTX");
llvm::Function *func =
getRuntimeFunction(e->loc, gIR->module, "_d_assocarrayliteralTX");
LLFunctionType *funcTy = func->getFunctionType();
LLValue *aaTypeInfo =
DtoBitCast(DtoTypeInfoOf(stripModifiers(aatype)),
Expand Down
64 changes: 24 additions & 40 deletions gen/tollvm.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,69 +257,53 @@ LLIntegerType *DtoSize_t() {

////////////////////////////////////////////////////////////////////////////////

LLValue *DtoGEP1(LLValue *ptr, LLValue *i0, const char *var,
llvm::BasicBlock *bb) {
namespace {
llvm::GetElementPtrInst *DtoGEP(LLValue *ptr, llvm::ArrayRef<LLValue *> indices,
bool inBounds, const char *name,
llvm::BasicBlock *bb) {
LLPointerType *p = isaPointer(ptr);
assert(p && "GEP expects a pointer type");
return llvm::GetElementPtrInst::Create(
auto gep = llvm::GetElementPtrInst::Create(
#if LDC_LLVM_VER >= 307
p->getElementType(),
#endif
ptr, i0, var, bb ? bb : gIR->scopebb());
ptr, indices, name, bb ? bb : gIR->scopebb());
gep->setIsInBounds(inBounds);
return gep;
}
}

////////////////////////////////////////////////////////////////////////////////

LLValue *DtoGEP(LLValue *ptr, LLValue *i0, LLValue *i1, const char *var,
llvm::BasicBlock *bb) {
LLPointerType *p = isaPointer(ptr);
assert(p && "GEP expects a pointer type");
LLValue *v[] = {i0, i1};
return llvm::GetElementPtrInst::Create(
#if LDC_LLVM_VER >= 307
p->getElementType(),
#endif
ptr, v, var, bb ? bb : gIR->scopebb());
LLValue *DtoGEP1(LLValue *ptr, LLValue *i0, bool inBounds, const char *name,
llvm::BasicBlock *bb) {
return DtoGEP(ptr, i0, inBounds, name, bb);
}

////////////////////////////////////////////////////////////////////////////////
LLValue *DtoGEP(LLValue *ptr, LLValue *i0, LLValue *i1, bool inBounds,
const char *name, llvm::BasicBlock *bb) {
LLValue *indices[] = {i0, i1};
return DtoGEP(ptr, indices, inBounds, name, bb);
}

LLValue *DtoGEPi1(LLValue *ptr, unsigned i, const char *var,
LLValue *DtoGEPi1(LLValue *ptr, unsigned i0, const char *name,
llvm::BasicBlock *bb) {
LLPointerType *p = isaPointer(ptr);
assert(p && "GEP expects a pointer type");
return llvm::GetElementPtrInst::Create(
#if LDC_LLVM_VER >= 307
p->getElementType(),
#endif
ptr, DtoConstUint(i), var, bb ? bb : gIR->scopebb());
return DtoGEP(ptr, DtoConstUint(i0), /* inBounds = */ true, name, bb);
}

////////////////////////////////////////////////////////////////////////////////

LLValue *DtoGEPi(LLValue *ptr, unsigned i0, unsigned i1, const char *var,
LLValue *DtoGEPi(LLValue *ptr, unsigned i0, unsigned i1, const char *name,
llvm::BasicBlock *bb) {
LLPointerType *p = isaPointer(ptr);
assert(p && "GEP expects a pointer type");
LLValue *v[] = {DtoConstUint(i0), DtoConstUint(i1)};
return llvm::GetElementPtrInst::Create(
#if LDC_LLVM_VER >= 307
p->getElementType(),
#endif
ptr, v, var, bb ? bb : gIR->scopebb());
LLValue *indices[] = {DtoConstUint(i0), DtoConstUint(i1)};
return DtoGEP(ptr, indices, /* inBounds = */ true, name, bb);
}

////////////////////////////////////////////////////////////////////////////////

LLConstant *DtoGEPi(LLConstant *ptr, unsigned i0, unsigned i1) {
LLPointerType *p = isaPointer(ptr);
assert(p && "GEP expects a pointer type");
LLValue *v[] = {DtoConstUint(i0), DtoConstUint(i1)};
LLValue *indices[] = {DtoConstUint(i0), DtoConstUint(i1)};
return llvm::ConstantExpr::getGetElementPtr(
#if LDC_LLVM_VER >= 307
p->getElementType(),
#endif
ptr, v, true);
ptr, indices, /* InBounds = */ true);
}

////////////////////////////////////////////////////////////////////////////////
Expand Down
8 changes: 4 additions & 4 deletions gen/tollvm.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,10 @@ LLStructType *DtoMutexType();
LLStructType *DtoModuleReferenceType();

// getelementptr helpers
LLValue *DtoGEP1(LLValue *ptr, LLValue *i0, const char *name = "",
llvm::BasicBlock *bb = nullptr);
LLValue *DtoGEP(LLValue *ptr, LLValue *i0, LLValue *i1, const char *name = "",
llvm::BasicBlock *bb = nullptr);
LLValue *DtoGEP1(LLValue *ptr, LLValue *i0, bool inBounds,
const char *name = "", llvm::BasicBlock *bb = nullptr);
LLValue *DtoGEP(LLValue *ptr, LLValue *i0, LLValue *i1, bool inBounds,
const char *name = "", llvm::BasicBlock *bb = nullptr);

LLValue *DtoGEPi1(LLValue *ptr, unsigned i0, const char *name = "",
llvm::BasicBlock *bb = nullptr);
Expand Down
2 changes: 1 addition & 1 deletion ir/irclass.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,7 @@ llvm::GlobalVariable *IrAggr::getInterfaceVtbl(BaseClass *b, bool new_instance,
: 1];
LLType *targetThisType = thisArg->getType();
thisArg = DtoBitCast(thisArg, getVoidPtrType());
thisArg = DtoGEP1(thisArg, DtoConstInt(-b->offset));
thisArg = DtoGEP1(thisArg, DtoConstInt(-b->offset), true);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure about this one either.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be fine, this is semantically the same as http://llvm.org/docs/LangRef.html#prefix-data, right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, looks similar. The docs also state that the base pointer (thisArg) must only point somewhere into the 'object' (whatever that exactly is), not exclusively to its start, so this should be fine.

thisArg = DtoBitCast(thisArg, targetThisType);

// call the real vtbl function.
Expand Down