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

Fix if-statement elision on constant true/false condition, for condit… #4559

Merged
merged 2 commits into from
Jan 10, 2024
Merged

Fix if-statement elision on constant true/false condition, for condit… #4559

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
2e02d8f
Fix if-statement elision on constant true/false condition, for condit…
JohanEngelen Jan 8, 2024
9068d9e
Turn codegen test case into runnable test, proving that the code does…
JohanEngelen Jan 10, 2024
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
21 changes: 14 additions & 7 deletions gen/statements.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -417,15 +417,26 @@ class ToIRVisitor : public Visitor {
}
DValue *cond_e = toElemDtor(stmt->condition);
LLValue *cond_val = DtoRVal(cond_e);
// Is it constant?
if (LLConstant *const_val = llvm::dyn_cast<LLConstant>(cond_val)) {
if (!cond_val->getType()->isIntegerTy(1)) {
IF_LOG Logger::cout() << "if conditional: " << *cond_val << std::endl;
cond_val = DtoRVal(DtoCast(stmt->loc, cond_e, Type::tbool));
}

// Is it constant, and is its value certainly `false` or `true`?
// Note: It must be a simple constant for which it is sufficient to
// determine true/false by calling `isZeroValue()`. This is not the case for
// complex llvm::Constant such as a CompareConstantExpr where `isZeroValue()
// == false` does not imply that the value is always `true`. See LDC issue
// #4556.
if (llvm::ConstantInt *const_val = llvm::dyn_cast<llvm::ConstantInt>(cond_val)) {
Statement *executed = stmt->ifbody;
Statement *skipped = stmt->elsebody;
if (const_val->isZeroValue()) {
std::swap(executed, skipped);
}
if (!containsLabel(skipped)) {
IF_LOG Logger::println("Constant true/false condition - elide.");
IF_LOG Logger::cout() << "Constant true/false condition - elide. (cond_val: " << *cond_val << ')' << std::endl;

if (executed) {
irs->DBuilder.EmitBlockStart(executed->loc);
}
Expand All @@ -447,10 +458,6 @@ class ToIRVisitor : public Visitor {
llvm::BasicBlock *elsebb =
stmt->elsebody ? irs->insertBBAfter(ifbb, "else") : endbb;

if (!cond_val->getType()->isIntegerTy(1)) {
IF_LOG Logger::cout() << "if conditional: " << *cond_val << '\n';
cond_val = DtoRVal(DtoCast(stmt->loc, cond_e, Type::tbool));
}
auto brinstr =
llvm::BranchInst::Create(ifbb, elsebb, cond_val, irs->scopebb());
PGO.addBranchWeights(brinstr, brweights);
Expand Down
37 changes: 37 additions & 0 deletions tests/codegen/const_cond.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,13 @@
// Also, verify it _does_ generate correct code when it is not constant.

// RUN: %ldc -O0 -output-ll -of=%t.ll %s && FileCheck %s < %t.ll
// RUN: %ldc -O0 -run %s
// RUN: %ldc -O3 -run %s

extern(C): //to avoid name mangling.

void g();

// CHECK-LABEL: @foo
void foo()
{
Expand Down Expand Up @@ -67,6 +71,17 @@ void only_ret2()
}
}

// CHECK-LABEL: @only_ret3
void only_ret3()
{
// CHECK-NEXT: ret void
// CHECK-NEXT: }
if (!cast(void*)&only_ret2)
{
int a = 1;
}
}

// CHECK-LABEL: @gen_br
void gen_br(immutable int a)
{
Expand All @@ -76,3 +91,25 @@ void gen_br(immutable int a)
int b = 1;
}
}

// CHECK-LABEL: @gh4556
void gh4556()
{
if ("a" is "b")
{
assert(false);
}

if ("a" !is "b")
{
return;
}
else
{
assert(false);
}
}

void main() {
gh4556();
}