To do this:
- Add fixup_riscv_relax fixup types which eventually will transfer to R_RISCV_RELAX relocation type.
- Insert R_RISCV_RELAX relocation type to auipc function call expression when linker relaxation enabled.
shiva0217 on Mar 25 2018, 6:30 PM.Authored by
To do this:
Could you please add tests that demonstrate the new logic in shouldForceRelocation? Specifically, it would be good to show that relocations are emitted even for local control flow, when they weren't before.
This seems like a sensible intermediate step, even if it's not a 100% match for gcc behaviour.
I note that assembling asm generated by linker-relaxation.ll results in a RISCV_RELAX being generated with gas attached to both the pcrel_hi20 and the pcrel_lo12_i. I think we definitely want to do the same thing and it probably makes sense to do so before switching to directly generating the RISCV_CALL relocation - do you think?
Given that Simon's D45181 will be dependent on this (as it should really query whether linker relaxation is enabled), it probably makes sense to get this merged sooner rather than later.
I think the options are:
The psabi doc doesn't discuss attaching RISCV_RELAX to relocations other than RISCV_CALL or RISCV_CALL_PLT, but presumably binutils ld will use it for other pairs?
If we merge as is will leave the relocation difference between assembly code and object file as you described.
The psabi doc didn't mention add RISCV_RELAX to pcrel_hi and pcrel_lo relocations and I have done some experiments. It seems that attaching RISCV_RELAX to pcrel_hi and pcrel_lo relocations can't relax aupic and jalr to jal by Binutils ld. So we have to use R_RISCV_CALL and R_RISCV_RELAX for function call relaxation.
I have updated the patch to generate call pseudoinstruction as your comments in https://reviews.llvm.org/D44886?id=140176#inline-398646.
Hi Shiva. Could you please split out the definition of shouldForceRelocation, enableLinkerRelax and FeatureRelax to a separate patch. That way we can commit that followed by all the patches that rely on querying whether linker relaxation is enabled, and then finally enable it via the RISCVMCCodeEmitter changes in this patch. Now there are a few patches we know we want to commit before linker relaxation is actually safe to use, I think that's probably the best way to structure things.
Thanks Shiva, a few minor comments but with those resolved it should be good to go.