[PATCH] D60697: [ARM] Allow "-march=foo+fp" to vary with foo.

Tim Northover via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Apr 15 07:16:39 PDT 2019

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.

  rG LLVM Github Monorepo



More information about the cfe-commits mailing list