[CIR][CIRGen] Emit cir.unreachable on implicit returns (llvm#486)

This patch changes the emission of implicit returns from functions whose
return type is not `void`. Instead of emitting `cir.return`, this PR
aligns to the original clang CodeGen and emits a `cir.unreachable`
operation.

Related issue: llvm#457 .

clang/lib/CIR/CodeGen/CIRGenFunction.cpp

@@ -319,22 +319,6 @@ void CIRGenFunction::LexicalScope::cleanup() {
   auto &builder = CGF.builder;
   auto *localScope = CGF.currLexScope;
 
-  auto buildReturn = [&](mlir::Location loc) {
-    // If we are on a coroutine, add the coro_end builtin call.
-    auto Fn = dyn_cast<mlir::cir::FuncOp>(CGF.CurFn);
-    assert(Fn && "other callables NYI");
-    if (Fn.getCoroutine())
-      CGF.buildCoroEndBuiltinCall(
-          loc, builder.getNullPtr(builder.getVoidPtrTy(), loc));
-
-    if (CGF.FnRetCIRTy.has_value()) {
-      // If there's anything to return, load it first.
-      auto val = builder.create<LoadOp>(loc, *CGF.FnRetCIRTy, *CGF.FnRetAlloca);
-      return builder.create<ReturnOp>(loc, llvm::ArrayRef(val.getResult()));
-    }
-    return builder.create<ReturnOp>(loc);
-  };
-
   // Handle pending gotos and the solved labels in this scope.
   while (!localScope->PendingGotos.empty()) {
     auto gotoInfo = localScope->PendingGotos.back();
@@ -377,15 +361,18 @@ void CIRGenFunction::LexicalScope::cleanup() {
     // Leverage and defers to RunCleanupsScope's dtor and scope handling.
     applyCleanup();
 
-    if (localScope->Depth != 0) { // end of any local scope != function
-      // Ternary ops have to deal with matching arms for yielding types
-      // and do return a value, it must do its own cir.yield insertion.
-      if (!localScope->isTernary()) {
-        !retVal ? builder.create<YieldOp>(localScope->EndLoc)
-                : builder.create<YieldOp>(localScope->EndLoc, retVal);
-      }
-    } else
-      (void)buildReturn(localScope->EndLoc);
+    if (localScope->Depth == 0) {
+      buildImplicitReturn();
+      return;
+    }
+
+    // End of any local scope != function
+    // Ternary ops have to deal with matching arms for yielding types
+    // and do return a value, it must do its own cir.yield insertion.
+    if (!localScope->isTernary()) {
+      !retVal ? builder.create<YieldOp>(localScope->EndLoc)
+              : builder.create<YieldOp>(localScope->EndLoc, retVal);
+    }
   };
 
   // If a cleanup block has been created at some point, branch to it
@@ -430,6 +417,64 @@ void CIRGenFunction::LexicalScope::cleanup() {
   insertCleanupAndLeave(currBlock);
 }
 
+mlir::cir::ReturnOp
+CIRGenFunction::LexicalScope::buildReturn(mlir::Location loc) {
+  auto &builder = CGF.getBuilder();
+
+  // If we are on a coroutine, add the coro_end builtin call.
+  auto Fn = dyn_cast<mlir::cir::FuncOp>(CGF.CurFn);
+  assert(Fn && "other callables NYI");
+  if (Fn.getCoroutine())
+    CGF.buildCoroEndBuiltinCall(
+        loc, builder.getNullPtr(builder.getVoidPtrTy(), loc));
+
+  if (CGF.FnRetCIRTy.has_value()) {
+    // If there's anything to return, load it first.
+    auto val = builder.create<LoadOp>(loc, *CGF.FnRetCIRTy, *CGF.FnRetAlloca);
+    return builder.create<ReturnOp>(loc, llvm::ArrayRef(val.getResult()));
+  }
+  return builder.create<ReturnOp>(loc);
+}
+
+void CIRGenFunction::LexicalScope::buildImplicitReturn() {
+  auto &builder = CGF.getBuilder();
+  auto *localScope = CGF.currLexScope;
+
+  const auto *FD = cast<clang::FunctionDecl>(CGF.CurGD.getDecl());
+
+  // C++11 [stmt.return]p2:
+  //   Flowing off the end of a function [...] results in undefined behavior
+  //   in a value-returning function.
+  // C11 6.9.1p12:
+  //   If the '}' that terminates a function is reached, and the value of the
+  //   function call is used by the caller, the behavior is undefined.
+  if (CGF.getLangOpts().CPlusPlus && !FD->hasImplicitReturnZero() &&
+      !CGF.SawAsmBlock && !FD->getReturnType()->isVoidType() &&
+      builder.getInsertionBlock()) {
+    bool shouldEmitUnreachable = CGF.CGM.getCodeGenOpts().StrictReturn ||
+                                 !CGF.CGM.MayDropFunctionReturn(
+                                     FD->getASTContext(), FD->getReturnType());
+
+    if (CGF.SanOpts.has(SanitizerKind::Return)) {
+      assert(!UnimplementedFeature::sanitizerReturn());
+      llvm_unreachable("NYI");
+    } else if (shouldEmitUnreachable) {
+      if (CGF.CGM.getCodeGenOpts().OptimizationLevel == 0) {
+        // TODO: buildTrapCall(llvm::Intrinsic::trap);
+        assert(!UnimplementedFeature::trap());
+      }
+    }
+
+    if (CGF.SanOpts.has(SanitizerKind::Return) || shouldEmitUnreachable) {
+      builder.create<mlir::cir::UnreachableOp>(localScope->EndLoc);
+      builder.clearInsertionPoint();
+      return;
+    }
+  }
+
+  (void)buildReturn(localScope->EndLoc);
+}
+
 void CIRGenFunction::finishFunction(SourceLocation EndLoc) {
   // CIRGen doesn't use a BreakContinueStack or evaluates OnlySimpleReturnStmts.
 
@@ -657,31 +702,6 @@ CIRGenFunction::generateCode(clang::GlobalDecl GD, mlir::cir::FuncOp Fn,
   if (mlir::failed(Fn.verifyBody()))
     return nullptr;
 
-  // C++11 [stmt.return]p2:
-  //   Flowing off the end of a function [...] results in undefined behavior
-  //   in a value-returning function.
-  // C11 6.9.1p12:
-  //   If the '}' that terminates a function is reached, and the value of the
-  //   function call is used by the caller, the behavior is undefined.
-  if (getLangOpts().CPlusPlus && !FD->hasImplicitReturnZero() && !SawAsmBlock &&
-      !FD->getReturnType()->isVoidType() && builder.getInsertionBlock()) {
-    bool shouldEmitUnreachable =
-        CGM.getCodeGenOpts().StrictReturn ||
-        !CGM.MayDropFunctionReturn(FD->getASTContext(), FD->getReturnType());
-
-    if (SanOpts.has(SanitizerKind::Return)) {
-      llvm_unreachable("NYI");
-    } else if (shouldEmitUnreachable) {
-      if (CGM.getCodeGenOpts().OptimizationLevel == 0)
-        ; // TODO: buildTrapCall(llvm::Intrinsic::trap);
-    }
-    if (SanOpts.has(SanitizerKind::Return) || shouldEmitUnreachable) {
-      // TODO: builder.createUnreachable();
-      assert(!UnimplementedFeature::unreachableOp());
-      builder.clearInsertionPoint();
-    }
-  }
-
   // Emit the standard function epilogue.
   finishFunction(BodyRange.getEnd());
 

clang/lib/CIR/CodeGen/CIRGenFunction.h

@@ -1918,6 +1918,9 @@ class CIRGenFunction : public CIRGenTypeCache {
       return b;
     }
 
+    mlir::cir::ReturnOp buildReturn(mlir::Location loc);
+    void buildImplicitReturn();
+
   public:
     void updateCurrentSwitchCaseRegion() { CurrentSwitchRegionIdx++; }
     llvm::ArrayRef<mlir::Block *> getRetBlocks() { return RetBlocks; }

clang/lib/CIR/CodeGen/UnimplementedFeatureGuarding.h

@@ -58,6 +58,7 @@ struct UnimplementedFeature {
   static bool pointerOverflowSanitizer() { return false; }
   static bool sanitizeDtor() { return false; }
   static bool sanitizeVLABound() { return false; }
+  static bool sanitizerReturn() { return false; }
 
   // ObjC
   static bool setObjCGCLValueClass() { return false; }
@@ -160,12 +161,12 @@ struct UnimplementedFeature {
   static bool emitScalarRangeCheck() { return false; }
   static bool stmtExprEvaluation() { return false; }
   static bool setCallingConv() { return false; }
-  static bool unreachableOp() { return false; }
   static bool tryMarkNoThrow() { return false; }
   static bool indirectBranch() { return false; }
   static bool escapedLocals() { return false; }
   static bool deferredReplacements() { return false; }
   static bool shouldInstrumentFunction() { return false; }
+  static bool trap() { return false; }
 };
 } // namespace cir
 

clang/test/CIR/CodeGen/implicit-return.cpp

@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir-enable -emit-cir %s -o %t.cir
+// RUN: FileCheck --input-file=%t.cir %s
+
+void ret_void() {}
+
+//      CHECK: cir.func @_Z8ret_voidv()
+// CHECK-NEXT:   cir.return
+// CHECK-NEXT: }
+
+int ret_non_void() {}
+
+//      CHECK: cir.func @_Z12ret_non_voidv() -> !s32i
+// CHECK-NEXT:   %0 = cir.alloca !s32i, cir.ptr <!s32i>, ["__retval"]
+// CHECK-NEXT:   cir.unreachable
+// CHECK-NEXT: }