SjoerdMeijer added inline comments.
================
Comment at: clang/lib/Driver/ToolChains/Arch/ARM.cpp:103
std::vector<StringRef> &Features,
+ std::vector<StringRef> &FeaturesAfter,
const llvm::Triple &Triple) {
----------------
Nitpick: can you comment in code somewhere what the purpose is of
`FeaturesAfter`? Can it be renamed to something more descriptive? I don't know
what it could be to be honest...
================
Comment at: clang/lib/Driver/ToolChains/Arch/ARM.cpp:449
// as well.
- for (std::string Feature : {"vfp2", "vfp3", "vfp4", "fp-armv8", "fullfp16",
- "neon", "crypto", "dotprod", "fp16fml"})
- if (std::find(std::begin(Features), std::end(Features), "+" + Feature)
!= std::end(Features))
- Features.push_back(Args.MakeArgString("-" + Feature));
-
- // Disable the base feature unconditionally, even if it was not
- // explicitly in the features list (e.g. if we had +vfp3, which
- // implies it).
- Features.push_back("-fpregs");
+ for (std::string Feature : {
+ "vfp2", "vfp2sp", "vfp2d16", "vfp2d16sp",
----------------
This function and this code is truly one of the worst I guess.... I mean, it's
not worse than it was before, but you'd really hope that there was a
(targetparser) API that gives you a list of FPUs, possibly matching some
criteria, rather than yet another list of hard coded strings. It gets worse,
this is repeated in `lib/Support/ARMTargetParser.cpp`. End of rant! ;-)
More on topic: do we need to add +mve.fp here?
================
Comment at: llvm/lib/Support/ARMTargetParser.cpp:178
+
+ {"+fpregs", "-fpregs", FPUVersion::VFPV2, FPURestriction::SP_D16},
+ {"+vfp2", "-vfp2", FPUVersion::VFPV2, FPURestriction::None},
----------------
This looks familiar.... I don't think we can easily simplify this? Or get rid
of? I guess that's best done in a follow up when we this working again? Perhaps
the only insignificant simplification is that we don't need the +<string> and
-<string> because they are the same.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D62998/new/
https://reviews.llvm.org/D62998
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
