[PATCH] D132541: [llvm-objcopy] Introduce 'ihex-flat' output format.

Tue Sep 6 01:40:38 PDT 2022

jhenderson added a comment.

llvm-objcopy CommandGuide docs (llvm/docs/CommandGuide) will need updating.

@MaskRay, any thoughts on this? I think it's a reasonable change, but am unsure whether it should be the "default" ihex output, or under the new format name. If the former, I'd be tempted to keep the old format around for the clients who need it, under a different name. The change should then definitely be mentioned in the release notes too.

================
Comment at: llvm/lib/ObjCopy/ELF/ELFObject.h:275
+  // Whether we're emitting segment:offset format at all
+  bool UseSegmentOffset;
+
----------------
I suggest `MayUseSegmentOffset` since, if I understand it correctly, you may need to use the other format even if this is `true`.

================
Comment at: llvm/tools/llvm-objcopy/ObjcopyOptions.cpp:645
                            .Case("ihex", FileFormat::IHex)
+                           .Case("ihex-flat", FileFormat::IHexFlat)
                            .Default(FileFormat::Unspecified);
----------------
To keep the code simpler, I think we should omit this `ihex-flat` as an input format: as far as I'm aware, there's no need to support it. Users can just use `ihex` for the input option. In my opinion, the symmetry isn't important: note that for `binary` format the input and output formats are essentially unrelated, so the "symmetry" there is, if anything, confusing, but a necessary evil of being compatible with GNU.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132541