[PATCH] D89711: [Coroutine] Prevent value reusing across coroutine suspensions in EarlyCSE and GVN

Mon Oct 19 11:27:46 PDT 2020

lxfind added a comment.

In D89711#2339394 <https://reviews.llvm.org/D89711#2339394>, @efriedma wrote:

> For the first point, if the IR definition isn't consistent, I'd prefer to actually fix that, instead of work around it. There are a lot of places that assume alias analysis is accurate.
>
>> For example, the glibc library pthread_self is defined with attribute((const)) (which would have readnone attribute in LLVM IR) even though it in fact reads global memory.
>
> Can we fix up the attributes on pthread_self in clang?  Or are we more generally concerned with people marking their functions const?
>
> On a related note, we probably need to change the representation of references to thread-local variables.
>
> -----
>
> For the second point, it isn't obvious to me that disabling CSE is universally profitable.  We can actually end up reducing the number of values live across the suspend point in some cases.  And it seems simpler to teach coroutine lowering to rematerialize instructions when it's profitable.

Thanks for the suggestions.
On the first point, I could certainly try fixing up pthread_self in Clang, but that would also mean we would never be able to optimize out redundant pthread_self() calls. I am not sure if that's acceptable in general. So far I only find pthread_self() to be problematic, but I am overall worried there might be more things like this that I am not aware of. Anyway, I could give it a try.

And yes thread local variables are another set of problems. I don't have a solution yet on how to handle them.

The second point makes sense to me.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89711