Skip to content

Commit 4864aac

Browse files
committed
Address review feedback
Add new CIR round trip test for inline attributes
1 parent 8412847 commit 4864aac

File tree

8 files changed

+109
-66
lines changed

8 files changed

+109
-66
lines changed

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

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -964,7 +964,7 @@ def CIR_TypeInfoAttr : CIR_Attr<"TypeInfo", "typeinfo", [TypedAttrInterface]> {
964964
//===----------------------------------------------------------------------===//
965965

966966
def CIR_InlineKind : CIR_I32EnumAttr<"InlineKind", "inlineKind", [
967-
I32EnumAttrCase<"NoInline", 1, "no">,
967+
I32EnumAttrCase<"NoInline", 1, "never">,
968968
I32EnumAttrCase<"AlwaysInline", 2, "always">,
969969
I32EnumAttrCase<"InlineHint", 3, "hint">
970970
]> {
@@ -974,7 +974,24 @@ def CIR_InlineKind : CIR_I32EnumAttr<"InlineKind", "inlineKind", [
974974
def CIR_InlineAttr : CIR_EnumAttr<CIR_InlineKind, "inline"> {
975975
let summary = "Inline attribute";
976976
let description = [{
977-
Inline attributes represents user directives.
977+
Inline attribute represents user directives for inlining behavior.
978+
This attribute is only used by `cir.func` operations.
979+
980+
Values:
981+
- `never`: Prevents the function from being inlined (__attribute__((noinline)))
982+
- `always`: Forces the function to be inlined (__attribute__((always_inline)))
983+
- `hint`: Suggests the function should be inlined (inline keyword)
984+
985+
Example:
986+
```
987+
cir.func @noinline_func(%arg0: !s32i) -> !s32i #cir.inline<never> {
988+
cir.return %arg0 : !s32i
989+
}
990+
cir.func @always_inline_func() -> !s32i #cir.inline<always> {
991+
%0 = cir.const #cir.int<42> : !s32i
992+
cir.return %0 : !s32i
993+
}
994+
```
978995
}];
979996

980997
let cppClassName = "InlineAttr";

clang/lib/CIR/CodeGen/CIRGenModule.cpp

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1890,7 +1890,7 @@ void CIRGenModule::setCIRFunctionAttributesForDefinition(
18901890
assert(!cir::MissingFeatures::opFuncUnwindTablesAttr());
18911891
assert(!cir::MissingFeatures::stackProtector());
18921892

1893-
auto existingInlineKind = f.getInlineKind();
1893+
std::optional<cir::InlineKind> existingInlineKind = f.getInlineKind();
18941894
bool isNoInline =
18951895
existingInlineKind && *existingInlineKind == cir::InlineKind::NoInline;
18961896
bool isAlwaysInline = existingInlineKind &&
@@ -1901,9 +1901,9 @@ void CIRGenModule::setCIRFunctionAttributesForDefinition(
19011901

19021902
if (!isAlwaysInline &&
19031903
codeGenOpts.getInlining() == CodeGenOptions::OnlyAlwaysInlining) {
1904-
// If we don't have a declaration to control inlining, the function isn't
1905-
// explicitly marked as alwaysinline for semantic reasons, and inlining is
1906-
// disabled, mark the function as noinline.
1904+
// If inlining is disabled and we don't have a declaration to control
1905+
// inlining, mark the function as 'noinline' unless it is explicitly
1906+
// marked as 'alwaysinline'.
19071907
f.setInlineKindAttr(
19081908
cir::InlineAttr::get(&getMLIRContext(), cir::InlineKind::NoInline));
19091909
}
@@ -1925,11 +1925,12 @@ void CIRGenModule::setCIRFunctionAttributesForDefinition(
19251925
f.setInlineKindAttr(
19261926
cir::InlineAttr::get(&getMLIRContext(), cir::InlineKind::NoInline));
19271927
} else if (decl->hasAttr<AlwaysInlineAttr>() && !isNoInline) {
1928-
// (noinline wins over always_inline, and we can't specify both in IR)
1928+
// Don't override AlwaysInline with NoInline, or vice versa, since we can't
1929+
// specify both in IR.
19291930
f.setInlineKindAttr(
19301931
cir::InlineAttr::get(&getMLIRContext(), cir::InlineKind::AlwaysInline));
19311932
} else if (codeGenOpts.getInlining() == CodeGenOptions::OnlyAlwaysInlining) {
1932-
// If we're not inlining, then force everything that isn't always_inline
1933+
// If inlining is disabled, force everything that isn't always_inline
19331934
// to carry an explicit noinline attribute.
19341935
if (!isAlwaysInline) {
19351936
f.setInlineKindAttr(
@@ -1940,6 +1941,9 @@ void CIRGenModule::setCIRFunctionAttributesForDefinition(
19401941
// absence to mark things as noinline.
19411942
// Search function and template pattern redeclarations for inline.
19421943
if (auto *fd = dyn_cast<FunctionDecl>(decl)) {
1944+
// TODO: Share this checkForInline implementation with classic codegen.
1945+
// This logic is likely to change over time, so sharing would help ensure
1946+
// consistency.
19431947
auto checkForInline = [](const FunctionDecl *decl) {
19441948
auto checkRedeclForInline = [](const FunctionDecl *redecl) {
19451949
return redecl->isInlineSpecified();

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

Lines changed: 9 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1720,21 +1720,12 @@ ParseResult cir::FuncOp::parse(OpAsmParser &parser, OperationState &state) {
17201720
hasAlias = true;
17211721
}
17221722

1723-
// Parse optional inline attribute: inline_never, inline_always, or
1724-
// inline_hint
1725-
if (parser.parseOptionalKeyword("inline_never").succeeded()) {
1726-
state.addAttribute(
1727-
getInlineKindAttrName(state.name),
1728-
cir::InlineAttr::get(builder.getContext(), cir::InlineKind::NoInline));
1729-
} else if (parser.parseOptionalKeyword("inline_always").succeeded()) {
1730-
state.addAttribute(getInlineKindAttrName(state.name),
1731-
cir::InlineAttr::get(builder.getContext(),
1732-
cir::InlineKind::AlwaysInline));
1733-
} else if (parser.parseOptionalKeyword("inline_hint").succeeded()) {
1734-
state.addAttribute(getInlineKindAttrName(state.name),
1735-
cir::InlineAttr::get(builder.getContext(),
1736-
cir::InlineKind::InlineHint));
1737-
}
1723+
// Parse optional inline attribute: #cir.inline<never|always|hint>
1724+
cir::InlineAttr inlineAttr;
1725+
OptionalParseResult inlineResult = parser.parseOptionalAttribute(
1726+
inlineAttr, getInlineKindAttrName(state.name), state.attributes);
1727+
if (inlineResult.has_value() && failed(*inlineResult))
1728+
return failure();
17381729

17391730
// Parse the optional function body.
17401731
auto *body = state.addRegion();
@@ -1817,14 +1808,9 @@ void cir::FuncOp::print(OpAsmPrinter &p) {
18171808
p << ")";
18181809
}
18191810

1820-
if (auto inlineKind = getInlineKind()) {
1821-
if (*inlineKind == cir::InlineKind::NoInline) {
1822-
p << " inline_never";
1823-
} else if (*inlineKind == cir::InlineKind::AlwaysInline) {
1824-
p << " inline_always";
1825-
} else if (*inlineKind == cir::InlineKind::InlineHint) {
1826-
p << " inline_hint";
1827-
}
1811+
if (auto inlineAttr = getInlineKindAttr()) {
1812+
p << ' ';
1813+
p.printAttribute(inlineAttr);
18281814
}
18291815

18301816
// Print the body if this is not an external function.

clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1626,7 +1626,7 @@ mlir::LogicalResult CIRToLLVMFuncOpLowering::matchAndRewrite(
16261626

16271627
// Add inline_kind attribute with "cir." prefix so amendOperation handles it
16281628
if (auto inlineKind = op.getInlineKind()) {
1629-
fn->setAttr("cir.inline_kind",
1629+
fn->setAttr("cir.inline_kind",
16301630
cir::InlineAttr::get(getContext(), *inlineKind));
16311631
}
16321632

clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVMIR.cpp

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -66,16 +66,23 @@ class CIRDialectLLVMIRTranslationInterface
6666
mlir::NamedAttribute attribute,
6767
mlir::LLVM::ModuleTranslation &moduleTranslation) const {
6868
llvm::Function *llvmFunc = moduleTranslation.lookupFunction(func.getName());
69-
if (auto inlineAttr = mlir::dyn_cast<cir::InlineAttr>(attribute.getValue())) {
70-
if (inlineAttr.isNoInline())
71-
llvmFunc->addFnAttr(llvm::Attribute::NoInline);
72-
else if (inlineAttr.isAlwaysInline())
73-
llvmFunc->addFnAttr(llvm::Attribute::AlwaysInline);
74-
else if (inlineAttr.isInlineHint())
75-
llvmFunc->addFnAttr(llvm::Attribute::InlineHint);
76-
else
77-
llvm_unreachable("Unknown inline kind");
78-
// Drop ammended CIR attribute from LLVM op.
69+
70+
if (auto inlineAttr =
71+
mlir::dyn_cast<cir::InlineAttr>(attribute.getValue())) {
72+
const auto toLLVMAttr =
73+
[](const cir::InlineAttr &attr) -> llvm::Attribute::AttrKind {
74+
switch (attr.getValue()) {
75+
case cir::InlineKind::NoInline:
76+
return llvm::Attribute::NoInline;
77+
case cir::InlineKind::AlwaysInline:
78+
return llvm::Attribute::AlwaysInline;
79+
case cir::InlineKind::InlineHint:
80+
return llvm::Attribute::InlineHint;
81+
}
82+
llvm_unreachable("Unknown cir::InlineKind value in inlineAttr");
83+
};
84+
85+
llvmFunc->addFnAttr(toLLVMAttr(inlineAttr));
7986
func->removeAttr(attribute.getName());
8087
}
8188

clang/test/CIR/CodeGen/inline-attributes.cpp

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -29,19 +29,17 @@ int (*inline_hint_ptr)(int) = &inline_hint_function;
2929
int (*noinline_ptr)(int) = &noinline_function;
3030
int (*regular_ptr)(int) = &regular_function;
3131

32-
// CIR-LABEL: cir.func dso_local @_Z17noinline_functioni(%arg0: !s32i {{.*}}) -> !s32i inline_never
32+
// CIR-LABEL: cir.func dso_local @_Z17noinline_functioni(%arg0: !s32i {{.*}}) -> !s32i #cir.inline<never>
3333

3434
// CIR-LABEL: cir.func dso_local @_Z16regular_functioni(%arg0: !s32i {{.*}}) -> !s32i
35-
// CIR-NOT: inline_never
36-
// CIR-NOT: inline_always
37-
// CIR-NOT: inline_hint
35+
// CIR-NOT: #cir.inline<never>
36+
// CIR-NOT: #cir.inline<always>
37+
// CIR-NOT: #cir.inline<hint>
3838
// CIR-SAME: {
3939

40-
// CIR-LABEL: cir.func {{.*}}@_Z22always_inline_functioni(%arg0: !s32i {{.*}}) -> !s32i inline_always
40+
// CIR-LABEL: cir.func {{.*}}@_Z22always_inline_functioni(%arg0: !s32i {{.*}}) -> !s32i #cir.inline<always>
4141

42-
// CIR-LABEL: cir.func {{.*}}@_Z20inline_hint_functioni(%arg0: !s32i {{.*}}) -> !s32i inline_hint
43-
44-
// LLVM: @global_var = external{{.*}} global i32
42+
// CIR-LABEL: cir.func {{.*}}@_Z20inline_hint_functioni(%arg0: !s32i {{.*}}) -> !s32i #cir.inline<hint>
4543

4644
// LLVM: ; Function Attrs:{{.*}} noinline
4745
// LLVM: define{{.*}} i32 @_Z17noinline_functioni
@@ -59,8 +57,6 @@ int (*regular_ptr)(int) = &regular_function;
5957
// LLVM: ; Function Attrs:{{.*}} inlinehint
6058
// LLVM: define{{.*}} i32 @_Z20inline_hint_functioni
6159

62-
// OGCG: @global_var = external{{.*}} global i32
63-
6460
// OGCG: ; Function Attrs:{{.*}} noinline
6561
// OGCG: define{{.*}} i32 @_Z17noinline_functioni
6662

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,42 +1,42 @@
11
// RUN: %clang_cc1 -std=c++20 -triple x86_64-unknown-linux-gnu -fclangir -emit-cir %s -o - 2>&1 | FileCheck %s
22

33
extern "C" void TopLevelC(){}
4-
// CHECK: cir.func dso_local @TopLevelC() inline_never {
4+
// CHECK: cir.func dso_local @TopLevelC() #cir.inline<never> {
55
extern "C++" void TopLevelCpp(){}
6-
// CHECK: cir.func dso_local @_Z11TopLevelCppv() inline_never {
6+
// CHECK: cir.func dso_local @_Z11TopLevelCppv() #cir.inline<never> {
77

88
extern "C++" {
99
void ExternCppEmpty(){}
10-
// CHECK: cir.func dso_local @_Z14ExternCppEmptyv() inline_never {
10+
// CHECK: cir.func dso_local @_Z14ExternCppEmptyv() #cir.inline<never> {
1111
extern "C" void ExternCpp_C(){}
12-
// CHECK: cir.func dso_local @ExternCpp_C() inline_never {
12+
// CHECK: cir.func dso_local @ExternCpp_C() #cir.inline<never> {
1313
extern "C++" void ExternCpp_Cpp(){}
14-
// CHECK: cir.func dso_local @_Z13ExternCpp_Cppv() inline_never {
14+
// CHECK: cir.func dso_local @_Z13ExternCpp_Cppv() #cir.inline<never> {
1515

1616
extern "C" {
1717
void ExternCpp_CEmpty(){}
18-
// CHECK: cir.func dso_local @ExternCpp_CEmpty() inline_never {
18+
// CHECK: cir.func dso_local @ExternCpp_CEmpty() #cir.inline<never> {
1919
extern "C" void ExternCpp_C_C(){}
20-
// CHECK: cir.func dso_local @ExternCpp_C_C() inline_never {
20+
// CHECK: cir.func dso_local @ExternCpp_C_C() #cir.inline<never> {
2121
extern "C++" void ExternCpp_C_Cpp(){}
22-
// CHECK: cir.func dso_local @_Z15ExternCpp_C_Cppv() inline_never {
22+
// CHECK: cir.func dso_local @_Z15ExternCpp_C_Cppv() #cir.inline<never> {
2323
}
2424
}
2525

2626
extern "C" {
2727
void ExternCEmpty(){}
28-
// CHECK: cir.func dso_local @ExternCEmpty() inline_never {
28+
// CHECK: cir.func dso_local @ExternCEmpty() #cir.inline<never> {
2929
extern "C" void ExternC_C(){}
30-
// CHECK: cir.func dso_local @ExternC_C() inline_never {
30+
// CHECK: cir.func dso_local @ExternC_C() #cir.inline<never> {
3131
extern "C++" void ExternC_Cpp(){}
32-
// CHECK: cir.func dso_local @_Z11ExternC_Cppv() inline_never {
32+
// CHECK: cir.func dso_local @_Z11ExternC_Cppv() #cir.inline<never> {
3333
extern "C++" {
3434
void ExternC_CppEmpty(){}
35-
// CHECK: cir.func dso_local @_Z16ExternC_CppEmptyv() inline_never {
35+
// CHECK: cir.func dso_local @_Z16ExternC_CppEmptyv() #cir.inline<never> {
3636
extern "C" void ExternC_Cpp_C(){}
37-
// CHECK: cir.func dso_local @ExternC_Cpp_C() inline_never {
37+
// CHECK: cir.func dso_local @ExternC_Cpp_C() #cir.inline<never> {
3838
extern "C++" void ExternC_Cpp_Cpp(){}
39-
// CHECK: cir.func dso_local @_Z15ExternC_Cpp_Cppv() inline_never {
39+
// CHECK: cir.func dso_local @_Z15ExternC_Cpp_Cppv() #cir.inline<never> {
4040
}
4141
}
4242

clang/test/CIR/IR/inline-attrs.cir

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
// RUN: cir-opt %s --verify-roundtrip | FileCheck %s
2+
3+
!s32i = !cir.int<s, 32>
4+
5+
module {
6+
cir.func @noinline_func(%arg0: !s32i) -> !s32i #cir.inline<never> {
7+
cir.return %arg0 : !s32i
8+
}
9+
cir.func @always_inline_func(%arg0: !s32i) -> !s32i #cir.inline<always> {
10+
cir.return %arg0 : !s32i
11+
}
12+
cir.func @inline_hint_func(%arg0: !s32i) -> !s32i #cir.inline<hint> {
13+
cir.return %arg0 : !s32i
14+
}
15+
cir.func @regular_func(%arg0: !s32i) -> !s32i {
16+
cir.return %arg0 : !s32i
17+
}
18+
cir.func dso_local @noinline_with_attrs(%arg0: !s32i) -> !s32i #cir.inline<never> {
19+
cir.return %arg0 : !s32i
20+
}
21+
cir.func @noinline_decl(%arg0: !s32i) -> !s32i #cir.inline<never>
22+
cir.func @always_inline_decl(%arg0: !s32i) -> !s32i #cir.inline<always>
23+
cir.func @inline_hint_decl(%arg0: !s32i) -> !s32i #cir.inline<hint>
24+
}
25+
26+
// CHECK: cir.func @noinline_func(%arg0: !s32i) -> !s32i #cir.inline<never>
27+
// CHECK: cir.func @always_inline_func(%arg0: !s32i) -> !s32i #cir.inline<always>
28+
// CHECK: cir.func @inline_hint_func(%arg0: !s32i) -> !s32i #cir.inline<hint>
29+
// CHECK: cir.func @regular_func(%arg0: !s32i) -> !s32i {
30+
// CHECK: cir.func dso_local @noinline_with_attrs(%arg0: !s32i) -> !s32i #cir.inline<never>
31+
// CHECK: cir.func @noinline_decl(%arg0: !s32i) -> !s32i #cir.inline<never>
32+
// CHECK: cir.func @always_inline_decl(%arg0: !s32i) -> !s32i #cir.inline<always>
33+
// CHECK: cir.func @inline_hint_decl(%arg0: !s32i) -> !s32i #cir.inline<hint>

0 commit comments

Comments
 (0)