[PATCH] D97656: [llvm-objcopy] Initial XCOFF32 support.

Esme Yi via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Sep 17 03:49:31 PDT 2021


Esme added inline comments.


================
Comment at: llvm/tools/llvm-objcopy/llvm-objcopy.cpp:209
+  } else if (auto *XCOFFBinary = dyn_cast<object::XCOFFObjectFile>(&In)) {
+    Expected<const XCOFFConfig &> XCOFFConfig = Config.getXCOFFConfig();
+    if (!XCOFFConfig)
----------------
jhenderson wrote:
> Are there likely to be any XCOFF-specific options added in the future? If not, I wonder if we could drop `XCOFFConfig` entirely. We'd still want the check for unsupported options, but perhaps that should be in a different function then (e.g. `Config.checkXCOFFOptions()`). What do you think?
> 
> (If there are XCOFF-specific options that are planned, having a separate config, even though empty for now, is fine).
There isn't a concrete plan yet, but I think it is quite possible to add XCOFF-specific options in the future. So I think it's fine to keep this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97656



More information about the llvm-commits mailing list