simon_tatham added inline comments.
================
Comment at: clang/lib/Driver/ToolChains/Arch/ARM.cpp:151
+                             const std::vector<StringRef> &Features) {
+  if (llvm::find(Features, "-fpregs") == Features.end())
+    return;
----------------
This whole patch hinges on the unspoken assumption that this is a correct way 
to test for the absence of FP registers, i.e. that `"-fpregs"` will //always// 
appear in `Features` if there are no FP registers.

Could you add a comment stating that assumption explicitly and explaining why 
it's valid? In particular, why you're confident that the absence of FP 
registers might not sometimes be signalled by the lack of either `"-fpregs"` or 
`"+fpregs"` in that list.

(In a quick look at the code I //think// you're right, but I'm not 100% 
confident, and if you show your working then people will be able to work out 
what went wrong later if things change.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150902

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to