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

[Petr Hosek via Phabricator via cfe-commits]

phosek accepted this revision.
phosek added a comment.

LGTM



================
Comment at: llvm/lib/InterfaceStub/IFSStub.cpp:70
+  }
+  llvm_unreachable("unknown bitwidth");
+}
----------------
This shouldn't be needed.


================
Comment at: llvm/lib/InterfaceStub/IFSStub.cpp:82
+  }
+  llvm_unreachable("unknown endianness");
+}
----------------
This shouldn't be needed.


================
Comment at: llvm/lib/InterfaceStub/IFSStub.cpp:65
+uint8_t elfabi::convertIFSBitWidthToELF(IFSBitWidthType BitWidth) {
+  return BitWidth == IFSBitWidthType::IFS32 ? ELF::ELFCLASS32 : ELF::ELFCLASS64;
+}
----------------
haowei wrote:
> phosek wrote:
> > I'd consider using `switch` here so if someone adds a new entry to the `IFSBitWidthType` enum, it gets caught by the compiler.
> I changed it to switch and I added `llvm_unreachable("unkown bitwidth");` to cases where an unknown bitwidth or endianness is used. Please let me know if it is OK to do so.
> 
> The Unknown enum is reserved for ifs file that has unknown bitwidth or endianness field and will trigger ifs tool's early termination. So it shouldn't be seen when IFS library is writing an ELF file.
LG


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99810