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

Artem Belevich via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Feb 2 10:01:32 PST 2021


tra added a comment.

In D71726#2536966 <https://reviews.llvm.org/D71726#2536966>, @jyknight wrote:

> My concern is that this is treating a backend _bug_ as if it were just an optional feature. But it's not the case that it might be reasonable to either implement or not implement this in a backend -- it should be implemented, and those that don't are buggy.
>
> I'd be happier with just having an ISEL failure when you try to use fp atomics on broken targets, rather than adding all this code and configuration to Clang in order to avoid that. (And, of course, the target maintainers should also fix them)

+1. I agree with James.

Removing code is often harder than adding it. When you're adding things, you're the only user. Once things are in, they will start growing dependencies that will need to be dealt with if you ever want to remove the code.

Clean solution that works for AMDGPU only for now is better than a potentially permanent workaround.


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

https://reviews.llvm.org/D71726



More information about the cfe-commits mailing list