[CIR] Better handling of void function return

Upstream llvm/clangir#1249
"Remove return !cir.void from IR and textual representation"

C/C++ functions returning `void` had an explicit `!cir.void` return type
while not having any returned value, which was breaking 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 satisfies the MLIR invariant about the same number of
return types and returned values.

Author: dkolsen-pgi

clang/include/clang/CIR/Dialect/IR/CIRTypes.td

@@ -273,29 +273,36 @@ def CIR_PointerType : CIR_Type<"Pointer", "ptr",
 def CIR_FuncType : CIR_Type<"Func", "func"> {
   let summary = "CIR function type";
   let description = [{
-    The `!cir.func` is a function type. It consists of a single return type, a
-    list of parameter types and can optionally be variadic.
+    The `!cir.func` is a function type. It consists of an optional return type,
+    a list of parameter types and can optionally be variadic.
 
     Example:
 
     ```mlir
+    !cir.func<()>
     !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, "bool":$varArg);
+                        "mlir::Type":$optionalReturnType, "bool":$varArg);
+  // Use a custom parser to handle the optional return and argument types
+  // without an optional anchor.
   let assemblyFormat = [{
-    `<` $returnType ` ` `(` custom<FuncTypeArgs>($inputs, $varArg) `>`
+    `<` custom<FuncType>($optionalReturnType, $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) ? nullptr : returnType,
+                     isVarArg);
     }]>
   ];
 
@@ -309,11 +316,15 @@ 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 actual return type or
+    /// explicit !cir.void
+    mlir::Type getReturnType() const;
+
     /// Returns the result type of the function as an ArrayRef, enabling better
     /// integration with generic MLIR utilities.
     llvm::ArrayRef<mlir::Type> getReturnTypes() 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

clang/lib/CIR/CodeGen/CIRGenTypes.cpp

@@ -60,7 +60,7 @@ bool CIRGenTypes::isFuncTypeConvertible(const FunctionType *ft) {
 mlir::Type CIRGenTypes::convertFunctionTypeInternal(QualType qft) {
   assert(qft.isCanonical());
   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.
   if (!isFuncTypeConvertible(ft)) {

clang/lib/CIR/Dialect/IR/CIRDialect.cpp

@@ -399,6 +399,11 @@ LogicalResult cir::FuncOp::verifyType() {
   if (!isa<cir::FuncType>(type))
     return emitOpError("requires '" + getFunctionTypeAttrName().str() +
                        "' attribute of function type");
+  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();
 }
 

clang/lib/CIR/Dialect/IR/CIRTypes.cpp

@@ -16,15 +16,19 @@
 #include "clang/CIR/Dialect/IR/CIRDialect.h"
 #include "llvm/ADT/TypeSwitch.h"
 
+#include <cassert>
+
 //===----------------------------------------------------------------------===//
 // CIR Custom Parser/Printer Signatures
 //===----------------------------------------------------------------------===//
 
-static mlir::ParseResult
-parseFuncTypeArgs(mlir::AsmParser &p, llvm::SmallVector<mlir::Type> &params,
-                  bool &isVarArg);
-static void printFuncTypeArgs(mlir::AsmPrinter &p,
-                              mlir::ArrayRef<mlir::Type> params, bool isVarArg);
+static mlir::ParseResult parseFuncType(mlir::AsmParser &p,
+                                       mlir::Type &optionalReturnTypes,
+                                       llvm::SmallVector<mlir::Type> &params,
+                                       bool &isVarArg);
+
+static void printFuncType(mlir::AsmPrinter &p, mlir::Type optionalReturnTypes,
+                          mlir::ArrayRef<mlir::Type> params, bool isVarArg);
 
 //===----------------------------------------------------------------------===//
 // Get autogenerated stuff
@@ -331,9 +335,38 @@ FuncType FuncType::clone(TypeRange inputs, TypeRange results) const {
   return get(llvm::to_vector(inputs), results[0], isVarArg());
 }
 
-mlir::ParseResult parseFuncTypeArgs(mlir::AsmParser &p,
-                                    llvm::SmallVector<mlir::Type> &params,
-                                    bool &isVarArg) {
+// A special parser is needed for function returning void to handle the missing
+// type.
+static mlir::ParseResult parseFuncTypeReturn(mlir::AsmParser &p,
+                                             mlir::Type &optionalReturnType) {
+  if (succeeded(p.parseOptionalLParen())) {
+    // If we have already a '(', the function has no return type
+    optionalReturnType = {};
+    return mlir::success();
+  }
+  mlir::Type type;
+  if (p.parseType(type))
+    return mlir::failure();
+  if (isa<cir::VoidType>(type))
+    // An explicit !cir.void means also no return type.
+    optionalReturnType = {};
+  else
+    // Otherwise use the actual type.
+    optionalReturnType = type;
+  return p.parseLParen();
+}
+
+// A special pretty-printer for function returning or not a result.
+static void printFuncTypeReturn(mlir::AsmPrinter &p,
+                                mlir::Type optionalReturnType) {
+  if (optionalReturnType)
+    p << optionalReturnType << ' ';
+  p << '(';
+}
+
+static mlir::ParseResult
+parseFuncTypeArgs(mlir::AsmParser &p, llvm::SmallVector<mlir::Type> &params,
+                  bool &isVarArg) {
   isVarArg = false;
   // `(` `)`
   if (succeeded(p.parseOptionalRParen()))
@@ -363,8 +396,9 @@ mlir::ParseResult parseFuncTypeArgs(mlir::AsmParser &p,
   return p.parseRParen();
 }
 
-void printFuncTypeArgs(mlir::AsmPrinter &p, mlir::ArrayRef<mlir::Type> params,
-                       bool isVarArg) {
+static void printFuncTypeArgs(mlir::AsmPrinter &p,
+                              mlir::ArrayRef<mlir::Type> params,
+                              bool isVarArg) {
   llvm::interleaveComma(params, p,
                         [&p](mlir::Type type) { p.printType(type); });
   if (isVarArg) {
@@ -375,11 +409,48 @@ void printFuncTypeArgs(mlir::AsmPrinter &p, mlir::ArrayRef<mlir::Type> params,
   p << ')';
 }
 
+// Use a custom parser to handle the optional return and argument types without
+// an optional anchor.
+static mlir::ParseResult parseFuncType(mlir::AsmParser &p,
+                                       mlir::Type &optionalReturnTypes,
+                                       llvm::SmallVector<mlir::Type> &params,
+                                       bool &isVarArg) {
+  if (failed(parseFuncTypeReturn(p, optionalReturnTypes)))
+    return failure();
+  return parseFuncTypeArgs(p, params, isVarArg);
+}
+
+static void printFuncType(mlir::AsmPrinter &p, mlir::Type optionalReturnTypes,
+                          mlir::ArrayRef<mlir::Type> params, bool isVarArg) {
+  printFuncTypeReturn(p, optionalReturnTypes);
+  printFuncTypeArgs(p, params, isVarArg);
+}
+
+// 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())->optionalReturnType;
+}
+
+/// Returns the result type of the function as an ArrayRef, enabling better
+/// integration with generic MLIR utilities.
 llvm::ArrayRef<mlir::Type> FuncType::getReturnTypes() const {
-  return static_cast<detail::FuncTypeStorage *>(getImpl())->returnType;
+  if (isVoid())
+    return {};
+  return static_cast<detail::FuncTypeStorage *>(getImpl())->optionalReturnType;
 }
 
-bool FuncType::isVoid() const { return mlir::isa<VoidType>(getReturnType()); }
+// Whether the function returns void
+bool FuncType::isVoid() const {
+  auto rt =
+      static_cast<detail::FuncTypeStorage *>(getImpl())->optionalReturnType;
+  assert((!rt || !mlir::isa<cir::VoidType>(rt)) &&
+         "The return type for a function returning void should be empty "
+         "instead of a real !cir.void");
+  return !rt;
+}
 
 //===----------------------------------------------------------------------===//
 // PointerType Definitions

clang/test/CIR/func-simple.cpp

@@ -2,12 +2,12 @@
 // RUN: %clang_cc1 -std=c++20 -triple x86_64-unknown-linux-gnu -fclangir -emit-cir %s -o -  | FileCheck %s
 
 void empty() { }
-// CHECK: cir.func @empty() -> !cir.void {
+// CHECK: cir.func @empty() {
 // CHECK:   cir.return
 // CHECK: }
 
 void voidret() { return; }
-// CHECK: cir.func @voidret() -> !cir.void {
+// CHECK: cir.func @voidret() {
 // CHECK:   cir.return
 // CHECK: }
 

clang/test/CIR/global-var-simple.cpp

@@ -89,10 +89,10 @@ char **cpp;
 // CHECK: cir.global @cpp : !cir.ptr<!cir.ptr<!cir.int<s, 8>>>
 
 void (*fp)();
-// CHECK: cir.global @fp : !cir.ptr<!cir.func<!cir.void ()>>
+// CHECK: cir.global @fp : !cir.ptr<!cir.func<()>>
 
 int (*fpii)(int) = 0;
 // CHECK: cir.global @fpii = #cir.ptr<null> : !cir.ptr<!cir.func<!cir.int<s, 32> (!cir.int<s, 32>)>>
 
 void (*fpvar)(int, ...);
-// CHECK: cir.global @fpvar : !cir.ptr<!cir.func<!cir.void (!cir.int<s, 32>, ...)>>
+// CHECK: cir.global @fpvar : !cir.ptr<!cir.func<(!cir.int<s, 32>, ...)>>