[Codegen] evaluate side effects properly for conditional operators

isuckatcs · Jul 1, 2024 · 3315ab3 · 3315ab3
1 parent 3a5a0c7
commit 3315ab3
Show file tree

Hide file tree

Showing 3 changed files with 111 additions and 13 deletions.
diff --git a/include/codegen.h b/include/codegen.h
@@ -33,6 +33,11 @@ class Codegen {
 
   llvm::Value *generateExpr(const ResolvedExpr &expr);
   llvm::Value *generateCallExpr(const ResolvedCallExpr &call);
+
+  void generateConditionalOperator(const ResolvedExpr &op,
+                                   llvm::BasicBlock *trueBlock,
+                                   llvm::BasicBlock *falseBlock);
+
   llvm::Value *generateBinaryOperator(const ResolvedBinaryOperator &binop);
   llvm::Value *generateUnaryOperator(const ResolvedUnaryOperator &unary);
 

diff --git a/src/codegen.cpp b/src/codegen.cpp
@@ -3,6 +3,21 @@
 #include <llvm/IR/Function.h>
 #include <llvm/IR/Module.h>
 
+// FIXME: Move this to utility
+namespace {
+const ResolvedBinaryOperator *getAsConditionalBinop(const ResolvedExpr *expr) {
+  const auto *binop = dynamic_cast<const ResolvedBinaryOperator *>(expr);
+  if (!binop)
+    return nullptr;
+
+  TokenKind op = binop->op;
+  if (op == TokenKind::PipePipe || op == TokenKind::AmpAmp)
+    return binop;
+
+  return nullptr;
+}
+} // namespace
+
 Codegen::Codegen(
     std::vector<std::unique_ptr<ResolvedFunctionDecl>> resolvedSourceFile)
     : resolvedSourceFile(std::move(resolvedSourceFile)), builder(context),
@@ -193,27 +208,82 @@ Codegen::generateUnaryOperator(const ResolvedUnaryOperator &unary) {
   llvm_unreachable("unknown unary op");
 }
 
+void Codegen::generateConditionalOperator(const ResolvedExpr &op,
+                                          llvm::BasicBlock *trueBlock,
+                                          llvm::BasicBlock *falseBlock) {
+  if (const auto *condBinop = getAsConditionalBinop(&op)) {
+    if (condBinop->op == TokenKind::PipePipe) {
+      llvm::BasicBlock *nextBlock = llvm::BasicBlock::Create(
+          context, "or.lhs.false", trueBlock->getParent());
+      generateConditionalOperator(*condBinop->lhs, trueBlock, nextBlock);
+
+      builder.SetInsertPoint(nextBlock);
+      generateConditionalOperator(*condBinop->rhs, trueBlock, falseBlock);
+      return;
+    }
+
+    if (condBinop->op == TokenKind::AmpAmp) {
+      llvm::BasicBlock *nextBlock = llvm::BasicBlock::Create(
+          context, "and.lhs.true", trueBlock->getParent());
+      generateConditionalOperator(*condBinop->lhs, nextBlock, falseBlock);
+
+      builder.SetInsertPoint(nextBlock);
+      generateConditionalOperator(*condBinop->rhs, trueBlock, falseBlock);
+      return;
+    }
+  }
+
+  llvm::Value *val = doubleToBool(generateExpr(op));
+  builder.CreateCondBr(val, trueBlock, falseBlock);
+};
+
 llvm::Value *
 Codegen::generateBinaryOperator(const ResolvedBinaryOperator &binop) {
+
+  TokenKind op = binop.op;
+  if (op == TokenKind::AmpAmp || op == TokenKind::PipePipe) {
+    bool isOr = op == TokenKind::PipePipe;
+
+    llvm::BasicBlock *rhsBlock =
+        llvm::BasicBlock::Create(context, isOr ? "or.rhs" : "and.rhs",
+                                 builder.GetInsertBlock()->getParent());
+    llvm::BasicBlock *mergeBlock =
+        llvm::BasicBlock::Create(context, isOr ? "or.merge" : "and.merge",
+                                 builder.GetInsertBlock()->getParent());
+
+    generateConditionalOperator(*binop.lhs, isOr ? mergeBlock : rhsBlock,
+                                isOr ? rhsBlock : mergeBlock);
+    builder.SetInsertPoint(rhsBlock);
+
+    llvm::Value *rhs = doubleToBool(generateExpr(*binop.rhs));
+    builder.CreateBr(mergeBlock);
+
+    builder.SetInsertPoint(mergeBlock);
+
+    llvm::PHINode *phi = builder.CreatePHI(builder.getInt1Ty(), 0);
+
+    for (llvm::pred_iterator pi = pred_begin(mergeBlock),
+                             pe = pred_end(mergeBlock);
+         pi != pe; ++pi)
+      phi->addIncoming(builder.getInt1(isOr), *pi);
+    phi->addIncoming(rhs, rhsBlock);
+
+    return boolToDouble(phi);
+  }
+
   llvm::Value *lhs = generateExpr(*binop.lhs);
   llvm::Value *rhs = generateExpr(*binop.rhs);
 
-  // FIXME: Refactor this!!!
-  if (binop.op == TokenKind::Lt)
+  if (op == TokenKind::Lt)
     return boolToDouble(builder.CreateFCmpOLT(lhs, rhs));
-  if (binop.op == TokenKind::Gt)
+
+  if (op == TokenKind::Gt)
     return boolToDouble(builder.CreateFCmpOGT(lhs, rhs));
-  if (binop.op == TokenKind::EqualEqual)
+
+  if (op == TokenKind::EqualEqual)
     return boolToDouble(builder.CreateFCmpOEQ(lhs, rhs));
-  if (binop.op == TokenKind::AmpAmp) {
-    return boolToDouble(
-        builder.CreateLogicalAnd(doubleToBool(lhs), doubleToBool(rhs)));
-  }
-  if (binop.op == TokenKind::PipePipe)
-    return boolToDouble(
-        builder.CreateLogicalOr(doubleToBool(lhs), doubleToBool(rhs)));
 
-  return builder.CreateBinOp(getOperatorKind(binop.op), lhs, rhs);
+  return builder.CreateBinOp(getOperatorKind(op), lhs, rhs);
 }
 
 llvm::Value *Codegen::doubleToBool(llvm::Value *v) {
@@ -358,4 +428,4 @@ std::unique_ptr<llvm::Module> Codegen::generateIR() {
   generateMainWrapper();
 
   return std::move(module);
-}
+}
diff --git a/test/codegen/cond_binop_side_effect.al b/test/codegen/cond_binop_side_effect.al
@@ -0,0 +1,23 @@
+// RUN: compiler %s -o cond_binop_side_effect && ./cond_binop_side_effect | grep -Plzx '1.000000\n2.000000\n3.000000\n4.000000\n5.000000\n7.000000\n10.000000\n13.000000\n14.000000\n15.000000\n16.000000\n'
+fn true(x: number): number {
+    print(x);
+    return 1.0;
+}
+
+fn false(x: number): number {
+    print(x);
+    return 0.0;
+}
+
+fn main(): void {
+    false(1.0) || true(2.0) && false(3.0);
+
+    false(4.0) || true(5.0) || true(6.0);
+
+    false(7.0) && false(8.0) && true(9.0);
+
+    true(10.0) || true(11.0) || true(12.0);
+
+    false(13.0) || true(14.0) && false(15.0) || true(16.0);
+
+}