From 3cf61568207cb19c04c1a3a1585c79a6fba407c3 Mon Sep 17 00:00:00 2001 From: Martin Kinkelin Date: Sat, 11 May 2024 15:52:52 +0200 Subject: [PATCH] Fix operand load order for binary comparison/equality/identity expressions Fixes #4651. --- gen/toir.cpp | 87 ++++++++++++++++++------------------------ tests/codegen/gh4651.d | 21 ++++++++++ 2 files changed, 59 insertions(+), 49 deletions(-) create mode 100644 tests/codegen/gh4651.d diff --git a/gen/toir.cpp b/gen/toir.cpp index 8b8203f90a..5196f92a84 100644 --- a/gen/toir.cpp +++ b/gen/toir.cpp @@ -1322,8 +1322,8 @@ class ToElemVisitor : public Visitor { auto &PGO = gIR->funcGen().pgo; PGO.setCurrentStmt(e); - DValue *l = toElem(e->e1); - DValue *r = toElem(e->e2); + LLValue *l = DtoRVal(e->e1); + LLValue *r = DtoRVal(e->e2); Type *t = e->e1->type->toBasetype(); @@ -1334,16 +1334,14 @@ class ToElemVisitor : public Visitor { tokToICmpPred(e->op, isLLVMUnsigned(t), &icmpPred, &eval); if (!eval) { - LLValue *a = DtoRVal(l); - LLValue *b = DtoRVal(r); IF_LOG { - Logger::cout() << "type 1: " << *a << '\n'; - Logger::cout() << "type 2: " << *b << '\n'; + Logger::cout() << "type 1: " << *l << '\n'; + Logger::cout() << "type 2: " << *r << '\n'; } - if (a->getType() != b->getType()) { - b = DtoBitCast(b, a->getType()); + if (l->getType() != r->getType()) { + r = DtoBitCast(r, l->getType()); } - eval = p->ir->CreateICmp(icmpPred, a, b); + eval = p->ir->CreateICmp(icmpPred, l, r); } } else if (t->isfloating()) { llvm::FCmpInst::Predicate cmpop; @@ -1364,7 +1362,7 @@ class ToElemVisitor : public Visitor { default: llvm_unreachable("Unsupported floating point comparison operator."); } - eval = p->ir->CreateFCmp(cmpop, DtoRVal(l), DtoRVal(r)); + eval = p->ir->CreateFCmp(cmpop, l, r); } else if (t->ty == TY::Taarray) { eval = LLConstantInt::getFalse(gIR->context()); } else if (t->ty == TY::Tdelegate) { @@ -1374,23 +1372,21 @@ class ToElemVisitor : public Visitor { if (!eval) { // First compare the function pointers, then the context ones. This is // what DMD does. - llvm::Value *lhs = DtoRVal(l); - llvm::Value *rhs = DtoRVal(r); llvm::BasicBlock *fptreq = p->insertBB("fptreq"); llvm::BasicBlock *fptrneq = p->insertBBAfter(fptreq, "fptrneq"); llvm::BasicBlock *dgcmpend = p->insertBBAfter(fptrneq, "dgcmpend"); - llvm::Value *lfptr = p->ir->CreateExtractValue(lhs, 1, ".lfptr"); - llvm::Value *rfptr = p->ir->CreateExtractValue(rhs, 1, ".rfptr"); + llvm::Value *lfptr = p->ir->CreateExtractValue(l, 1, ".lfptr"); + llvm::Value *rfptr = p->ir->CreateExtractValue(r, 1, ".rfptr"); llvm::Value *fptreqcmp = p->ir->CreateICmp(llvm::ICmpInst::ICMP_EQ, lfptr, rfptr, ".fptreqcmp"); llvm::BranchInst::Create(fptreq, fptrneq, fptreqcmp, p->scopebb()); p->ir->SetInsertPoint(fptreq); - llvm::Value *lctx = p->ir->CreateExtractValue(lhs, 0, ".lctx"); - llvm::Value *rctx = p->ir->CreateExtractValue(rhs, 0, ".rctx"); + llvm::Value *lctx = p->ir->CreateExtractValue(l, 0, ".lctx"); + llvm::Value *rctx = p->ir->CreateExtractValue(r, 0, ".rctx"); llvm::Value *ctxcmp = p->ir->CreateICmp(icmpPred, lctx, rctx, ".ctxcmp"); llvm::BranchInst::Create(dgcmpend, p->scopebb()); @@ -1423,10 +1419,10 @@ class ToElemVisitor : public Visitor { auto &PGO = gIR->funcGen().pgo; PGO.setCurrentStmt(e); - DValue *l = toElem(e->e1); - DValue *r = toElem(e->e2); + Expression *l = e->e1; + Expression *r = e->e2; - Type *t = e->e1->type->toBasetype(); + Type *t = l->type->toBasetype(); LLValue *eval = nullptr; @@ -1457,20 +1453,22 @@ class ToElemVisitor : public Visitor { } eval = p->ir->CreateICmp(cmpop, lv, rv); } else if (t->isfloating()) { // includes iscomplex - eval = DtoBinNumericEquals(e->loc, l, r, e->op); + eval = DtoBinNumericEquals(e->loc, toElem(l)->getRVal(), + toElem(r)->getRVal(), e->op); } else if (t->ty == TY::Tsarray || t->ty == TY::Tarray) { Logger::println("static or dynamic array"); - eval = DtoArrayEquals(e->loc, e->op, l, r); + eval = DtoArrayEquals(e->loc, e->op, toElem(l), toElem(r)); } else if (t->ty == TY::Taarray) { Logger::println("associative array"); - eval = DtoAAEquals(e->loc, e->op, l, r); + eval = DtoAAEquals(e->loc, e->op, toElem(l)->getRVal(), + toElem(r)->getRVal()); } else if (t->ty == TY::Tdelegate) { Logger::println("delegate"); eval = DtoDelegateEquals(e->op, DtoRVal(l), DtoRVal(r)); } else if (t->ty == TY::Tstruct) { Logger::println("struct"); // when this is reached it means there is no opEquals overload. - eval = DtoStructEquals(e->op, l, r); + eval = DtoStructEquals(e->op, toElem(l), toElem(r)); } else { llvm_unreachable("Unsupported EqualExp type."); } @@ -2028,45 +2026,35 @@ class ToElemVisitor : public Visitor { e->type->toChars()); LOG_SCOPE; - DValue *l = toElem(e->e1); - DValue *r = toElem(e->e2); - - Type *t1 = e->e1->type->toBasetype(); + Expression *l = e->e1; + Expression *r = e->e2; - // handle dynarray specially - if (t1->ty == TY::Tarray) { - result = zextBool(DtoDynArrayIs(e->op, l, r), e->type); - return; - } - // also structs - if (t1->ty == TY::Tstruct) { - result = zextBool(DtoStructEquals(e->op, l, r), e->type); - return; - } + Type *t1 = l->type->toBasetype(); - // FIXME this stuff isn't pretty LLValue *eval = nullptr; - if (t1->ty == TY::Tdelegate) { + if (t1->ty == TY::Tarray) { + eval = DtoDynArrayIs(e->op, toElem(l), toElem(r)); + } else if (t1->ty == TY::Tstruct) { + eval = DtoStructEquals(e->op, toElem(l), toElem(r)); + } else if (t1->ty == TY::Tdelegate) { LLValue *lv = DtoRVal(l); + DValue *rhs = toElem(r); + LLValue *rv = nullptr; - if (!r->isNull()) { - rv = DtoRVal(r); + if (!rhs->isNull()) { + rv = DtoRVal(rhs); assert(lv->getType() == rv->getType()); } + eval = DtoDelegateEquals(e->op, lv, rv); } else if (t1->isfloating()) { // includes iscomplex - eval = DtoBinNumericEquals(e->loc, l, r, e->op); + eval = DtoBinNumericEquals(e->loc, toElem(l)->getRVal(), + toElem(r)->getRVal(), e->op); } else if (t1->ty == TY::Tpointer || t1->ty == TY::Tclass) { LLValue *lv = DtoRVal(l); LLValue *rv = DtoRVal(r); - if (lv->getType() != rv->getType()) { - if (r->isNull()) { - rv = llvm::ConstantPointerNull::get(isaPointer(lv)); - } else { - rv = DtoBitCast(rv, lv->getType()); - } - } + rv = DtoBitCast(rv, lv->getType()); eval = (e->op == EXP::identity) ? p->ir->CreateICmpEQ(lv, rv) : p->ir->CreateICmpNE(lv, rv); } else if (t1->ty == TY::Tsarray) { @@ -2086,6 +2074,7 @@ class ToElemVisitor : public Visitor { : EXP::notEqual); } } + result = zextBool(eval, e->type); } diff --git a/tests/codegen/gh4651.d b/tests/codegen/gh4651.d new file mode 100644 index 0000000000..bd2aaac860 --- /dev/null +++ b/tests/codegen/gh4651.d @@ -0,0 +1,21 @@ +// RUN: %ldc -run %s + +void main() +{ + int x; + + assert(!(++x < x++)); // CmpExp: !(1 < 1) + assert(x == 2); + + assert(!(++x > x++)); // CmpExp: !(3 > 3) + assert(x == 4); + + assert(++x == x++); // EqualExp: 5 == 5 + assert(x == 6); + + assert(++x is x++); // IdentityExp: 7 is 7 + assert(x == 8); + + assert(x++ !is x); // IdentityExp: 8 !is 9 + assert(x == 9); +}