Skip to content
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.

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

[coro][pgo] Remove redundant coroutine test files #89620

Merged
merged 1 commit into from
Apr 25, 2024

Conversation

zmodem
Copy link
Collaborator

@zmodem zmodem commented Apr 22, 2024

Each of these is a copy of a test under Transforms/Coroutines/

The PGO functionality is already covered by multiple runlines (pgo and no pgo) in the original files, so the copies add no new coverage, only maintenance problems.

Each of these is a copy of a test under Transforms/Coroutines/

The PGO functionality is already covered by multiple runlines (PGO and
NOPGO) in the original files, so the copies add no new coverage, only
maintenance problems.
@zmodem zmodem added the coroutines C++20 coroutines label Apr 22, 2024
@llvmbot llvmbot added the PGO Profile Guided Optimizations label Apr 22, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Apr 22, 2024

@llvm/pr-subscribers-pgo

@llvm/pr-subscribers-coroutines

Author: Hans (zmodem)

Changes

Each of these is a copy of a test under Transforms/Coroutines/

The PGO functionality is already covered by multiple runlines (pgo and no pgo) in the original files, so the copies add no new coverage, only maintenance problems.


Patch is 36.04 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/89620.diff

12 Files Affected:

  • (removed) llvm/test/Instrumentation/InstrProfiling/Coro/coro-split-musttail.ll (-63)
  • (removed) llvm/test/Instrumentation/InstrProfiling/Coro/coro-split-musttail1.ll (-97)
  • (removed) llvm/test/Instrumentation/InstrProfiling/Coro/coro-split-musttail10.ll (-55)
  • (removed) llvm/test/Instrumentation/InstrProfiling/Coro/coro-split-musttail11.ll (-55)
  • (removed) llvm/test/Instrumentation/InstrProfiling/Coro/coro-split-musttail12.ll (-85)
  • (removed) llvm/test/Instrumentation/InstrProfiling/Coro/coro-split-musttail13.ll (-76)
  • (removed) llvm/test/Instrumentation/InstrProfiling/Coro/coro-split-musttail2.ll (-68)
  • (removed) llvm/test/Instrumentation/InstrProfiling/Coro/coro-split-musttail3.ll (-91)
  • (removed) llvm/test/Instrumentation/InstrProfiling/Coro/coro-split-musttail4.ll (-66)
  • (removed) llvm/test/Instrumentation/InstrProfiling/Coro/coro-split-musttail5.ll (-63)
  • (removed) llvm/test/Instrumentation/InstrProfiling/Coro/coro-split-musttail6.ll (-112)
  • (removed) llvm/test/Instrumentation/InstrProfiling/Coro/coro-split-musttail7.ll (-115)
diff --git a/llvm/test/Instrumentation/InstrProfiling/Coro/coro-split-musttail.ll b/llvm/test/Instrumentation/InstrProfiling/Coro/coro-split-musttail.ll
deleted file mode 100644
index a7321833d74843..00000000000000
--- a/llvm/test/Instrumentation/InstrProfiling/Coro/coro-split-musttail.ll
+++ /dev/null
@@ -1,63 +0,0 @@
-; Tests that instrumentation doesn't interfere with lowering (coro-split).
-; It should convert coro.resume followed by a suspend to a musttail call.
-
-; RUN: opt < %s -passes='pgo-instr-gen,cgscc(coro-split),simplifycfg,early-cse' -S | FileCheck %s
-
-define void @f() #0 {
-entry:
-  %id = call token @llvm.coro.id(i32 0, ptr null, ptr null, ptr null)
-  %alloc = call ptr @malloc(i64 16) #3
-  %vFrame = call noalias nonnull ptr @llvm.coro.begin(token %id, ptr %alloc)
-
-  %save = call token @llvm.coro.save(ptr null)
-  %addr1 = call ptr @llvm.coro.subfn.addr(ptr null, i8 0)
-  call fastcc void %addr1(ptr null)
-
-  %suspend = call i8 @llvm.coro.suspend(token %save, i1 false)
-  switch i8 %suspend, label %exit [
-    i8 0, label %await.ready
-    i8 1, label %exit
-  ]
-await.ready:
-  %save2 = call token @llvm.coro.save(ptr null)
-  %addr2 = call ptr @llvm.coro.subfn.addr(ptr null, i8 0)
-  call fastcc void %addr2(ptr null)
-
-  %suspend2 = call i8 @llvm.coro.suspend(token %save2, i1 false)
-  switch i8 %suspend2, label %exit [
-    i8 0, label %exit
-    i8 1, label %exit
-  ]
-exit:
-  call i1 @llvm.coro.end(ptr null, i1 false, token none)
-  ret void
-}
-
-; Verify that in the initial function resume is not marked with musttail.
-; CHECK-LABEL: @f(
-; CHECK: %[[addr1:.+]] = call ptr @llvm.coro.subfn.addr(ptr null, i8 0)
-; CHECK-NOT: musttail call fastcc void %[[addr1]](ptr null)
-
-; Verify that in the resume part resume call is marked with musttail.
-; CHECK-LABEL: @f.resume(
-; CHECK: %[[addr2:.+]] = call ptr @llvm.coro.subfn.addr(ptr null, i8 0)
-; CHECK: call void @llvm.instrprof
-; CHECK-NEXT: musttail call fastcc void %[[addr2]](ptr null)
-; CHECK-NEXT: ret void
-
-declare token @llvm.coro.id(i32, ptr readnone, ptr nocapture readonly, ptr) #1
-declare i1 @llvm.coro.alloc(token) #2
-declare i64 @llvm.coro.size.i64() #3
-declare ptr @llvm.coro.begin(token, ptr writeonly) #2
-declare token @llvm.coro.save(ptr) #2
-declare ptr @llvm.coro.frame() #3
-declare i8 @llvm.coro.suspend(token, i1) #2
-declare ptr @llvm.coro.free(token, ptr nocapture readonly) #1
-declare i1 @llvm.coro.end(ptr, i1, token) #2
-declare ptr @llvm.coro.subfn.addr(ptr nocapture readonly, i8) #1
-declare ptr @malloc(i64)
-
-attributes #0 = { presplitcoroutine }
-attributes #1 = { argmemonly nounwind readonly }
-attributes #2 = { nounwind }
-attributes #3 = { nounwind readnone }
diff --git a/llvm/test/Instrumentation/InstrProfiling/Coro/coro-split-musttail1.ll b/llvm/test/Instrumentation/InstrProfiling/Coro/coro-split-musttail1.ll
deleted file mode 100644
index 6098dee9a58035..00000000000000
--- a/llvm/test/Instrumentation/InstrProfiling/Coro/coro-split-musttail1.ll
+++ /dev/null
@@ -1,97 +0,0 @@
-; Tests that instrumentation doesn't interfere with lowering (coro-split).
-; It should convert coro.resume followed by a suspend to a musttail call.
-
-; RUN: opt < %s -passes='pgo-instr-gen,cgscc(coro-split),simplifycfg,early-cse' -S | FileCheck %s
-
-define void @f() #0 {
-entry:
-  %id = call token @llvm.coro.id(i32 0, ptr null, ptr null, ptr null)
-  %alloc = call ptr @malloc(i64 16) #3
-  %vFrame = call noalias nonnull ptr @llvm.coro.begin(token %id, ptr %alloc)
-
-  %save = call token @llvm.coro.save(ptr null)
-  %addr1 = call ptr @llvm.coro.subfn.addr(ptr null, i8 0)
-  call fastcc void %addr1(ptr null)
-
-  %suspend = call i8 @llvm.coro.suspend(token %save, i1 false)
-  switch i8 %suspend, label %exit [
-    i8 0, label %await.suspend
-    i8 1, label %exit
-  ]
-await.suspend:
-  %save2 = call token @llvm.coro.save(ptr null)
-  %br0 = call i8 @switch_result()
-  switch i8 %br0, label %unreach [
-    i8 0, label %await.resume3
-    i8 1, label %await.resume1
-    i8 2, label %await.resume2
-  ]
-await.resume1:
-  %hdl = call ptr @g()
-  %addr2 = call ptr @llvm.coro.subfn.addr(ptr %hdl, i8 0)
-  call fastcc void %addr2(ptr %hdl)
-  br label %final.suspend
-await.resume2:
-  %hdl2 = call ptr @h()
-  %addr3 = call ptr @llvm.coro.subfn.addr(ptr %hdl2, i8 0)
-  call fastcc void %addr3(ptr %hdl2)
-  br label %final.suspend
-await.resume3:
-  %addr4 = call ptr @llvm.coro.subfn.addr(ptr null, i8 0)
-  call fastcc void %addr4(ptr null)
-  br label %final.suspend
-final.suspend:
-  %suspend2 = call i8 @llvm.coro.suspend(token %save2, i1 false)
-  switch i8 %suspend2, label %exit [
-    i8 0, label %pre.exit
-    i8 1, label %exit
-  ]
-pre.exit:
-  br label %exit
-exit:
-  call i1 @llvm.coro.end(ptr null, i1 false, token none)
-  ret void
-unreach:
-  unreachable
-}
-
-; Verify that in the initial function resume is not marked with musttail.
-; CHECK-LABEL: @f(
-; CHECK: %[[addr1:.+]] = call ptr @llvm.coro.subfn.addr(ptr null, i8 0)
-; CHECK-NOT: musttail call fastcc void %[[addr1]](ptr null)
-
-; Verify that in the resume part resume call is marked with musttail.
-; CHECK-LABEL: @f.resume(
-; CHECK: %[[hdl:.+]] = call ptr @g()
-; CHECK-NEXT: %[[addr2:.+]] = call ptr @llvm.coro.subfn.addr(ptr %[[hdl]], i8 0)
-; CHECK: musttail call fastcc void %[[addr2]](ptr %[[hdl]])
-; CHECK-NEXT: ret void
-; CHECK: %[[hdl2:.+]] = call ptr @h()
-; CHECK-NEXT: %[[addr3:.+]] = call ptr @llvm.coro.subfn.addr(ptr %[[hdl2]], i8 0)
-; CHECK: musttail call fastcc void %[[addr3]](ptr %[[hdl2]])
-; CHECK-NEXT: ret void
-; CHECK: %[[addr4:.+]] = call ptr @llvm.coro.subfn.addr(ptr null, i8 0)
-; CHECK: musttail call fastcc void %[[addr4]](ptr null)
-; CHECK-NEXT: ret void
-
-
-
-declare token @llvm.coro.id(i32, ptr readnone, ptr nocapture readonly, ptr) #1
-declare i1 @llvm.coro.alloc(token) #2
-declare i64 @llvm.coro.size.i64() #3
-declare ptr @llvm.coro.begin(token, ptr writeonly) #2
-declare token @llvm.coro.save(ptr) #2
-declare ptr @llvm.coro.frame() #3
-declare i8 @llvm.coro.suspend(token, i1) #2
-declare ptr @llvm.coro.free(token, ptr nocapture readonly) #1
-declare i1 @llvm.coro.end(ptr, i1, token) #2
-declare ptr @llvm.coro.subfn.addr(ptr nocapture readonly, i8) #1
-declare ptr @malloc(i64)
-declare i8 @switch_result()
-declare ptr @g()
-declare ptr @h()
-
-attributes #0 = { presplitcoroutine }
-attributes #1 = { argmemonly nounwind readonly }
-attributes #2 = { nounwind }
-attributes #3 = { nounwind readnone }
diff --git a/llvm/test/Instrumentation/InstrProfiling/Coro/coro-split-musttail10.ll b/llvm/test/Instrumentation/InstrProfiling/Coro/coro-split-musttail10.ll
deleted file mode 100644
index f43b10ebf42e5a..00000000000000
--- a/llvm/test/Instrumentation/InstrProfiling/Coro/coro-split-musttail10.ll
+++ /dev/null
@@ -1,55 +0,0 @@
-; Tests that instrumentation doesn't interfere with lowering (coro-split).
-; It should convert coro.resume followed by a suspend to a musttail call.
-
-; RUN: opt < %s -passes='pgo-instr-gen,cgscc(coro-split),simplifycfg,early-cse' -S | FileCheck %s
-
-target triple = "wasm64-unknown-unknown"
-
-define void @f() #0 {
-entry:
-  %id = call token @llvm.coro.id(i32 0, ptr null, ptr null, ptr null)
-  %alloc = call ptr @malloc(i64 16) #3
-  %vFrame = call noalias nonnull ptr @llvm.coro.begin(token %id, ptr %alloc)
-
-  %save = call token @llvm.coro.save(ptr null)
-  %addr1 = call ptr @llvm.coro.subfn.addr(ptr null, i8 0)
-  call fastcc void %addr1(ptr null)
-
-  %suspend = call i8 @llvm.coro.suspend(token %save, i1 false)
-  switch i8 %suspend, label %exit [
-    i8 0, label %await.ready
-    i8 1, label %exit
-  ]
-await.ready:
-  %save2 = call token @llvm.coro.save(ptr null)
-  %addr2 = call ptr @llvm.coro.subfn.addr(ptr null, i8 0)
-  call fastcc void %addr2(ptr null)
-
-  %suspend2 = call i8 @llvm.coro.suspend(token %save2, i1 false)
-  switch i8 %suspend2, label %exit [
-    i8 0, label %exit
-    i8 1, label %exit
-  ]
-exit:
-  call i1 @llvm.coro.end(ptr null, i1 false, token none)
-  ret void
-}
-
-; CHECK: musttail call
-
-declare token @llvm.coro.id(i32, ptr readnone, ptr nocapture readonly, ptr) #1
-declare i1 @llvm.coro.alloc(token) #2
-declare i64 @llvm.coro.size.i64() #3
-declare ptr @llvm.coro.begin(token, ptr writeonly) #2
-declare token @llvm.coro.save(ptr) #2
-declare ptr @llvm.coro.frame() #3
-declare i8 @llvm.coro.suspend(token, i1) #2
-declare ptr @llvm.coro.free(token, ptr nocapture readonly) #1
-declare i1 @llvm.coro.end(ptr, i1, token) #2
-declare ptr @llvm.coro.subfn.addr(ptr nocapture readonly, i8) #1
-declare ptr @malloc(i64)
-
-attributes #0 = { presplitcoroutine "target-features"="+tail-call" }
-attributes #1 = { argmemonly nounwind readonly }
-attributes #2 = { nounwind }
-attributes #3 = { nounwind readnone }
diff --git a/llvm/test/Instrumentation/InstrProfiling/Coro/coro-split-musttail11.ll b/llvm/test/Instrumentation/InstrProfiling/Coro/coro-split-musttail11.ll
deleted file mode 100644
index fc5bb9a1b20b3d..00000000000000
--- a/llvm/test/Instrumentation/InstrProfiling/Coro/coro-split-musttail11.ll
+++ /dev/null
@@ -1,55 +0,0 @@
-; Tests that instrumentation doesn't interfere with lowering (coro-split).
-; It should convert coro.resume followed by a suspend to a musttail call.
-
-; RUN: opt < %s -passes='pgo-instr-gen,cgscc(coro-split),simplifycfg,early-cse' -S | FileCheck %s
-
-target triple = "wasm32-unknown-unknown"
-
-define void @f() #0 {
-entry:
-  %id = call token @llvm.coro.id(i32 0, ptr null, ptr null, ptr null)
-  %alloc = call ptr @malloc(i64 16) #3
-  %vFrame = call noalias nonnull ptr @llvm.coro.begin(token %id, ptr %alloc)
-
-  %save = call token @llvm.coro.save(ptr null)
-  %addr1 = call ptr @llvm.coro.subfn.addr(ptr null, i8 0)
-  call fastcc void %addr1(ptr null)
-
-  %suspend = call i8 @llvm.coro.suspend(token %save, i1 false)
-  switch i8 %suspend, label %exit [
-    i8 0, label %await.ready
-    i8 1, label %exit
-  ]
-await.ready:
-  %save2 = call token @llvm.coro.save(ptr null)
-  %addr2 = call ptr @llvm.coro.subfn.addr(ptr null, i8 0)
-  call fastcc void %addr2(ptr null)
-
-  %suspend2 = call i8 @llvm.coro.suspend(token %save2, i1 false)
-  switch i8 %suspend2, label %exit [
-    i8 0, label %exit
-    i8 1, label %exit
-  ]
-exit:
-  call i1 @llvm.coro.end(ptr null, i1 false, token none)
-  ret void
-}
-
-; CHECK: musttail call
-
-declare token @llvm.coro.id(i32, ptr readnone, ptr nocapture readonly, ptr) #1
-declare i1 @llvm.coro.alloc(token) #2
-declare i64 @llvm.coro.size.i64() #3
-declare ptr @llvm.coro.begin(token, ptr writeonly) #2
-declare token @llvm.coro.save(ptr) #2
-declare ptr @llvm.coro.frame() #3
-declare i8 @llvm.coro.suspend(token, i1) #2
-declare ptr @llvm.coro.free(token, ptr nocapture readonly) #1
-declare i1 @llvm.coro.end(ptr, i1, token) #2
-declare ptr @llvm.coro.subfn.addr(ptr nocapture readonly, i8) #1
-declare ptr @malloc(i64)
-
-attributes #0 = { presplitcoroutine "target-features"="+tail-call" }
-attributes #1 = { argmemonly nounwind readonly }
-attributes #2 = { nounwind }
-attributes #3 = { nounwind readnone }
diff --git a/llvm/test/Instrumentation/InstrProfiling/Coro/coro-split-musttail12.ll b/llvm/test/Instrumentation/InstrProfiling/Coro/coro-split-musttail12.ll
deleted file mode 100644
index 634d0106a2e6ae..00000000000000
--- a/llvm/test/Instrumentation/InstrProfiling/Coro/coro-split-musttail12.ll
+++ /dev/null
@@ -1,85 +0,0 @@
-; Tests that instrumentation doesn't interfere with lowering (coro-split).
-; It should convert coro.resume followed by a suspend to a musttail call.
-
-; RUN: opt < %s -passes='pgo-instr-gen,cgscc(coro-split),simplifycfg,early-cse' -S | FileCheck %s
-
-declare void @fakeresume1(ptr)
-declare void @print()
-
-define void @f(i1 %cond) #0 {
-entry:
-  %id = call token @llvm.coro.id(i32 0, ptr null, ptr null, ptr null)
-  %alloc = call ptr @malloc(i64 16) #3
-  %vFrame = call noalias nonnull ptr @llvm.coro.begin(token %id, ptr %alloc)
-
-  %save = call token @llvm.coro.save(ptr null)
-
-  %init_suspend = call i8 @llvm.coro.suspend(token %save, i1 false)
-  switch i8 %init_suspend, label %coro.end [
-    i8 0, label %await.ready
-    i8 1, label %coro.end
-  ]
-await.ready:
-  %save2 = call token @llvm.coro.save(ptr null)
-  br i1 %cond, label %then, label %else
-
-then:
-  call fastcc void @fakeresume1(ptr align 8 null)
-  br label %merge
-
-else:
-  br label %merge
-
-merge:
-  %v0 = phi i1 [0, %then], [1, %else]
-  br label %compare
-
-compare:
-  %cond.cmp = icmp eq i1 %v0, 0
-  br i1 %cond.cmp, label %ready, label %prepare
-
-prepare:
-  call void @print()
-  br label %ready
-
-ready:
-  %suspend = call i8 @llvm.coro.suspend(token %save2, i1 true)
-  %switch = icmp ult i8 %suspend, 2
-  br i1 %switch, label %cleanup, label %coro.end
-
-cleanup:
-  %free.handle = call ptr @llvm.coro.free(token %id, ptr %vFrame)
-  %.not = icmp eq ptr %free.handle, null
-  br i1 %.not, label %coro.end, label %coro.free
-
-coro.free:
-  call void @delete(ptr nonnull %free.handle) #2
-  br label %coro.end
-
-coro.end:
-  call i1 @llvm.coro.end(ptr null, i1 false, token none)
-  ret void
-}
-
-; CHECK-LABEL: @f.resume(
-; CHECK-NOT:      }
-; CHECK:          call void @print()
-
-
-declare token @llvm.coro.id(i32, ptr readnone, ptr nocapture readonly, ptr) #1
-declare i1 @llvm.coro.alloc(token) #2
-declare i64 @llvm.coro.size.i64() #3
-declare ptr @llvm.coro.begin(token, ptr writeonly) #2
-declare token @llvm.coro.save(ptr) #2
-declare ptr @llvm.coro.frame() #3
-declare i8 @llvm.coro.suspend(token, i1) #2
-declare ptr @llvm.coro.free(token, ptr nocapture readonly) #1
-declare i1 @llvm.coro.end(ptr, i1, token) #2
-declare ptr @llvm.coro.subfn.addr(ptr nocapture readonly, i8) #1
-declare ptr @malloc(i64)
-declare void @delete(ptr nonnull) #2
-
-attributes #0 = { presplitcoroutine }
-attributes #1 = { argmemonly nounwind readonly }
-attributes #2 = { nounwind }
-attributes #3 = { nounwind readnone }
diff --git a/llvm/test/Instrumentation/InstrProfiling/Coro/coro-split-musttail13.ll b/llvm/test/Instrumentation/InstrProfiling/Coro/coro-split-musttail13.ll
deleted file mode 100644
index 2f9a14c9010719..00000000000000
--- a/llvm/test/Instrumentation/InstrProfiling/Coro/coro-split-musttail13.ll
+++ /dev/null
@@ -1,76 +0,0 @@
-; Tests that instrumentation doesn't interfere with lowering (coro-split).
-; It should convert coro.resume followed by a suspend to a musttail call.
-
-; RUN: opt < %s -passes='pgo-instr-gen,cgscc(coro-split),simplifycfg,early-cse' -S | FileCheck %s
-
-declare void @fakeresume1(ptr)
-declare void @may_throw(ptr)
-declare void @print()
-
-define void @f(i1 %cond) #0 personality i32 3 {
-entry:
-  %id = call token @llvm.coro.id(i32 0, ptr null, ptr null, ptr null)
-  %alloc = call ptr @malloc(i64 16) #3
-  %vFrame = call noalias nonnull ptr @llvm.coro.begin(token %id, ptr %alloc)
-
-  %save = call token @llvm.coro.save(ptr null)
-
-  %init_suspend = call i8 @llvm.coro.suspend(token %save, i1 false)
-  switch i8 %init_suspend, label %coro.end [
-    i8 0, label %await.ready
-    i8 1, label %coro.end
-  ]
-await.ready:
-  call fastcc void @fakeresume1(ptr align 8 null)
-  invoke void @may_throw(ptr null)
-    to label %ready unwind label %lpad
-
-ready:
-  %save2 = call token @llvm.coro.save(ptr null)
-  %suspend = call i8 @llvm.coro.suspend(token %save2, i1 true)
-  %switch = icmp ult i8 %suspend, 2
-  br i1 %switch, label %cleanup, label %coro.end
-
-cleanup:
-  %free.handle = call ptr @llvm.coro.free(token %id, ptr %vFrame)
-  %.not = icmp eq ptr %free.handle, null
-  br i1 %.not, label %coro.end, label %coro.free
-
-lpad:
-  %lpval = landingpad { ptr, i32 }
-     cleanup
-
-  %need.resume = call i1 @llvm.coro.end(ptr null, i1 true, token none)
-  resume { ptr, i32 } %lpval
-
-coro.free:
-  call void @delete(ptr nonnull %free.handle) #2
-  br label %coro.end
-
-coro.end:
-  call i1 @llvm.coro.end(ptr null, i1 false, token none)
-  ret void
-}
-
-; CHECK-LABEL: @f.resume(
-; CHECK-NOT:          musttail call fastcc void @fakeresume1(
-; CHECK:     }
-
-
-declare token @llvm.coro.id(i32, ptr readnone, ptr nocapture readonly, ptr) #1
-declare i1 @llvm.coro.alloc(token) #2
-declare i64 @llvm.coro.size.i64() #3
-declare ptr @llvm.coro.begin(token, ptr writeonly) #2
-declare token @llvm.coro.save(ptr) #2
-declare ptr @llvm.coro.frame() #3
-declare i8 @llvm.coro.suspend(token, i1) #2
-declare ptr @llvm.coro.free(token, ptr nocapture readonly) #1
-declare i1 @llvm.coro.end(ptr, i1, token) #2
-declare ptr @llvm.coro.subfn.addr(ptr nocapture readonly, i8) #1
-declare ptr @malloc(i64)
-declare void @delete(ptr nonnull) #2
-
-attributes #0 = { presplitcoroutine }
-attributes #1 = { argmemonly nounwind readonly }
-attributes #2 = { nounwind }
-attributes #3 = { nounwind readnone }
diff --git a/llvm/test/Instrumentation/InstrProfiling/Coro/coro-split-musttail2.ll b/llvm/test/Instrumentation/InstrProfiling/Coro/coro-split-musttail2.ll
deleted file mode 100644
index 61b61a200e704d..00000000000000
--- a/llvm/test/Instrumentation/InstrProfiling/Coro/coro-split-musttail2.ll
+++ /dev/null
@@ -1,68 +0,0 @@
-; Tests that instrumentation doesn't interfere with lowering (coro-split).
-; It should convert coro.resume followed by a suspend to a musttail call.
-
-; RUN: opt < %s -passes='pgo-instr-gen,cgscc(coro-split),simplifycfg,early-cse' -S | FileCheck %s
-
-define void @fakeresume1(ptr)  {
-entry:
-  ret void;
-}
-
-define void @fakeresume2(ptr align 8)  {
-entry:
-  ret void;
-}
-
-define void @g() #0 {
-entry:
-  %id = call token @llvm.coro.id(i32 0, ptr null, ptr null, ptr null)
-  %alloc = call ptr @malloc(i64 16) #3
-  %vFrame = call noalias nonnull ptr @llvm.coro.begin(token %id, ptr %alloc)
-
-  %save = call token @llvm.coro.save(ptr null)
-  call fastcc void @fakeresume1(ptr null)
-
-  %suspend = call i8 @llvm.coro.suspend(token %save, i1 false)
-  switch i8 %suspend, label %exit [
-    i8 0, label %await.ready
-    i8 1, label %exit
-  ]
-await.ready:
-  %save2 = call token @llvm.coro.save(ptr null)
-  call fastcc void @fakeresume2(ptr align 8 null)
-
-  %suspend2 = call i8 @llvm.coro.suspend(token %save2, i1 false)
-  switch i8 %suspend2, label %exit [
-    i8 0, label %exit
-    i8 1, label %exit
-  ]
-exit:
-  call i1 @llvm.coro.end(ptr null, i1 false, token none)
-  ret void
-}
-
-; Verify that in the initial function resume is not marked with musttail.
-; CHECK-LABEL: @g(
-; CHECK-NOT: musttail call fastcc void @fakeresume1(ptr null)
-
-; Verify that in the resume part resume call is marked with musttail.
-; CHECK-LABEL: @g.resume(
-; CHECK: musttail call fastcc void @fakeresume2(ptr align 8 null)
-; CHECK-NEXT: ret void
-
-declare token @llvm.coro.id(i32, ptr readnone, ptr nocapture readonly, ptr) #1
-declare i1 @llvm.coro.alloc(token) #2
-declare i64 @llvm.coro.size.i64() #3
-declare ptr @llvm.coro.begin(token, ptr writeonly) #2
-declare token @llvm.coro.save(ptr) #2
-declare ptr @llvm.coro.frame() #3
-declare i8 @llvm.coro.suspend(token, i1) #2
-declare ptr @llvm.coro.free(token, ptr nocapture readonly) #1
-declare i1 @llvm.coro.end(ptr, i1, token) #2
-declare ptr @llvm.coro.subfn.addr(ptr nocapture readonly, i8) #1
-declare ptr @malloc(i64)
-
-attributes #0 = { presplitcoroutine }
-attributes #1 = { argmemonly nounwind readonly }
-attributes #2 = { nounwind }
-attributes #3 = { nounwind readnone }
diff --git a/llvm/test/Instrumentation/InstrProfiling/Coro/coro-split-musttail3.ll b/llvm/test/Instrumentation/InstrProfiling/Coro/coro-split-musttail3.ll
deleted file mode 100644
index 82176b8085e6c7..00000000000000
--- a/llvm/test/Instrumentation/InstrProfiling/Coro/coro-split-musttail3.ll
+++ /dev/null
@@ -1,91 +0,0 @@
-; Tests that instrumentation doesn't interfere with lowering (coro-split).
-; It should convert coro.resume followed by a suspend to a musttail call.
-
-; RUN: opt < %s -passes='pgo-instr-gen,cgscc(coro-split),simplifycfg,early-cse' -S | FileCheck %s
-
-define void @f() #0 {
-entry:
-  %id = call token @llvm.coro.id(i32 0, ptr null, ptr null, ptr null)
-  %alloc = call ptr @malloc(i64 16) #3
-  %vFrame = call noalias nonnull ptr @llvm.coro.begin(token %id, ptr %alloc)
-
-  %save = call token @llvm.coro.save(ptr null)
-  %addr1 = call ptr @llvm.coro.subfn.addr(ptr null, i8 0)
-  call fastcc void %addr1(ptr null)
-
-  %suspend = call i8 @llvm.coro.suspend(token %save, i1 false)
-  %cmp = icmp eq i8 %suspend, 0
-  br i1 %cmp, label %await.suspend, label %exit
-await.suspend:
-  %sav...
[truncated]

@mtrofin mtrofin requested a review from david-xl April 22, 2024 16:23
@mtrofin
Copy link
Member

mtrofin commented Apr 22, 2024

This was intentional, see the comment that introduced them (#71262 (review))

An advantage is that the tests cover 2 concerns, really: coro lowering, and PGO instrumentation. They can potentially diverge if needed.

Other than needing to duplicate a fix in both places, is there any other shortcoming of the double presence?

+david-xl who suggested the change originally.

@zmodem
Copy link
Collaborator Author

zmodem commented Apr 22, 2024

This was intentional, see the comment that introduced them (#71262 (review))

I saw that, but I don't agree with the outcome :-)

An advantage is that the tests cover 2 concerns, really: coro lowering, and PGO instrumentation. They can potentially diverge if needed.

The original tests (e.g. llvm/test/Transforms/Coroutines/coro-split-musttail.ll) already cover both concerns nicely.

If there's a need to diverge, new tests could be created when that need arises.

Other than needing to duplicate a fix in both places, is there any other shortcoming of the double presence?

Needing to update two almost identical copies of 12 tests is a pretty big pain.

@mtrofin
Copy link
Member

mtrofin commented Apr 22, 2024

This was intentional, see the comment that introduced them (#71262 (review))

I saw that, but I don't agree with the outcome :-)

An advantage is that the tests cover 2 concerns, really: coro lowering, and PGO instrumentation. They can potentially diverge if needed.

The original tests (e.g. llvm/test/Transforms/Coroutines/coro-split-musttail.ll) already cover both concerns nicely.

If there's a need to diverge, new tests could be created when that need arises.

Other than needing to duplicate a fix in both places, is there any other shortcoming of the double presence?

Needing to update two almost identical copies of 12 tests is a pretty big pain.

How about removing the PGO RUN: from the coro side and leaving these on the PGO side handle that? (still want to hear from @david-xl if he had any other concerns addressed by the duplication tho)

One reason I could think of, to keep the PGO side, is for code review auto-notification (i.e. because the pgo pr "team" is auto-added)

@yuxuanchen1997
Copy link
Contributor

yuxuanchen1997 commented Apr 22, 2024

Are there any lit tricks to make it read from the equivalent tests under Transforms/Coroutines directory instead of %s? So that we only have one copy of the test body, but still give you notifications for PGO team.

@ChuanqiXu9
Copy link
Member

No objection from me. I'd like to leave this to @mtrofin and @zmodem

@zmodem
Copy link
Collaborator Author

zmodem commented Apr 23, 2024

How about removing the PGO RUN: from the coro side and leaving these on the PGO side handle that?

I think that would be a step in the wrong direction. It would still be a copy-paste of the test code, which is a definite code smell.

The current setup (on the coro side) with multiple run-lines for running each test in different configs is the way to go.

One reason I could think of, to keep the PGO side, is for code review auto-notification (i.e. because the pgo pr "team" is auto-added)

It sounds like auto notifications is the main motivation here? In that case, I think the pgo team should subscribe to the coro dir.

Are there any lit tricks to make it read from the equivalent tests under Transforms/Coroutines directory instead of %s? So that we only have one copy of the test body, but still give you notifications for PGO team.

Just subscribing to the coro test dir seems easier.

@zmodem
Copy link
Collaborator Author

zmodem commented Apr 24, 2024

Any more comments? I'd like to go ahead and merge this.

@zmodem zmodem merged commit 0894946 into llvm:main Apr 25, 2024
7 checks passed
@zmodem zmodem deleted the remove_redundant_coro_Tests branch April 25, 2024 07:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
coroutines C++20 coroutines PGO Profile Guided Optimizations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants