[PATCH] D127383: Don't treat readnone call in presplit coroutine as not access memory

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

Do we also need to upgrade argmemonly to inaccessibleorargmemonly? Assuming that the thread ID counts as inaccessible memory.



================
Comment at: llvm/include/llvm/IR/InstrTypes.h:1854
+           // If the call lives in presplit coroutine, we couldn't assume the
+           // call wouldn't access memory even if it has readnone attribute.
+           // Since readnone could be used for thread identification and
----------------
couldn't -> can't, wouldn't -> won't


================
Comment at: llvm/include/llvm/IR/InstrTypes.h:1871
+    return hasImpliedFnAttr(Attribute::WriteOnly) &&
+           // See the comments in doesNotAccessMemory. Due to readnone implies
+           // writeonly.
----------------
Due to -> Because


================
Comment at: llvm/test/Transforms/Coroutines/coro-readnone-01.ll:5
+; RUN: opt < %s -S -passes='gvn' -opaque-pointers | FileCheck %s --check-prefixes=CHECK,CHECK_UNSPLITTED
+; RUN: opt < %s -S -passes='newgvn' -opaque-pointers | FileCheck %s --check-prefixes=CHECK,CHECK_UNSPLITTED
+
----------------
It would be better to consistently use `ptr` in the test and drop the `-opaque-pointers` flag. Currently it mixed `ptr` and `i8*`...


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

https://reviews.llvm.org/D127383