[PATCH] D124363: [Coroutines] Don't optimize readnone function before we split coroutine (4/5)

[John McCall via Phabricator via llvm-commits]

rjmccall added inline comments.


================
Comment at: clang/lib/CodeGen/CGCall.cpp:2132
     if (TargetDecl->hasAttr<ConstAttr>()) {
-      FuncAttrs.addAttribute(llvm::Attribute::ReadNone);
+      if (!LangOpts.Coroutines)
+        FuncAttrs.addAttribute(llvm::Attribute::ReadNone);
----------------
This is a global setting and will affect every function in every file that has coroutines enabled, which is presumably every file compiled with `-std=c++20` or later.  So you're radically changing optimization for a ton of C++ code that doesn't use coroutines.

It should be fine to just add `coro_readnone` instead of `readnone` to call sites from coroutine bodies, right?  Oh, but I guess we could have a `readnone` call in an inlined function body.  Can we just suppress this kind of code motion in coroutine bodies, or is that too invasive to the optimizer?

Actually, maybe your change to this patch was a misunderstanding.  What I was trying to say in Discourse was that I wasn't sure that your early pass approach would work for thread-local variables, because addresses of thread-locals are LLVM constants and can get moved around implicitly; you need the frontend to be involved there so that there's something hooked into the right place in the emitted function.  But I think it should work for this `readnone` annotation.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124363/new/

https://reviews.llvm.org/D124363