Skip to content

Fix evaluation order issues (continuation of #1620) #1623

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

Merged
JohanEngelen merged 15 commits into from
Jul 21, 2016
Merged

Fix evaluation order issues (continuation of #1620) #1623

Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
cb15d21
Fix evaluation order of bitwise operators
JohanEngelen Jul 12, 2016
f5935ff
Fix evaluation order of bitwise-assign operators.
JohanEngelen Jul 13, 2016
e4dec4d
Deduplicate code for multiply, divide, and module. [NFC]
JohanEngelen Jul 13, 2016
f9f373e
Fix evaluation order of =* =/ and =%
JohanEngelen Jul 13, 2016
52bddf2
Fix evaluation order of `+=` and `-=`, and update testsuite.
JohanEngelen Jul 13, 2016
8ba8baf
Fix bug: remove casts that snuck in from bitwise operator code.
JohanEngelen Jul 13, 2016
c44db68
Fix another evaluation order bug with `@=` expressions and extend the…
JohanEngelen Jul 13, 2016
df34beb
Refactor binops and binassigns
kinke Jul 15, 2016
7e770f7
Refactor bitwise binary operators
kinke Jul 16, 2016
88b19d3
Binassign: Elide apparently superfluous cast
kinke Jul 16, 2016
d6dcc7a
Enforce type consistency for most DValues
kinke Jul 17, 2016
add49d5
Fix evaluation order for pointer += and -=
kinke Jul 17, 2016
ffaeb09
Binassign: Always use the lvalue subexpression for the binop lhs
kinke Jul 17, 2016
951040c
Get rid of local AST pre-evaluation
kinke Jul 18, 2016
2a0a663
Clarify some non-obvious points wrt. binops/binassign
kinke Jul 18, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions ddmd/expression.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2362,11 +2362,6 @@ public:
ubyte size; // # of bytes in Expression so we can copy() it
ubyte parens; // if this is a parenthesized expression

version(IN_LLVM)
{
void* cachedLvalue; // llvm::Value*
}

final extern (D) this(Loc loc, TOK op, int size)
{
//printf("Expression::Expression(op = %d) this = %p\n", op, this);
Expand Down
4 changes: 0 additions & 4 deletions ddmd/expression.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,10 +135,6 @@ class Expression : public RootObject
unsigned char size; // # of bytes in Expression so we can copy() it
unsigned char parens; // if this is a parenthesized expression

#if IN_LLVM
llvm::Value* cachedLvalue;
#endif

Expression(Loc loc, TOK op, int size);
static void _init();
Expression *copy();
Expand Down
8 changes: 4 additions & 4 deletions gen/arrays.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -590,11 +590,11 @@ void initializeArrayLiteral(IRState *p, ArrayLiteralExp *ale, LLValue *dstMem) {
} else {
// Store the elements one by one.
for (size_t i = 0; i < elemCount; ++i) {
DValue *e = toElem(indexArrayLiteral(ale, i));
DValue *rhs = toElem(indexArrayLiteral(ale, i));

LLValue *elemAddr = DtoGEPi(dstMem, 0, i, "", p->scopebb());
auto vv = new DLValue(e->type, elemAddr);
DtoAssign(ale->loc, vv, e, TOKconstruct, true);
LLValue *lhsPtr = DtoGEPi(dstMem, 0, i, "", p->scopebb());
DLValue lhs(rhs->type, DtoBitCast(lhsPtr, DtoPtrToType(rhs->type)));
DtoAssign(ale->loc, &lhs, rhs, TOKconstruct, true);
}
}
}
Expand Down
284 changes: 240 additions & 44 deletions gen/binops.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,102 +7,298 @@
//
//===----------------------------------------------------------------------===//

#include "gen/llvm.h"
#include "gen/binops.h"
#include "declaration.h"
#include "gen/complex.h"
#include "gen/dvalue.h"
#include "gen/irstate.h"
#include "gen/llvm.h"
#include "gen/llvmhelpers.h"
#include "gen/logger.h"
#include "gen/tollvm.h"

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

DImValue *DtoBinAdd(DRValue *lhs, DRValue *rhs) {
Type *t = lhs->type;
LLValue *l = DtoRVal(lhs);
LLValue *r = DtoRVal(rhs);
dinteger_t undoStrideMul(Loc &loc, Type *t, dinteger_t offset) {
assert(t->ty == Tpointer);
d_uns64 elemSize = t->nextOf()->size(loc);
assert((offset % elemSize) == 0 &&
"Expected offset by an integer amount of elements");

LLValue *res;
if (t->isfloating()) {
res = gIR->ir->CreateFAdd(l, r);
return offset / elemSize;
}

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

namespace {
struct RVals {
DRValue *lhs, *rhs;
};

RVals evalSides(DValue *lhs, Expression *rhs, bool loadLhsAfterRhs) {
RVals rvals;

if (!loadLhsAfterRhs) {
rvals.lhs = lhs->getRVal();
rvals.rhs = toElem(rhs)->getRVal();
} else {
res = gIR->ir->CreateAdd(l, r);
rvals.rhs = toElem(rhs)->getRVal();
rvals.lhs = lhs->getRVal();
}

return new DImValue(t, res);
return rvals;
}

//////////////////////////////////////////////////////////////////////////////
/// Tries to remove a MulExp by a constant value of baseSize from e. Returns
/// NULL if not possible.
Expression *extractNoStrideInc(Expression *e, d_uns64 baseSize, bool &negate) {
MulExp *mul;
while (true) {
if (e->op == TOKneg) {
negate = !negate;
e = static_cast<NegExp *>(e)->e1;
continue;
}

DImValue *DtoBinSub(DRValue *lhs, DRValue *rhs) {
Type *t = lhs->type;
LLValue *l = DtoRVal(lhs);
LLValue *r = DtoRVal(rhs);
if (e->op == TOKmul) {
mul = static_cast<MulExp *>(e);
break;
}

LLValue *res;
if (t->isfloating()) {
res = gIR->ir->CreateFSub(l, r);
return nullptr;
}

if (!mul->e2->isConst()) {
return nullptr;
}
dinteger_t stride = mul->e2->toInteger();

if (stride != baseSize) {
return nullptr;
}

return mul->e1;
}

DValue *emitPointerOffset(Loc loc, DValue *base, Expression *offset,
bool negateOffset, Type *resultType,
bool loadLhsAfterRhs) {
// The operand emitted by the frontend is in units of bytes, and not
// pointer elements. We try to undo this before resorting to
// temporarily bitcasting the pointer to i8.

LLValue *llBase = nullptr;
LLValue *llOffset = nullptr;
LLValue *llResult = nullptr;

if (offset->isConst()) {
llBase = DtoRVal(base);
dinteger_t byteOffset = offset->toInteger();
if (byteOffset == 0) {
llResult = llBase;
} else {
llOffset = DtoConstSize_t(undoStrideMul(loc, base->type, byteOffset));
}
} else {
res = gIR->ir->CreateSub(l, r);
Expression *noStrideInc = extractNoStrideInc(
offset, base->type->nextOf()->size(loc), negateOffset);
auto rvals =
evalSides(base, noStrideInc ? noStrideInc : offset, loadLhsAfterRhs);
llBase = DtoRVal(rvals.lhs);
llOffset = DtoRVal(rvals.rhs);
if (!noStrideInc) // byte offset => cast base to i8*
llBase = DtoBitCast(llBase, getVoidPtrType());
}

if (!llResult) {
if (negateOffset)
llOffset = gIR->ir->CreateNeg(llOffset);
llResult = DtoGEP1(llBase, llOffset, false);
}

return new DImValue(t, res);
return new DImValue(resultType, DtoBitCast(llResult, DtoType(resultType)));
}
}

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

DImValue *DtoBinMul(Type *targettype, DRValue *lhs, DRValue *rhs) {
Type *t = lhs->type;
LLValue *l = DtoRVal(lhs);
LLValue *r = DtoRVal(rhs);
DValue *binAdd(Loc &loc, Type *type, DValue *lhs, Expression *rhs,
bool loadLhsAfterRhs) {
Type *lhsType = lhs->type->toBasetype();
Type *rhsType = rhs->type->toBasetype();

LLValue *res;
if (t->isfloating()) {
res = gIR->ir->CreateFMul(l, r);
} else {
res = gIR->ir->CreateMul(l, r);
if (lhsType != rhsType && lhsType->ty == Tpointer && rhsType->isintegral()) {
Logger::println("Adding integer to pointer");
return emitPointerOffset(loc, lhs, rhs, false, type, loadLhsAfterRhs);
}

auto rvals = evalSides(lhs, rhs, loadLhsAfterRhs);

if (type->iscomplex())
return DtoComplexAdd(loc, type, rvals.lhs, rvals.rhs);

LLValue *l = DtoRVal(DtoCast(loc, rvals.lhs, type));
LLValue *r = DtoRVal(DtoCast(loc, rvals.rhs, type));
LLValue *res = (type->isfloating() ? gIR->ir->CreateFAdd(l, r)
: gIR->ir->CreateAdd(l, r));

return new DImValue(type, res);
}

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

DValue *binMin(Loc &loc, Type *type, DValue *lhs, Expression *rhs,
bool loadLhsAfterRhs) {
Type *lhsType = lhs->type->toBasetype();
Type *rhsType = rhs->type->toBasetype();

if (lhsType != rhsType && lhsType->ty == Tpointer && rhsType->isintegral()) {
Logger::println("Subtracting integer from pointer");
return emitPointerOffset(loc, lhs, rhs, true, type, loadLhsAfterRhs);
}

auto rvals = evalSides(lhs, rhs, loadLhsAfterRhs);

if (lhsType->ty == Tpointer && rhsType->ty == Tpointer) {
LLValue *l = DtoRVal(rvals.lhs);
LLValue *r = DtoRVal(rvals.rhs);
LLType *llSizeT = DtoSize_t();
l = gIR->ir->CreatePtrToInt(l, llSizeT);
r = gIR->ir->CreatePtrToInt(r, llSizeT);
LLValue *diff = gIR->ir->CreateSub(l, r);
LLType *llType = DtoType(type);
if (diff->getType() != llType)
diff = gIR->ir->CreateIntToPtr(diff, llType);
return new DImValue(type, diff);
}

return new DImValue(targettype, res);
if (type->iscomplex())
return DtoComplexMin(loc, type, rvals.lhs, rvals.rhs);

LLValue *l = DtoRVal(DtoCast(loc, rvals.lhs, type));
LLValue *r = DtoRVal(DtoCast(loc, rvals.rhs, type));
LLValue *res = (type->isfloating() ? gIR->ir->CreateFSub(l, r)
: gIR->ir->CreateSub(l, r));

return new DImValue(type, res);
}

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

DImValue *DtoBinDiv(Type *targettype, DRValue *lhs, DRValue *rhs) {
Type *t = lhs->type;
LLValue *l = DtoRVal(lhs);
LLValue *r = DtoRVal(rhs);
DValue *binMul(Loc &loc, Type *type, DValue *lhs, Expression *rhs,
bool loadLhsAfterRhs) {
auto rvals = evalSides(lhs, rhs, loadLhsAfterRhs);

if (type->iscomplex())
return DtoComplexMul(loc, type, rvals.lhs, rvals.rhs);

LLValue *l = DtoRVal(DtoCast(loc, rvals.lhs, type));
LLValue *r = DtoRVal(DtoCast(loc, rvals.rhs, type));
LLValue *res = (type->isfloating() ? gIR->ir->CreateFMul(l, r)
: gIR->ir->CreateMul(l, r));

return new DImValue(type, res);
}

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

DValue *binDiv(Loc &loc, Type *type, DValue *lhs, Expression *rhs,
bool loadLhsAfterRhs) {
auto rvals = evalSides(lhs, rhs, loadLhsAfterRhs);

if (type->iscomplex())
return DtoComplexDiv(loc, type, rvals.lhs, rvals.rhs);

LLValue *l = DtoRVal(DtoCast(loc, rvals.lhs, type));
LLValue *r = DtoRVal(DtoCast(loc, rvals.rhs, type));
LLValue *res;
if (t->isfloating()) {
if (type->isfloating()) {
res = gIR->ir->CreateFDiv(l, r);
} else if (!isLLVMUnsigned(t)) {
} else if (!isLLVMUnsigned(type)) {
res = gIR->ir->CreateSDiv(l, r);
} else {
res = gIR->ir->CreateUDiv(l, r);
}

return new DImValue(targettype, res);
return new DImValue(type, res);
}

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

DImValue *DtoBinRem(Type *targettype, DRValue *lhs, DRValue *rhs) {
Type *t = lhs->type;
LLValue *l = DtoRVal(lhs);
LLValue *r = DtoRVal(rhs);
DValue *binMod(Loc &loc, Type *type, DValue *lhs, Expression *rhs,
bool loadLhsAfterRhs) {
auto rvals = evalSides(lhs, rhs, loadLhsAfterRhs);

if (type->iscomplex())
return DtoComplexMod(loc, type, rvals.lhs, rvals.rhs);

LLValue *l = DtoRVal(DtoCast(loc, rvals.lhs, type));
LLValue *r = DtoRVal(DtoCast(loc, rvals.rhs, type));
LLValue *res;
if (t->isfloating()) {
if (type->isfloating()) {
res = gIR->ir->CreateFRem(l, r);
} else if (!isLLVMUnsigned(t)) {
} else if (!isLLVMUnsigned(type)) {
res = gIR->ir->CreateSRem(l, r);
} else {
res = gIR->ir->CreateURem(l, r);
}

return new DImValue(targettype, res);
return new DImValue(type, res);
}

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

namespace {
template <llvm::Instruction::BinaryOps binOp>
Copy link
Member

@dnadlinger dnadlinger Jul 18, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Does it really buy us anything to make this a template? It seems like inlining should be sufficient, even if this was performance critical.

Copy link
Member Author

@kinke kinke Jul 18, 2016

Choose a reason for hiding this comment

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

Yeah no need for a template here anymore.

DValue *binBitwise(Loc &loc, Type *type, DValue *lhs, Expression *rhs,
bool loadLhsAfterRhs) {
auto rvals = evalSides(lhs, rhs, loadLhsAfterRhs);

LLValue *l = DtoRVal(DtoCast(loc, rvals.lhs, type));
LLValue *r = DtoRVal(DtoCast(loc, rvals.rhs, type));
LLValue *res = llvm::BinaryOperator::Create(binOp, l, r, "", gIR->scopebb());

return new DImValue(type, res);
}
}

DValue *binAnd(Loc &loc, Type *type, DValue *lhs, Expression *rhs,
bool loadLhsAfterRhs) {
return binBitwise<llvm::Instruction::And>(loc, type, lhs, rhs,
loadLhsAfterRhs);
}

DValue *binOr(Loc &loc, Type *type, DValue *lhs, Expression *rhs,
bool loadLhsAfterRhs) {
return binBitwise<llvm::Instruction::Or>(loc, type, lhs, rhs,
loadLhsAfterRhs);
}

DValue *binXor(Loc &loc, Type *type, DValue *lhs, Expression *rhs,
bool loadLhsAfterRhs) {
return binBitwise<llvm::Instruction::Xor>(loc, type, lhs, rhs,
loadLhsAfterRhs);
}

DValue *binShl(Loc &loc, Type *type, DValue *lhs, Expression *rhs,
bool loadLhsAfterRhs) {
return binBitwise<llvm::Instruction::Shl>(loc, type, lhs, rhs,
loadLhsAfterRhs);
}

DValue *binShr(Loc &loc, Type *type, DValue *lhs, Expression *rhs,
bool loadLhsAfterRhs) {
if (isLLVMUnsigned(type))
return binUshr(loc, type, lhs, rhs);

return binBitwise<llvm::Instruction::AShr>(loc, type, lhs, rhs,
loadLhsAfterRhs);
}

DValue *binUshr(Loc &loc, Type *type, DValue *lhs, Expression *rhs,
bool loadLhsAfterRhs) {
return binBitwise<llvm::Instruction::LShr>(loc, type, lhs, rhs,
loadLhsAfterRhs);
}

//////////////////////////////////////////////////////////////////////////////
Expand Down
Loading