[PATCH] D71101: [lld][RISCV] Use an e_flags of 0 if there are only binary input files.

Alex Bradbury via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Dec 12 05:30:43 PST 2019


asb added inline comments.


================
Comment at: lld/ELF/Arch/RISCV.cpp:109
 uint32_t RISCV::calcEFlags() const {
-  assert(!objectFiles.empty());
+  if (objectFiles.empty())
+    return 0;
----------------
ruiu wrote:
> jrtc27 wrote:
> > MaskRay wrote:
> > > For reasonable change that applies to other targets as well, we should try making them generic.
> > There are only 6 architectures that override calcEFlags to be non-zero:
> > 
> > AMDGPU: Also has the same assert
> > ARM: Conditioned solely on config
> > Hexagon: Also has the same assert
> > Mips: Defaults to 0 if the list is empty
> > PPC64: Hard-coded to 2 (for the ABI version), but checks any input if present (allowed to be empty)
> > RISC-V: This
> > 
> > calcEFlags is already very target-specific and inconsistent, so I don't know how you intend to make this generic?
> I think defaulting to 0 is fine, as that is the best that we could do if no object files are given. A short comment explaining that objectFiles is empty if for example the only file given to the linker is `-b <somefile>` and we don't want to crash but instead create an output would help readers of the code, though.
I agree with others on this review that defaulting to 0 seems safe here, but I feel the rationale and reasoning needs to be captured:
* In the commit message if/when this lands
* At least in brief as a comment here in the code


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71101





More information about the llvm-commits mailing list