Skip to content

Commit

Permalink
[CIR][CIRGen] Emit cir.unreachable on implicit returns (#486)
Browse files Browse the repository at this point in the history
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: #457 .
  • Loading branch information
Lancern authored and lanza committed Nov 2, 2024
1 parent a9dfe60 commit 7faf56d
Show file tree
Hide file tree
Showing 4 changed files with 90 additions and 51 deletions.
120 changes: 70 additions & 50 deletions clang/lib/CIR/CodeGen/CIRGenFunction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -323,22 +323,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();
Expand Down Expand Up @@ -381,15 +365,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
Expand Down Expand Up @@ -434,6 +421,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.

Expand Down Expand Up @@ -661,31 +706,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());

Expand Down
3 changes: 3 additions & 0 deletions clang/lib/CIR/CodeGen/CIRGenFunction.h
Original file line number Diff line number Diff line change
Expand Up @@ -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; }
Expand Down
3 changes: 2 additions & 1 deletion clang/lib/CIR/CodeGen/UnimplementedFeatureGuarding.h
Original file line number Diff line number Diff line change
Expand Up @@ -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; }
Expand Down Expand Up @@ -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

Expand Down
15 changes: 15 additions & 0 deletions clang/test/CIR/CodeGen/implicit-return.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -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: }

0 comments on commit 7faf56d

Please sign in to comment.