Changed Opts.EABIVersion type string to llvm::EABI enum class

[yamaguchi]

Summary

Changed EABIVersion type from string to llvm::EABI.
It seems it was just a typo and this is intended implementation.

[ruiu]

Nice find!

Looks like the original intention of this code is to convert a string to an enum value and then initialize Opts.EABIVersion with the enum. So I wonder if we should change the type of Opts.EABIVersion (which is defined in clang/include/clang/Basic/TargetOptions.h) from std::string to llvm::EABI and keep the StringSwitch.

[yamaguchi]

Update review and diff.

[ruiu]

clang/lib/Frontend/CompilerInvocation.cpp

2550 ↗

(On Diff #104976)

How about assigning directly to Opts.EABIVersion? If you do, you can eliminate this temporary variable.

[yamaguchi]

clang/lib/Frontend/CompilerInvocation.cpp

2550 ↗

(On Diff #104976)

@ruiu
I thought about it, but I was not sure if we can set llvm::EABI::Unknown to Opts.EABIVersion.

[ruiu]

clang/lib/Frontend/CompilerInvocation.cpp

2550 ↗

(On Diff #104976)

In terms of your system, you can. And if you abort when it is Unknown, it doesn't matter whether it's unknown or not, no?

[ruiu]

clang/lib/Frontend/CompilerInvocation.cpp

2550 ↗

(On Diff #104976)

Sorry for the silly typo. I meant "in terms of the type system".

[yamaguchi]

clang/lib/Frontend/CompilerInvocation.cpp

2550 ↗

(On Diff #104976)

I'm not very happy about setting invalid value to Opts.EABIVersion, but I will change this if you strongly feel so.

[ruiu]

LGTM

clang/lib/Frontend/CompilerInvocation.cpp

2550 ↗

(On Diff #104976)

I'm fine with your code. Mine was just a suggestion.

[teemperor]

LGTM.

cfe/trunk/include/clang/Basic/TargetOptions.h

 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_BASIC_TARGETOPTIONS_H
 #define LLVM_CLANG_BASIC_TARGETOPTIONS_H
 
 #include <string>
 #include <vector>
 #include "clang/Basic/OpenCLOptions.h"
+#include "llvm/Target/TargetOptions.h"
 
 namespace clang {
 
 /// \brief Options for controlling the target.
 class TargetOptions {
 public:
   /// The name of the target triple to compile for.
   std::string Triple;
 
   /// When compiling for the device side, contains the triple used to compile
   /// for the host.
   std::string HostTriple;
 
   /// If given, the name of the target CPU to generate code for.
   std::string CPU;
 
   /// If given, the unit to use for floating point math.
   std::string FPMath;
 
   /// If given, the name of the target ABI to use.
   std::string ABI;
 
   /// The EABI version to use
-  std::string EABIVersion;
+  llvm::EABI EABIVersion;
 
   /// If given, the version string of the linker in use.
   std::string LinkerVersion;
 
   /// \brief The list of target specific features to enable or disable, as written on the command line.
   std::vector<std::string> FeaturesAsWritten;
 
   /// The list of target specific features to enable or disable -- this should

cfe/trunk/lib/Basic/Targets.cpp

ARMTargetInfo(const llvm::Triple &Triple, const TargetOptions &Opts)
     // the alignment of the zero-length bitfield is greater than the member
     // that follows it, `bar', `bar' will be aligned as the  type of the
     // zero length bitfield.
     UseZeroLengthBitfieldAlignment = true;
 
     if (Triple.getOS() == llvm::Triple::Linux ||
         Triple.getOS() == llvm::Triple::UnknownOS)
       this->MCountName =
-          Opts.EABIVersion == "gnu" ? "\01__gnu_mcount_nc" : "\01mcount";
+          Opts.EABIVersion == llvm::EABI::GNU ? "\01__gnu_mcount_nc" : "\01mcount";
   }
 
   StringRef getABI() const override { return ABI; }
 
   bool setABI(const std::string &Name) override {
     ABI = Name;
 
     // The defaults (above) are for AAPCS, check if we need to change them.
AArch64TargetInfo(const llvm::Triple &Triple, const TargetOptions &Opts)
     UseZeroLengthBitfieldAlignment = true;
 
     // AArch64 targets default to using the ARM C++ ABI.
     TheCXXABI.set(TargetCXXABI::GenericAArch64);
 
     if (Triple.getOS() == llvm::Triple::Linux)
       this->MCountName = "\01_mcount";
     else if (Triple.getOS() == llvm::Triple::UnknownOS)
-      this->MCountName = Opts.EABIVersion == "gnu" ? "\01_mcount" : "mcount";
+      this->MCountName = Opts.EABIVersion == llvm::EABI::GNU ? "\01_mcount" : "mcount";
   }
 
   StringRef getABI() const override { return ABI; }
   bool setABI(const std::string &Name) override {
     if (Name != "aapcs" && Name != "darwinpcs")
       return false;
 
     ABI = Name;

cfe/trunk/lib/CodeGen/BackendUtil.cpp

static void initTargetOptions(llvm::TargetOptions &Options,
   }
 
   Options.UseInitArray = CodeGenOpts.UseInitArray;
   Options.DisableIntegratedAS = CodeGenOpts.DisableIntegratedAS;
   Options.CompressDebugSections = CodeGenOpts.getCompressDebugSections();
   Options.RelaxELFRelocations = CodeGenOpts.RelaxELFRelocations;
 
   // Set EABI version.
-  Options.EABIVersion = llvm::StringSwitch<llvm::EABI>(TargetOpts.EABIVersion)
-                            .Case("4", llvm::EABI::EABI4)
-                            .Case("5", llvm::EABI::EABI5)
-                            .Case("gnu", llvm::EABI::GNU)
-                            .Default(llvm::EABI::Default);
+  Options.EABIVersion = TargetOpts.EABIVersion;
 
   if (LangOpts.SjLjExceptions)
     Options.ExceptionModel = llvm::ExceptionHandling::SjLj;
 
   Options.NoInfsFPMath = CodeGenOpts.NoInfsFPMath;
   Options.NoNaNsFPMath = CodeGenOpts.NoNaNsFPMath;
   Options.NoZerosInBSS = CodeGenOpts.NoZeroInitializedInBSS;
   Options.UnsafeFPMath = CodeGenOpts.UnsafeFPMath;

cfe/trunk/lib/Frontend/CompilerInvocation.cpp

llvm::EABI EABIVersion = llvm::StringSwitch<llvm::EABI>(Value)
                                  .Case("4", llvm::EABI::EABI4)
                                  .Case("5", llvm::EABI::EABI5)
                                  .Case("gnu", llvm::EABI::GNU)
                                  .Default(llvm::EABI::Unknown);
     if (EABIVersion == llvm::EABI::Unknown)
       Diags.Report(diag::err_drv_invalid_value) << A->getAsString(Args)
                                                 << Value;
     else
-      Opts.EABIVersion = Value;
+      Opts.EABIVersion = EABIVersion;
   }
   Opts.CPU = Args.getLastArgValue(OPT_target_cpu);
   Opts.FPMath = Args.getLastArgValue(OPT_mfpmath);
   Opts.FeaturesAsWritten = Args.getAllArgValues(OPT_target_feature);
   Opts.LinkerVersion = Args.getLastArgValue(OPT_target_linker_version);
   Opts.Triple = llvm::Triple::normalize(Args.getLastArgValue(OPT_triple));
   Opts.Reciprocals = Args.getAllArgValues(OPT_mrecip_EQ);
   // Use the default target triple if unspecified.