[CIR] Add nothrow attribute to the call operation #145178
Open
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
Lancern
requested review from
andykaylor
and removed request for
bcardosolopes and
lanza
llvmbot
added
clang
|
@llvm/pr-subscribers-clangir @llvm/pr-subscribers-clang Author: Sirui Mu (Lancern) ChangesThis patch adds extra function attributes to the Full diff: https://github.com/llvm/llvm-project/pull/145178.diff 8 Files Affected:
diff --git a/clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h b/clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h
index 3e052c564112e..520ac32dccebc 100644
--- a/clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h
+++ b/clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h
@@ -228,25 +228,41 @@ class CIRBaseBuilderTy : public mlir::OpBuilder {
cir::CallOp createCallOp(mlir::Location loc, mlir::SymbolRefAttr callee,
mlir::Type returnType, mlir::ValueRange operands,
- cir::SideEffect sideEffect = cir::SideEffect::All) {
- return create<cir::CallOp>(loc, callee, returnType, operands, sideEffect);
+ cir::SideEffect sideEffect = cir::SideEffect::All,
+ cir::ExtraFuncAttributesAttr extraFuncAttr = {}) {
+ auto op =
+ create<cir::CallOp>(loc, callee, returnType, operands, sideEffect);
+
+ if (extraFuncAttr) {
+ op->setAttr("extra_attrs", extraFuncAttr);
+ } else {
+ mlir::NamedAttrList empty;
+ op->setAttr("extra_attrs", cir::ExtraFuncAttributesAttr::get(
+ empty.getDictionary(getContext())));
+ }
+
+ return op;
}
cir::CallOp createCallOp(mlir::Location loc, cir::FuncOp callee,
mlir::ValueRange operands,
- cir::SideEffect sideEffect = cir::SideEffect::All) {
+ cir::SideEffect sideEffect = cir::SideEffect::All,
+ cir::ExtraFuncAttributesAttr extraFuncAttr = {}) {
return createCallOp(loc, mlir::SymbolRefAttr::get(callee),
callee.getFunctionType().getReturnType(), operands,
- sideEffect);
+ sideEffect, extraFuncAttr);
}
- cir::CallOp createIndirectCallOp(mlir::Location loc,
- mlir::Value indirectTarget,
- cir::FuncType funcType,
- mlir::ValueRange operands,
- cir::SideEffect sideEffect) {
- return create<cir::CallOp>(loc, indirectTarget, funcType.getReturnType(),
- operands, sideEffect);
+ cir::CallOp
+ createIndirectCallOp(mlir::Location loc, mlir::Value indirectTarget,
+ cir::FuncType funcType, mlir::ValueRange operands,
+ cir::SideEffect sideEffect,
+ cir::ExtraFuncAttributesAttr extraFuncAttr = {}) {
+ llvm::SmallVector<mlir::Value> resOperands{indirectTarget};
+ resOperands.append(operands.begin(), operands.end());
+
+ return createCallOp(loc, mlir::SymbolRefAttr(), funcType.getReturnType(),
+ resOperands, sideEffect, extraFuncAttr);
}
//===--------------------------------------------------------------------===//
diff --git a/clang/include/clang/CIR/Dialect/IR/CIRAttrs.td b/clang/include/clang/CIR/Dialect/IR/CIRAttrs.td
index 9e01dde379d7a..5a03bd0c135cd 100644
--- a/clang/include/clang/CIR/Dialect/IR/CIRAttrs.td
+++ b/clang/include/clang/CIR/Dialect/IR/CIRAttrs.td
@@ -368,4 +368,34 @@ def CIR_VisibilityAttr : CIR_Attr<"Visibility", "visibility"> {
}];
}
+//===----------------------------------------------------------------------===//
+// ExtraFuncAttributesAttr
+//===----------------------------------------------------------------------===//
+
+def ExtraFuncAttributesAttr : CIR_Attr<"ExtraFuncAttributes", "extra"> {
+ let summary = "Represents aggregated attributes for a function";
+ let description = [{
+ This is a wrapper of attribute dictionary that contains extra attributes of
+ a function.
+ }];
+
+ let parameters = (ins "mlir::DictionaryAttr":$elements);
+
+ let builders = [
+ AttrBuilderWithInferredContext<(ins "mlir::DictionaryAttr":$elements), [{
+ return $_get(elements.getContext(), elements);
+ }]>
+ ];
+
+ let assemblyFormat = [{ `(` $elements `)` }];
+}
+
+//===----------------------------------------------------------------------===//
+// Unit Function Attributes
+//===----------------------------------------------------------------------===//
+
+def NoThrowAttr : CIRUnitAttr<"NoThrow", "nothrow"> {
+ let storageType = "NoThrowAttr";
+}
+
#endif // LLVM_CLANG_CIR_DIALECT_IR_CIRATTRS_TD
diff --git a/clang/include/clang/CIR/Dialect/IR/CIROps.td b/clang/include/clang/CIR/Dialect/IR/CIROps.td
index 852d3aa131148..dfa0079758dc9 100644
--- a/clang/include/clang/CIR/Dialect/IR/CIROps.td
+++ b/clang/include/clang/CIR/Dialect/IR/CIROps.td
@@ -1942,7 +1942,8 @@ class CIR_CallOpBase<string mnemonic, list<Trait> extra_traits = []>
dag commonArgs = (ins OptionalAttr<FlatSymbolRefAttr>:$callee,
Variadic<CIR_AnyType>:$args,
- DefaultValuedAttr<CIR_SideEffect, "SideEffect::All">:$side_effect);
+ DefaultValuedAttr<CIR_SideEffect, "SideEffect::All">:$side_effect,
+ ExtraFuncAttributesAttr:$extra_attrs);
}
def CallOp : CIR_CallOpBase<"call", [NoRegionArguments]> {
@@ -1971,29 +1972,17 @@ def CallOp : CIR_CallOpBase<"call", [NoRegionArguments]> {
let arguments = commonArgs;
let builders = [
- // Build a call op for a direct call
OpBuilder<(ins "mlir::SymbolRefAttr":$callee, "mlir::Type":$resType,
"mlir::ValueRange":$operands,
CArg<"SideEffect", "SideEffect::All">:$sideEffect), [{
- assert(callee && "callee attribute is required for direct call");
$_state.addOperands(operands);
- $_state.addAttribute("callee", callee);
+ if (callee)
+ $_state.addAttribute("callee", callee);
$_state.addAttribute("side_effect",
SideEffectAttr::get($_builder.getContext(), sideEffect));
if (resType && !isa<VoidType>(resType))
$_state.addTypes(resType);
- }]>,
- // Build a call op for an indirect call
- OpBuilder<(ins "mlir::Value":$calleePtr, "mlir::Type":$resType,
- "mlir::ValueRange":$operands,
- CArg<"SideEffect", "SideEffect::All">:$sideEffect), [{
- $_state.addOperands(calleePtr);
- $_state.addOperands(operands);
- if (resType && !isa<VoidType>(resType))
- $_state.addTypes(resType);
- $_state.addAttribute("side_effect",
- SideEffectAttr::get($_builder.getContext(), sideEffect));
- }]>,
+ }]>
];
}
diff --git a/clang/lib/CIR/CodeGen/CIRGenCall.cpp b/clang/lib/CIR/CodeGen/CIRGenCall.cpp
index 9c9c96604c168..17aea1a29aeca 100644
--- a/clang/lib/CIR/CodeGen/CIRGenCall.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenCall.cpp
@@ -77,17 +77,43 @@ void CIRGenFunction::emitAggregateStore(mlir::Value value, Address dest) {
builder.createStore(*currSrcLoc, value, dest);
}
+static void addAttributesFromFunctionProtoType(CIRGenBuilderTy &builder,
+ mlir::NamedAttrList &attrs,
+ const FunctionProtoType *fpt) {
+ if (!fpt)
+ return;
+
+ if (!isUnresolvedExceptionSpec(fpt->getExceptionSpecType()) &&
+ fpt->isNothrow()) {
+ auto nothrowAttr = cir::NoThrowAttr::get(builder.getContext());
+ attrs.set(nothrowAttr.getMnemonic(), nothrowAttr);
+ }
+}
+
/// Construct the CIR attribute list of a function or call.
void CIRGenModule::constructAttributeList(CIRGenCalleeInfo calleeInfo,
+ mlir::NamedAttrList &attrs,
cir::SideEffect &sideEffect) {
assert(!cir::MissingFeatures::opCallCallConv());
sideEffect = cir::SideEffect::All;
- assert(!cir::MissingFeatures::opCallAttrs());
+ addAttributesFromFunctionProtoType(getBuilder(), attrs,
+ calleeInfo.getCalleeFunctionProtoType());
const Decl *targetDecl = calleeInfo.getCalleeDecl().getDecl();
if (targetDecl) {
+ if (targetDecl->hasAttr<NoThrowAttr>()) {
+ auto nothrowAttr = cir::NoThrowAttr::get(&getMLIRContext());
+ attrs.set(nothrowAttr.getMnemonic(), nothrowAttr);
+ }
+
+ if (const FunctionDecl *func = dyn_cast<FunctionDecl>(targetDecl)) {
+ addAttributesFromFunctionProtoType(
+ getBuilder(), attrs, func->getType()->getAs<FunctionProtoType>());
+ assert(!cir::MissingFeatures::opCallAttrs());
+ }
+
assert(!cir::MissingFeatures::opCallAttrs());
// 'const', 'pure' and 'noalias' attributed functions are also nounwind.
@@ -411,12 +437,11 @@ CIRGenTypes::arrangeFunctionDeclaration(const FunctionDecl *fd) {
return arrangeFreeFunctionType(funcTy.castAs<FunctionProtoType>());
}
-static cir::CIRCallOpInterface
-emitCallLikeOp(CIRGenFunction &cgf, mlir::Location callLoc,
- cir::FuncType indirectFuncTy, mlir::Value indirectFuncVal,
- cir::FuncOp directFuncOp,
- const SmallVectorImpl<mlir::Value> &cirCallArgs,
- cir::SideEffect sideEffect) {
+static cir::CIRCallOpInterface emitCallLikeOp(
+ CIRGenFunction &cgf, mlir::Location callLoc, cir::FuncType indirectFuncTy,
+ mlir::Value indirectFuncVal, cir::FuncOp directFuncOp,
+ const SmallVectorImpl<mlir::Value> &cirCallArgs, cir::SideEffect sideEffect,
+ cir::ExtraFuncAttributesAttr extraFuncAttrs) {
CIRGenBuilderTy &builder = cgf.getBuilder();
assert(!cir::MissingFeatures::opCallSurroundingTry());
@@ -427,11 +452,13 @@ emitCallLikeOp(CIRGenFunction &cgf, mlir::Location callLoc,
if (indirectFuncTy) {
// TODO(cir): Set calling convention for indirect calls.
assert(!cir::MissingFeatures::opCallCallConv());
- return builder.createIndirectCallOp(
- callLoc, indirectFuncVal, indirectFuncTy, cirCallArgs, sideEffect);
+ return builder.createIndirectCallOp(callLoc, indirectFuncVal,
+ indirectFuncTy, cirCallArgs, sideEffect,
+ extraFuncAttrs);
}
- return builder.createCallOp(callLoc, directFuncOp, cirCallArgs, sideEffect);
+ return builder.createCallOp(callLoc, directFuncOp, cirCallArgs, sideEffect,
+ extraFuncAttrs);
}
const CIRGenFunctionInfo &
@@ -545,7 +572,7 @@ RValue CIRGenFunction::emitCall(const CIRGenFunctionInfo &funcInfo,
assert(!cir::MissingFeatures::opCallCallConv());
assert(!cir::MissingFeatures::opCallAttrs());
cir::SideEffect sideEffect;
- cgm.constructAttributeList(callee.getAbstractInfo(), sideEffect);
+ cgm.constructAttributeList(callee.getAbstractInfo(), attrs, sideEffect);
assert(!cir::MissingFeatures::invokeOp());
@@ -566,12 +593,13 @@ RValue CIRGenFunction::emitCall(const CIRGenFunctionInfo &funcInfo,
indirectFuncVal = calleePtr->getResult(0);
}
- assert(!cir::MissingFeatures::opCallAttrs());
+ auto extraFuncAttrs =
+ cir::ExtraFuncAttributesAttr::get(attrs.getDictionary(&getMLIRContext()));
mlir::Location callLoc = loc;
cir::CIRCallOpInterface theCall =
emitCallLikeOp(*this, loc, indirectFuncTy, indirectFuncVal, directFuncOp,
- cirCallArgs, sideEffect);
+ cirCallArgs, sideEffect, extraFuncAttrs);
if (callOp)
*callOp = theCall;
diff --git a/clang/lib/CIR/CodeGen/CIRGenCall.h b/clang/lib/CIR/CodeGen/CIRGenCall.h
index 56c76c51a46d8..bd113293fdafd 100644
--- a/clang/lib/CIR/CodeGen/CIRGenCall.h
+++ b/clang/lib/CIR/CodeGen/CIRGenCall.h
@@ -33,7 +33,8 @@ class CIRGenCalleeInfo {
CIRGenCalleeInfo(const clang::FunctionProtoType *calleeProtoTy,
clang::GlobalDecl calleeDecl)
: calleeProtoTy(calleeProtoTy), calleeDecl(calleeDecl) {}
- CIRGenCalleeInfo(clang::GlobalDecl calleeDecl) : calleeDecl(calleeDecl) {}
+ CIRGenCalleeInfo(clang::GlobalDecl calleeDecl)
+ : calleeProtoTy(nullptr), calleeDecl(calleeDecl) {}
const clang::FunctionProtoType *getCalleeFunctionProtoType() const {
return calleeProtoTy;
diff --git a/clang/lib/CIR/CodeGen/CIRGenModule.h b/clang/lib/CIR/CodeGen/CIRGenModule.h
index 71806e3c5de21..c435c14c08f57 100644
--- a/clang/lib/CIR/CodeGen/CIRGenModule.h
+++ b/clang/lib/CIR/CodeGen/CIRGenModule.h
@@ -166,6 +166,7 @@ class CIRGenModule : public CIRGenTypeCache {
/// constructed for. If valid, the attributes applied to this decl may
/// contribute to the function attributes and calling convention.
void constructAttributeList(CIRGenCalleeInfo calleeInfo,
+ mlir::NamedAttrList &attrs,
cir::SideEffect &sideEffect);
/// Return a constant array for the given string.
diff --git a/clang/lib/CIR/Dialect/IR/CIRDialect.cpp b/clang/lib/CIR/Dialect/IR/CIRDialect.cpp
index 16248059c4975..dadae709b3b7f 100644
--- a/clang/lib/CIR/Dialect/IR/CIRDialect.cpp
+++ b/clang/lib/CIR/Dialect/IR/CIRDialect.cpp
@@ -65,6 +65,11 @@ struct CIROpAsmDialectInterface : public OpAsmDialectInterface {
os << (boolAttr.getValue() ? "true" : "false");
return AliasResult::FinalAlias;
}
+ if (auto extraFuncAttr =
+ mlir::dyn_cast<cir::ExtraFuncAttributesAttr>(attr)) {
+ os << "fn_attr";
+ return AliasResult::FinalAlias;
+ }
return AliasResult::NoAlias;
}
};
@@ -549,7 +554,8 @@ unsigned cir::CallOp::getNumArgOperands() {
}
static mlir::ParseResult parseCallCommon(mlir::OpAsmParser &parser,
- mlir::OperationState &result) {
+ mlir::OperationState &result,
+ llvm::StringRef extraAttrsAttrName) {
llvm::SmallVector<mlir::OpAsmParser::UnresolvedOperand, 4> ops;
llvm::SMLoc opsLoc;
mlir::FlatSymbolRefAttr calleeAttr;
@@ -586,6 +592,21 @@ static mlir::ParseResult parseCallCommon(mlir::OpAsmParser &parser,
result.addAttribute("side_effect", attr);
}
+ Attribute extraAttrs;
+ if (mlir::succeeded(parser.parseOptionalKeyword("extra"))) {
+ if (parser.parseLParen().failed())
+ return failure();
+ if (parser.parseAttribute(extraAttrs).failed())
+ return failure();
+ if (parser.parseRParen().failed())
+ return failure();
+ } else {
+ NamedAttrList empty;
+ extraAttrs = cir::ExtraFuncAttributesAttr::get(
+ empty.getDictionary(parser.getContext()));
+ }
+ result.addAttribute(extraAttrsAttrName, extraAttrs);
+
if (parser.parseOptionalAttrDict(result.attributes))
return ::mlir::failure();
@@ -609,6 +630,7 @@ static void printCallCommon(mlir::Operation *op,
mlir::FlatSymbolRefAttr calleeSym,
mlir::Value indirectCallee,
mlir::OpAsmPrinter &printer,
+ cir::ExtraFuncAttributesAttr extraAttrs,
cir::SideEffect sideEffect) {
printer << ' ';
@@ -631,7 +653,14 @@ static void printCallCommon(mlir::Operation *op,
printer << ")";
}
- printer.printOptionalAttrDict(op->getAttrs(), {"callee", "side_effect"});
+ if (!extraAttrs.getElements().empty()) {
+ printer << " extra(";
+ printer.printAttributeWithoutType(extraAttrs);
+ printer << ")";
+ }
+
+ printer.printOptionalAttrDict(op->getAttrs(),
+ {"callee", "extra_attrs", "side_effect"});
printer << " : ";
printer.printFunctionalType(op->getOperands().getTypes(),
@@ -640,13 +669,14 @@ static void printCallCommon(mlir::Operation *op,
mlir::ParseResult cir::CallOp::parse(mlir::OpAsmParser &parser,
mlir::OperationState &result) {
- return parseCallCommon(parser, result);
+ return parseCallCommon(parser, result, getExtraAttrsAttrName(result.name));
}
void cir::CallOp::print(mlir::OpAsmPrinter &p) {
mlir::Value indirectCallee = isIndirect() ? getIndirectCall() : nullptr;
cir::SideEffect sideEffect = getSideEffect();
- printCallCommon(*this, getCalleeAttr(), indirectCallee, p, sideEffect);
+ printCallCommon(*this, getCalleeAttr(), indirectCallee, p, getExtraAttrs(),
+ sideEffect);
}
static LogicalResult
diff --git a/clang/test/CIR/CodeGen/call.cpp b/clang/test/CIR/CodeGen/call.cpp
index cc25afce1e5a4..0941655df2aab 100644
--- a/clang/test/CIR/CodeGen/call.cpp
+++ b/clang/test/CIR/CodeGen/call.cpp
@@ -3,6 +3,8 @@
// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -Wno-unused-value -fclangir -emit-llvm %s -o %t-cir.ll
// RUN: FileCheck --input-file=%t-cir.ll %s -check-prefix=LLVM
+// CIR: #[[FN_ATTR:.+]] = #cir<extra({nothrow = #cir.nothrow})>
+
void f1() {}
void f2() {
f1();
@@ -102,3 +104,16 @@ void f12() {
// LLVM: %[[#slot:]] = alloca %struct.S, i64 1, align 4
// LLVM-NEXT: %[[#ret:]] = call %struct.S @_Z3f10v()
// LLVM-NEXT: store %struct.S %[[#ret]], ptr %[[#slot]], align 4
+
+void f13() noexcept;
+void f14() {
+ f13();
+}
+
+// CIR-LABEL: cir.func @_Z3f14v()
+// CIR: cir.call @_Z3f13v() extra(#[[FN_ATTR]]) : () -> ()
+// CIR: }
+
+// LLVM-LABEL: define void @_Z3f14v()
+// LLVM: call void @_Z3f13v()
+// LLVM: }
|
| // ExtraFuncAttributesAttr | ||
| //===----------------------------------------------------------------------===// | ||
|
|
||
| def ExtraFuncAttributesAttr : CIR_Attr<"ExtraFuncAttributes", "extra"> { |
There was a problem hiding this comment.
@xlauko and I start discussing about whether we really need ExtraFuncAttributesAttr. Maybe this is a good point in time to decide on it, and perhaps we don't need to introduce this upstream. The original idea was to use this to share attributes that later in LLVM could be placed in both functions and calls, but currently there isn't anything special about it (IIRC) and LLVMDialect uses a more natural approach to represent these, seems like we could do similar?
There was a problem hiding this comment.
+1 for not upstreaming this if it isn't needed.
There was a problem hiding this comment.
LGTM to not upstream this if it's unnecessary, but then should we still upstream the #cir.nothrow attribute as it is now? For now #cir.nothrow is defined as a CIRUnitAttr, but maybe we could just use a simple UnitAttr for it instead?
There was a problem hiding this comment.
Good catch, I think CIRUnitAttr is in a similar boat!
There was a problem hiding this comment.
Yeah, I am trying to get it work without custom unit attributes and ExtraFuncAttributesAttr but it will take me a bit more time. I first need to finish assembly format for FuncOp, so its easier to pull out these extra attrs. But I am almost there.
There was a problem hiding this comment.
Removed this attribute from the patch, and changed NothrowAttr to a UnitAttr.
andykaylor
reviewed
Jun 23, 2025
clang/test/CIR/CodeGen/call.cpp
Outdated
| // CIR: } | ||
|
|
||
| // LLVM-LABEL: define void @_Z3f14v() | ||
| // LLVM: call void @_Z3f13v() |
There was a problem hiding this comment.
Is this capturing the complete LLVM IR. It appears that the incubator doesn't set a callsite attribute here, but classic codegen does. That would seem to indicate that we missed something in lowering.
| // ExtraFuncAttributesAttr | ||
| //===----------------------------------------------------------------------===// | ||
|
|
||
| def ExtraFuncAttributesAttr : CIR_Attr<"ExtraFuncAttributes", "extra"> { |
There was a problem hiding this comment.
+1 for not upstreaming this if it isn't needed.
This patch adds nothrow attribute to call operations to indicate the callee will not throw. This patch also contains a minor refactor. The construction of call operations is now split into two steps. The CallOp::build function and the create function in CIR builder now only accepts basic parameters including the callee and the arguments. Other extra arguments, including any additional attributes, are set later on the constructed op in a separate step.
Lancern
force-pushed
the
cir/call-extra-attrs
branch
from
ca4ea5e to
183dad8
Compare
Lancern
changed the title
|
I conducted a minor refactor in the latest commit. Prior to the latest commit, CallOp build functions (namely, |
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.
This patch adds extra function attributes to the
cir.calloperation. The extra attributes now may contain a singlecir.nothrowattribute that indicates whether the callee throws.