[PATCH] D127549: RISCV: handle 64-bit PCREL data relocations

[Luís Marques via Phabricator via llvm-commits]

luismarques added a comment.

I know that this issue arose in the context of `-fprofile-instr-generate` but can you please clean up the test to focus on the bug? Something like this:

  	.section	sx,"aw", at progbits
  	.globl	x
  x:
  
  	.section	sy,"aw", at progbits
  	.globl	y
  y:
  	.8byte	x-y



================
Comment at: llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp:232-234
+    // the fixups here as we should relax the pair-wise relocation to a
+    // R_RISCV_[32|64|128]_PCREL instead.
+
----------------
This point is a bit of a nit-pick but that optimization isn't limited to when we are generating code for the compact code model. Rather, to define the compact code mode it's necessary to define that new relocation. Once we have support for that relocation (in the object code generator and the linker) we should be able to use it, whatever code model we are targeting. (The same point applies to the patch summary).


================
Comment at: llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp:236-242
+    if (A.isInSection())
+      return !IsDebugOrEHFrameSection(A.getSection());
+    if (B.isInSection())
+      return !IsDebugOrEHFrameSection(B.getSection());
+    // as well as for absolute symbols.
+    return !A.getName().empty() || !B.getName().empty();
   }
----------------
luismarques wrote:
> compnerd wrote:
> > craig.topper wrote:
> > > compnerd wrote:
> > > > craig.topper wrote:
> > > > > Should relaxation here be relocation?
> > > > In the case of a compact code model, we can relax the pair of relocations to a single relocation.  You would still have a relocation, it just is a nominally smaller file, and fewer relocations for the linker to process.
> > > I was asking about “Avoid relaxation for symbolic differences…”. Is relaxation the right word there?
> > Right; I believe that is the term here - we are technically relaxing the sequence by replacing the two relocations with one.
> I've never seen "relaxation" being used that way. While that seems a reasonable generalization of how the term is typically used in compiler land, I suspect most people are going to be confused by the comment.
Are we testing all of these conditions?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127549