This is an archive of the discontinued LLVM Phabricator instance.

[ELF] - Eliminate Target::isPicRel method.
ClosedPublic

Authored by grimar on Apr 4 2018, 4:16 AM.

Details

Summary

As was mentioned in comments for D45158,
isPicRel's name does not make much sense,
because what this method does is checks if
we need to create the dynamic relocation or not.

Instead of renaming it to something different,
I suggest eliminating 'isPicRel' completely.

We can reuse the getDynRel method.
They are logically very close, getDynRel can just return
R_*_NONE in case no dynamic relocation should be produced
and that would simplify things and avoid functionality
correlation/duplication with 'isPicRel'.

Diff Detail

Repository
rLLD LLVM Linker

Event Timeline

grimar created this revision.Apr 4 2018, 4:16 AM
grimar updated this revision to Diff 140932.Apr 4 2018, 4:20 AM
  • Drop unnecessary llvm:: prefixes.
grimar added a comment.Apr 4 2018, 4:58 AM

btw, instead of Optional<> it could just return RelType R_*_NONE I think,
which is always zero for all targets it seems.
I am not sure that would make more sense though.

I like the idea! Please rebase and use R_*_NONE.

grimar updated this revision to Diff 140969.Apr 4 2018, 8:35 AM
  • Rebased.
  • Use R_*_NONE,
grimar edited the summary of this revision. (Show Details)Apr 4 2018, 8:38 AM
This revision is now accepted and ready to land.Apr 4 2018, 8:46 AM
This revision was automatically updated to reflect the committed changes.