This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG7dccdd76b374: [Coroutines] Don't mark the parameter
attribute of resume function as noalias (authored by ChuanqiXu).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D139295/new/
https://reviews.llvm.org/D139295
Files:
clang/test/CodeGenCoroutines/pr59221.cpp
llvm/lib/Transforms/Coroutines/CoroSplit.cpp
llvm/test/Transforms/Coroutines/coro-debug-dbg.addr.ll
llvm/test/Transforms/Coroutines/coro-debug-dbg.values-not_used_in_frame.ll
llvm/test/Transforms/Coroutines/coro-debug-dbg.values.ll
llvm/test/Transforms/Coroutines/coro-debug.ll
llvm/test/Transforms/Coroutines/coro-frame.ll
Index: llvm/test/Transforms/Coroutines/coro-frame.ll
===================================================================
--- llvm/test/Transforms/Coroutines/coro-frame.ll
+++ llvm/test/Transforms/Coroutines/coro-frame.ll
@@ -44,7 +44,7 @@
; CHECK: ret i8* %hdl
; See if the float was loaded from the frame
-; CHECK-LABEL: @f.resume(%f.Frame* noalias nonnull align 8
+; CHECK-LABEL: @f.resume(%f.Frame* noundef nonnull align 8
; CHECK: %r.reload = load double, double* %r.reload.addr
; CHECK: call double @print(double %r.reload)
; CHECK: ret void
Index: llvm/test/Transforms/Coroutines/coro-debug.ll
===================================================================
--- llvm/test/Transforms/Coroutines/coro-debug.ll
+++ llvm/test/Transforms/Coroutines/coro-debug.ll
@@ -172,7 +172,7 @@
!32 = !DILocalVariable(name: "inline_asm", scope: !6, file: !7, line: 55, type: !11)
; CHECK: define i8* @f(i32 %x) #0 personality i32 0 !dbg ![[ORIG:[0-9]+]]
-; CHECK: define internal fastcc void @f.resume(%f.Frame* noalias nonnull align 8 dereferenceable(40) %FramePtr) #0 personality i32 0 !dbg ![[RESUME:[0-9]+]]
+; CHECK: define internal fastcc void @f.resume(%f.Frame* noundef nonnull align 8 dereferenceable(40) %FramePtr) #0 personality i32 0 !dbg ![[RESUME:[0-9]+]]
; CHECK: entry.resume:
; CHECK: %[[DBG_PTR:.*]] = alloca %f.Frame*
; CHECK: call void @llvm.dbg.declare(metadata %f.Frame** %[[DBG_PTR]], metadata ![[RESUME_COROHDL:[0-9]+]], metadata !DIExpression(DW_OP_deref, DW_OP_plus_uconst,
@@ -194,8 +194,8 @@
; CHECK-NEXT: to label %[[DEFAULT_DEST:.+]] [label
; CHECK: [[DEFAULT_DEST]]:
; CHECK-NEXT: call void @llvm.dbg.declare(metadata i32 %[[CALLBR_RES]]
-; CHECK: define internal fastcc void @f.destroy(%f.Frame* noalias nonnull align 8 dereferenceable(40) %FramePtr) #0 personality i32 0 !dbg ![[DESTROY:[0-9]+]]
-; CHECK: define internal fastcc void @f.cleanup(%f.Frame* noalias nonnull align 8 dereferenceable(40) %FramePtr) #0 personality i32 0 !dbg ![[CLEANUP:[0-9]+]]
+; CHECK: define internal fastcc void @f.destroy(%f.Frame* noundef nonnull align 8 dereferenceable(40) %FramePtr) #0 personality i32 0 !dbg ![[DESTROY:[0-9]+]]
+; CHECK: define internal fastcc void @f.cleanup(%f.Frame* noundef nonnull align 8 dereferenceable(40) %FramePtr) #0 personality i32 0 !dbg ![[CLEANUP:[0-9]+]]
; CHECK: ![[ORIG]] = distinct !DISubprogram(name: "f", linkageName: "flink"
Index: llvm/test/Transforms/Coroutines/coro-debug-dbg.values.ll
===================================================================
--- llvm/test/Transforms/Coroutines/coro-debug-dbg.values.ll
+++ llvm/test/Transforms/Coroutines/coro-debug-dbg.values.ll
@@ -2,7 +2,7 @@
; RUN: opt < %s -passes='module(coro-early),cgscc(coro-split,coro-split)' -S | FileCheck %s
;
; This file is based on coro-debug-frame-variable.ll.
-; CHECK: define internal fastcc void @f.resume(%f.Frame* noalias nonnull align 16 dereferenceable(80) %FramePtr) !dbg ![[RESUME_FN_DBG_NUM:[0-9]+]]
+; CHECK: define internal fastcc void @f.resume(%f.Frame* noundef nonnull align 16 dereferenceable(80) %FramePtr) !dbg ![[RESUME_FN_DBG_NUM:[0-9]+]]
; CHECK: init.ready:
; CHECK: call void @llvm.dbg.value(metadata %f.Frame** %FramePtr.debug, metadata ![[XVAR_RESUME:[0-9]+]],
; CHECK: await.ready:
Index: llvm/test/Transforms/Coroutines/coro-debug-dbg.values-not_used_in_frame.ll
===================================================================
--- llvm/test/Transforms/Coroutines/coro-debug-dbg.values-not_used_in_frame.ll
+++ llvm/test/Transforms/Coroutines/coro-debug-dbg.values-not_used_in_frame.ll
@@ -2,7 +2,7 @@
; RUN: opt < %s -passes='module(coro-early),cgscc(coro-split,coro-split)' -S | FileCheck %s
;
; This file is based on coro-debug-frame-variable.ll.
-; CHECK: define internal fastcc void @f.resume(%f.Frame* noalias nonnull align 16 dereferenceable(80) %FramePtr) !dbg ![[RESUME_FN_DBG_NUM:[0-9]+]]
+; CHECK: define internal fastcc void @f.resume(%f.Frame* noundef nonnull align 16 dereferenceable(80) %FramePtr) !dbg ![[RESUME_FN_DBG_NUM:[0-9]+]]
; CHECK: await.ready:
; CHECK: call void @llvm.dbg.value(metadata i32 undef, metadata ![[IVAR_RESUME:[0-9]+]], metadata !DIExpression(
; CHECK: call void @llvm.dbg.value(metadata i32 undef, metadata ![[JVAR_RESUME:[0-9]+]], metadata !DIExpression(
Index: llvm/test/Transforms/Coroutines/coro-debug-dbg.addr.ll
===================================================================
--- llvm/test/Transforms/Coroutines/coro-debug-dbg.addr.ll
+++ llvm/test/Transforms/Coroutines/coro-debug-dbg.addr.ll
@@ -8,20 +8,20 @@
; RUN: opt < %s -passes='module(coro-early),cgscc(coro-split,coro-split)' -S | FileCheck %s
; This file is based on coro-debug-frame-variable.ll.
-; CHECK: define internal fastcc void @f.resume(%f.Frame* noalias nonnull align 16 dereferenceable(80) %FramePtr) !dbg ![[RESUME_FN_DBG_NUM:[0-9]+]]
+; CHECK: define internal fastcc void @f.resume(%f.Frame* noundef nonnull align 16 dereferenceable(80) %FramePtr) !dbg ![[RESUME_FN_DBG_NUM:[0-9]+]]
; CHECK-NEXT: entry.resume:
; CHECK: call void @llvm.dbg.addr(metadata %f.Frame** %FramePtr.debug, metadata ![[XVAR_RESUME:[0-9]+]],
; CHECK: call void @llvm.dbg.addr(metadata %f.Frame** %FramePtr.debug, metadata ![[YVAR_RESUME:[0-9]+]],
; CHECK: call void @llvm.dbg.addr(metadata %f.Frame** %FramePtr.debug, metadata ![[ZVAR_RESUME:[0-9]+]],
-; CHECK: define internal fastcc void @f.destroy(%f.Frame* noalias nonnull align 16 dereferenceable(80) %FramePtr) !dbg ![[DESTROY_FN_DBG_NUM:[0-9]+]] {
+; CHECK: define internal fastcc void @f.destroy(%f.Frame* noundef nonnull align 16 dereferenceable(80) %FramePtr) !dbg ![[DESTROY_FN_DBG_NUM:[0-9]+]] {
; CHECK-NEXT: entry.destroy:
; CHECK-NEXT: %FramePtr.debug = alloca
; CHECK: call void @llvm.dbg.addr(metadata %f.Frame** %FramePtr.debug, metadata ![[XVAR_DESTROY:[0-9]+]],
; CHECK: call void @llvm.dbg.addr(metadata %f.Frame** %FramePtr.debug, metadata ![[YVAR_DESTROY:[0-9]+]],
; CHECK: call void @llvm.dbg.addr(metadata %f.Frame** %FramePtr.debug, metadata ![[ZVAR_DESTROY:[0-9]+]],
-; CHECK: define internal fastcc void @f.cleanup(%f.Frame* noalias nonnull align 16 dereferenceable(80) %FramePtr) !dbg ![[CLEANUP_FN_DBG_NUM:[0-9]+]] {
+; CHECK: define internal fastcc void @f.cleanup(%f.Frame* noundef nonnull align 16 dereferenceable(80) %FramePtr) !dbg ![[CLEANUP_FN_DBG_NUM:[0-9]+]] {
; CHECK: entry.cleanup:
; CHECK: call void @llvm.dbg.addr(metadata %f.Frame** %FramePtr.debug, metadata ![[XVAR_CLEANUP:[0-9]+]],
; CHECK: call void @llvm.dbg.addr(metadata %f.Frame** %FramePtr.debug, metadata ![[YVAR_CLEANUP:[0-9]+]],
Index: llvm/lib/Transforms/Coroutines/CoroSplit.cpp
===================================================================
--- llvm/lib/Transforms/Coroutines/CoroSplit.cpp
+++ llvm/lib/Transforms/Coroutines/CoroSplit.cpp
@@ -516,13 +516,6 @@
Function *NewF =
Function::Create(FnTy, GlobalValue::LinkageTypes::InternalLinkage,
OrigF.getName() + Suffix);
- if (Shape.ABI != coro::ABI::Async)
- NewF->addParamAttr(0, Attribute::NonNull);
-
- // For the async lowering ABI we can't guarantee that the context argument is
- // not access via a different pointer not based on the argument.
- if (Shape.ABI != coro::ABI::Async)
- NewF->addParamAttr(0, Attribute::NoAlias);
M->getFunctionList().insert(InsertBefore, NewF);
@@ -849,11 +842,15 @@
}
static void addFramePointerAttrs(AttributeList &Attrs, LLVMContext &Context,
- unsigned ParamIndex,
- uint64_t Size, Align Alignment) {
+ unsigned ParamIndex, uint64_t Size,
+ Align Alignment, bool NoAlias) {
AttrBuilder ParamAttrs(Context);
ParamAttrs.addAttribute(Attribute::NonNull);
- ParamAttrs.addAttribute(Attribute::NoAlias);
+ ParamAttrs.addAttribute(Attribute::NoUndef);
+
+ if (NoAlias)
+ ParamAttrs.addAttribute(Attribute::NoAlias);
+
ParamAttrs.addAlignmentAttr(Alignment);
ParamAttrs.addDereferenceableAttr(Size);
Attrs = Attrs.addParamAttributes(Context, ParamIndex, ParamAttrs);
@@ -959,8 +956,8 @@
NewAttrs = NewAttrs.addFnAttributes(
Context, AttrBuilder(Context, OrigAttrs.getFnAttrs()));
- addFramePointerAttrs(NewAttrs, Context, 0,
- Shape.FrameSize, Shape.FrameAlign);
+ addFramePointerAttrs(NewAttrs, Context, 0, Shape.FrameSize,
+ Shape.FrameAlign, /*NoAlias=*/false);
break;
case coro::ABI::Async: {
auto *ActiveAsyncSuspend = cast<CoroSuspendAsyncInst>(ActiveSuspend);
@@ -989,9 +986,12 @@
// full-stop.
NewAttrs = Shape.RetconLowering.ResumePrototype->getAttributes();
+ /// FIXME: Is it really good to add the NoAlias attribute?
addFramePointerAttrs(NewAttrs, Context, 0,
Shape.getRetconCoroId()->getStorageSize(),
- Shape.getRetconCoroId()->getStorageAlignment());
+ Shape.getRetconCoroId()->getStorageAlignment(),
+ /*NoAlias=*/true);
+
break;
}
Index: clang/test/CodeGenCoroutines/pr59221.cpp
===================================================================
--- /dev/null
+++ clang/test/CodeGenCoroutines/pr59221.cpp
@@ -0,0 +1,88 @@
+// Test for PR59221. Tests the compiler wouldn't misoptimize the final result.
+//
+// REQUIRES: x86-registered-target
+//
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -std=c++20 %s -O3 -S -emit-llvm -o - | FileCheck %s
+
+#include "Inputs/coroutine.h"
+
+template <typename T> struct task {
+ struct promise_type {
+ T value{123};
+ std::coroutine_handle<> caller{std::noop_coroutine()};
+
+ struct final_awaiter: std::suspend_always {
+ auto await_suspend(std::coroutine_handle<promise_type> me) const noexcept {
+ return me.promise().caller;
+ }
+ };
+
+ constexpr auto initial_suspend() const noexcept {
+ return std::suspend_always();
+ }
+ constexpr auto final_suspend() const noexcept {
+ return final_awaiter{};
+ }
+ auto unhandled_exception() noexcept {
+ // ignore
+ }
+ constexpr void return_value(T v) noexcept {
+ value = v;
+ }
+ constexpr auto & get_return_object() noexcept {
+ return *this;
+ }
+ };
+
+ using coroutine_handle = std::coroutine_handle<promise_type>;
+
+ promise_type & promise{nullptr};
+
+ task(promise_type & p) noexcept: promise{p} { }
+
+ ~task() noexcept {
+ coroutine_handle::from_promise(promise).destroy();
+ }
+
+ auto await_ready() noexcept {
+ return false;
+ }
+
+ auto await_suspend(std::coroutine_handle<> caller) noexcept {
+ promise.caller = caller;
+ return coroutine_handle::from_promise(promise);
+ }
+
+ constexpr auto await_resume() const noexcept {
+ return promise.value;
+ }
+
+ // non-coroutine access to result
+ auto get() noexcept {
+ const auto handle = coroutine_handle::from_promise(promise);
+
+ if (!handle.done()) {
+ handle.resume();
+ }
+
+ return promise.value;
+ }
+};
+
+
+static inline auto a() noexcept -> task<int> {
+ co_return 42;
+}
+
+static inline auto test() noexcept -> task<int> {
+ co_return co_await a();
+}
+
+int foo() {
+ return test().get();
+}
+
+// Checks that the store for the result value 42 is not misoptimized out.
+// CHECK: define{{.*}}_Z3foov(
+// CHECK: store i32 42, ptr %{{.*}}
+// CHECK: }
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
