[PATCH] D47755: [RISCV] Insert R_RISCV_ALIGN relocation type and Nops for code alignment when linker relaxation enabled

Shiva Chen via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jan 11 20:16:43 PST 2019


shiva0217 added a comment.

In D47755#1352910 <https://reviews.llvm.org/D47755#1352910>, @asb wrote:

> In D47755#1334053 <https://reviews.llvm.org/D47755#1334053>, @shiva0217 wrote:
>
> > Hi Alex, I have added a constant pool test case at the end of align.s to verify the padding behavior for constant pool. Once D45961 <https://reviews.llvm.org/D45961> landing and changing the behavior, we should be able to aware. It seems that GCC will insert Nops for padding constant pool. Could you illustrate more about "use .align with a value to fill with"? I could test on GCC.
>
>
> I meant that .align/.p2align in GNU as accept a padding value https://sourceware.org/binutils/docs/as/Align.html#Align. It seems that for RISC-V, it accepts and uses this padding value even when relaxation is enabled, but won't emit the R_RISCV_RELAX in that case (which I suppose makes sense, as the linker wouldn't respect the padding value). I'm not saying we should definitely replicate GNU as here - obviously performing alignment without generating R_RISCV_RELAX is quite liable to break things unexpectedly. But it might not be a bad starting point if it's easy to support. Either way, we should have tests that demonsrate how we handle those cases.


Hi Alex,
Thanks for the explanation. I tested on GNU with `.p2align 4, 1`, as you said, GNU won't emit relocation types, so the alignment requirement may break when relaxation is enabled. For the same case with this patch, LLVM will emit relocation types, so the alignment requirement will be satisfied. I have added the test case. Thanks for catching this.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D47755





More information about the llvm-commits mailing list