From 556fae2d77c07cd9ce0eb29d3faf0b2b14d797de Mon Sep 17 00:00:00 2001 From: Sirui Mu Date: Wed, 6 Mar 2024 03:10:19 +0800 Subject: [PATCH] [CIR][CIRGen] Emit `cir.unreachable` on implicit returns (#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: #457 . --- clang/lib/CIR/CodeGen/CIRGenFunction.cpp | 120 ++++++++++-------- clang/lib/CIR/CodeGen/CIRGenFunction.h | 3 + .../CodeGen/UnimplementedFeatureGuarding.h | 3 +- clang/test/CIR/CodeGen/implicit-return.cpp | 15 +++ 4 files changed, 90 insertions(+), 51 deletions(-) create mode 100644 clang/test/CIR/CodeGen/implicit-return.cpp diff --git a/clang/lib/CIR/CodeGen/CIRGenFunction.cpp b/clang/lib/CIR/CodeGen/CIRGenFunction.cpp index 6d0b04afa4ca..7a37eec5cc15 100644 --- a/clang/lib/CIR/CodeGen/CIRGenFunction.cpp +++ b/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(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(loc, *CGF.FnRetCIRTy, *CGF.FnRetAlloca); - return builder.create(loc, llvm::ArrayRef(val.getResult())); - } - return builder.create(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(localScope->EndLoc) - : builder.create(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(localScope->EndLoc) + : builder.create(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(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(loc, *CGF.FnRetCIRTy, *CGF.FnRetAlloca); + return builder.create(loc, llvm::ArrayRef(val.getResult())); + } + return builder.create(loc); +} + +void CIRGenFunction::LexicalScope::buildImplicitReturn() { + auto &builder = CGF.getBuilder(); + auto *localScope = CGF.currLexScope; + + const auto *FD = cast(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(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()); diff --git a/clang/lib/CIR/CodeGen/CIRGenFunction.h b/clang/lib/CIR/CodeGen/CIRGenFunction.h index c1a2c1843730..056fe393d191 100644 --- a/clang/lib/CIR/CodeGen/CIRGenFunction.h +++ b/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 getRetBlocks() { return RetBlocks; } diff --git a/clang/lib/CIR/CodeGen/UnimplementedFeatureGuarding.h b/clang/lib/CIR/CodeGen/UnimplementedFeatureGuarding.h index d6a7e1d89433..1b75451d9174 100644 --- a/clang/lib/CIR/CodeGen/UnimplementedFeatureGuarding.h +++ b/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 diff --git a/clang/test/CIR/CodeGen/implicit-return.cpp b/clang/test/CIR/CodeGen/implicit-return.cpp new file mode 100644 index 000000000000..c41dd1e7e4d8 --- /dev/null +++ b/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 , ["__retval"] +// CHECK-NEXT: cir.unreachable +// CHECK-NEXT: }