Skip to content

[ClR] Set optnone attribute for functions. #115

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 15, 2023
Merged

[ClR] Set optnone attribute for functions. #115

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions clang/include/clang/CIR/Dialect/IR/CIRAttrs.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,14 @@ class CIR_Attr<string name, string attrMnemonic, list<Trait> traits = []>
let mnemonic = attrMnemonic;
}

class CIRUnitAttr<string name, string attrMnemonic, list<Trait> traits = []>
: CIR_Attr<name, attrMnemonic, traits> {
let returnType = "bool";
let defaultValue = "false";
let valueType = NoneType;
let isOptional = 1;
}

//===----------------------------------------------------------------------===//
// LangAttr
//===----------------------------------------------------------------------===//
Expand Down Expand Up @@ -440,4 +448,8 @@ def InlineAttr : CIR_Attr<"Inline", "inline"> {
}];
}

def OptNoneAttr : CIRUnitAttr<"OptNone", "optnone"> {
let storageType = [{ OptNoneAttr }];
}

#endif // MLIR_CIR_DIALECT_CIR_ATTRS
19 changes: 19 additions & 0 deletions clang/lib/CIR/CodeGen/CIRGenModule.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1905,7 +1905,26 @@ void CIRGenModule::setExtraAttributesForFunc(FuncOp f,
mlir::cir::InlineKind::NoInline);
attrs.set(attr.getMnemonic(), attr);
}
}

// Track whether we need to add the optnone attribute,
// starting with the default for this optimization level.
bool ShouldAddOptNone =
!codeGenOpts.DisableO0ImplyOptNone && codeGenOpts.OptimizationLevel == 0;
if (FD) {
ShouldAddOptNone &= !FD->hasAttr<MinSizeAttr>();
ShouldAddOptNone &= !FD->hasAttr<AlwaysInlineAttr>();
ShouldAddOptNone |= FD->hasAttr<OptimizeNoneAttr>();
}

if (ShouldAddOptNone) {
auto optNoneAttr = mlir::cir::OptNoneAttr::get(builder.getContext());
attrs.set(optNoneAttr.getMnemonic(), optNoneAttr);

// OptimizeNone implies noinline; we should not be inlining such functions.
auto noInlineAttr = mlir::cir::InlineAttr::get(
builder.getContext(), mlir::cir::InlineKind::NoInline);
attrs.set(noInlineAttr.getMnemonic(), noInlineAttr);
}

f.setExtraAttrsAttr(mlir::cir::ExtraFuncAttributesAttr::get(
Expand Down
2 changes: 2 additions & 0 deletions clang/lib/CIR/Lowering/DirectToLLVM/LowerAttrToLLVMIR.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ class CIRDialectLLVMIRTranslationInterface
llvmFunc->addFnAttr(llvm::Attribute::InlineHint);
else
llvm_unreachable("Unknown inline kind");
} else if (attr.getValue().dyn_cast<mlir::cir::OptNoneAttr>()) {
llvmFunc->addFnAttr(llvm::Attribute::OptimizeNone);
}
}
}
Expand Down
25 changes: 25 additions & 0 deletions clang/test/CIR/CodeGen/optnone.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// RUN: %clang_cc1 -O0 -triple x86_64-unknown-linux-gnu -fclangir-enable -emit-cir %s -o %t.cir
// RUN: FileCheck --input-file=%t.cir %s -check-prefix=CIR-O0
// RUN: %clang_cc1 -O0 -triple x86_64-unknown-linux-gnu -fclangir-enable -emit-llvm %s -o %t.ll
// RUN: FileCheck --input-file=%t.ll %s -check-prefix=LLVM-O0

// RUN: %clang_cc1 -O2 -triple x86_64-unknown-linux-gnu -fclangir-enable -emit-cir %s -o %t2.cir
// RUN: FileCheck --input-file=%t2.cir %s -check-prefix=CIR-O2
// RUN: %clang_cc1 -O2 -triple x86_64-unknown-linux-gnu -fclangir-enable -emit-llvm %s -o %t2.ll
// RUN: FileCheck --input-file=%t2.ll %s -check-prefix=LLVM-O2

int s0(int a, int b) {
int x = a + b;
if (x > 0)
x = 0;
else
x = 1;
return x;
}

// CIR-O0: cir.func @_Z2s0ii(%arg0:{{.*}}, %arg1:{{.*}} -> {{.*}} extra( {inline = #cir.inline<no>, optnone = #cir.optnone} )
Copy link
Member

@bcardosolopes bcardosolopes Aug 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see the output is:

extra( {inline = #cir.inline<no>, optnone = #cir.optnone} )

Given that the idea is to be unit-attr-like, it seems too verbose to have optnone = #cir.optnone, can we instead have (this builds on the assumption that nothing will show up if this CIRUnitAttr doesn't get created, which is how unit-attr usually works):

extra( {inline = #cir.inline<no>, #cir.optnone} )

?

Copy link
Contributor Author

@htyu htyu Aug 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Talked offline. It looks like MLIR has a hack that only works for the MLIR UnitAttr



clangir/mlir/lib/IR/AsmPrinter.cpp

Line 2584 in 8e37695

if (llvm::isa<UnitAttr>(attr.getValue()))
. 

I can think to create a subclass to override that method (will need to change it to virtual first). The parser (

clangir/mlir/lib/AsmParser/AttributeParser.cpp

Line 323 in 8e37695

attributes.push_back({*nameId, builder.getUnitAttr()});
) will need to be overriden too. Doesn’t sound trivial. 

The problem is that extra func attr is a list of named attributes, so no direct way to control how a CIR attribute is printed there. The name will always be printed first.

// CIR-O2-NOT: cir.func @_Z2s0ii(%arg0:{{.*}}, %arg1:{{.*}} -> {{.*}} optnone

// LLVM-O0: define i32 @_Z2s0ii(i32 %0, i32 %1) #[[#ATTR:]]
// LLVM-O0: attributes #[[#ATTR]] = { noinline optnone }
// LLVM-O2-NOT: attributes #[[#]] = { noinline optnone }