[PATCH] D99810: [ifs] Prepare llvm-ifs for elfabi/ifs merging.

Petr Hosek via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jun 14 00:55:34 PDT 2021


phosek accepted this revision.
phosek added inline comments.
This revision is now accepted and ready to land.


================
Comment at: llvm/include/llvm/InterfaceStub/ELFObjHandler.h:25
 
 namespace elfabi {
 
----------------
Shouldn't this be in namespace `ifs`?


================
Comment at: llvm/include/llvm/InterfaceStub/IFSStub.h:82-83
+  return true;
+}
+inline bool operator!=(const IFSTarget &Lhs, const IFSTarget &Rhs) {
+  return !(Lhs == Rhs);
----------------
Use empty line between functions.


================
Comment at: llvm/lib/InterfaceStub/IFSStub.cpp:57-61
+  if (!Triple && !ObjectFormat && !Arch && !ArchString && !Endianness &&
+      !BitWidth) {
+    return true;
+  }
+  return false;
----------------
This can be simplified.


================
Comment at: llvm/lib/InterfaceStub/IFSStub.cpp:65
+uint8_t elfabi::convertIFSBitWidthToELF(IFSBitWidthType BitWidth) {
+  return BitWidth == IFSBitWidthType::IFS32 ? ELF::ELFCLASS32 : ELF::ELFCLASS64;
+}
----------------
I'd consider using `switch` here so if someone adds a new entry to the `IFSBitWidthType` enum, it gets caught by the compiler.


================
Comment at: llvm/lib/InterfaceStub/IFSStub.cpp:69
+uint8_t elfabi::convertIFSEndiannessToELF(IFSEndiannessType Endianness) {
+  return Endianness == IFSEndiannessType::Little ? ELF::ELFDATA2LSB
+                                                 : ELF::ELFDATA2MSB;
----------------
Ditto here.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D99810/new/

https://reviews.llvm.org/D99810



More information about the cfe-commits mailing list