[PATCH] D52416: Allow FP types for atomicrmw xchg

[Matt Arsenault via Phabricator via llvm-commits]

arsenm marked an inline comment as done.
arsenm added a comment.

xxz



================
Comment at: lib/CodeGen/AtomicExpandPass.cpp:500
+
+  bool NeedBitcast = OrigTy->isFloatingPointTy();
+  if (NeedBitcast) {
----------------
jfb wrote:
> arsenm wrote:
> > jfb wrote:
> > > jyknight wrote:
> > > > arsenm wrote:
> > > > > jyknight wrote:
> > > > > > Add a comment here that this code can go away if the cmpxchg instruction adds support for floating point types.
> > > > > I think there might need to be a separate fcmpxchg instruction for that, unless you mean there will also be a version that treats the FP type here as integer in memory
> > > > No -- the intent is not to compare for floating-point-equality ala fcmp, but rather just as bit equality. (e.g. NaNs are equal to each-other when they have the same bit representation, and unequal if they do not)
> > > This is what various ISAs and programming languages do, so I support what James says :)
> > AMDGPU does also have fcmpxchg, which does an FP compare, with nans failing etc.
> Off topic... but wat?!? Once you have a NaN your fcmpxchg infloops? Count me confused.
It doesn't let you put the NaN in? As long as the memory was initialized with something not-NaN before I think it works? Otherwise you're stuck with NaNs forever. I haven't tried using it, but that's my interpretation of the manual.


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

https://reviews.llvm.org/D52416