[PATCH] D92661: [RFC] Fix TLS and Coroutine

[Hongtao Yu via Phabricator via cfe-commits]

hoy added inline comments.


================
Comment at: llvm/include/llvm/IR/Intrinsics.td:1309
+// Intrinsic to obtain the address of a thread_local variable.
+def int_threadlocal : Intrinsic<[llvm_ptr_ty], [llvm_ptr_ty]>;
+
----------------
lxfind wrote:
> hoy wrote:
> > lxfind wrote:
> > > hoy wrote:
> > > > hoy wrote:
> > > > > With the intrinsic, can TLS variable reference in the same coroutine or regular routine be DCE-ed anymore?
> > > > Sorry, I meant CSE-ed.
> > > Since the intrinsics does not have readnone attribute, it won't be CSE-ed before CoroSplit.
> > > However after CoroSplit, it will be lowered back to the direct reference of the TLS, and will be CSE-ed by latter passes.
> > > I can add a test function to demonstrate that too.
> > Sounds good. Can you please point out what optimization passes CSE-ed tls reference without this implementation? I'm wondering if those optimizations can be postponed to after CoroSplit. 
> To clarify, it wasn't just CSE that would merge the references of the same TLS.
> For instance, without this patch, a reference to "tls_variable" will just be "@tls_variable". For code like this:
> 
>   @tls_variable = internal thread_local global i32 0, align 4
> 
>   define i32* @foo(){
>     ret i32* @tls_variable
>   }
>   
>   define void @bar() {
>     %tls1 = call i32* @foo()
>     ..coro.suspend..
>     %tls2 = call i32* @foo()
>     %cond = icmp eq i32* %tls1, %tls2
>   }
> 
> When foo() is inlined into bar(), all uses of %tls1 will be replaced with @tls_variable.
Thanks for the explanation. I have a dumb question. Why isn't corosplit placed at the very beginning of the pipeline?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92661