[JITLink][RISCV] Initial Support RISCV64 in JITLink

[StephenFan]

Summary

This patch is the initial support, it implements translation from object file to JIT link graph, and very few relocations were supported. Currently, the test file ELF_pc_indirect.s is passed, the HelloWorld program(compiled with mno-relax flag) can be linked correctly and run on instruction emulator correctly.

In the downstream implementation, I have implemented the GOT, PLT function, and EHFrame and some optimization will be implement soon. I will organize the code in to patches, then gradually send it to upstream.

[lhames]

Hi Stephen,

LGTM. This is great stuff -- Thank you for working on this!

llvm/include/llvm/ExecutionEngine/JITLink/ELF_riscv64.h

23–33 ↗

(On Diff #356483)

Could you please move these edge kinds into their own file (e.g. 'RISCV.h') following the convention established in llvm/include/llvm/ExecutionEngine/JITLink/x86_64.h?

I don't mind if the initial edge set very closely mirrors the ELF relocation set, but the idea is to try to capture the generic fixups that the architecture requires. This simplifies the implementation of new linker backends for the architecture, and allows Plugins to remain as format-agnostic as possible.

Sadly ELF_x86_64 hasn't been moved to the generic kinds in x86_64.h yet, so it's not an ideal example here. The MachO_x86_64 backend is probably a better example.

llvm/lib/ExecutionEngine/JITLink/ELF_riscv64.cpp

24–33 ↗

(On Diff #356483)

Is this needed? You should be able to use the isDwarfSection method in the base class.

[jrtc27]

As a general point I don't like that this is dubbed a "riscv64" JITLink implementation. Almost all the code is the same between RV32 and RV64, the only difference is whether you have ELFCLASS32 or ELFCLASS64. Renaming ELFJITLinker_riscv64 to ELFJITLinker_riscv, plus renaming ELFLinkGraphBuilder_riscv64 to ELFLinkGraphBuilder_riscv and templating it on the ELFType, should make it "just work" for RV32. At least name the files and public interfaces riscv not riscv64 so adding RV32 support in future won't create avoidable churn.

[jrtc27]

llvm/include/llvm/ExecutionEngine/JITLink/ELF_riscv64.h

28 ↗

(On Diff #356483)

I-type but not S-type is a bit odd

llvm/lib/ExecutionEngine/JITLink/CMakeLists.txt

21

Keep sorted

llvm/lib/ExecutionEngine/JITLink/ELF.cpp

72

Sort

89

Sort

llvm/lib/ExecutionEngine/JITLink/ELF_riscv64.cpp

24–33 ↗

(On Diff #356483)

Can this be moved somewhere so it's shared with x86?

40–42 ↗

(On Diff #356483)

Dodgy grammar; this is more natural, and also shorter (though you'll need to change the message to R_RISCV_PCREL_LO12_[IS] once you support R_RISCV_PCREL_LO12_S).

45–52 ↗

(On Diff #356483)

This is strange. You find the first edge that's >= the edge, then walk backwards. That is wrong for two reasons:

1. You only want to find an R_RISCV_PCREL_HI20 at precisely the right location, not keep walking back until you find one, so should use std::equal_range and iterate over the returned range to look at the relocations _only_ at that specific location.
2. This is totally the wrong way to do R_RISCV_PCREL_LO12_[IS]; it's _indirect_, so you need to find the relocation by looking at E,getTarget().getAddress() + E.getAddend(), otherwise you'll incorrectly handle things like:

1: auipc a0, %pcrel_hi(foo)
2: auipc a1, %pcrel_hi(bar)
   addi a0, a0, %pcrel_lo(1b)
   addi a1, a1, %pcrel_lo(2b)

1: auipc a0, %pcrel_hi(foo)
   ld t0, %pcrel_lo(1b)(a0)
2: auipc a1, %pcrel_hi(bar)
   ld t1, %pcrel_lo(2b)(a1)
   add t0, t0, t1
   sd t0, %pcrel_lo(1b)(a0)

and

   j 2f
1: addi a0, a0, %pcrel_lo(2f)
   j 3f
2: auipc a0, a0, %pcrel_hi(foo)
   j 1b
3:

all of which are legal, if silly in the case of that last one, though currently LLVM happens to not generate those kinds of things (GCC will generate things like that with -mexplicit-relocs - see https://godbolt.org/z/jPn3M5bM8 - though that's not a default option, and is discouraged, because it's overly aggressive and will do unsound optimisations in certain cases).

88 ↗

(On Diff #356483)

As discussed in the other review, Lo & 0xFFF is just Value & 0xFFF.

99 ↗

(On Diff #356483)

Lo needs masking like LO12_I, but also this is then just Value & 0xFFF.

116 ↗

(On Diff #356483)

Ditto

146 ↗

(On Diff #356483)

Copy-paste error

152–158 ↗

(On Diff #356483)

These comments are copied from x86_64 but I don't think they make sense

186 ↗

(On Diff #356483)

Please follow these clang-tidy suggestions, marking pointers as pointers with auto is the LLVM style

195 ↗

(On Diff #356483)

Missing return at the front

222 ↗

(On Diff #356483)

Addends are signed.

llvm/test/ExecutionEngine/JITLink/RISCV/ELF_pc_indirect.s

14

Why is everything 16-byte aligned? And why are you padding with x86 NOPs?

28

Make this lw from a .4byte then the same test could be used for RV32 too

[jrtc27]

llvm/lib/ExecutionEngine/JITLink/ELF_riscv64.cpp

244 ↗

(On Diff #356483)

OSes other than Linux exist

[StephenFan]

Address @lhames and @jrtc27 's comment

[StephenFan]

llvm/include/llvm/ExecutionEngine/JITLink/ELF_riscv64.h

23–33 ↗

(On Diff #356483)

Currently, the edge set is the mirror of the ELF relocation set. After a while, I will capture the generic fixups and replace the edge set with generic edge set.

[StephenFan]

Fix error

[jrtc27]

llvm/include/llvm/ExecutionEngine/JITLink/ELF_riscv64.h

1 ↗

(On Diff #359602)

Should be ELF_riscv.h

1 ↗

(On Diff #359602)

Also copy-paste error with that x86-64

llvm/lib/ExecutionEngine/JITLink/ELF.cpp

82

And riscv32

llvm/lib/ExecutionEngine/JITLink/ELF_riscv.cpp

267

This still hard-codes Linux, as well as that it's 64-bit now the class is just ELFLinkGraphBuilder_riscv.

283–284

1. Asserting it's 64-bit _after_ you've cast it is unlikely to end well
2. Just add the handful of lines to have an ELF32LE case?

290–293

This can just be link_ELF_riscv, nothing here cares about RV32 vs RV64

[StephenFan]

1. Address @jrtc27 's comment.

2. Support the RV32, and add test cases to test it

[StephenFan]

llvm/lib/ExecutionEngine/JITLink/ELF_riscv.cpp

267

To avoid hard-codes, I add an argument to initialize the triple information. Is there any better way to avoid hard-codes ? @jrtc27

[lhames]

llvm/lib/ExecutionEngine/JITLink/ELF_riscv.cpp

267

Passing an argument in here seems like a reasonable solution to me. You can probably use the triple returned by ObjectFile::makeTriple when you construct the graph builder in createLinkGraphFromELFObject_riscv below.

[StephenFan]

Address @lhames 's comment.

[lhames]

@StephenFan -- Thanks for splitting those edge kinds out!

Now that you've moved the edge kinds into their own header the getELFRISCVRelocationKindName function should be renamed/declared as getEdgeKindName in the riscv namespace in riscv.h, and the implementation moved to a new riscv.cpp file (to mirror x86_64::getEdgeKindName). This is a minor request though: I don't mind whether it happens as part of this review, before you commit, or in-tree after it lands. From my point of view this is new functionality, and it'd be a win to land it in-tree sooner rather than later so that other people can start checking it out.

@jrtc27 -- did you have any further fixes that you want before this lands?

[StephenFan]

Address @lhames 's comments

Replace getELFRISCVRelocationKindName with getEdgeKindName, and moved to riscv.cpp.

[jrtc27]

llvm/lib/ExecutionEngine/JITLink/ELF.cpp

82–83

Sort

llvm/lib/ExecutionEngine/JITLink/ELF_riscv.cpp

58

Should say PCREL in there otherwise it can be confused with the absolute HI20/LO12 relocations that are more normal (ie don't have this indirection)

91

This is still using the old inefficient "backwards" way of doing it, just mask Value with 0xFFF and be done with it.

100

Ditto

119–120

makes more sense to me, though Expected<const Edge &> would remove one level of indirection for callers so you can just do RelHI20->getTarget() etc.

[StephenFan]

Address @jrtc27 's comments

[lhames]

LGTM.

@StephenFan -- Do you have commit access? If so, please go ahead and commit. If not, are you happy for me to commit this on your behalf?

[thakis]

llvm/lib/ExecutionEngine/JITLink/CMakeLists.txt

23

Did you forget to git add this file perchance? :)

[StephenFan]

In D105429#2899490, @lhames wrote:

LGTM.

@StephenFan -- Do you have commit access? If so, please go ahead and commit. If not, are you happy for me to commit this on your behalf?

Thanks Lang! I have commit this patch.

[yaxunl]

This caused regressions in https://lab.llvm.org/buildbot/#/builders/165/builds/4168

CMake Error at cmake/modules/AddLLVM.cmake:538 (add_library):

Cannot find source file:
  riscv.cpp
Tried extensions .c .C .c++ .cc .cpp .cxx .cu .mpp .m .M .mm .h .hh .h++
.hm .hpp .hxx .in .txx .f .F .for .f77 .f90 .f95 .f03 .ispc

Call Stack (most recent call first):

cmake/modules/AddLLVM.cmake:794 (llvm_add_library)
cmake/modules/AddLLVM.cmake:769 (add_llvm_library)
lib/ExecutionEngine/JITLink/CMakeLists.txt:1 (add_llvm_component_library)

[StephenFan]

llvm/lib/ExecutionEngine/JITLink/CMakeLists.txt

23

I'm sorry. Now, I have made up for my fault. Is it ok now?

[thakis]

llvm/lib/ExecutionEngine/JITLink/CMakeLists.txt

23

Yes, bots are happy again. Thanks :)

[thakis]

The test fails in bootstrap builds on all our bots, e.g.:
https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8840851478629466864/+/u/package_clang/stdout?format=raw (linux)
https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8840851478629466816/+/u/package_clang/stdout?format=raw (windows)

The error is: /b/s/w/ir/cache/builder/src/third_party/llvm-bootstrap/bin/llvm-mc: error: : error: unable to get target for 'riscv64', see --version and --triple.

We don't build in riscv support, so I guess the test needs a REQUIRES line. Oh, actually it's in a RISCV subfolder, so a lit.local.cfg is more appropriate. Added one in 75077f46e7e4d5c89a6d7cd9a8ae7d740df2f4cd, hopefully that helps.

[MaskRay]

Perhaps revert this patch since it may be good to fix formatting nits in the initial RISC-V JIT patch.

llvm/include/llvm/ExecutionEngine/JITLink/riscv.h

29

I know this copied from the existing x86_64.h but the documentation style is quite verbose. The blank line and Fixup expression: are redundant.

It is better to use the regular ELF relocation format where symbols like S, A are used to describe operands.

llvm/lib/ExecutionEngine/JITLink/ELF_riscv.cpp

61

lld/ELF/RISCV.cpp is not a good example. High before Low is weird.

I may use low&size to represent bits [low, low+size).

135

previous relocations don't add a blank line.

[lhames]

In D105429#2903344, @MaskRay wrote:

Perhaps revert this patch since it may be good to fix formatting nits in the initial RISC-V JIT patch.

Please don't do that. Formatting nits can easily be fixed in-tree. Getting the code out to people (and testers) is more important.

[MaskRay]

In D105429#2903436, @lhames wrote:

In D105429#2903344, @MaskRay wrote:

Perhaps revert this patch since it may be good to fix formatting nits in the initial RISC-V JIT patch.

Please don't do that. Formatting nits can easily be fixed in-tree. Getting the code out to people (and testers) is more important.

The test issues and formatting are not the only problems. The tests insufficient. The absolute and branch relocation types are not tested.

From the description it's clear this is a WIP and not blocking any project.

For initial check-in of some architecture, I think we should set up a higher bar. It looks multiple fixups are not needed for the patch.
It's just cleaner to revert it.

[lhames]

In D105429#2903448, @MaskRay wrote:

In D105429#2903436, @lhames wrote:

In D105429#2903344, @MaskRay wrote:

Perhaps revert this patch since it may be good to fix formatting nits in the initial RISC-V JIT patch.

Please don't do that. Formatting nits can easily be fixed in-tree. Getting the code out to people (and testers) is more important.

The test issues and formatting are not the only problems. The tests insufficient. The absolute and branch relocation types are not tested.

From the description it's clear this is a WIP and not blocking any project.

For initial check-in of some architecture, I think we should set up a higher bar. It looks multiple fixups are not needed for the patch.
It's just cleaner to revert it.

As long as the existing tests are passing it's not regressing anything, and represents a monotonic improvement in functionality. More test coverage is better, and a discussion of where testing can/should be improved is good, but at this stage I think this patch is a good fit for in-tree development. I would like it to remain there, and future development to happen in-tree.

[luismarques]

If this has landed and isn't going to be reverted, can we please mark this as closed?

[StephenFan]

In D105429#3003862, @luismarques wrote:

If this has landed and isn't going to be reverted, can we please mark this as closed?

There doesn't seem to have the close revision option in the Add Action... bar as usual. Do you have another way to close the revision?

[xgupta]

It think someone need to accept this revision now to close it.

[xgupta]

no luck ):

[StephenFan]

In D105429#3005524, @xgupta wrote:

no luck ):

Although no luck, thanks!

[sgraenitz]

In D105429#3005522, @xgupta wrote:

It think someone need to accept this revision now to close it.

Apparently, @jrtc27 needs to withdraw their request for changes first.

llvm/include/llvm/ExecutionEngine/JITLink/ELF_riscv.h

+//===--- ELF_riscv64.h - JIT link functions for ELF/riscv ---*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+//===----------------------------------------------------------------------===//
+//
+// jit-link functions for ELF/riscv.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_EXECUTIONENGINE_JITLINK_ELF_RISCV_H
+#define LLVM_EXECUTIONENGINE_JITLINK_ELF_RISCV_H
+
+#include "llvm/ExecutionEngine/JITLink/JITLink.h"
+
+namespace llvm {
+namespace jitlink {
+
+/// Create a LinkGraph from an ELF/riscv64 relocatable object
+///
+/// Note: The graph does not take ownership of the underlying buffer, nor copy
+/// its contents. The caller is responsible for ensuring that the object buffer
+/// outlives the graph.
+Expected<std::unique_ptr<LinkGraph>>
+createLinkGraphFromELFObject_riscv(MemoryBufferRef ObjectBuffer);
+
+/// jit-link the given object buffer, which must be a ELF riscv64 object file.
+void link_ELF_riscv(std::unique_ptr<LinkGraph> G,
+                    std::unique_ptr<JITLinkContext> Ctx);
+
+/// Return the string name of the given ELF riscv64 edge kind.
+const char *getELFRISCVRelocationKindName(Edge::Kind R);
+} // end namespace jitlink
+} // end namespace llvm
+
+#endif // LLVM_EXECUTIONENGINE_JITLINK_ELF_RISCV64_H

llvm/include/llvm/ExecutionEngine/JITLink/riscv.h

+//===--  riscv.h  - Generic JITLink riscv edge kinds, utilities -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// Generic utilities for graphs representing riscv objects.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_EXECUTIONENGINE_JITLINK_RISCV_H
+#define LLVM_EXECUTIONENGINE_JITLINK_RISCV_H
+
+#include "llvm/ExecutionEngine/JITLink/JITLink.h"
+
+namespace llvm {
+namespace jitlink {
+namespace riscv {
+
+/// Represets riscv fixups
+enum EdgeKind_riscv : Edge::Kind {
+
+  // TODO: Capture and replace to generic fixups
+  /// A plain 32-bit pointer value relocation
+  ///
+  /// Fixup expression:
+  ///   Fixup <= Target + Addend : uint32
+  ///
+  R_RISCV_32 = Edge::FirstRelocation,
+
+  /// A plain 64-bit pointer value relocation
+  ///
+  /// Fixup expression:
+  ///   Fixup <- Target + Addend : uint32
+  ///
+  R_RISCV_64,
+
+  /// High 20 bits of 32-bit pointer value relocation
+  ///
+  /// Fixup expression
+  ///   Fixup <- (Target + Addend + 0x800) >> 12
+  R_RISCV_HI20,
+
+  /// Low 12 bits of 32-bit pointer value relocation
+  ///
+  /// Fixup expression
+  ///   Fixup <- (Target + Addend) & 0xFFF
+  R_RISCV_LO12_I,
+  /// High 20 bits of PC relative relocation
+  ///
+  /// Fixup expression:
+  ///   Fixup <- (Target - Fixup + Addend + 0x800) >> 12
+  R_RISCV_PCREL_HI20,
+
+  /// Low 12 bits of PC relative relocation, used by I type instruction format
+  ///
+  /// Fixup expression:
+  ///   Fixup <- (Target - Fixup + Addend) & 0xFFF
+  R_RISCV_PCREL_LO12_I,
+
+  /// Low 12 bits of PC relative relocation, used by S type instruction format
+  ///
+  /// Fixup expression:
+  ///   Fixup <- (Target - Fixup + Addend) & 0xFFF
+  R_RISCV_PCREL_LO12_S,
+
+  /// PC relative call
+  ///
+  /// Fixup expression:
+  ///   Fixup <- (Target - Fixup + Addend)
+  R_RISCV_CALL
+
+};
+} // namespace riscv
+} // namespace jitlink
+} // namespace llvm
+
+#endif

llvm/lib/ExecutionEngine/JITLink/CMakeLists.txt

add_llvm_component_library(LLVMJITLink
   MachO_arm64.cpp
   MachO_x86_64.cpp
   MachOLinkGraphBuilder.cpp
 
   # ELF
 
   ELF.cpp
   ELFLinkGraphBuilder.cpp
+  ELF_riscv.cpp
   ELF_x86_64.cpp
 
   # Architectures:
   x86_64.cpp
 
   ADDITIONAL_HEADER_DIRS
   ${LLVM_MAIN_INCLUDE_DIR}/llvm/ExecutionEngine/JITLink
 
   DEPENDS
   intrinsics_gen
 
   LINK_COMPONENTS

llvm/lib/ExecutionEngine/JITLink/ELF.cpp

 //===-------------- ELF.cpp - JIT linker function for ELF -------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // ELF jit-link function.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/ExecutionEngine/JITLink/ELF.h"
 
 #include "llvm/BinaryFormat/ELF.h"
+#include "llvm/ExecutionEngine/JITLink/ELF_riscv.h"
 #include "llvm/ExecutionEngine/JITLink/ELF_x86_64.h"
 #include "llvm/Object/ELF.h"
 #include "llvm/Support/Endian.h"
 #include "llvm/Support/Format.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include <cstring>
 
 using namespace llvm;
createLinkGraphFromELFObject(MemoryBufferRef ObjectBuffer) {
   if (memcmp(Buffer.data(), ELF::ElfMagic, strlen(ELF::ElfMagic)) != 0)
     return make_error<JITLinkError>("ELF magic not valid");
 
   Expected<uint16_t> TargetMachineArch = readTargetMachineArch(Buffer);
   if (!TargetMachineArch)
     return TargetMachineArch.takeError();
 
   switch (*TargetMachineArch) {
+  case ELF::EM_RISCV:
+    return createLinkGraphFromELFObject_riscv(ObjectBuffer);
   case ELF::EM_X86_64:
     return createLinkGraphFromELFObject_x86_64(ObjectBuffer);
   default:
     return make_error<JITLinkError>(
         "Unsupported target machine architecture in ELF object " +
         ObjectBuffer.getBufferIdentifier());
   }
 }
 
 void link_ELF(std::unique_ptr<LinkGraph> G,
               std::unique_ptr<JITLinkContext> Ctx) {
   switch (G->getTargetTriple().getArch()) {
+  case Triple::riscv64:
+  case Triple::riscv32:
+    link_ELF_riscv(std::move(G), std::move(Ctx));
+    return;
   case Triple::x86_64:
     link_ELF_x86_64(std::move(G), std::move(Ctx));
     return;
   default:
     Ctx->notifyFailed(make_error<JITLinkError>(
         "Unsupported target machine architecture in ELF link graph " +
         G->getName()));
     return;
   }
 }
 
 } // end namespace jitlink
 } // end namespace llvm

llvm/lib/ExecutionEngine/JITLink/ELF_riscv.cpp

+//===---- ELF_riscv64.cpp -JIT linker implementation for ELF/riscv64 ----===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// ELF/riscv64 jit-link implementation.
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/ExecutionEngine/JITLink/ELF_riscv.h"
+#include "llvm/ExecutionEngine/JITLink/JITLink.h"
+#include "llvm/ExecutionEngine/JITLink/riscv.h"
+#include "llvm/Object/ELF.h"
+#include "llvm/Object/ELFObjectFile.h"
+
+#include "ELFLinkGraphBuilder.h"
+#include "JITLinkGeneric.h"
+
+#define DEBUG_TYPE "jitlink"
+using namespace llvm;
+
+namespace llvm {
+namespace jitlink {
+
+static Expected<const Edge *> getRISCVPCRelHi20(const Edge &E) {
+  using namespace riscv;
+  assert((E.getKind() == R_RISCV_PCREL_LO12_I ||
+          E.getKind() == R_RISCV_PCREL_LO12_S) &&
+         "Can only have high relocation for R_RISCV_PCREL_LO12_I or "
+         "R_RISCV_PCREL_LO12_S");
+
+  const Symbol &Sym = E.getTarget();
+  const Block &B = Sym.getBlock();
+  JITTargetAddress Offset = Sym.getOffset();
+
+  struct Comp {
+    bool operator()(const Edge &Lhs, JITTargetAddress Offset) {
+      return Lhs.getOffset() < Offset;
+    }
+    bool operator()(JITTargetAddress Offset, const Edge &Rhs) {
+      return Offset < Rhs.getOffset();
+    }
+  };
+
+  auto Bound =
+      std::equal_range(B.edges().begin(), B.edges().end(), Offset, Comp{});
+
+  for (auto It = Bound.first; It != Bound.second; ++It) {
+    if (It->getKind() == R_RISCV_PCREL_HI20)
+      return &*It;
+  }
+
+  return make_error<JITLinkError>(
+      "No HI20 relocation type be found for LO12 relocation type");
+}
+
+static uint32_t extractBits(uint64_t Num, unsigned High, unsigned Low) {
+  return (Num & ((1ULL << (High + 1)) - 1)) >> Low;
+}
+
+class ELFJITLinker_riscv : public JITLinker<ELFJITLinker_riscv> {
+  friend class JITLinker<ELFJITLinker_riscv>;
+
+public:
+  ELFJITLinker_riscv(std::unique_ptr<JITLinkContext> Ctx,
+                     std::unique_ptr<LinkGraph> G, PassConfiguration PassConfig)
+      : JITLinker(std::move(Ctx), std::move(G), std::move(PassConfig)) {}
+
+private:
+  Error applyFixup(LinkGraph &G, Block &B, const Edge &E) const {
+    using namespace riscv;
+    using namespace llvm::support;
+
+    char *BlockWorkingMem = B.getAlreadyMutableContent().data();
+    char *FixupPtr = BlockWorkingMem + E.getOffset();
+    JITTargetAddress FixupAddress = B.getAddress() + E.getOffset();
+    switch (E.getKind()) {
+    case R_RISCV_HI20: {
+      int64_t Value = E.getTarget().getAddress() + E.getAddend();
+      int32_t Hi = (Value + 0x800) & 0xFFFFF000;
+      uint32_t RawInstr = *(little32_t *)FixupPtr;
+      *(little32_t *)FixupPtr = (RawInstr & 0xFFF) | static_cast<uint32_t>(Hi);
+      break;
+    }
+    case R_RISCV_LO12_I: {
+      int64_t Value = E.getTarget().getAddress() + E.getAddend();
+      int32_t Lo = Value - ((Value + 0x800) & 0xFFFFF000);
+      uint32_t RawInstr = *(little32_t *)FixupPtr;
+      *(little32_t *)FixupPtr =
+          (RawInstr & 0xFFFFF) | (static_cast<uint32_t>(Lo & 0xFFF) << 20);
+      break;
+    }
+    case R_RISCV_CALL: {
+      int64_t Value = E.getTarget().getAddress() + E.getAddend() - FixupAddress;
+      int32_t Hi = (Value + 0x800) & 0xFFFFF000;
+      int32_t Lo = Value - Hi;
+      uint32_t RawInstrAuipc = *(little32_t *)FixupPtr;
+      uint32_t RawInstrJalr = *(little32_t *)(FixupPtr + 4);
+      *(little32_t *)FixupPtr = RawInstrAuipc | static_cast<uint32_t>(Hi);
+      *(little32_t *)(FixupPtr + 4) =
+          RawInstrJalr | (static_cast<uint32_t>(Lo) << 20);
+      break;
+    }
+    case R_RISCV_PCREL_HI20: {
+      int64_t Value = E.getTarget().getAddress() + E.getAddend() - FixupAddress;
+      int32_t Hi = (Value + 0x800) & 0xFFFFF000;
+      uint32_t RawInstr = *(little32_t *)FixupPtr;
+      *(little32_t *)FixupPtr = (RawInstr & 0xFFF) | static_cast<uint32_t>(Hi);
+      break;
+    }
+    case R_RISCV_PCREL_LO12_I: {
+      auto RelHI20 = getRISCVPCRelHi20(E);
+      if (!RelHI20)
+        return RelHI20.takeError();
+      int64_t Value = (**RelHI20).getTarget().getAddress() +
+                      (**RelHI20).getAddend() - E.getTarget().getAddress();
+      int64_t Lo = Value & 0xFFF;
+      uint32_t RawInstr = *(little32_t *)FixupPtr;
+      *(little32_t *)FixupPtr =
+          (RawInstr & 0xFFFFF) | (static_cast<uint32_t>(Lo & 0xFFF) << 20);
+      break;
+    }
+    case R_RISCV_PCREL_LO12_S: {
+      auto RelHI20 = getRISCVPCRelHi20(E);
+      int64_t Value = (**RelHI20).getTarget().getAddress() +
+                      (**RelHI20).getAddend() - E.getTarget().getAddress();
+      int64_t Lo = Value & 0xFFF;
+      uint32_t Imm31_25 = extractBits(Lo, 11, 5) << 25;
+      uint32_t Imm11_7 = extractBits(Lo, 4, 0) << 7;
+      uint32_t RawInstr = *(little32_t *)FixupPtr;
+
+      *(little32_t *)FixupPtr = (RawInstr & 0x1FFF07F) | Imm31_25 | Imm11_7;
+      break;
+    }
+    }
+    return Error::success();
+  }
+};
+
+template <typename ELFT>
+class ELFLinkGraphBuilder_riscv : public ELFLinkGraphBuilder<ELFT> {
+private:
+  static Expected<riscv::EdgeKind_riscv>
+  getRelocationKind(const uint32_t Type) {
+    using namespace riscv;
+    switch (Type) {
+    case ELF::R_RISCV_32:
+      return EdgeKind_riscv::R_RISCV_32;
+    case ELF::R_RISCV_64:
+      return EdgeKind_riscv::R_RISCV_64;
+    case ELF::R_RISCV_HI20:
+      return EdgeKind_riscv::R_RISCV_HI20;
+    case ELF::R_RISCV_LO12_I:
+      return EdgeKind_riscv::R_RISCV_LO12_I;
+    case ELF::R_RISCV_CALL:
+      return EdgeKind_riscv::R_RISCV_CALL;
+    case ELF::R_RISCV_PCREL_HI20:
+      return EdgeKind_riscv::R_RISCV_PCREL_HI20;
+    case ELF::R_RISCV_PCREL_LO12_I:
+      return EdgeKind_riscv::R_RISCV_PCREL_LO12_I;
+    case ELF::R_RISCV_PCREL_LO12_S:
+      return EdgeKind_riscv::R_RISCV_PCREL_LO12_S;
+    }
+
+    return make_error<JITLinkError>("Unsupported riscv relocation:" +
+                                    formatv("{0:d}", Type));
+  }
+
+  Error addRelocations() override {
+    using Base = ELFLinkGraphBuilder<ELFT>;
+    LLVM_DEBUG(dbgs() << "Adding relocations\n");
+
+    // TODO a partern is forming of iterate some sections but only give me
+    // ones I am interested, I should abstract that concept some where
+    for (auto &SecRef : Base::Sections) {
+      if (SecRef.sh_type != ELF::SHT_RELA && SecRef.sh_type != ELF::SHT_REL)
+        continue;
+      auto RelSectName = Base::Obj.getSectionName(SecRef);
+      if (!RelSectName)
+        return RelSectName.takeError();
+
+      LLVM_DEBUG({
+        dbgs() << "Adding relocations from section " << *RelSectName << "\n";
+      });
+
+      auto UpdateSection = Base::Obj.getSection(SecRef.sh_info);
+      if (!UpdateSection)
+        return UpdateSection.takeError();
+
+      auto UpdateSectionName = Base::Obj.getSectionName(**UpdateSection);
+      if (!UpdateSectionName)
+        return UpdateSectionName.takeError();
+      // Don't process relocations for debug sections.
+      if (Base::isDwarfSection(*UpdateSectionName)) {
+        LLVM_DEBUG({
+          dbgs() << "  Target is dwarf section " << *UpdateSectionName
+                 << ". Skipping.\n";
+        });
+        continue;
+      } else
+        LLVM_DEBUG({
+          dbgs() << "  For target section " << *UpdateSectionName << "\n";
+        });
+
+      auto *JITSection = Base::G->findSectionByName(*UpdateSectionName);
+      if (!JITSection)
+        return make_error<llvm::StringError>(
+            "Refencing a section that wasn't added to graph" +
+                *UpdateSectionName,
+            llvm::inconvertibleErrorCode());
+
+      auto Relocations = Base::Obj.relas(SecRef);
+      if (!Relocations)
+        return Relocations.takeError();
+
+      for (const auto &Rela : *Relocations) {
+        auto Type = Rela.getType(false);
+
+        LLVM_DEBUG({
+          dbgs() << "Relocation Type: " << Type << "\n"
+                 << "Name: " << Base::Obj.getRelocationTypeName(Type) << "\n";
+        });
+
+        auto SymbolIndex = Rela.getSymbol(false);
+        auto Symbol = Base::Obj.getRelocationSymbol(Rela, Base::SymTabSec);
+        if (!Symbol)
+          return Symbol.takeError();
+
+        auto BlockToFix = *(JITSection->blocks().begin());
+        auto *TargetSymbol = Base::getGraphSymbol(SymbolIndex);
+
+        if (!TargetSymbol) {
+          return make_error<llvm::StringError>(
+              "Could not find symbol at given index, did you add it to "
+              "JITSymbolTable? index: " +
+                  std::to_string(SymbolIndex) + ", shndx: " +
+                  std::to_string((*Symbol)->st_shndx) + " Size of table: " +
+                  std::to_string(Base::GraphSymbols.size()),
+              llvm::inconvertibleErrorCode());
+        }
+        int64_t Addend = Rela.r_addend;
+        JITTargetAddress FixupAddress =
+            (*UpdateSection)->sh_addr + Rela.r_offset;
+
+        LLVM_DEBUG({
+          dbgs() << "Processing relocation at "
+                 << format("0x%016" PRIx64, FixupAddress) << "\n";
+        });
+        auto Kind = getRelocationKind(Type);
+        if (!Kind)
+          return Kind.takeError();
+
+        BlockToFix->addEdge(*Kind, FixupAddress - BlockToFix->getAddress(),
+                            *TargetSymbol, Addend);
+      }
+    }
+    return Error::success();
+  }
+
+public:
+  ELFLinkGraphBuilder_riscv(StringRef FileName,
+                            const object::ELFFile<ELFT> &Obj, const Triple T)
+      : ELFLinkGraphBuilder<ELFT>(Obj, std::move(T), FileName,
+                                  getELFRISCVRelocationKindName) {}
+};
+
+Expected<std::unique_ptr<LinkGraph>>
+createLinkGraphFromELFObject_riscv(MemoryBufferRef ObjectBuffer) {
+  LLVM_DEBUG({
+    dbgs() << "Building jitlink graph for new input "
+           << ObjectBuffer.getBufferIdentifier() << "...\n";
+  });
+
+  auto ELFObj = object::ObjectFile::createELFObjectFile(ObjectBuffer);
+  if (!ELFObj)
+    return ELFObj.takeError();
+
+  if ((*ELFObj)->getArch() == Triple::riscv64) {
+    auto &ELFObjFile = cast<object::ELFObjectFile<object::ELF64LE>>(**ELFObj);
+    return ELFLinkGraphBuilder_riscv<object::ELF64LE>(
+               (*ELFObj)->getFileName(), ELFObjFile.getELFFile(),
+               Triple("riscv64-unknown-linux"))
+        .buildGraph();
+  } else {
+    assert((*ELFObj)->getArch() == Triple::riscv32 &&
+           "Invalid triple for RISCV ELF object file");
+    auto &ELFObjFile = cast<object::ELFObjectFile<object::ELF32LE>>(**ELFObj);
+    return ELFLinkGraphBuilder_riscv<object::ELF32LE>(
+               (*ELFObj)->getFileName(), ELFObjFile.getELFFile(),
+               Triple("riscv32-unknown-linux"))
+        .buildGraph();
+  }
+}
+
+void link_ELF_riscv(std::unique_ptr<LinkGraph> G,
+                    std::unique_ptr<JITLinkContext> Ctx) {
+  PassConfiguration Config;
+  const Triple &TT = G->getTargetTriple();
+  if (Ctx->shouldAddDefaultTargetPasses(TT)) {
+    if (auto MarkLive = Ctx->getMarkLivePass(TT))
+      Config.PrePrunePasses.push_back(std::move(MarkLive));
+    else
+      Config.PrePrunePasses.push_back(markAllSymbolsLive);
+  }
+  if (auto Err = Ctx->modifyPassConfig(*G, Config))
+    return Ctx->notifyFailed(std::move(Err));
+
+  ELFJITLinker_riscv::link(std::move(Ctx), std::move(G), std::move(Config));
+}
+
+const char *getELFRISCVRelocationKindName(Edge::Kind R) {
+  using namespace riscv;
+  switch (R) {
+  case riscv::R_RISCV_32:
+    return "R_RISCV_32";
+  case riscv::R_RISCV_64:
+    return "R_RISCV_64";
+  case riscv::R_RISCV_CALL:
+    return "R_RISCV_CALL";
+  case riscv::R_RISCV_HI20:
+    return "R_RISCV_HI20";
+  case riscv::R_RISCV_LO12_I:
+    return "R_RISCV_LO12_I";
+  case riscv::R_RISCV_PCREL_HI20:
+    return "R_RISCV_PCREL_HI20";
+  case riscv::R_RISCV_PCREL_LO12_I:
+    return "R_RISCV_PCREL_LO12_I";
+  }
+  return getGenericEdgeKindName(R);
+}
+} // namespace jitlink
+} // namespace llvm

llvm/test/ExecutionEngine/JITLink/RISCV/ELF_pc_indirect.s

+# RUN: rm -rf %t && mkdir -p %t
+# RUN: llvm-mc -triple=riscv64 -position-independent -filetype=obj -o %t/elf_riscv64_sm_pic_reloc.o %s
+# RUN: llvm-mc -triple=riscv32 -position-independent -filetype=obj -o %t/elf_riscv32_sm_pic_reloc.o %s
+# RUN: llvm-jitlink -noexec -slab-allocate 100Kb -slab-address 0xfff00000 \
+# RUN:              -check %s %t/elf_riscv64_sm_pic_reloc.o
+# RUN: llvm-jitlink -noexec -slab-allocate 100Kb -slab-address 0xfff00000 \
+# RUN:              -check %s %t/elf_riscv32_sm_pic_reloc.o
+#
+# Test ELF small/PIC relocations
+
+        .text
+        .file   "testcase.c"
+
+# Empty main entry point.
+        .globl  main
+        .p2align  1
+        .type   main,@function
+main:
+        ret
+
+        .size   main, .-main
+
+# Test R_RISCV_PCREL_HI20 and R_RISCV_PCREL_LO
+# jitlink-check: decode_operand(test_pcrel32, 1) = ((named_data - test_pcrel32) + 0x800)[31:12] 
+        .globl  test_pcrel32
+        .p2align  1
+        .type   test_pcrel32,@function
+test_pcrel32:
+        auipc a0, %pcrel_hi(named_data)
+        lw  a0, %pcrel_lo(test_pcrel32)(a0)
+
+        .size   test_pcrel32, .-test_pcrel32
+
+        .data
+        .type   named_data,@object
+        .p2align  1
+named_data:
+        .quad   42
+        .size   named_data, 4