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

[Xun Li via Phabricator via cfe-commits]

lxfind 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]>;
+
----------------
hoy wrote:
> 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?
The coroutine frame size is determined during CoroSplit. So if CoroSplit happens too early without any optimizations, the frame size will always be very big and there is no chance to optimize it.
This is indeed a fundamental trade-off. If CoroSplit happens much earlier then it will be immune to this kind of problem.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92661