[cfe-dev] [RFC] Should an illegal C++11 memory model in an atomic op be an error?

[Sean Silva]

Looks like it was deliberately demoted to a warning:

Sean:~/pg/llvm/tools/clang/include/clang/Basic % git log
-Swarn_atomic_op_has_invalid_memory_order

commit 7533ae94377aed5aa7866ebd67cbcf616efabb3c

Author: Tim Northover <tnorthover at apple.com>

Date:   Tue Mar 11 11:35:10 2014 +0000


    Sema: demote invalid atomic ordering message to warning.



    Someone could write:

      if (0) {

        __c11_atomic_load(ptr, memory_order_release);

      }



    or the equivalent, which is perfectly valid, so we shouldn't outright
reject

    invalid orderings on purely static grounds.



    rdar://problem/16242991



    git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@203564
91177308-0d34-0410-b5e6-96231b3b80d8


Tim, could you shed a bit more light on the context of this patch in
rdar://problem/16242991 ?

-- Sean Silva

On Fri, Jan 30, 2015 at 9:48 AM, Ristow, Warren <
warren_ristow at playstation.sony.com> wrote:

>  Hi,
>
>
>
> Using the builtin functions for atomic operations that honor the C++11
> memory model requirements will result in undefined behavior if an illegal
> memory model is specified.  Consequently, we can optimize-away code that
> uses the result of one of these incorrectly invoked atomic ops.
>
>
>
> For example, for the function:
>
>
>
>   type __atomic_load_n (type *ptr, int memmodel)
>
>
>
> the models __ATOMIC_RELEASE and __ATOMIC_ACQ_REL are not legal.  So in a
> test-case like:
>
>
>
>   int test_good(int* p_atomic, int* p1, int* p2) {
>
>     // __ATOMIC_RELAXED memmodel is legal for __atomic_load_n()
>
>     int v1 = *p1;
>
>     int atomic_val = __atomic_load_n(p_atomic, __ATOMIC_RELAXED);
>
>     int v2 = *p2;
>
>     return v1 + v2 + atomic_val;
>
>   }
>
>
>
>   int test_bad(int* p_atomic, int* p1, int* p2) {
>
>     // __ATOMIC_RELEASE memmodel is illegal for __atomic_load_n()
>
>     int v1 = *p1;
>
>     int atomic_val = __atomic_load_n(p_atomic, __ATOMIC_RELEASE);
>
>     int v2 = *p2;
>
>     return v1 + v2 + atomic_val;
>
>   }
>
>
>
> since the 'test_bad()' function is using an illegal memmodel parameter,
> the behavior is undefined.  Given that that function uses the result
> ('atomic_val') of that illegal atomic op in the return expression, we
> notice that the return-value of the function is undefined, and therefore at
> -O2, we legitimately optimize-away anything that feeds into computing that
> return value.
>
>
>
> Prior to Clang 3.5, this illegal memmodel was accepted without a
> diagnostic (even with -Weverything).  Beginning with 3.5, we produce:
>
>
>
>   warning: memory order argument to atomic operation is invalid
>
>
>
> So it's now better for users.  But since we freely optimize-away
> computations that are based on the result of one of these illegal memmodel
> uses, it seems like it would be reasonable to treat it as an error, rather
> than a warning.
>
>
>
> What do people think?
>
>
>
> FTR, trying this with g++ (version 4.8.2), shows that GNU considers it an
> error:
>
>
>
>   error: invalid memory model for `__atomic_load'
>
>
>
> Thanks,
>
> -Warren
>
>
>
> --
>
> Warren Ristow
>
> SN Systems - Sony Computer Entertainment Group
>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20150202/76eeb560/attachment.html>