[PATCH] D150985: [clang] Allow fp in atomic fetch max/min builtins

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.


================
Comment at: clang/lib/Sema/SemaChecking.cpp:6576-6578
       if (!ValType->isFloatingType())
         return false;
+      if (!(AllowedType & AOAVT_FP))
----------------
Collapse into a single if statement: `if (!(ValType->isFloatingType() && (AllowedType & AOAVT_FP)))`


================
Comment at: clang/lib/Sema/SemaChecking.cpp:6588
+    if (!IsAllowedValueType(ValType, ArithAllows)) {
+      assert(ArithAllows & AOAVT_Integer);
+      auto DID = ArithAllows & AOAVT_FP
----------------
Why do we expect a failed `IsAllowedValueType` check to fail only if we were allowed integers? Is that because we assume that all atomic instructions support integers?

If that's the case, I'd hoist the assertion and apply it right after we're done setting `ArithAllows`. Alternatively, we could discard `AOAVT_Integer` and call the enum `ArithOpExtraValueType`. Tracking a bit that's always set does not buy us much, though it does make the code a bit more uniform. Up to you.





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

https://reviews.llvm.org/D150985