[PATCH] D71843: [ARM][MVE] MVE-I should not be disabled by -mfpu=none

[Simon Tatham via Phabricator via llvm-commits]

simon_tatham accepted this revision.
simon_tatham added a comment.
This revision is now accepted and ready to land.

LGTM, with a request for an extra comment.



================
Comment at: clang/lib/Driver/ToolChains/Arch/ARM.cpp:471
+        !llvm::is_contained(Features, "+mve.fp"))
+      Features.emplace_back("-fpregs");
   }
----------------
This confused me on the first three readings – you carefully add `-mve.fp`, and then test whether `+mve.fp` is in the list?

It made sense to me on the fourth reading. If I've understood correctly, the point is that if `mve.fp` was previously enabled, and we have just disabled it, then `mve` (integer-only) is still required, and hence, so are `fpregs`. And the same goes if `mve` itself is enabled in the feature list.

So I think this is right, but I also think it could use a comment spelling out the reasoning.


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

https://reviews.llvm.org/D71843