[CIR] Add inline function attributes #162866
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
mmha
requested review from
andykaylor,
bcardosolopes,
lanza and
xlauko
as code owners
llvmbot
added
clang
Member
|
@llvm/pr-subscribers-clangir @llvm/pr-subscribers-clang Author: Morris Hafner (mmha) ChangesUnlike the incubator, this adds the inline attribute directly to FuncOp instead of adding the ExtraFnAttr dict. This adds three new optional keywords to CIR: inline_always, inline_never and inline_hint. Just like in OGCG -O0 implies inline_never on functions withoutt the C++ This patch also adapts all tests that use functions so they account for LLVM attributes being attached now. Patch is 67.05 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/162866.diff 32 Files Affected:
diff --git a/clang/include/clang/CIR/Dialect/IR/CIRAttrs.td b/clang/include/clang/CIR/Dialect/IR/CIRAttrs.td
index bb62223d9e152..7ec8dc6c08376 100644
--- a/clang/include/clang/CIR/Dialect/IR/CIRAttrs.td
+++ b/clang/include/clang/CIR/Dialect/IR/CIRAttrs.td
@@ -959,5 +959,31 @@ def CIR_TypeInfoAttr : CIR_Attr<"TypeInfo", "typeinfo", [TypedAttrInterface]> {
`<` custom<RecordMembers>($data) `>`
}];
}
+//===----------------------------------------------------------------------===//
+// InlineAttr
+//===----------------------------------------------------------------------===//
+
+def CIR_InlineKind : CIR_I32EnumAttr<"InlineKind", "inlineKind", [
+ I32EnumAttrCase<"NoInline", 1, "no">,
+ I32EnumAttrCase<"AlwaysInline", 2, "always">,
+ I32EnumAttrCase<"InlineHint", 3, "hint">
+]> {
+ let genSpecializedAttr = 0;
+}
+
+def CIR_InlineAttr : CIR_EnumAttr<CIR_InlineKind, "inline"> {
+ let summary = "Inline attribute";
+ let description = [{
+ Inline attributes represents user directives.
+ }];
+
+ let cppClassName = "InlineAttr";
+
+ let extraClassDeclaration = [{
+ bool isNoInline() const { return getValue() == InlineKind::NoInline; };
+ bool isAlwaysInline() const { return getValue() == InlineKind::AlwaysInline; };
+ bool isInlineHint() const { return getValue() == InlineKind::InlineHint; };
+ }];
+}
#endif // 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 27fe0cc46d7cf..5d4aa6ac782ec 100644
--- a/clang/include/clang/CIR/Dialect/IR/CIROps.td
+++ b/clang/include/clang/CIR/Dialect/IR/CIROps.td
@@ -2449,6 +2449,10 @@ def CIR_FuncOp : CIR_Op<"func", [
without a prototype and, consequently, may contain calls with invalid
arguments and undefined behavior.
+ The `inline_never` keyword marks a function that should not be inlined.
+ The `inline_always` keyword marks a function that should always be inlined.
+ The `inline_hint` keyword suggests that the function should be inlined.
+
Example:
```mlir
@@ -2483,6 +2487,7 @@ def CIR_FuncOp : CIR_Op<"func", [
UnitAttr:$dso_local,
DefaultValuedAttr<CIR_GlobalLinkageKind,
"cir::GlobalLinkageKind::ExternalLinkage">:$linkage,
+ OptionalAttr<CIR_InlineAttr>:$inline_kind,
OptionalAttr<StrAttr>:$sym_visibility,
UnitAttr:$comdat,
OptionalAttr<DictArrayAttr>:$arg_attrs,
diff --git a/clang/include/clang/CIR/MissingFeatures.h b/clang/include/clang/CIR/MissingFeatures.h
index ace20868532f0..12af322c4ca54 100644
--- a/clang/include/clang/CIR/MissingFeatures.h
+++ b/clang/include/clang/CIR/MissingFeatures.h
@@ -70,23 +70,30 @@ struct MissingFeatures {
static bool opAllocaCaptureByInit() { return false; }
// FuncOp handling
- static bool opFuncOpenCLKernelMetadata() { return false; }
+ static bool opFuncArmNewAttr() { return false; }
+ static bool opFuncArmStreamingAttr() { return false; }
static bool opFuncAstDeclAttr() { return false; }
- static bool opFuncAttributesForDefinition() { return false; }
static bool opFuncCallingConv() { return false; }
+ static bool opFuncColdHotAttr() { return false; }
static bool opFuncCPUAndFeaturesAttributes() { return false; }
static bool opFuncExceptions() { return false; }
static bool opFuncExtraAttrs() { return false; }
static bool opFuncMaybeHandleStaticInExternC() { return false; }
+ static bool opFuncMinSizeAttr() { return false; }
static bool opFuncMultipleReturnVals() { return false; }
+ static bool opFuncNakedAttr() { return false; }
+ static bool opFuncNoDuplicateAttr() { return false; }
static bool opFuncNoUnwind() { return false; }
+ static bool opFuncOpenCLKernelMetadata() { return false; }
static bool opFuncOperandBundles() { return false; }
+ static bool opFuncOptNoneAttr() { return false; }
static bool opFuncParameterAttributes() { return false; }
static bool opFuncReadOnly() { return false; }
static bool opFuncSection() { return false; }
+ static bool opFuncUnwindTablesAttr() { return false; }
static bool opFuncWillReturn() { return false; }
- static bool setLLVMFunctionFEnvAttributes() { return false; }
static bool setFunctionAttributes() { return false; }
+ static bool setLLVMFunctionFEnvAttributes() { return false; }
// CallOp handling
static bool opCallAggregateArgs() { return false; }
@@ -265,6 +272,7 @@ struct MissingFeatures {
static bool objCBlocks() { return false; }
static bool objCGC() { return false; }
static bool objCLifetime() { return false; }
+ static bool hlsl() { return false; }
static bool openCL() { return false; }
static bool openMP() { return false; }
static bool opTBAA() { return false; }
@@ -282,6 +290,7 @@ struct MissingFeatures {
static bool sourceLanguageCases() { return false; }
static bool stackBase() { return false; }
static bool stackSaveOp() { return false; }
+ static bool stackProtector() { return false; }
static bool targetCIRGenInfoArch() { return false; }
static bool targetCIRGenInfoOS() { return false; }
static bool targetCodeGenInfoGetNullPointer() { return false; }
diff --git a/clang/lib/CIR/CodeGen/CIRGenCXX.cpp b/clang/lib/CIR/CodeGen/CIRGenCXX.cpp
index 274d11b8c7629..171ce1c950907 100644
--- a/clang/lib/CIR/CodeGen/CIRGenCXX.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenCXX.cpp
@@ -171,7 +171,8 @@ cir::FuncOp CIRGenModule::codegenCXXStructor(GlobalDecl gd) {
curCGF = nullptr;
setNonAliasAttributes(gd, fn);
- assert(!cir::MissingFeatures::opFuncAttributesForDefinition());
+ setCIRFunctionAttributesForDefinition(mlir::cast<FunctionDecl>(gd.getDecl()),
+ fn);
return fn;
}
diff --git a/clang/lib/CIR/CodeGen/CIRGenModule.cpp b/clang/lib/CIR/CodeGen/CIRGenModule.cpp
index fe1ea5617b8cd..fdaad2f4f9df9 100644
--- a/clang/lib/CIR/CodeGen/CIRGenModule.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenModule.cpp
@@ -449,7 +449,7 @@ void CIRGenModule::emitGlobalFunctionDefinition(clang::GlobalDecl gd,
curCGF = nullptr;
setNonAliasAttributes(gd, funcOp);
- assert(!cir::MissingFeatures::opFuncAttributesForDefinition());
+ setCIRFunctionAttributesForDefinition(funcDecl, funcOp);
if (funcDecl->getAttr<ConstructorAttr>())
errorNYI(funcDecl->getSourceRange(), "constructor attribute");
@@ -1885,6 +1885,87 @@ void CIRGenModule::setFunctionAttributes(GlobalDecl globalDecl,
}
}
+void CIRGenModule::setCIRFunctionAttributesForDefinition(
+ const clang::FunctionDecl *decl, cir::FuncOp f) {
+ assert(!cir::MissingFeatures::opFuncUnwindTablesAttr());
+ assert(!cir::MissingFeatures::stackProtector());
+
+ auto existingInlineKind = f.getInlineKind();
+ bool isNoInline =
+ existingInlineKind && *existingInlineKind == cir::InlineKind::NoInline;
+ bool isAlwaysInline = existingInlineKind &&
+ *existingInlineKind == cir::InlineKind::AlwaysInline;
+
+ if (!decl) {
+ assert(!cir::MissingFeatures::hlsl());
+
+ if (!isAlwaysInline &&
+ codeGenOpts.getInlining() == CodeGenOptions::OnlyAlwaysInlining) {
+ // If we don't have a declaration to control inlining, the function isn't
+ // explicitly marked as alwaysinline for semantic reasons, and inlining is
+ // disabled, mark the function as noinline.
+ f.setInlineKindAttr(
+ cir::InlineAttr::get(&getMLIRContext(), cir::InlineKind::NoInline));
+ }
+
+ return;
+ }
+
+ assert(!cir::MissingFeatures::opFuncArmStreamingAttr());
+ assert(!cir::MissingFeatures::opFuncArmNewAttr());
+ assert(!cir::MissingFeatures::opFuncOptNoneAttr());
+ assert(!cir::MissingFeatures::opFuncMinSizeAttr());
+ assert(!cir::MissingFeatures::opFuncNakedAttr());
+ assert(!cir::MissingFeatures::opFuncNoDuplicateAttr());
+ assert(!cir::MissingFeatures::hlsl());
+
+ // Handle inline attributes
+ if (decl->hasAttr<NoInlineAttr>() && !isAlwaysInline) {
+ // Add noinline if the function isn't always_inline.
+ f.setInlineKindAttr(
+ cir::InlineAttr::get(&getMLIRContext(), cir::InlineKind::NoInline));
+ } else if (decl->hasAttr<AlwaysInlineAttr>() && !isNoInline) {
+ // (noinline wins over always_inline, and we can't specify both in IR)
+ f.setInlineKindAttr(
+ cir::InlineAttr::get(&getMLIRContext(), cir::InlineKind::AlwaysInline));
+ } else if (codeGenOpts.getInlining() == CodeGenOptions::OnlyAlwaysInlining) {
+ // If we're not inlining, then force everything that isn't always_inline
+ // to carry an explicit noinline attribute.
+ if (!isAlwaysInline) {
+ f.setInlineKindAttr(
+ cir::InlineAttr::get(&getMLIRContext(), cir::InlineKind::NoInline));
+ }
+ } else {
+ // Otherwise, propagate the inline hint attribute and potentially use its
+ // absence to mark things as noinline.
+ // Search function and template pattern redeclarations for inline.
+ if (auto *fd = dyn_cast<FunctionDecl>(decl)) {
+ auto checkForInline = [](const FunctionDecl *decl) {
+ auto checkRedeclForInline = [](const FunctionDecl *redecl) {
+ return redecl->isInlineSpecified();
+ };
+ if (any_of(decl->redecls(), checkRedeclForInline))
+ return true;
+ const FunctionDecl *pattern = decl->getTemplateInstantiationPattern();
+ if (!pattern)
+ return false;
+ return any_of(pattern->redecls(), checkRedeclForInline);
+ };
+ if (checkForInline(fd)) {
+ f.setInlineKindAttr(cir::InlineAttr::get(&getMLIRContext(),
+ cir::InlineKind::InlineHint));
+ } else if (codeGenOpts.getInlining() ==
+ CodeGenOptions::OnlyHintInlining &&
+ !fd->isInlined() && !isAlwaysInline) {
+ f.setInlineKindAttr(
+ cir::InlineAttr::get(&getMLIRContext(), cir::InlineKind::NoInline));
+ }
+ }
+ }
+
+ assert(!cir::MissingFeatures::opFuncColdHotAttr());
+}
+
cir::FuncOp CIRGenModule::getOrCreateCIRFunction(
StringRef mangledName, mlir::Type funcType, GlobalDecl gd, bool forVTable,
bool dontDefer, bool isThunk, ForDefinition_t isForDefinition,
diff --git a/clang/lib/CIR/CodeGen/CIRGenModule.h b/clang/lib/CIR/CodeGen/CIRGenModule.h
index f627bae9f87f9..d03c9f37a158e 100644
--- a/clang/lib/CIR/CodeGen/CIRGenModule.h
+++ b/clang/lib/CIR/CodeGen/CIRGenModule.h
@@ -422,6 +422,10 @@ class CIRGenModule : public CIRGenTypeCache {
void setFunctionAttributes(GlobalDecl gd, cir::FuncOp f,
bool isIncompleteFunction, bool isThunk);
+ /// Set extra attributes (inline, etc.) for a function.
+ void setCIRFunctionAttributesForDefinition(const clang::FunctionDecl *fd,
+ cir::FuncOp f);
+
void emitGlobalDefinition(clang::GlobalDecl gd,
mlir::Operation *op = nullptr);
void emitGlobalFunctionDefinition(clang::GlobalDecl gd, mlir::Operation *op);
diff --git a/clang/lib/CIR/Dialect/IR/CIRDialect.cpp b/clang/lib/CIR/Dialect/IR/CIRDialect.cpp
index 5f88590c48d30..06543fc9a1ed4 100644
--- a/clang/lib/CIR/Dialect/IR/CIRDialect.cpp
+++ b/clang/lib/CIR/Dialect/IR/CIRDialect.cpp
@@ -1720,6 +1720,22 @@ ParseResult cir::FuncOp::parse(OpAsmParser &parser, OperationState &state) {
hasAlias = true;
}
+ // Parse optional inline attribute: inline_never, inline_always, or
+ // inline_hint
+ if (parser.parseOptionalKeyword("inline_never").succeeded()) {
+ state.addAttribute(
+ getInlineKindAttrName(state.name),
+ cir::InlineAttr::get(builder.getContext(), cir::InlineKind::NoInline));
+ } else if (parser.parseOptionalKeyword("inline_always").succeeded()) {
+ state.addAttribute(getInlineKindAttrName(state.name),
+ cir::InlineAttr::get(builder.getContext(),
+ cir::InlineKind::AlwaysInline));
+ } else if (parser.parseOptionalKeyword("inline_hint").succeeded()) {
+ state.addAttribute(getInlineKindAttrName(state.name),
+ cir::InlineAttr::get(builder.getContext(),
+ cir::InlineKind::InlineHint));
+ }
+
// Parse the optional function body.
auto *body = state.addRegion();
OptionalParseResult parseResult = parser.parseOptionalRegion(
@@ -1801,6 +1817,16 @@ void cir::FuncOp::print(OpAsmPrinter &p) {
p << ")";
}
+ if (auto inlineKind = getInlineKind()) {
+ if (*inlineKind == cir::InlineKind::NoInline) {
+ p << " inline_never";
+ } else if (*inlineKind == cir::InlineKind::AlwaysInline) {
+ p << " inline_always";
+ } else if (*inlineKind == cir::InlineKind::InlineHint) {
+ p << " inline_hint";
+ }
+ }
+
// Print the body if this is not an external function.
Region &body = getOperation()->getRegion(0);
if (!body.empty()) {
diff --git a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
index a1ecfc7a70909..ec3100d8e336d 100644
--- a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
+++ b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
@@ -1539,6 +1539,7 @@ void CIRToLLVMFuncOpLowering::lowerFuncAttributes(
attr.getName() == getLinkageAttrNameString() ||
attr.getName() == func.getGlobalVisibilityAttrName() ||
attr.getName() == func.getDsoLocalAttrName() ||
+ attr.getName() == func.getInlineKindAttrName() ||
(filterArgAndResAttrs &&
(attr.getName() == func.getArgAttrsAttrName() ||
attr.getName() == func.getResAttrsAttrName())))
@@ -1623,6 +1624,12 @@ mlir::LogicalResult CIRToLLVMFuncOpLowering::matchAndRewrite(
assert(!cir::MissingFeatures::opFuncMultipleReturnVals());
+ // Add inline_kind attribute with "cir." prefix so amendOperation handles it
+ if (auto inlineKind = op.getInlineKind()) {
+ fn->setAttr("cir.inline_kind",
+ cir::InlineAttr::get(getContext(), *inlineKind));
+ }
+
fn.setVisibility_Attr(mlir::LLVM::VisibilityAttr::get(
getContext(), lowerCIRVisibilityToLLVMVisibility(
op.getGlobalVisibilityAttr().getValue())));
diff --git a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVMIR.cpp b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVMIR.cpp
index 30b9eaaca2d37..95698c48deeae 100644
--- a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVMIR.cpp
+++ b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVMIR.cpp
@@ -34,6 +34,18 @@ class CIRDialectLLVMIRTranslationInterface
public:
using LLVMTranslationDialectInterface::LLVMTranslationDialectInterface;
+ /// Any named attribute in the CIR dialect, i.e, with name started with
+ /// "cir.", will be handled here.
+ virtual mlir::LogicalResult amendOperation(
+ mlir::Operation *op, llvm::ArrayRef<llvm::Instruction *> instructions,
+ mlir::NamedAttribute attribute,
+ mlir::LLVM::ModuleTranslation &moduleTranslation) const override {
+ if (auto func = mlir::dyn_cast<mlir::LLVM::LLVMFuncOp>(op)) {
+ amendFunction(func, instructions, attribute, moduleTranslation);
+ }
+ return mlir::success();
+ }
+
/// Translates the given operation to LLVM IR using the provided IR builder
/// and saving the state in `moduleTranslation`.
mlir::LogicalResult convertOperation(
@@ -47,6 +59,32 @@ class CIRDialectLLVMIRTranslationInterface
return mlir::success();
}
+
+ // Translate CIR's inline attribute to LLVM's function attributes.
+ void amendFunction(mlir::LLVM::LLVMFuncOp func,
+ llvm::ArrayRef<llvm::Instruction *> instructions,
+ mlir::NamedAttribute attribute,
+ mlir::LLVM::ModuleTranslation &moduleTranslation) const {
+ llvm::Function *llvmFunc = moduleTranslation.lookupFunction(func.getName());
+ if (auto inlineAttr = mlir::dyn_cast<cir::InlineAttr>(attribute.getValue())) {
+ if (inlineAttr.isNoInline())
+ llvmFunc->addFnAttr(llvm::Attribute::NoInline);
+ else if (inlineAttr.isAlwaysInline())
+ llvmFunc->addFnAttr(llvm::Attribute::AlwaysInline);
+ else if (inlineAttr.isInlineHint())
+ llvmFunc->addFnAttr(llvm::Attribute::InlineHint);
+ else
+ llvm_unreachable("Unknown inline kind");
+ // Drop ammended CIR attribute from LLVM op.
+ func->removeAttr(attribute.getName());
+ }
+
+ assert(!cir::MissingFeatures::opFuncOptNoneAttr());
+ assert(!cir::MissingFeatures::opFuncNoUnwind());
+ assert(!cir::MissingFeatures::opFuncColdHotAttr());
+ assert(!cir::MissingFeatures::opFuncUnwindTablesAttr());
+ assert(!cir::MissingFeatures::openCL());
+ }
};
void registerCIRDialectTranslation(mlir::DialectRegistry ®istry) {
diff --git a/clang/test/CIR/CodeGen/array.cpp b/clang/test/CIR/CodeGen/array.cpp
index d7488bfb258f8..82add4b347e72 100644
--- a/clang/test/CIR/CodeGen/array.cpp
+++ b/clang/test/CIR/CodeGen/array.cpp
@@ -123,7 +123,7 @@ void func() {
// CIR: %[[TMP:.*]] = cir.load{{.*}} %[[ELE_PTR]] : !cir.ptr<!s32i>, !s32i
// CIR" cir.store %[[TMP]], %[[INIT_2]] : !s32i, !cir.ptr<!s32i>
-// LLVM: define{{.*}} void @_Z4funcv()
+// LLVM: define{{.*}} void @_Z4funcv(){{.*}}
// LLVM-NEXT: %[[ARR:.*]] = alloca [10 x i32], i64 1, align 16
// LLVM-NEXT: %[[INIT:.*]] = alloca i32, i64 1, align 4
// LLVM-NEXT: %[[INIT_2:.*]] = alloca i32, i64 1, align 4
@@ -174,7 +174,7 @@ void func2() {
// CIR: cir.condition(%[[CMP]])
// CIR: }
-// LLVM: define{{.*}} void @_Z5func2v()
+// LLVM: define{{.*}} void @_Z5func2v(){{.*}}
// LLVM: %[[ARR:.*]] = alloca [2 x i32], i64 1, align 4
// LLVM: %[[TMP:.*]] = alloca ptr, i64 1, align 8
// LLVM: %[[ARR_PTR:.*]] = getelementptr i32, ptr %[[ARR]], i32 0
@@ -224,7 +224,7 @@ void func3() {
// CIR: %[[ELE_TMP:.*]] = cir.load{{.*}} %[[ELE_PTR]] : !cir.ptr<!s32i>, !s32i
// CIR: cir.store{{.*}} %[[ELE_TMP]], %[[INIT]] : !s32i, !cir.ptr<!s32i>
-// LLVM: define{{.*}} void @_Z5func3v()
+// LLVM: define{{.*}} void @_Z5func3v(){{.*}}
// LLVM: %[[ARR:.*]] = alloca [2 x i32], i64 1, align 4
// LLVM: %[[IDX:.*]] = alloca i32, i64 1, align 4
// LLVM: %[[INIT:.*]] = alloca i32, i64 1, align 4
@@ -276,7 +276,7 @@ void func4() {
// CIR: %[[TMP:.*]] = cir.load{{.*}} %[[ELE_0]] : !cir.ptr<!s32i>, !s32i
// CIR: cir.store{{.*}} %[[TMP]], %[[INIT]] : !s32i, !cir.ptr<!s32i>
-// LLVM: define{{.*}} void @_Z5func4v()
+// LLVM: define{{.*}} void @_Z5func4v(){{.*}}
// LLVM: %[[ARR:.*]] = alloca [2 x [1 x i32]], i64 1, align 4
// LLVM: %[[INIT:.*]] = alloca i32, i64 1, align 4
// LLVM: %[[ARR_PTR:.*]] = getelementptr [1 x i32], ptr %[[ARR]], i32 0
@@ -329,7 +329,7 @@ void func5() {
// CIR: cir.condition(%[[CMP]])
// CIR: }
-// LLVM: define{{.*}} void @_Z5func5v()
+// LLVM: define{{.*}} void @_Z5func5v(){{.*}}
// LLVM: %[[ARR:.*]] = alloca [2 x [1 x i32]], i64 1, align 4
// LLVM: %[[TMP:.*]] = alloca ptr, i64 1, align 8
// LLVM: %[[ARR_PTR:.*]] = getelementptr [1 x i32], ptr %[[ARR]], i32 0
@@ -372,7 +372,7 @@ void func6() {
// CIR: %[[V1:.*]] = cir.const #cir.int<5> : !s32i
// CIR: cir.store{{.*}} %[[V1]], %[[ELE_PTR]] : !s32i, !cir.ptr<!s32i>
-// LLVM: define{{.*}} void @_Z5func6v()
+// LLVM: define{{.*}} void @_Z5func6v(){{.*}}
// LLVM: %[[VAR:.*]] = alloca i32, i64 1, align 4
// LLVM: %[[ARR:.*]] = alloca [2 x i32], i64 1, align 4
// LLVM: store i32 4, ptr %[[VAR]], align 4
@@ -414,7 +414,7 @@ void func7() {
// CIR: cir.condition(%[[CMP]])
// CIR: }
-// LLVM: define{{.*}} void @_Z5func7v()
+// LLVM: define{{.*}} void @_Z5func7v(){{.*}}
// LLVM: %[[ARR:.*]] = alloca [1 x ptr], i64 1, align 8
// LLVM: %[[TMP:.*]] = alloca ptr, i64 1, align 8
// LLVM: %[[ARR_PTR:.*]] = getelementptr ptr, ptr %[[ARR]], i32 0
@@ -458,7 +458,7 @@ void func8(int arr[10]) {
// CIR: %[[TMP_4:.*]] = cir.load{{.*}} %[[ELE_1]] : !cir.ptr<!s32i>, !s32i
// CIR: cir.store{{.*}} %[[TMP_4]], %[[INIT_2]] : !s32i, !cir.ptr<!s32i>
-// LLVM: define{{.*}} void @_Z5func8Pi(ptr %[[ARG:.*]])
+// LLVM: define{{.*}} void @_Z5func8Pi(ptr %[[ARG:.*]]){{.*}}
// LLVM: %[[ARR:.*]] = alloca ptr, i64 1, align 8
// LLVM: %[[INIT:.*]] = alloca i32, i64 1, align 4
// LLVM: %[[INIT_2:.*]] = alloca i32, i64 1, align 4
@@ -502,7 +502,7 @@ void func9(int arr[10][5]) {
// CIR: %[[TMP_2:....
[truncated]
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
Member
bcardosolopes
left a comment
bcardosolopes
left a comment
There was a problem hiding this comment.
Overall looks good, some comments regarding the printing/parsing of the attribute
andykaylor
reviewed
Oct 10, 2025
Contributor
andykaylor
left a comment
andykaylor
left a comment
There was a problem hiding this comment.
It's great to see function attribute support get started! This looks good overall.
Comment on lines
1904
to
1906
| // If we don't have a declaration to control inlining, the function isn't | ||
| // explicitly marked as alwaysinline for semantic reasons, and inlining is | ||
| // disabled, mark the function as noinline. |
Contributor
There was a problem hiding this comment.
Suggested change
| // If we don't have a declaration to control inlining, the function isn't | |
| // explicitly marked as alwaysinline for semantic reasons, and inlining is | |
| // disabled, mark the function as noinline. | |
| // If inlining is disabled and we don't have a declaration to control | |
| // inlining, mark the function as 'noinline' unless it is explicitly marked | |
| // as 'alwaysinline'. |
I found the original comment confusing. Do you think this is better? It doesn't match the order in which the code checks things, but I think it corresponds better to the actual logic of the situation.
| f.setInlineKindAttr( | ||
| cir::InlineAttr::get(&getMLIRContext(), cir::InlineKind::NoInline)); | ||
| } else if (decl->hasAttr<AlwaysInlineAttr>() && !isNoInline) { | ||
| // (noinline wins over always_inline, and we can't specify both in IR) |
Contributor
There was a problem hiding this comment.
This comment doesn't seem quite right. If the declaration has NoInlineAttr and the function previously had the cir::InlineKind::AlwaysInline attribute, the case above does not set noinline. So it seems that the existing inline kind attribute wins (unless it's 'InlineHint').
xlauko
reviewed
Oct 10, 2025
Comment on lines
69
to
80
| if (auto inlineAttr = mlir::dyn_cast<cir::InlineAttr>(attribute.getValue())) { | ||
| if (inlineAttr.isNoInline()) | ||
| llvmFunc->addFnAttr(llvm::Attribute::NoInline); | ||
| else if (inlineAttr.isAlwaysInline()) | ||
| llvmFunc->addFnAttr(llvm::Attribute::AlwaysInline); | ||
| else if (inlineAttr.isInlineHint()) | ||
| llvmFunc->addFnAttr(llvm::Attribute::InlineHint); | ||
| else | ||
| llvm_unreachable("Unknown inline kind"); | ||
| // Drop ammended CIR attribute from LLVM op. | ||
| func->removeAttr(attribute.getName()); | ||
| } |
Contributor
There was a problem hiding this comment.
I would prefer switch over branching code to always safely cover all posibilities and create a helper method for the translation, e.g.:
llvm::Attribute toLLVMAttr(cir::InlineAttr attr) {
switch (attr.getValue()) {
case InlineKind::NoInline: return llvm::Attribute::NoInline;
...
}
}
Comment on lines
1821
to
1827
| if (*inlineKind == cir::InlineKind::NoInline) { | ||
| p << " inline_never"; | ||
| } else if (*inlineKind == cir::InlineKind::AlwaysInline) { | ||
| p << " inline_always"; | ||
| } else if (*inlineKind == cir::InlineKind::InlineHint) { | ||
| p << " inline_hint"; | ||
| } |
Contributor
There was a problem hiding this comment.
I believe this should use stringify method from the generate CIR_InlineAttr instead of manual listing.
The attribute needs to be fixed though to specify expected stringified names.
Unlike the incubator, this adds the inline attribute directly to FuncOp instead of adding the ExtraFnAttr dict. This adds three new optional keywords to CIR: inline_always, inline_never and inline_hint. Just like in OGCG -O0 implies inline_never on functions withoutt the C++ `inline` keyword and no other inlining-related attribute. This patch also adapts all tests that use functions so they account for LLVM attributes being attached now.
Add new CIR round trip test for inline attributes
mmha
force-pushed
the
cir-function-inlining
branch
from
99189fc to
65fc3a0
Compare
bcardosolopes
approved these changes
Oct 16, 2025
Member
bcardosolopes
left a comment
bcardosolopes
left a comment
There was a problem hiding this comment.
Nothing else to add, LGTM!
…o attr is present Fix remaining tests outside CIR/CodeGen
Contributor
Author
|
I changed it back to manual parsing. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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.
Unlike the incubator, this adds the inline attribute directly to FuncOp instead of adding the ExtraFnAttr dict.
This adds three new optional keywords to CIR: inline_always, inline_never and inline_hint. Just like in OGCG -O0 implies inline_never on functions withoutt the C++
inlinekeyword and no other inlining-related attribute.This patch also adapts all tests that use functions so they account for LLVM attributes being attached now.