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

[Xun Li via Phabricator via llvm-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:
> > > 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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92661