t.p.northover added a comment.
This needs some tests. I'm also not quite sure when you'd use bare "+fp", if
it's the default anyway.
================
Comment at: llvm/lib/Support/ARMTargetParser.cpp:476
-StringRef ARM::getArchExtFeature(StringRef ArchExt) {
- if (ArchExt.startswith("no")) {
- StringRef ArchExtBase(ArchExt.substr(2));
- for (const auto AE : ARCHExtNames) {
- if (AE.NegFeature && ArchExtBase == AE.getName())
- return StringRef(AE.NegFeature);
- }
+static bool wasNegated(StringRef &Name) {
+ if (Name.startswith("no")) {
----------------
I think `isNegated` is probably more in line with existing naming.
================
Comment at: llvm/lib/Support/ARMTargetParser.cpp:504-507
+ if (ArchExt == "fp" || ArchExt == "fp.dp") {
+ unsigned FPUKind;
+ if (Negated) {
+ FPUKind = ARM::FK_NONE;
----------------
Doesn't this mean `+nofp.dp` disables all floats? That seems surprising
behaviour to me, but I can't find any existing tests covering it.
================
Comment at: llvm/lib/Support/ARMTargetParser.cpp:516-517
+ const FPUName &DefaultFPU = FPUNames[FPUKind];
+ if (DefaultFPU.Restriction != FPURestriction::SP_D16)
+ return false; // meaningless for this arch
+
----------------
Doesn't this silently disable the FPU entirely if we decide "fp.dp" is useless?
That seems unlikely to be what a user wants, especially without a diagnostic.
Could you also expand on the comment a bit more. I had to look up exactly what
FPURestrictions existed to get this, and I'm not even 100% sure I'm right now.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D60697/new/
https://reviews.llvm.org/D60697
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
