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

[Ristow, Warren]

I'm convinced that it's best to keep it as a warning.

In reading over the responses, one thing that comes to mind which I think is analogous is
a shift by a literal amount that's too large.  For example, a shift of a 4-byte 'int' by
the literal 32 results in a warning, not an error:

  unsigned int foo(unsigned int a0, unsigned int a1)
  {
    return (a0 >> 32) + a1;
  }

gets:

  warning: shift count >= width of type [-Wshift-count-overflow]

So for this, we give a warning.  And at -O2, we legitimately optimize-away all expressions
based on the result of the undefined-behavior-shift (producing just a 'return' in the
above case).  I wouldn't consider changing the warning that comes for the above shift to
an error, and so by the same token, I no longer consider an incorrect (literal)
memory-model parameter for an atomic op to be a good candidate for an error.  Keeping it
a warning keeps us consistent in this regard.

Thanks all, for the feedback.
-Warren

--
Warren Ristow
SN Systems - Sony Computer Entertainment Group

From: metafoo at gmail.com [mailto:metafoo at gmail.com] On Behalf Of Richard Smith
Sent: Monday, February 02, 2015 5:07 PM
To: Tim Northover
Cc: Reid Kleckner; cfe-dev at cs.uiuc.edu; Ristow, Warren
Subject: Re: [cfe-dev] [RFC] Should an illegal C++11 memory model in an atomic op be an error?

On Mon, Feb 2, 2015 at 1:50 PM, Tim Northover <tnorthover at apple.com<mailto:tnorthover at apple.com>> wrote:
> I'd be in favor or promoting it to an error. We recently started rejecting this code, for example:

I'm not sure I would. Deciding on the diagnostics for our own intrinsics is one thing, but this would trigger on well-formed programs according to the standard.

I agree that this should not be an error. The builtin does not require a constant; we shouldn't say it's ill-formed if we happen to be able to evaluate the order parameter and find it's out-of-range.

I think this sort of thing is completely reasonable (perhaps in the implementation of a C++11-compatible atomics library):

#define atomic_builtin(kind, order, ...) \
  ({ switch(order) { \
    case memory_order_relaxed: kind(__VA_ARGS__, __ATOMIC_RELAXED); break; \
    [...]

That's rather a bigger step to take, and we should probably be comparing it to situations with that similarity (the only one I know of is ARM64 complaining more strongly about functions without prototypes).

Cheers.

Tim.
_______________________________________________
cfe-dev mailing list
cfe-dev at cs.uiuc.edu<mailto: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/20150203/d27a8c8e/attachment.html>