[PATCH] D130155: [DeadStoreElimination] Handle null accessing

Tue Jul 26 02:52:05 PDT 2022

ChuanqiXu added a comment.

In D130155#3678987 <https://reviews.llvm.org/D130155#3678987>, @nikic wrote:

> In D130155#3678941 <https://reviews.llvm.org/D130155#3678941>, @ChuanqiXu wrote:
>
>> @nikic would you mind landing this? Since 15.x branch is going to be branched and the thread identification problem is really important to users of coroutines, I want to have a workaround for it. And there is more than one month long testing time so that we could revert it if we find anything bad.
>
> Would it be "good enough" to mark coro.tls.wrapper as reading memory for LLVM 15, if we're adding a temporary intrinsic anyway?

Do you mean to mark `coro.tls.wrapper` as readonly?

> Or is it very important that `__attribute__((const))` functions also get "fixed"?

Yeah, it is important. Many users need to call pthread_self() or getid() to get the current the thread.

And this should be fixed/workarounded in D127383 <https://reviews.llvm.org/D127383>. Do you mean to not land D130142 <https://reviews.llvm.org/D130142> in 15.x? From my understanding, if we're concerning it breaks some assumptions, it should be easy to test. And if we're concerning about the impact to the IR model, it depends on whether we agree the consensus we made in previous discussions.

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

https://reviews.llvm.org/D130155