Skip to content

Commit

Permalink
[CIR][Asm] Remove duplicated lambda & coroutine attributes (#580)
Browse files Browse the repository at this point in the history
Do not print in cir.func definition the 'attr { ... }' with coroutine or
lambda attributes since they are already printed before the function
name. Otherwise redundancy breaks a future parsing. Sort the attributes
to be skipped so it is more obvious to see the list of attributes.
Improve the tests to check there are no spurious attributes anymore.
  • Loading branch information
keryell authored May 3, 2024
1 parent 795925f commit 5961a8d
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 14 deletions.
20 changes: 16 additions & 4 deletions clang/lib/CIR/Dialect/IR/CIRDialect.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2025,6 +2025,9 @@ ::mlir::Region *cir::FuncOp::getCallableRegion() {
void cir::FuncOp::print(OpAsmPrinter &p) {
p << ' ';

// When adding a specific keyword here, do not forget to omit it in
// printFunctionAttributes below or there will be a syntax error when
// parsing
if (getBuiltin())
p << "builtin ";

Expand Down Expand Up @@ -2058,10 +2061,19 @@ void cir::FuncOp::print(OpAsmPrinter &p) {
function_interface_impl::printFunctionAttributes(
p, *this,
// These are all omitted since they are custom printed already.
{getSymVisibilityAttrName(), getAliaseeAttrName(),
getFunctionTypeAttrName(), getLinkageAttrName(), getBuiltinAttrName(),
getNoProtoAttrName(), getGlobalCtorAttrName(), getGlobalDtorAttrName(),
getExtraAttrsAttrName()});
{
getAliaseeAttrName(),
getBuiltinAttrName(),
getCoroutineAttrName(),
getExtraAttrsAttrName(),
getFunctionTypeAttrName(),
getGlobalCtorAttrName(),
getGlobalDtorAttrName(),
getLambdaAttrName(),
getLinkageAttrName(),
getNoProtoAttrName(),
getSymVisibilityAttrName(),
});

if (auto aliaseeName = getAliasee()) {
p << " alias(";
Expand Down
14 changes: 7 additions & 7 deletions clang/test/CIR/CodeGen/coro-task.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ VoidTask silly_task() {
co_await std::suspend_always();
}

// CHECK: cir.func coroutine @_Z10silly_taskv() -> ![[VoidTask]] {{.*}} {
// CHECK: cir.func coroutine @_Z10silly_taskv() -> ![[VoidTask]] extra{{.*}}{

// Allocate promise.

Expand Down Expand Up @@ -274,7 +274,7 @@ folly::coro::Task<int> byRef(const std::string& s) {
}

// FIXME: this could be less redundant than two allocas + reloads
// CHECK: cir.func coroutine @_Z5byRefRKSt6string(%arg0: !cir.ptr<![[StdString]]>
// CHECK: cir.func coroutine @_Z5byRefRKSt6string(%arg0: !cir.ptr<![[StdString]]> {{.*}}22 extra{{.*}}{
// CHECK: %[[#AllocaParam:]] = cir.alloca !cir.ptr<![[StdString]]>, {{.*}} ["s", init]
// CHECK: %[[#AllocaFnUse:]] = cir.alloca !cir.ptr<![[StdString]]>, {{.*}} ["s", init]

Expand All @@ -291,7 +291,7 @@ folly::coro::Task<void> silly_coro() {
// Make sure we properly handle OnFallthrough coro body sub stmt and
// check there are not multiple co_returns emitted.

// CHECK: cir.func coroutine @_Z10silly_corov()
// CHECK: cir.func coroutine @_Z10silly_corov() {{.*}}22 extra{{.*}}{
// CHECK: cir.await(init, ready : {
// CHECK: cir.call @_ZN5folly4coro4TaskIvE12promise_type11return_voidEv
// CHECK-NOT: cir.call @_ZN5folly4coro4TaskIvE12promise_type11return_voidEv
Expand All @@ -303,7 +303,7 @@ folly::coro::Task<int> go1() {
co_return co_await task;
}

// CHECK: cir.func coroutine @_Z3go1v()
// CHECK: cir.func coroutine @_Z3go1v() {{.*}}22 extra{{.*}}{
// CHECK: %[[#IntTaskAddr:]] = cir.alloca ![[IntTask]], !cir.ptr<![[IntTask]]>, ["task", init]

// CHECK: cir.await(init, ready : {
Expand Down Expand Up @@ -338,16 +338,16 @@ folly::coro::Task<int> go1_lambda() {
co_return co_await task;
}

// CHECK: cir.func coroutine lambda internal private @_ZZ10go1_lambdavENK3$_0clEv
// CHECK: cir.func coroutine @_Z10go1_lambdav()
// CHECK: cir.func coroutine lambda internal private @_ZZ10go1_lambdavENK3$_0clEv{{.*}}22 extra{{.*}}{
// CHECK: cir.func coroutine @_Z10go1_lambdav() {{.*}}22 extra{{.*}}{

folly::coro::Task<int> go4() {
auto* fn = +[](int const& i) -> folly::coro::Task<int> { co_return i; };
auto task = fn(3);
co_return co_await std::move(task);
}

// CHECK: cir.func coroutine @_Z3go4v()
// CHECK: cir.func coroutine @_Z3go4v() {{.*}}22 extra{{.*}}{

// CHECK: cir.await(init, ready : {
// CHECK: }, suspend : {
Expand Down
6 changes: 3 additions & 3 deletions clang/test/CIR/CodeGen/lambda.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ void fn() {
// CHECK: !ty_22anon2E222 = !cir.struct<class "anon.2" {!cir.int<u, 8>}>
// CHECK-DAG: module

// CHECK: cir.func lambda internal private @_ZZ2fnvENK3$_0clEv
// CHECK: cir.func lambda internal private @_ZZ2fnvENK3$_0clEv{{.*}}) extra

// CHECK: cir.func @_Z2fnv()
// CHECK-NEXT: %0 = cir.alloca !ty_22anon2E222, !cir.ptr<!ty_22anon2E222>, ["a"]
Expand All @@ -21,7 +21,7 @@ void l0() {
a();
}

// CHECK: cir.func lambda internal private @_ZZ2l0vENK3$_0clEv(
// CHECK: cir.func lambda internal private @_ZZ2l0vENK3$_0clEv({{.*}}) extra

// CHECK: %0 = cir.alloca !cir.ptr<!ty_22anon2E422>, !cir.ptr<!cir.ptr<!ty_22anon2E422>>, ["this", init] {alignment = 8 : i64}
// CHECK: cir.store %arg0, %0 : !cir.ptr<!ty_22anon2E422>, !cir.ptr<!cir.ptr<!ty_22anon2E422>>
Expand Down Expand Up @@ -99,7 +99,7 @@ int g3() {
}

// lambda operator()
// CHECK: cir.func lambda internal private @_ZZ2g3vENK3$_0clERKi
// CHECK: cir.func lambda internal private @_ZZ2g3vENK3$_0clERKi{{.*}}!s32i extra

// lambda __invoke()
// CHECK: cir.func internal private @_ZZ2g3vEN3$_08__invokeERKi
Expand Down

0 comments on commit 5961a8d

Please sign in to comment.