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

Kyle Evans via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 16 20:54:44 PST 2019


kevans added inline comments.


================
Comment at: lld/ELF/Arch/RISCV.cpp:109
 uint32_t RISCV::calcEFlags() const {
-  assert(!objectFiles.empty());
+  if (objectFiles.empty())
+    return 0;
----------------
asb wrote:
> 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
Minor correction: mips new-ish behavior is to derive eflags from emulation string if the input list is empty, which was introduced initially to fix FreeBSD kmod builds on mips n32, since ABI is described here and lld refuses to link later due to ABI mismatch otherwise.


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