[PATCH] D96659: [WebAssembly] Add new relocation for pc-rel data

[Yuta Saito via Phabricator via llvm-commits]

kateinoigakukun added a comment.

@sbc100 Thanks for taking a look!

> Is the "pc-rel" in the title a little misleading? How about just "relative" or "location relative"?

You are right. it seems "location relative" is suitable. I'll change naming in my patch also.

> Thinking a little more about this, would it not make sense to have these relative relocations in the code section instead (or in addition)?     I assume the idea is to do stuff like this:
>
>   i32.load <some_base_pointer>
>   i32.load <selfrel_location>
>   i32.add
>   .. use absolute pointer ..
>
> In that case wouldn't it be better to just do:
>
>   i32.load <some_base_pointer>
>   i32.const <relative_relocation>
>   i32.add
>   .. use absolute pointer ..
>
> This would save using memory at all and just embed the relative offset into the i32.const instruction.

In the latter case, the relocation needs to have two symbols to calculate the difference. On the other hand, the former case requires a single symbol to calculate the diff because it's relative to `selfrel_location` itself.



================
Comment at: llvm/test/MC/WebAssembly/selfrel.ll:48
+    i32 ptrtoint (i32* @x_sec to i32)
+), section ".sec1"
+
----------------
sbc100 wrote:
> What happens today with this code?  (without this change would this fail in some way?).
> What happens today with this code?  (without this change would this fail in some way?).




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96659/new/

https://reviews.llvm.org/D96659