This is an archive of the discontinued LLVM Phabricator instance.

[JITLink] Adopt forEachRelocation() helper in ELF x86-64 backend (NFC)
ClosedPublic

Authored by sgraenitz on Sep 9 2021, 9:22 AM.

Details

Summary

Following D109516, this patch re-uses the new helper function for ELF relocation traversal in the x86-64 backend.

Diff Detail

Event Timeline

sgraenitz created this revision.Sep 9 2021, 9:22 AM
sgraenitz requested review of this revision.Sep 9 2021, 9:22 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 9 2021, 9:22 AM

The patch aims to be NFC (except debug output format). Please ping me if I introduced a functional change by accident.

sgraenitz updated this revision to Diff 372230.Sep 13 2021, 6:19 AM

Account for modifications in the underlying patch && address pre-merge checks

Ping! I was hoping to land this together with D109522.

StephenFan accepted this revision.Sep 17 2021, 9:07 AM

LGTM. Thanks @sgraenitz!

This revision is now accepted and ready to land.Sep 17 2021, 9:07 AM
This revision was landed with ongoing or failed builds.Sep 20 2021, 6:47 AM
This revision was automatically updated to reflect the committed changes.
foad added a subscriber: foad.Sep 20 2021, 6:50 AM
foad added inline comments.
llvm/lib/ExecutionEngine/JITLink/ELF_x86_64.cpp
371

This causes a warning when I do a release build with clang 10.0.0:

ELF_x86_64.cpp:308:13: warning: enumeration value 'PCRel32TLV' not handled in switch [-Wswitch]
GMNGeoffrey added inline comments.
llvm/lib/ExecutionEngine/JITLink/ELF_x86_64.cpp
371

I'm also seeing this and build with -Werror

GMNGeoffrey added inline comments.
llvm/lib/ExecutionEngine/JITLink/ELF_x86_64.cpp
371

Looks like a bad merge with https://reviews.llvm.org/D109293 (thanks @bkramer for pointing this out). I'll send a fix