[PATCH] D47126: [RISCV] Add missing fixup_riscv_call in MCFixupKindInfo table

Alex Bradbury via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon May 28 05:03:43 PDT 2018


asb added inline comments.


================
Comment at: test/CodeGen/RISCV/function-call.ll:1-3
+; RUN: llc -mtriple=riscv32 -filetype=obj < %s\
+; RUN: | llvm-objdump -triple=riscv32 -mattr=-relax -d -riscv-no-aliases -\
+; RUN: | FileCheck -check-prefix=CHECK %s
----------------
asb wrote:
> Now this test has changed to just checking the fixup is resolved when -mattr=-relax I think it makes much more sense as an MC layer test. Perhaps expanding test/MC/RISCV/fixups.s? We need some reassurance that the fixup calculation is correct, even for offsets > 12 bits.
> 
> It looks like you have to use .space as in fixups.s, as the following still results in a relocation, even though I would have expected it not to:
> ```
> .equ addr, 0xdeadbeef
> call addr
> ```
Sorry - I didn't turn my brain on for that comment.  It's absolutely correct that the call to an absolute target results in a relocation, as of course call is pc-relative. Any way, more tests in test/MC/RISCV/fixups.s to give reassurance that resolvable fixups are encoded correctly would be really welcome.


Repository:
  rL LLVM

https://reviews.llvm.org/D47126





More information about the llvm-commits mailing list