Page MenuHomePhabricator

[JITLink] Adopt forEachRelocation() helper in ELF RISCV backend (NFC)
ClosedPublic

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

Details

Summary

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

Diff Detail

Event Timeline

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

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

jrtc27 added inline comments.Sep 9 2021, 9:29 AM
llvm/lib/ExecutionEngine/JITLink/ELF_riscv.cpp
303

std::move of a lambda is a strange thing to do

StephenFan added inline comments.Sep 9 2021, 7:28 PM
llvm/lib/ExecutionEngine/JITLink/ELF_riscv.cpp
297

x64 -> riscv

sgraenitz updated this revision to Diff 372241.Sep 13 2021, 6:47 AM
sgraenitz marked 2 inline comments as done.

Account for modifications in the underlying patch

llvm/lib/ExecutionEngine/JITLink/ELF_riscv.cpp
297

Good catch! Actually, I made a copy/paste mistake here (from x86-64). This check didn't exist for RISCV. It'd be a functional change and thus I am going to drop it (even in case it's accurate for RISCV).

303

Agree. It's been a leftover from my previous std::function sketches. Latest version passes the member function pointer directly.

StephenFan accepted this revision.Sep 13 2021, 8:35 AM
This revision is now accepted and ready to land.Sep 13 2021, 8:35 AM