Skip to content

Commit

Permalink
[CIR] Remove return !cir.void from IR and textual representation
Browse files Browse the repository at this point in the history
C/C++ functions returning void had an explicit !cir.void return type while not
having any returned value, which was breaking a lot of MLIR invariants when the
CIR dialect is used in a greater context, for example with the inliner.

Now, a C/C++ function returning void has no return type and no return values,
which does not break the MLIR invariant about the same number of return types
and returned values.

This change does not keeps the same parsing/pretty-printed syntax as before for
compatibility like in #1203 because it
requires some new features from the MLIR parser infrastructure itself, which is
not great.

For now use a list of return types.
  • Loading branch information
keryell committed Jan 8, 2025
1 parent aceb0b0 commit afde518
Show file tree
Hide file tree
Showing 7 changed files with 58 additions and 24 deletions.
13 changes: 8 additions & 5 deletions clang/include/clang/CIR/Dialect/IR/CIROps.td
Original file line number Diff line number Diff line change
Expand Up @@ -3474,8 +3474,6 @@ def FuncOp : CIR_Op<"func", [
/// Returns the results types that the callable region produces when
/// executed.
llvm::ArrayRef<mlir::Type> getCallableResults() {
if (::llvm::isa<cir::VoidType>(getFunctionType().getReturnType()))
return {};
return getFunctionType().getReturnTypes();
}

Expand All @@ -3492,10 +3490,15 @@ def FuncOp : CIR_Op<"func", [
}

/// Returns the argument types of this function.
llvm::ArrayRef<mlir::Type> getArgumentTypes() { return getFunctionType().getInputs(); }
llvm::ArrayRef<mlir::Type> getArgumentTypes() {
return getFunctionType().getInputs();
}

/// Returns the result types of this function.
llvm::ArrayRef<mlir::Type> getResultTypes() { return getFunctionType().getReturnTypes(); }
/// Returns 0 or 1 result type of this function (0 in the case of a function
/// returing void)
llvm::ArrayRef<mlir::Type> getResultTypes() {
return getFunctionType().getReturnTypes();
}

/// Hook for OpTrait::FunctionOpInterfaceTrait, called after verifying that
/// the 'type' attribute is present and checks if it holds a function type.
Expand Down
19 changes: 12 additions & 7 deletions clang/include/clang/CIR/Dialect/IR/CIRTypes.td
Original file line number Diff line number Diff line change
Expand Up @@ -379,22 +379,27 @@ def CIR_FuncType : CIR_Type<"Func", "func"> {

```mlir
!cir.func<!bool ()>
!cir.func<(!s8i, !s8i)>
!cir.func<!s32i (!s8i, !s8i)>
!cir.func<!s32i (!s32i, ...)>
```
}];

let parameters = (ins ArrayRefParameter<"mlir::Type">:$inputs, "mlir::Type":$returnType,
let parameters = (ins ArrayRefParameter<"mlir::Type">:$inputs, ArrayRefParameter<"mlir::Type">:$returnTypes,
"bool":$varArg);
let assemblyFormat = [{
`<` $returnType ` ` `(` custom<FuncTypeArgs>($inputs, $varArg) `>`
`<` $returnTypes ` ` `(` custom<FuncTypeArgs>($inputs, $varArg) `>`
}];

let builders = [
// Construct with an actual return type or explicit !cir.void
TypeBuilderWithInferredContext<(ins
"llvm::ArrayRef<mlir::Type>":$inputs, "mlir::Type":$returnType,
CArg<"bool", "false">:$isVarArg), [{
return $_get(returnType.getContext(), inputs, returnType, isVarArg);
return $_get(returnType.getContext(), inputs,
::mlir::isa<::cir::VoidType>(returnType) ? llvm::ArrayRef<mlir::Type>{}
: llvm::ArrayRef{returnType},
isVarArg);
}]>
];

Expand All @@ -408,11 +413,11 @@ def CIR_FuncType : CIR_Type<"Func", "func"> {
/// Returns the number of arguments to the function.
unsigned getNumInputs() const { return getInputs().size(); }

/// Returns the result type of the function as an ArrayRef, enabling better
/// integration with generic MLIR utilities.
llvm::ArrayRef<mlir::Type> getReturnTypes() const;
/// Returns the result type of the function as an actual return type or
/// explicit !cir.void
mlir::Type getReturnType() const;

/// Returns whether the function is returns void.
/// Returns whether the function returns void.
bool isVoid() const;

/// Returns a clone of this function type with the given argument
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/CIR/CodeGen/CIRGenTypes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ mlir::Type CIRGenTypes::ConvertFunctionTypeInternal(QualType QFT) {
assert(QFT.isCanonical());
const Type *Ty = QFT.getTypePtr();
const FunctionType *FT = cast<FunctionType>(QFT.getTypePtr());
// First, check whether we can build the full fucntion type. If the function
// First, check whether we can build the full function type. If the function
// type depends on an incomplete type (e.g. a struct or enum), we cannot lower
// the function type.
assert(isFuncTypeConvertible(FT) && "NYI");
Expand Down
14 changes: 7 additions & 7 deletions clang/lib/CIR/Dialect/IR/CIRDialect.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2489,13 +2489,8 @@ void cir::FuncOp::print(OpAsmPrinter &p) {
p.printSymbolName(getSymName());
auto fnType = getFunctionType();
llvm::SmallVector<Type, 1> resultTypes;
if (!fnType.isVoid())
function_interface_impl::printFunctionSignature(
p, *this, fnType.getInputs(), fnType.isVarArg(),
fnType.getReturnTypes());
else
function_interface_impl::printFunctionSignature(
p, *this, fnType.getInputs(), fnType.isVarArg(), {});
function_interface_impl::printFunctionSignature(
p, *this, fnType.getInputs(), fnType.isVarArg(), fnType.getReturnTypes());

if (mlir::ArrayAttr annotations = getAnnotationsAttr()) {
p << ' ';
Expand Down Expand Up @@ -2564,6 +2559,11 @@ LogicalResult cir::FuncOp::verifyType() {
if (!getNoProto() && type.isVarArg() && type.getNumInputs() == 0)
return emitError()
<< "prototyped function must have at least one non-variadic input";
if (auto rt = type.getReturnTypes();
!rt.empty() && mlir::isa<cir::VoidType>(rt.front()))
return emitOpError("The return type for a function returning void should "
"be empty instead of an explicit !cir.void");

return success();
}

Expand Down
18 changes: 15 additions & 3 deletions clang/lib/CIR/Dialect/IR/CIRTypes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
#include "llvm/ADT/TypeSwitch.h"
#include "llvm/Support/ErrorHandling.h"
#include "llvm/Support/MathExtras.h"
#include <cassert>
#include <optional>

using cir::MissingFeatures;
Expand Down Expand Up @@ -957,11 +958,22 @@ void printFuncTypeArgs(mlir::AsmPrinter &p, mlir::ArrayRef<mlir::Type> params,
p << ')';
}

llvm::ArrayRef<mlir::Type> FuncType::getReturnTypes() const {
return static_cast<detail::FuncTypeStorage *>(getImpl())->returnType;
// Return the actual return type or an explicit !cir.void if the function does
// not return anything
mlir::Type FuncType::getReturnType() const {
if (isVoid())
return cir::VoidType::get(getContext());
return static_cast<detail::FuncTypeStorage *>(getImpl())->returnTypes.front();
}

bool FuncType::isVoid() const { return mlir::isa<VoidType>(getReturnType()); }
bool FuncType::isVoid() const {
auto rt = static_cast<detail::FuncTypeStorage *>(getImpl())->returnTypes;
assert(rt.empty() ||
!mlir::isa<cir::VoidType>(rt.front()) &&
"The return type for a function returning void should be empty "
"instead of a real !cir.void");
return rt.empty();
}

//===----------------------------------------------------------------------===//
// MethodType Definitions
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ FuncType LowerTypes::getFunctionType(const LowerFunctionInfo &FI) {
}
}

return FuncType::get(getMLIRContext(), ArgTypes, resultType, FI.isVariadic());
return FuncType::get(ArgTypes, resultType, FI.isVariadic());
}

/// Convert a CIR type to its ABI-specific default form.
Expand Down
14 changes: 14 additions & 0 deletions clang/test/CIR/IR/being_and_nothingness.cir
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// RUN: cir-opt %s | FileCheck %s
// Exercise different ways to encode a function returning void
!s32i = !cir.int<s, 32>
!fnptr3 = !cir.ptr<!cir.func<(!s32i)>>
module {
cir.func @ind3(%fnptr: !fnptr3, %a : !s32i) {
// CHECK: cir.func @ind3(%arg0: !cir.ptr<!cir.func<(!s32i)>>, %arg1: !s32i) {
cir.return
}
cir.func @f3() {
// CHECK: cir.func @f3() {
cir.return
}
}

0 comments on commit afde518

Please sign in to comment.