[RISCV] Avoid save-restore target feature warning

[lenary]

Summary

LLVM issues a warning if passed unknown target features. Neither I nor
@asb noticed this until after https://reviews.llvm.org/D63498 landed.

This patch stops passing the (unknown) "save-restore" target feature to
the LLVM backend, but continues to emit a warning if a driver asks for
-msave-restore. The default of assuming -mno-save-restore (and
emitting no warnings) remains.

[asb]

LGTM, thanks.

[jrtc27]

For historical documentation purposes:

I don't think this ever actually fully worked. Whilst it stops passing the feature explicitly here, and thus means no target feature is passed when you don't specify either driver option, -m(no-)save-restore is still in m_riscv_Features_Group and so the later call to handleTargetFeaturesGroup still automatically adds [+-]save-restore if you specify one of the options on the command line, meaning -mno-save-restore still warned in Clang 9 and 10 despite having this patch. Clang 11 stopped warning as the feature was finally implemented by then.

https://godbolt.org/z/Gqv3GG6n4

clang/lib/Driver/ToolChains/Arch/RISCV.cpp

void riscv::getRISCVTargetFeatures(const Driver &D, const ArgList &Args,
   }
 
   // -mrelax is default, unless -mno-relax is specified.
   if (Args.hasFlag(options::OPT_mrelax, options::OPT_mno_relax, true))
     Features.push_back("+relax");
   else
     Features.push_back("-relax");
 
-  // -mno-save-restore is default, unless -msave-restore is specified.
+  // GCC Compatibility: -mno-save-restore is default, unless -msave-restore is
+  // specified...
   if (Args.hasFlag(options::OPT_msave_restore, options::OPT_mno_save_restore, false)) {
-    Features.push_back("+save-restore");
-    // ... but we don't yet support +save-restore, so issue a warning.
+    // ... but we don't support -msave-restore, so issue a warning.
     D.Diag(diag::warn_drv_clang_unsupported)
       << Args.getLastArg(options::OPT_msave_restore)->getAsString(Args);
-  } else {
-    Features.push_back("-save-restore");
   }
 
   // Now add any that the user explicitly requested on the command line,
   // which may override the defaults.
   handleTargetFeaturesGroup(Args, Features, options::OPT_m_riscv_Features_Group);
 }
 
 StringRef riscv::getRISCVABI(const ArgList &Args, const llvm::Triple &Triple) {
   if (Arg *A = Args.getLastArg(options::OPT_mabi_EQ))
     return A->getValue();
 
   return Triple.getArch() == llvm::Triple::riscv32 ? "ilp32" : "lp64";
 }

clang/test/Driver/riscv-features.c

 // NO-RELAX: "-target-feature" "-relax"
 // DEFAULT: "-target-feature" "+relax"
 // DEFAULT-NOT: "-target-feature" "-relax"
 
 // RUN: %clang -target riscv32-unknown-elf -### %s -msave-restore 2>&1 | FileCheck %s -check-prefix=SAVE-RESTORE
 // RUN: %clang -target riscv32-unknown-elf -### %s -mno-save-restore 2>&1 | FileCheck %s -check-prefix=NO-SAVE-RESTORE
 
 // SAVE-RESTORE: warning: the clang compiler does not support '-msave-restore'
-// DEFAULT-NOT: warning: the clang compiler does not support
-
-// SAVE-RESTORE: "-target-feature" "+save-restore"
-// NO-SAVE-RESTORE: "-target-feature" "-save-restore"
-// DEFAULT: "-target-feature" "-save-restore"
-// DEFAULT-NOT: "-target-feature" "+save-restore"
+// NO-SAVE-RESTORE-NOT: warning: the clang compiler does not support
+// DEFAULT-NOT: warning: the clang compiler does not support