[PATCH] D34781: Introduce a MCReloc class

[Rafael Avila de Espindola via llvm-commits]

Reid Kleckner via Phabricator <reviews at reviews.llvm.org> writes:

> rnk added a comment.
>
> I read through this, but it's hard to see exactly the simplifications that this will yield. I like the follow-up patch to merge PCRel handling between ELF, COFF, and wasm, though.

I think a good example is ARMELFObjectWriter::GetRelocTypeInner. It will
not need the IsPCRel argument since the kind will be different for pcrel
and non pcrel cases.

> ================
> Comment at: include/llvm/MC/MCFixup.h:120
> +  SMLoc Loc;
> +  uint32_t Offset;
> +  const MCSymbolRefExpr *RefA;
> ----------------
> Can you bring along the doxygen comment on MCFixup::Offset? This is typically a small value, it's an offset into the last instruction, not an offset into a fragment or a section.
>
> In general, documenting all these fields would help explain what an MCReloc *is*.

Will do.

> ================
> Comment at: lib/MC/ELFObjectWriter.cpp:630
>                   MCFixupKindInfo::FKF_IsPCRel;
> +  MCReloc &Target = Fixup;
> +  uint64_t &FixedValue = Fixup.getConstant();
> ----------------
> I take it you plan to rename these things soon, this is just a hack to avoid tons of renames in the diff.

Correct. Without it it is very hard to see what actually changes:

* The Kind value is always current.
* The scattered relocation code is in charge of adding the offset in the
  file, not just the offset of the section.

Cheers,
Rafael