[PATCH] D14184: [clang] Add initial support for -meabi flag
Vinicius Tinti via cfe-commits
cfe-commits at lists.llvm.org
Fri Oct 30 10:24:52 PDT 2015
tinti added inline comments.
================
Comment at: lib/CodeGen/BackendUtil.cpp:524
@@ +523,3 @@
+ .Case("5", llvm::EABI::Eabi5)
+ .Case("gnu", llvm::EABI::Gnu)
+ .Default(llvm::EABI::Default);
----------------
compnerd wrote:
> I'd really rather see this written in the LLVM Style:
>
> EABI4, EABI5, GNU
>
> since all of them are initialisms.
>
> BTW, please clang-format this change.
Agreed.
================
Comment at: lib/Frontend/CompilerInvocation.cpp:458
@@ +457,3 @@
+ StringRef Value = A->getValue();
+ bool Valid = llvm::StringSwitch<bool>(Value)
+ .Case("default", true)
----------------
compnerd wrote:
> If `llvm::EABI::EABIVersionType` had an `Invalid` value in the enumeration, you could convert directly to the value here, and report the error if the value was `Invalid`.
I chose this way because none of the other target options have it [1].
Do you prefer with it?
[1] https://github.com/llvm-mirror/llvm/blob/master/include/llvm/Target/TargetOptions.h#L27
http://reviews.llvm.org/D14184
More information about the cfe-commits
mailing list