keith.walker.arm added inline comments.
================
Comment at: lib/Driver/ToolChains/Arch/ARM.cpp:406
+ const bool HasVFPv4 = (std::find(ItBegin, ItEnd, "+vfpv4") != ItEnd);
+ const bool HasFParmv8 = (std::find(ItBegin, ItEnd, "+fp-armv8") != ItEnd);
+ const bool HasFullFP16 = (std::find(ItBegin, ItEnd, "+fullfp16") !=
ItEnd);
----------------
efriedma wrote:
> I don't like explicitly enumerating the features like this; it'll mess up if
> there's ever a new feature which isn't explicitly enumerated here. Can we
> just do `Features.push_back("-vfpv2")` and depend on that to implicitly
> disable all the other vfp features?
I'll check on whether disabling a feature on which other features depend
automatically disables the other features is something that can be relied upon
(It would seem sensible that one could .... but need to check).
That would certainly mean the code could be simplified, although I would also
need to check the impact on the what is checked in the testing (only test for
the base feature being disabled because the dependent features are
automatically disabled).
================
Comment at: lib/Driver/ToolChains/Arch/ARM.cpp:419
+ Features.push_back("-fullfp16");
+
+ const bool HasNeon = (std::find(ItBegin, ItEnd, "+neon") != ItEnd);
----------------
compnerd wrote:
> It would be nice to not have these explicitly listed. But at the very least,
> I think that having a list and looping through it would be better:
>
> for (const auto Feature : {"vfpv2", "vfpv3", "vfpv4", "fp-armv8",
> "fullfp16"})
> if (std::find(std::begin(Features), std::end(Features), "+" + Feature)
> == std::end(Features))
> continue;
> else
> Features.push_back("-" + Feature);
This certainly looks a better way to do it if we do need to provide a list of
features rather than relying on disabling a base feature on which the other
features depend.
https://reviews.llvm.org/D40256
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
