[PATCH] D71726: Let clang atomic builtins fetch add/sub support floating point types

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

@jyknight - James, do you have further concerns about the patch?



================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:6454
+    bool DiagAtomicLibCall = true;
+    for (auto *A : Args.filtered(options::OPT_W_Joined)) {
+      if (StringRef(A->getValue()) == "no-error=atomic-alignment")
----------------
If we rely on promoting the warnings to errors for correctness, I think we may need a more robust mechanism to enforce that than trying to guess the state based on provided options.
E.g. can these diagnostics be enabled/disabled with a wider scope option like `-W[no-]extra` or `-W[no-]all`?

Maybe we should add a cc1-only option `--enforce-atomic-alignment` and use that to determine if misalignment should be an error at the point where we issue the diagnostics?



================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:6457
+        DiagAtomicLibCall = false;
+      if (StringRef(A->getValue()) == "error=atomic-alignment")
+        DiagAtomicLibCall = true;
----------------
This should be `else if`, or,  maybe use `llvm::StringSwitch()`instead:
```
DiagAtomicLibCall = llvm::StringSwitch<bool>(A->getValue())
   .Case("no-error=atomic-alignment", false)
   .Case("error=atomic-alignment", true)
   .Default(DiagAtomicLibCall)
```


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

https://reviews.llvm.org/D71726