[CIR] Upstream simple function bodies #127674
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Enable ClangIR generation for very simple functions. The functions have to return `void` or an integral type, contain only compound statements or `return` statements, and `return` statement expressions can only be integral literals of the correct type. The functions can have parameters, but those are currently ignored because there is no way to access them. This change intentionally focuses on breadth (introducing scopes, statements, and expressions) rather than depth, because it enables people to work on upstreaming in parallel without interference. The new ClangIR ops in this change are `ReturnOp`, `YieldOp`, `ScopeOp`, and `TrapOp`. These operations are complete (except for the `ParentOneOf` property) and shouldn't require further upstreaming changes. Significant additions were made to `FuncOp`, adding a type and a region, but that operation is still far from complete. The classes `ScalarExprEmitter` and `CIRGenFunction`, along with the `emit*` functions in `CIRGenFunction` that generate ClangIR for statements, are new in this change. All of these are very incomplete and will be filled out in later upstreaming patches. Existing test `hello.c` is removed and replaced by the new test `func-simple.cpp`. This tests all forms of functions that are currently supported.
llvmbot
added
clang
|
@llvm/pr-subscribers-clangir @llvm/pr-subscribers-clang Author: David Olsen (dkolsen-pgi) ChangesEnable ClangIR generation for very simple functions. The functions have to return This change intentionally focuses on breadth (introducing scopes, statements, and expressions) rather than depth, because it enables people to work on upstreaming in parallel without interference. The new ClangIR ops in this change are The classes Existing test Patch is 53.46 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/127674.diff 15 Files Affected:
diff --git a/clang/include/clang/CIR/Dialect/IR/CIROps.td b/clang/include/clang/CIR/Dialect/IR/CIROps.td
index b15e0415360ea..45d39807b35c8 100644
--- a/clang/include/clang/CIR/Dialect/IR/CIROps.td
+++ b/clang/include/clang/CIR/Dialect/IR/CIROps.td
@@ -11,8 +11,8 @@
///
//===----------------------------------------------------------------------===//
-#ifndef LLVM_CLANG_CIR_DIALECT_IR_CIROPS
-#define LLVM_CLANG_CIR_DIALECT_IR_CIROPS
+#ifndef CLANG_CIR_DIALECT_IR_CIROPS_TD
+#define CLANG_CIR_DIALECT_IR_CIROPS_TD
include "clang/CIR/Dialect/IR/CIRDialect.td"
include "clang/CIR/Dialect/IR/CIRTypes.td"
@@ -115,6 +115,165 @@ def ConstantOp : CIR_Op<"const",
let hasFolder = 1;
}
+//===----------------------------------------------------------------------===//
+// ReturnOp
+//===----------------------------------------------------------------------===//
+
+def ReturnOp : CIR_Op<"return", [ParentOneOf<["FuncOp", "ScopeOp"]>,
+ Terminator]> {
+ let summary = "Return from function";
+ let description = [{
+ The "return" operation represents a return operation within a function.
+ The operation takes an optional operand and produces no results.
+ The operand type must match the signature of the function that contains
+ the operation.
+
+ ```mlir
+ func @foo() -> i32 {
+ ...
+ cir.return %0 : i32
+ }
+ ```
+ }];
+
+ // The return operation takes an optional input operand to return. This
+ // value must match the return type of the enclosing function.
+ let arguments = (ins Variadic<CIR_AnyType>:$input);
+
+ // The return operation only emits the input in the format if it is present.
+ let assemblyFormat = "($input^ `:` type($input))? attr-dict ";
+
+ // Allow building a ReturnOp with no return operand.
+ let builders = [
+ OpBuilder<(ins), [{ build($_builder, $_state, std::nullopt); }]>
+ ];
+
+ // Provide extra utility definitions on the c++ operation class definition.
+ let extraClassDeclaration = [{
+ bool hasOperand() { return getNumOperands() != 0; }
+ }];
+
+ let hasVerifier = 1;
+}
+
+//===----------------------------------------------------------------------===//
+// YieldOp
+//===----------------------------------------------------------------------===//
+
+def YieldOp : CIR_Op<"yield", [ReturnLike, Terminator,
+ ParentOneOf<["ScopeOp"]>]> {
+ let summary = "Represents the default branching behaviour of a region";
+ let description = [{
+ The `cir.yield` operation terminates regions on different CIR operations,
+ and it is used to represent the default branching behaviour of a region.
+ Said branching behaviour is determinted by the parent operation. For
+ example, a yield in a `switch-case` region implies a fallthrough, while
+ a yield in a `cir.if` region implies a branch to the exit block, and so
+ on.
+
+ In some cases, it might yield an SSA value and the semantics of how the
+ values are yielded is defined by the parent operation. For example, a
+ `cir.ternary` operation yields a value from one of its regions.
+
+ As a general rule, `cir.yield` must be explicitly used whenever a region has
+ more than one block and no terminator, or within `cir.switch` regions not
+ `cir.return` terminated.
+
+ Examples:
+ ```mlir
+ cir.if %4 {
+ ...
+ cir.yield
+ }
+
+ cir.switch (%5) [
+ case (equal, 3) {
+ ...
+ cir.yield
+ }, ...
+ ]
+
+ cir.scope {
+ ...
+ cir.yield
+ }
+
+ %x = cir.scope {
+ ...
+ cir.yield %val
+ }
+
+ %y = cir.ternary {
+ ...
+ cir.yield %val : i32
+ } : i32
+ ```
+ }];
+
+ let arguments = (ins Variadic<CIR_AnyType>:$args);
+ let assemblyFormat = "($args^ `:` type($args))? attr-dict";
+ let builders = [
+ OpBuilder<(ins), [{ /* nothing to do */ }]>,
+ ];
+}
+
+//===----------------------------------------------------------------------===//
+// ScopeOp
+//===----------------------------------------------------------------------===//
+
+def ScopeOp : CIR_Op<"scope", [
+ DeclareOpInterfaceMethods<RegionBranchOpInterface>,
+ RecursivelySpeculatable, AutomaticAllocationScope, NoRegionArguments]> {
+ let summary = "Represents a C/C++ scope";
+ let description = [{
+ `cir.scope` contains one region and defines a strict "scope" for all new
+ values produced within its blocks.
+
+ The region can contain an arbitrary number of blocks but usually defaults
+ to one and can optionally return a value (useful for representing values
+ coming out of C++ full-expressions) via `cir.yield`:
+
+
+ ```mlir
+ %rvalue = cir.scope {
+ ...
+ cir.yield %value
+ }
+ ```
+
+ The blocks can be terminated by `cir.yield`, `cir.return` or `cir.throw`.
+ If `cir.scope` yields no value, the `cir.yield` can be left out, and
+ will be inserted implicitly.
+ }];
+
+ let results = (outs Optional<CIR_AnyType>:$results);
+ let regions = (region AnyRegion:$scopeRegion);
+
+ let hasVerifier = 1;
+ let skipDefaultBuilders = 1;
+ let assemblyFormat = [{
+ custom<OmittedTerminatorRegion>($scopeRegion) (`:` type($results)^)? attr-dict
+ }];
+
+ let extraClassDeclaration = [{
+ /// Determine whether the scope is empty, meaning it contains a single block
+ /// terminated by a cir.yield.
+ bool isEmpty() {
+ auto &entry = getRegion().front();
+ return getRegion().hasOneBlock() &&
+ llvm::isa<YieldOp>(entry.front());
+ }
+ }];
+
+ let builders = [
+ // Scopes for yielding values.
+ OpBuilder<(ins
+ "llvm::function_ref<void(mlir::OpBuilder &, mlir::Type &, mlir::Location)>":$scopeBuilder)>,
+ // Scopes without yielding values.
+ OpBuilder<(ins "llvm::function_ref<void(mlir::OpBuilder &, mlir::Location)>":$scopeBuilder)>
+ ];
+}
+
//===----------------------------------------------------------------------===//
// GlobalOp
//===----------------------------------------------------------------------===//
@@ -158,25 +317,86 @@ def GlobalOp : CIR_Op<"global"> {
// FuncOp
//===----------------------------------------------------------------------===//
-// TODO(CIR): For starters, cir.func has only name, nothing else. The other
-// properties of a function will be added over time as more of ClangIR is
-// upstreamed.
+// TODO(CIR): FuncOp is still a tiny shell of what it will become. Many more
+// properties and attributes will be added as upstreaming continues.
-def FuncOp : CIR_Op<"func"> {
+def FuncOp : CIR_Op<"func", [
+ AutomaticAllocationScope, CallableOpInterface, FunctionOpInterface,
+ IsolatedFromAbove
+]> {
let summary = "Declare or define a function";
let description = [{
The `cir.func` operation defines a function, similar to the `mlir::FuncOp`
built-in.
}];
- let arguments = (ins SymbolNameAttr:$sym_name);
+ let arguments = (ins SymbolNameAttr:$sym_name,
+ TypeAttrOf<CIR_FuncType>:$function_type,
+ OptionalAttr<DictArrayAttr>:$arg_attrs,
+ OptionalAttr<DictArrayAttr>:$res_attrs);
+
+ let regions = (region AnyRegion:$body);
let skipDefaultBuilders = 1;
- let builders = [OpBuilder<(ins "llvm::StringRef":$sym_name)>];
+ let builders = [OpBuilder<(ins "llvm::StringRef":$sym_name,
+ "FuncType":$type)>];
+
+ let extraClassDeclaration = [{
+ /// Returns the region on the current operation that is callable. This may
+ /// return null in the case of an external callable object, e.g. an external
+ /// function.
+ ::mlir::Region *getCallableRegion();
+
+ /// Returns the results types that the callable region produces when
+ /// executed.
+ llvm::ArrayRef<mlir::Type> getCallableResults() {
+ return getFunctionType().getReturnTypes();
+ }
+
+ /// Returns the argument types of this function.
+ llvm::ArrayRef<mlir::Type> getArgumentTypes() {
+ return getFunctionType().getInputs();
+ }
+
+ /// 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.
+ /// Ensures getType, getNumFuncArguments, and getNumFuncResults can be
+ /// called safely.
+ llvm::LogicalResult verifyType();
+
+ //===------------------------------------------------------------------===//
+ // SymbolOpInterface Methods
+ //===------------------------------------------------------------------===//
+
+ bool isDeclaration();
+ }];
let hasCustomAssemblyFormat = 1;
let hasVerifier = 1;
}
-#endif // LLVM_CLANG_CIR_DIALECT_IR_CIROPS
+//===----------------------------------------------------------------------===//
+// TrapOp
+//===----------------------------------------------------------------------===//
+
+def TrapOp : CIR_Op<"trap", [Terminator]> {
+ let summary = "Exit the program abnormally";
+ let description = [{
+ The cir.trap operation causes the program to exit abnormally. The
+ implementations may implement this operation with different mechanisms. For
+ example, an implementation may implement this operation by calling abort,
+ while another implementation may implement this operation by executing an
+ illegal instruction.
+ }];
+
+ let assemblyFormat = "attr-dict";
+}
+
+#endif // CLANG_CIR_DIALECT_IR_CIROPS_TD
diff --git a/clang/include/clang/CIR/TypeEvaluationKind.h b/clang/include/clang/CIR/TypeEvaluationKind.h
new file mode 100644
index 0000000000000..5d65eeb9d25b9
--- /dev/null
+++ b/clang/include/clang/CIR/TypeEvaluationKind.h
@@ -0,0 +1,21 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef CLANG_CIR_TYPEEVALUATIONKIND_H
+#define CLANG_CIR_TYPEEVALUATIONKIND_H
+
+namespace cir {
+
+// This is copied from clang/lib/CodeGen/CodeGenFunction.h. That file (1) is
+// not available as an include from ClangIR files, and (2) has lots of stuff
+// that we don't want in ClangIR.
+enum TypeEvaluationKind { TEK_Scalar, TEK_Complex, TEK_Aggregate };
+
+} // namespace cir
+
+#endif // CLANG_CIR_TYPEEVALUATIONKIND_H
diff --git a/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp b/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp
new file mode 100644
index 0000000000000..b802705ca8fdc
--- /dev/null
+++ b/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp
@@ -0,0 +1,70 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// Emit Expr nodes with scalar CIR types as CIR code.
+//
+//===----------------------------------------------------------------------===//
+
+#include "CIRGenFunction.h"
+
+#include "clang/AST/Expr.h"
+#include "clang/AST/StmtVisitor.h"
+
+#include "mlir/IR/Value.h"
+
+#include <cassert>
+
+using namespace clang;
+using namespace clang::CIRGen;
+
+namespace {
+
+class ScalarExprEmitter : public StmtVisitor<ScalarExprEmitter, mlir::Value> {
+ CIRGenFunction &cgf;
+ CIRGenBuilderTy &builder;
+ bool ignoreResultAssign;
+
+public:
+ ScalarExprEmitter(CIRGenFunction &cgf, CIRGenBuilderTy &builder,
+ bool ira = false)
+ : cgf(cgf), builder(builder), ignoreResultAssign(ira) {}
+
+ //===--------------------------------------------------------------------===//
+ // Visitor Methods
+ //===--------------------------------------------------------------------===//
+
+ mlir::Value Visit(Expr *e) {
+ return StmtVisitor<ScalarExprEmitter, mlir::Value>::Visit(e);
+ }
+
+ mlir::Value VisitStmt(Stmt *s) {
+ llvm_unreachable("Statement passed to ScalarExprEmitter");
+ }
+
+ mlir::Value VisitExpr(Expr *e) {
+ cgf.getCIRGenModule().errorNYI(
+ e->getSourceRange(), "scalar expression kind: ", e->getStmtClassName());
+ return {};
+ }
+
+ mlir::Value VisitIntegerLiteral(const IntegerLiteral *e) {
+ mlir::Type type = cgf.convertType(e->getType());
+ return builder.create<cir::ConstantOp>(
+ cgf.getLoc(e->getExprLoc()), type,
+ builder.getAttr<cir::IntAttr>(type, e->getValue()));
+ }
+};
+} // namespace
+
+/// Emit the computation of the specified expression of scalar type.
+mlir::Value CIRGenFunction::emitScalarExpr(const Expr *e) {
+ assert(e && hasScalarEvaluationKind(e->getType()) &&
+ "Invalid scalar expression to emit");
+
+ return ScalarExprEmitter(*this, builder).Visit(const_cast<Expr *>(e));
+}
diff --git a/clang/lib/CIR/CodeGen/CIRGenFunction.cpp b/clang/lib/CIR/CodeGen/CIRGenFunction.cpp
new file mode 100644
index 0000000000000..a6d176166773d
--- /dev/null
+++ b/clang/lib/CIR/CodeGen/CIRGenFunction.cpp
@@ -0,0 +1,203 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// Internal per-function state used for AST-to-ClangIR code gen
+//
+//===----------------------------------------------------------------------===//
+
+#include "CIRGenFunction.h"
+
+#include "clang/AST/GlobalDecl.h"
+
+#include <cassert>
+
+namespace clang::CIRGen {
+
+CIRGenFunction::CIRGenFunction(CIRGenModule &cgm, CIRGenBuilderTy &builder,
+ bool suppressNewContext)
+ : CIRGenTypeCache(cgm), cgm{cgm}, builder(builder) {}
+
+CIRGenFunction::~CIRGenFunction() {}
+
+// This is copied from clang/lib/CodeGen/CodeGenFunction.cpp
+cir::TypeEvaluationKind CIRGenFunction::getEvaluationKind(QualType type) {
+ type = type.getCanonicalType();
+ while (true) {
+ switch (type->getTypeClass()) {
+#define TYPE(name, parent)
+#define ABSTRACT_TYPE(name, parent)
+#define NON_CANONICAL_TYPE(name, parent) case Type::name:
+#define DEPENDENT_TYPE(name, parent) case Type::name:
+#define NON_CANONICAL_UNLESS_DEPENDENT_TYPE(name, parent) case Type::name:
+#include "clang/AST/TypeNodes.inc"
+ llvm_unreachable("non-canonical or dependent type in IR-generation");
+
+ case Type::ArrayParameter:
+ case Type::HLSLAttributedResource:
+ llvm_unreachable("NYI");
+
+ case Type::Auto:
+ case Type::DeducedTemplateSpecialization:
+ llvm_unreachable("undeduced type in IR-generation");
+
+ // Various scalar types.
+ case Type::Builtin:
+ case Type::Pointer:
+ case Type::BlockPointer:
+ case Type::LValueReference:
+ case Type::RValueReference:
+ case Type::MemberPointer:
+ case Type::Vector:
+ case Type::ExtVector:
+ case Type::ConstantMatrix:
+ case Type::FunctionProto:
+ case Type::FunctionNoProto:
+ case Type::Enum:
+ case Type::ObjCObjectPointer:
+ case Type::Pipe:
+ case Type::BitInt:
+ return cir::TEK_Scalar;
+
+ // Complexes.
+ case Type::Complex:
+ return cir::TEK_Complex;
+
+ // Arrays, records, and Objective-C objects.
+ case Type::ConstantArray:
+ case Type::IncompleteArray:
+ case Type::VariableArray:
+ case Type::Record:
+ case Type::ObjCObject:
+ case Type::ObjCInterface:
+ return cir::TEK_Aggregate;
+
+ // We operate on atomic values according to their underlying type.
+ case Type::Atomic:
+ type = cast<AtomicType>(type)->getValueType();
+ continue;
+ }
+ llvm_unreachable("unknown type kind!");
+ }
+}
+
+mlir::Type CIRGenFunction::convertTypeForMem(QualType t) {
+ return cgm.getTypes().convertTypeForMem(t);
+}
+
+mlir::Type CIRGenFunction::convertType(QualType t) {
+ return cgm.getTypes().convertType(t);
+}
+
+mlir::Location CIRGenFunction::getLoc(SourceLocation srcLoc) {
+ // Some AST nodes might contain invalid source locations (e.g.
+ // CXXDefaultArgExpr), workaround that to still get something out.
+ if (srcLoc.isValid()) {
+ const SourceManager &sm = getContext().getSourceManager();
+ PresumedLoc pLoc = sm.getPresumedLoc(srcLoc);
+ StringRef filename = pLoc.getFilename();
+ return mlir::FileLineColLoc::get(builder.getStringAttr(filename),
+ pLoc.getLine(), pLoc.getColumn());
+ } else {
+ // Do our best...
+ assert(currSrcLoc && "expected to inherit some source location");
+ return *currSrcLoc;
+ }
+}
+
+mlir::Location CIRGenFunction::getLoc(SourceRange srcLoc) {
+ // Some AST nodes might contain invalid source locations (e.g.
+ // CXXDefaultArgExpr), workaround that to still get something out.
+ if (srcLoc.isValid()) {
+ mlir::Location beg = getLoc(srcLoc.getBegin());
+ mlir::Location end = getLoc(srcLoc.getEnd());
+ SmallVector<mlir::Location, 2> locs = {beg, end};
+ mlir::Attribute metadata;
+ return mlir::FusedLoc::get(locs, metadata, &getMLIRContext());
+ } else if (currSrcLoc) {
+ return *currSrcLoc;
+ }
+
+ // We're brave, but time to give up.
+ return builder.getUnknownLoc();
+}
+
+mlir::Location CIRGenFunction::getLoc(mlir::Location lhs, mlir::Location rhs) {
+ SmallVector<mlir::Location, 2> locs = {lhs, rhs};
+ mlir::Attribute metadata;
+ return mlir::FusedLoc::get(locs, metadata, &getMLIRContext());
+}
+
+void CIRGenFunction::startFunction(GlobalDecl gd, QualType returnType,
+ cir::FuncOp fn, cir::FuncType funcType,
+ SourceLocation loc,
+ SourceLocation startLoc) {
+ assert(!curFn &&
+ "CIRGenFunction can only be used for one function at a time");
+
+ fnRetTy = returnType;
+ curFn = fn;
+
+ mlir::Block *entryBB = &fn.getBlocks().front();
+ builder.setInsertionPointToStart(entryBB);
+}
+
+void CIRGenFunction::finishFunction(SourceLocation endLoc) {}
+
+mlir::LogicalResult CIRGenFunction::emitFunctionBody(const clang::Stmt *body) {
+ auto result = mlir::LogicalResult::success();
+ if (const CompoundStmt *block = dyn_cast<CompoundStmt>(body))
+ emitCompoundStmtWithoutScope(*block);
+ else
+ result = emitStmt(body, /*useCurrentScope=*/true);
+ return result;
+}
+
+cir::FuncOp CIRGenFunction::generateCode(clang::GlobalDecl gd, cir::FuncOp fn,
+ cir::FuncType funcType) {
+ const auto funcDecl = cast<FunctionDecl>(gd.getDecl());
+ SourceLocation loc = funcDecl->getLocation();
+ Stmt *body = funcDecl->getBody();
+ SourceRange bodyRange =
+ body ? body->getSourceRange() : funcDecl->getLocation();
+
+ SourceLocRAIIObject fnLoc{*this, loc.isValid() ? getLoc(loc)
+ : builder.getUnknownLoc()};
+
+ mlir::Block *entryBB = fn.addEntryBlock();
+ builder.setInsertionPointToStart(entryBB);
+
+ startFunction(gd, funcDecl->getReturnType(), fn, funcType, loc,
+ bodyRange.getBegin());
+ if (mlir::failed(emitFunctionBody(body))) {
+ fn.erase();
+ return nullptr;
+ }
+
+ // This code to insert a cir.return or cir.trap at the end of the function is
+ // temporary until the function return code, including
+ // CIRGenFunction::LexicalScope::emitImplicitReturn(), is upstreamed.
+ mlir::Block &lastBlock = fn.getRegion().back();
+ if (lastBlock.empty() || !lastBlock.mightHaveTerminator() ||
+ !lastBlock.getTerminator()->hasTrait<mlir::OpTrait::IsTerminator>()) {
+ builder.setInsertionPointToEnd(&lastBlock);
+ if (mlir::isa<cir::VoidType>(funcType.getReturnType())) {
+ builder.create<cir::ReturnOp>(getLoc(bodyRange.getEnd()));
+ } else {
+ builder.create<cir::TrapOp>(getLoc(bodyRange.getEnd()));
+ }
+ }
+
+ if (mlir::failed(fn.verifyBody()))
+ return nullptr;
+
+ f...
[truncated]
|
andykaylor
reviewed
Feb 18, 2025
| StringRef filename = pLoc.getFilename(); | ||
| return mlir::FileLineColLoc::get(builder.getStringAttr(filename), | ||
| pLoc.getLine(), pLoc.getColumn()); | ||
| } else { |
There was a problem hiding this comment.
No need for an else after return.
| SmallVector<mlir::Location, 2> locs = {beg, end}; | ||
| mlir::Attribute metadata; | ||
| return mlir::FusedLoc::get(locs, metadata, &getMLIRContext()); | ||
| } else if (currSrcLoc) { |
| return builder.getUnknownLoc(); | ||
| } | ||
|
|
||
| mlir::Location CIRGenFunction::getLoc(mlir::Location lhs, mlir::Location rhs) { |
There was a problem hiding this comment.
Do we have a way to test these getLoc functions?
There was a problem hiding this comment.
Not that I know of.
There was a problem hiding this comment.
When printing mlir you can setup mlir::OpPrintingFlags and set enableDebugInfo to print locations.
| SourceLocation loc = funcDecl->getLocation(); | ||
| Stmt *body = funcDecl->getBody(); | ||
| SourceRange bodyRange = | ||
| body ? body->getSourceRange() : funcDecl->getLocation(); |
There was a problem hiding this comment.
It seems like not having a body should be a major condition for this function, not just for the bodyRange. How much of the code below makes sense with a null body?
There was a problem hiding this comment.
Good point. I'll have to look into this a little more, to see what the incubator does here. I think the code only gets here if there is a function body (because this is only used for function definitions, not declarations). So it may be that the check here for a nullptr body should be an assert. But I'll have to confirm that.
There was a problem hiding this comment.
In the incubator there are a bunch of special cases that cover all the situations where a function body might not exist (such as a = default function). I am duplicating all that logic in this PR, even though all the branches other than the normal case are errorNYI. That should make it more clear how a non-existent function body will be handled.
| emitCompoundStmt(cast<CompoundStmt>(*s)); | ||
| break; | ||
| case Stmt::ReturnStmtClass: | ||
| return emitReturnStmt(cast<ReturnStmt>(*s)); |
There was a problem hiding this comment.
Why do some emitXXX() statements return LogicalResult while others are void?
There was a problem hiding this comment.
In the incubator emitCompoundStmt and emitCompoundStmtWithoutScope return Address, while all the other statement emitter functions return LogicalResult. That matches (sort of) the way that Clang CodeGen does it. Type Address hasn't been upstreamed, and the Address return value is only used for the GNU extension of statement-expressions. So I have temporarily changed the return types from Address to void. If we want to change things to be more self-consistent and to deviate from Clang CodeGen, that should happen in the incubator, not as part of upstreaming.
| /// Convert type T into an mlir::Type. This differs from convertType in that | ||
| /// it is used to convert to the memory representation for a type. For | ||
| /// example, the scalar representation for bool is i1, but the memory | ||
| /// representation is usually i8 or i32, depending on the target. |
There was a problem hiding this comment.
When is the memory type used in ClangIR? Given the target-dependence, it seems like we'd at least want this deferred until the lowering phase.
There was a problem hiding this comment.
convertTypeForMem is not currently used in the upstreamed code. Earlier in this change when I was still sorting out exactly what code needed to be upstreamed, I thought it would be needed, so I included it here. It is small enough and self contained enough and it completes the upstreaming of all the convertType functions that I decided to leave it in even though it isn't used yet.
| // 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 { |
There was a problem hiding this comment.
Why aren't the function names mangled?
There was a problem hiding this comment.
The names aren't mangled because the name mangling code hasn't been upstreamed yet. (Name mangling is part of the ABI which is part of CodeGen which means the existing Clang code for that can't be used and ClangIR needs its own copy of the ABI code even though there is nothing ClangIR-specific about it. At least that's how I think it works.)
There was a problem hiding this comment.
Ideally once there will be abi-lib: https://discourse.llvm.org/t/llvm-abi-a-library-for-generating-abi-compliant-llvm-ir/37196/11 to be used across codegens.
xlauko
reviewed
Feb 19, 2025
| : builder.getUnknownLoc()}; | ||
|
|
||
| mlir::Block *entryBB = fn.addEntryBlock(); | ||
| builder.setInsertionPointToStart(entryBB); |
There was a problem hiding this comment.
This sets insertion point and immediately startFunction sets it to the same block probably unnecessarily?
There was a problem hiding this comment.
I think you are right, in the incubator as well, not just here. I am removing this call to setInsertionPointToStart (assuming nothing breaks).
xlauko
reviewed
Feb 19, 2025
| cir::FuncType funcType); | ||
|
|
||
| /// Emit code for the start of a function. | ||
| /// \param Loc The location to be associated with the function. |
There was a problem hiding this comment.
Suggested change
| /// \param Loc The location to be associated with the function. | |
| /// \param loc The location to be associated with the function. |
xlauko
reviewed
Feb 19, 2025
|
|
||
| /// Emit code for the start of a function. | ||
| /// \param Loc The location to be associated with the function. | ||
| /// \param StartLoc The location of the function body. |
There was a problem hiding this comment.
Suggested change
| /// \param StartLoc The location of the function body. | |
| /// \param startLoc The location of the function body. |
xlauko
reviewed
Feb 19, 2025
| @@ -99,19 +100,30 @@ void CIRGenModule::emitGlobal(clang::GlobalDecl gd) { | |||
| void CIRGenModule::emitGlobalFunctionDefinition(clang::GlobalDecl gd, | |||
There was a problem hiding this comment.
Does it make sense to pass gd as clang::GlobalDecl when it is always clang::FunctionDecl. It would eliminate multiple casts using clang::FunctionDecl directly as function codegen parameters.
There was a problem hiding this comment.
The equivalent function in Clang CodeGen has a GlobalDecl parameter, and there are some calls to that function where a FunctionDecl is not readily available. So I'm not anxious to make this change right now.
xlauko
reviewed
Feb 19, 2025
| func = builder.create<cir::FuncOp>(loc, name, funcType); | ||
| theModule.push_back(func); | ||
| } | ||
| return func; |
There was a problem hiding this comment.
This should be same as:
mlir::OpBuilder::InsertionGuard guard(builder);
cir::FuncOp func = builder.create<cir::FuncOp>(loc, name, funcType);
theModule.push_back(func);
return func;
xlauko
approved these changes
Feb 19, 2025
| // | ||
| //===----------------------------------------------------------------------===// | ||
|
|
||
| #ifndef LLVM_CLANG_LIB_CIR_CODEGEN_CIRGENFUNCTION_H |
There was a problem hiding this comment.
Suggested change
| #ifndef LLVM_CLANG_LIB_CIR_CODEGEN_CIRGENFUNCTION_H | |
| #ifndef CLANG_LIB_CIR_CODEGEN_CIRGENFUNCTION_H |
| //===----------------------------------------------------------------------===// | ||
|
|
||
| #ifndef LLVM_CLANG_LIB_CIR_CODEGEN_CIRGENFUNCTION_H | ||
| #define LLVM_CLANG_LIB_CIR_CODEGEN_CIRGENFUNCTION_H |
There was a problem hiding this comment.
Suggested change
| #define LLVM_CLANG_LIB_CIR_CODEGEN_CIRGENFUNCTION_H | |
| #define CLANG_LIB_CIR_CODEGEN_CIRGENFUNCTION_H |
erichkeane
reviewed
Feb 19, 2025
| bool forBitField) { | ||
| assert(!qualType->isConstantMatrixType() && "Matrix types NYI"); | ||
|
|
||
| mlir::Type convertedType = convertType(qualType); |
There was a problem hiding this comment.
We don't have bool implemented yet, right? One of the things that convertTypeForMem does is switch a bool from an i1 to an i8, so I presume we're just missing bool so far?
There was a problem hiding this comment.
CIRGenTypes::convertTypeForMem in the incubator doesn't have any special handling for bool, despite the comment on the declaration specifically pointing to bool as a type that needs special treatment. The implementation here is an exact copy of what is in the incubator. I created llvm/clangir#1371 in the incubator project to sort this out.
andykaylor
approved these changes
Feb 19, 2025
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/51/builds/11171 Here is the relevant piece of the build log for the reference |
bcardosolopes
pushed a commit
to bcardosolopes/llvm-project
that referenced
this pull request
Mar 6, 2025
This reverts commit f8bdbed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Enable ClangIR generation for very simple functions. The functions have to return
voidor an integral type, contain only compound statements orreturnstatements, andreturnstatement expressions can only be integral literals of the correct type. The functions can have parameters, but those are currently ignored because there is no way to access them.This change intentionally focuses on breadth (introducing scopes, statements, and expressions) rather than depth, because it enables people to work on upstreaming in parallel without interference.
The new ClangIR ops in this change are
ReturnOp,YieldOp,ScopeOp, andTrapOp. These operations are complete (except for theParentOneOfproperty) and shouldn't require further upstreaming changes. Significant additions were made toFuncOp, adding a type and a region, but that operation is still far from complete.The classes
ScalarExprEmitterandCIRGenFunction, along with theemit*functions inCIRGenFunctionthat generate ClangIR for statements, are new in this change. All of these are very incomplete and will be filled out in later upstreaming patches.Existing test
hello.cis removed and replaced by the new testfunc-simple.cpp. This tests all forms of functions that are currently supported.