This is an archive of the discontinued LLVM Phabricator instance.

[lanai] Add ELF enum value and relocations
ClosedPublic

Authored by jpienaar on Feb 8 2016, 4:35 PM.

Details

Summary

Add ELF enum value and relocations for Lanai backed.

General Lanai backend discussion on llvm-dev thread "[RFC] Lanai backend" (http://lists.llvm.org/pipermail/llvm-dev/2016-February/095118.html).

Diff Detail

Repository
rL LLVM

Event Timeline

jpienaar updated this revision to Diff 47270.Feb 8 2016, 4:35 PM
jpienaar retitled this revision from to [lanai] Add MCExpr and ELF relocations.
jpienaar updated this object.
jyknight added inline comments.
include/llvm/MC/MCExpr.h
294 ↗(On Diff #47270)

It looks like half the backends put target specific expr kinds in a subclass of MCTargetExpr, instead of in here. I'm not sure, but maybe that's the way you're supposed to do it now?

include/llvm/Object/RelocVisitor.h
324 ↗(On Diff #47270)

No call to getELFAddend needed here?

lib/MC/MCExpr.cpp
316 ↗(On Diff #47270)

This, in particular, seems an unfortunate consequence of not using a LanaiMCExpr: a very generic name, assigned a target-specific variant.

jpienaar updated this revision to Diff 47463.Feb 10 2016, 9:02 AM
jpienaar retitled this revision from [lanai] Add MCExpr and ELF relocations to [lanai] Add ELF enum value and relocations.
jpienaar updated this object.
jpienaar marked an inline comment as done.

Moved MCExpr changes to subclass of MCTargetExpr and use that instead in the backend.

jpienaar marked an inline comment as done.Feb 10 2016, 9:05 AM

Good suggestion on subclassing instead.

eliben added a subscriber: eliben.Feb 16 2016, 9:49 AM
eliben added inline comments.
include/llvm/Support/ELFRelocs/Lanai.def
6 ↗(On Diff #47463)

Would this be a good place for a few lines of comments explaining each relocation type?

jpienaar marked an inline comment as done.Feb 18 2016, 1:26 PM
jpienaar added inline comments.
include/llvm/Support/ELFRelocs/Lanai.def
6 ↗(On Diff #47463)

Looking at the other backends, the relocation types are not normally documented in ELFRelocs/X.def. Rather the explanation of relocation types is commonly done in lib/Target/X/MCTargetDesc/XFixupKinds.h.

jpienaar updated this object.Feb 24 2016, 9:34 AM

One inline comment. Are there any more testcases to test RelocToApply, readobj, etc?

-eric

include/llvm/Support/ELF.h
322 ↗(On Diff #47463)

Add the extra comma at the end here...

t.p.northover added inline comments.Feb 26 2016, 7:27 AM
include/llvm/Object/RelocVisitor.h
323 ↗(On Diff #47463)

What's the user of this function? It appears to be untested.

include/llvm/Support/ELFRelocs/Lanai.def
6 ↗(On Diff #47463)

Fixups aren't usually in 1-1 correspondence with relocations. Normally relocs are documented in a platform's ABI document, but I Lanai's intending to do it in-tree isn't it?

If so, this file looks like a reasonable place since they're all together here.

test/Object/Lanai/yaml2obj-elf-lanai-rel.yaml
6 ↗(On Diff #47463)

The other relocs should probably be tested too. It'd be good to test the actual numbers as well, if Lanai has any ABI stability expectations.

jpienaar updated this revision to Diff 49216.Feb 26 2016, 11:44 AM
jpienaar marked 4 inline comments as done.

Addressed comments, added readobj and dwarfdump tests.

Updated, thanks

include/llvm/Object/RelocVisitor.h
323 ↗(On Diff #47463)

It is used by dwarfdump. Adding test for it.

t.p.northover edited edge metadata.Feb 29 2016, 7:06 AM

Thanks, this looks fine now.

Tim.

t.p.northover accepted this revision.Feb 29 2016, 7:07 AM
t.p.northover edited edge metadata.
This revision is now accepted and ready to land.Feb 29 2016, 7:07 AM

Thanks, if none of the other reviewers have an objection, I'll submit this tomorrow.

echristo edited edge metadata.Mar 1 2016, 9:54 AM

It's good. Go ahead. :)

This revision was automatically updated to reflect the committed changes.