[clang] [llvm] [Coroutines] Change `llvm.coro.noop` to accept `llvm_anyptr_ty` instead (PR #102096)

Joseph Huber via cfe-commits cfe-commits at lists.llvm.org
Tue Aug 6 08:18:05 PDT 2024


https://github.com/jhuber6 updated https://github.com/llvm/llvm-project/pull/102096

>From 51b6f620c03eb5b4b00cfea22142850dc1ac51b8 Mon Sep 17 00:00:00 2001
From: Joseph Huber <huberjn at outlook.com>
Date: Mon, 5 Aug 2024 22:31:41 -0500
Subject: [PATCH] [Transforms] Fix Coroutine transform on non-default
 addressspaces

Summary:
This pass tries to get a pointer from the global. Some targets, like
AMDGPU, emit globals to a qualified address space. This would result in
an invalid cast. To fix this, we simply apply an addrspace cast, which
is a no-op if none exit.
---
 clang/lib/CodeGen/CGCoroutine.cpp             |  4 +++-
 clang/test/CodeGenCoroutines/coro-builtins.c  |  2 +-
 llvm/include/llvm/IR/Intrinsics.td            |  2 +-
 llvm/lib/Transforms/Coroutines/CoroEarly.cpp  | 15 ++++++++-------
 llvm/lib/Transforms/Coroutines/CoroInternal.h |  1 +
 llvm/lib/Transforms/Coroutines/Coroutines.cpp | 11 +++++++++++
 llvm/test/Transforms/Coroutines/coro-noop.ll  |  6 +++---
 7 files changed, 28 insertions(+), 13 deletions(-)

diff --git a/clang/lib/CodeGen/CGCoroutine.cpp b/clang/lib/CodeGen/CGCoroutine.cpp
index a8a70186c2c5a..99218621cb08e 100644
--- a/clang/lib/CodeGen/CGCoroutine.cpp
+++ b/clang/lib/CodeGen/CGCoroutine.cpp
@@ -1019,7 +1019,9 @@ RValue CodeGenFunction::EmitCoroutineIntrinsic(const CallExpr *E,
   if (IID == llvm::Intrinsic::coro_end)
     Args.push_back(llvm::ConstantTokenNone::get(getLLVMContext()));
 
-  llvm::Function *F = CGM.getIntrinsic(IID);
+  llvm::Function *F = IID == llvm::Intrinsic::coro_noop
+                          ? CGM.getIntrinsic(IID, {CGM.GlobalsVoidPtrTy})
+                          : CGM.getIntrinsic(IID);
   llvm::CallInst *Call = Builder.CreateCall(F, Args);
 
   // Note: The following code is to enable to emit coro.id and coro.begin by
diff --git a/clang/test/CodeGenCoroutines/coro-builtins.c b/clang/test/CodeGenCoroutines/coro-builtins.c
index 79f119b2b60ff..b66e7a7fd65a5 100644
--- a/clang/test/CodeGenCoroutines/coro-builtins.c
+++ b/clang/test/CodeGenCoroutines/coro-builtins.c
@@ -14,7 +14,7 @@ void f(int n) {
   // CHECK-NEXT: call i1 @llvm.coro.alloc(token %[[COROID]])
   __builtin_coro_alloc();
 
-  // CHECK-NEXT: call ptr @llvm.coro.noop()
+  // CHECK-NEXT: call ptr @llvm.coro.noop.p0()
   __builtin_coro_noop();
 
   // CHECK-NEXT: %[[SIZE:.+]] = call i64 @llvm.coro.size.i64()
diff --git a/llvm/include/llvm/IR/Intrinsics.td b/llvm/include/llvm/IR/Intrinsics.td
index b4e758136b39f..3ecf6a03d19bd 100644
--- a/llvm/include/llvm/IR/Intrinsics.td
+++ b/llvm/include/llvm/IR/Intrinsics.td
@@ -1713,7 +1713,7 @@ def int_coro_end_async
     : Intrinsic<[llvm_i1_ty], [llvm_ptr_ty, llvm_i1_ty, llvm_vararg_ty], []>;
 
 def int_coro_frame : Intrinsic<[llvm_ptr_ty], [], [IntrNoMem]>;
-def int_coro_noop : Intrinsic<[llvm_ptr_ty], [], [IntrNoMem]>;
+def int_coro_noop : Intrinsic<[llvm_anyptr_ty], [], [IntrNoMem]>;
 def int_coro_size : Intrinsic<[llvm_anyint_ty], [], [IntrNoMem]>;
 def int_coro_align : Intrinsic<[llvm_anyint_ty], [], [IntrNoMem]>;
 
diff --git a/llvm/lib/Transforms/Coroutines/CoroEarly.cpp b/llvm/lib/Transforms/Coroutines/CoroEarly.cpp
index d8e827e9cebcf..999895daac7a0 100644
--- a/llvm/lib/Transforms/Coroutines/CoroEarly.cpp
+++ b/llvm/lib/Transforms/Coroutines/CoroEarly.cpp
@@ -152,8 +152,7 @@ void Lowerer::lowerCoroNoop(IntrinsicInst *II) {
   }
 
   Builder.SetInsertPoint(II);
-  auto *NoopCoroVoidPtr = Builder.CreateBitCast(NoopCoro, Int8Ptr);
-  II->replaceAllUsesWith(NoopCoroVoidPtr);
+  II->replaceAllUsesWith(NoopCoro);
   II->eraseFromParent();
 }
 
@@ -249,11 +248,13 @@ void Lowerer::lowerEarlyIntrinsics(Function &F) {
 
 static bool declaresCoroEarlyIntrinsics(const Module &M) {
   return coro::declaresIntrinsics(
-      M, {"llvm.coro.id", "llvm.coro.id.retcon", "llvm.coro.id.retcon.once",
-          "llvm.coro.id.async", "llvm.coro.destroy", "llvm.coro.done",
-          "llvm.coro.end", "llvm.coro.end.async", "llvm.coro.noop",
-          "llvm.coro.free", "llvm.coro.promise", "llvm.coro.resume",
-          "llvm.coro.suspend"});
+      M, DenseSet<Intrinsic::ID>{
+             Intrinsic::coro_id, Intrinsic::coro_id_retcon,
+             Intrinsic::coro_id_retcon_once, Intrinsic::coro_id_async,
+             Intrinsic::coro_destroy, Intrinsic::coro_done, Intrinsic::coro_end,
+             Intrinsic::coro_end_async, Intrinsic::coro_noop,
+             Intrinsic::coro_free, Intrinsic::coro_promise,
+             Intrinsic::coro_resume, Intrinsic::coro_suspend});
 }
 
 PreservedAnalyses CoroEarlyPass::run(Module &M, ModuleAnalysisManager &) {
diff --git a/llvm/lib/Transforms/Coroutines/CoroInternal.h b/llvm/lib/Transforms/Coroutines/CoroInternal.h
index 5716fd0ea4ab9..b99eb325a892d 100644
--- a/llvm/lib/Transforms/Coroutines/CoroInternal.h
+++ b/llvm/lib/Transforms/Coroutines/CoroInternal.h
@@ -24,6 +24,7 @@ namespace coro {
 bool declaresAnyIntrinsic(const Module &M);
 bool declaresIntrinsics(const Module &M,
                         const std::initializer_list<StringRef>);
+bool declaresIntrinsics(const Module &M, const DenseSet<llvm::Intrinsic::ID> &);
 void replaceCoroFree(CoroIdInst *CoroId, bool Elide);
 
 /// Attempts to rewrite the location operand of debug intrinsics in terms of
diff --git a/llvm/lib/Transforms/Coroutines/Coroutines.cpp b/llvm/lib/Transforms/Coroutines/Coroutines.cpp
index 1a92bc1636257..46f68546dc491 100644
--- a/llvm/lib/Transforms/Coroutines/Coroutines.cpp
+++ b/llvm/lib/Transforms/Coroutines/Coroutines.cpp
@@ -123,6 +123,17 @@ bool coro::declaresIntrinsics(const Module &M,
   return false;
 }
 
+// Verifies if a module has any intrinsics.
+bool coro::declaresIntrinsics(const Module &M,
+                              const DenseSet<Intrinsic::ID> &Identifiers) {
+  for (const Function &F : M.functions()) {
+    if (Identifiers.contains(F.getIntrinsicID()))
+      return true;
+  }
+
+  return false;
+}
+
 // Replace all coro.frees associated with the provided CoroId either with 'null'
 // if Elide is true and with its frame parameter otherwise.
 void coro::replaceCoroFree(CoroIdInst *CoroId, bool Elide) {
diff --git a/llvm/test/Transforms/Coroutines/coro-noop.ll b/llvm/test/Transforms/Coroutines/coro-noop.ll
index 1e4f19a2ef66e..69ea076043c59 100644
--- a/llvm/test/Transforms/Coroutines/coro-noop.ll
+++ b/llvm/test/Transforms/Coroutines/coro-noop.ll
@@ -1,5 +1,5 @@
 ; Tests that CoroEarly pass correctly lowers coro.noop
-; RUN: opt < %s -S -passes=coro-early | FileCheck %s
+; RUN: opt -S -passes=coro-early < %s | FileCheck %s
 
 ; CHECK: %NoopCoro.Frame = type { ptr, ptr }
 ; CHECK: @NoopCoro.Frame.Const = private constant %NoopCoro.Frame { ptr @__NoopCoro_ResumeDestroy, ptr @__NoopCoro_ResumeDestroy }
@@ -10,11 +10,11 @@ define ptr @noop() {
 ; CHECK-NEXT: entry
 entry:
 ; CHECK-NEXT: ret ptr @NoopCoro.Frame.Const
-  %n = call ptr @llvm.coro.noop()
+  %n = call ptr @llvm.coro.noop.p1()
   ret ptr %n
 }
 
-declare ptr @llvm.coro.noop()
+declare ptr @llvm.coro.noop.p1()
 
 !llvm.dbg.cu = !{!0}
 !llvm.module.flags = !{!3, !4}



More information about the cfe-commits mailing list