[PATCH] D71225: [OpenMP][WIP] atomic update only evaluate once for expression having side effect

Alexey Bataev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Dec 9 16:25:46 PST 2019


ABataev added a comment.

In D71225#1776113 <https://reviews.llvm.org/D71225#1776113>, @jdoerfert wrote:

> In D71225#1776105 <https://reviews.llvm.org/D71225#1776105>, @cchen wrote:
>
> > Oops, accidentally remove my own comment. I'm not sure why `iarr[foo(), foo(), 0]` violate the rule since it will be evaluated as `iarr[0]`, and the counter `foo()` modified is also in the same location for both left hand side and right hand side.
>
>
> It's not a violation but it taps into the unspecified behavior, which in combination with side effects seems to be worth a warning.
>
> So my reasoning is:
>
> //x := `iarr[foo(), foo(), 0]`//
>
> which is fine on its own. Using it in the atomic update of the form,
>
> // x = x + 1//
>
> also fine.
>
> However, the standard says the number of times //x// is evaluated is unspecified. 
>  Thus, transforming the above to
>
> // x += 1 //
>
> should be valid. So should be,
>
> // t = &x; *t = *t + 1 //
>
> and even
>
> // x = (x, x, x, x+1) //


Agree, missed that he used commas instead of array expressions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71225





More information about the cfe-commits mailing list