[PATCH] D130142: [Coroutines] Introduce @llvm.coro.tls.wrapper to block optimizations

[Chuanqi Xu via Phabricator via llvm-commits]

ChuanqiXu added a comment.

In D130142#3667305 <https://reviews.llvm.org/D130142#3667305>, @ychen wrote:

> In D130142#3667280 <https://reviews.llvm.org/D130142#3667280>, @ChuanqiXu wrote:
>
>> In D130142#3666355 <https://reviews.llvm.org/D130142#3666355>, @ychen wrote:
>>
>>> John mentioned this test case,
>>>
>>>   thread_local int x = 0;
>>>   void helper(int *x, int y);
>>>   my_coro coroutine() {
>>>     helper(&x, co_await something_suspending());
>>>   }
>>>
>>> Is replacing the TLS variable with `llvm.coro.tls.wrapper` in CoroEarly too late to fix the issue?
>>
>> This example may be fine since C++ standard specifies that the order of evaluation of function arguments is unspecified: https://en.cppreference.com/w/cpp/language/eval_order.
>
> That's my understanding as well. Sorry I should've changed it a little,
>
>   thread_local int x = 0;
>   void helper(int *x, int y);
>   my_coro coroutine() {
>     helper(&x, (&x, co_await something_suspending()));
>   }
>
> The comma operator should have the left-to-right evaluation order. Personally, I hope there is a workaround for Clang 15. I'm just trying to convince myself that this workaround solves both issues in principle (even with potential pessimization with the TLS variables). But this test case makes me hesitate.

For the specific example, it is fine since `&x` would be deprecated due to it has no side effect as a comma operator. I guess this example fits your intention more:

  thread_local int tls_variable = 0;
  void helper(int *x);
  int *helper1(int *x);
  task coro() {
      helper((helper1(&tls_variable), co_await MyAwaiter()));
  }

In this example, `&tls_variable` would be evaluated before the await expression: https://godbolt.org/z/xse1q57ze


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130142