HomePhabricator

[RISCV] Properly evaluate fixup_riscv_pcrel_lo12

Description

[RISCV] Properly evaluate fixup_riscv_pcrel_lo12

This is a update to D43157 to correctly handle fixup_riscv_pcrel_lo12.

Notable changes:

Rebased onto trunk
Handle and test S-type
Test case pcrel-hilo.s is merged into relocations.s

D43157 description:
VK_RISCV_PCREL_LO has to be handled specially. The MCExpr inside is
actually the location of an auipc instruction with a VK_RISCV_PCREL_HI fixup
pointing to the real target.

Differential Revision: https://reviews.llvm.org/D54029
Patch by Chih-Mao Chen and Michael Spencer.

Details

Committed
asbDec 20 2018, 6:52 AM
Differential Revision
D54029: [RISCV] Properly evaluate fixup_riscv_pcrel_lo12
Parents
rL349763: [X86][AVX512] Don't custom lower v16i8 rotations.
Branches
Unknown
Tags
Unknown

Event Timeline

Hi Alex,

I found an odd behaviour occurring after this change went in.

Consider the following minimised testcase:

	.text
	.globl	main
	.type	main,@function
main:
.Lpic_label:
	auipc	a0, %pcrel_hi(timeout_handler)
	addi	a1, a0, %pcrel_lo(.Lpic_label)
	.p2align	2 ## This seems to be the culprit
	.type	timeout_handler,@function
timeout_handler:
	ret

Checking the relocations in the final binary I see this

$ ./bin/clang --target=riscv64 -c t.s && ./bin/llvm-objdump -d -r t.o
t.o:	file format ELF64-riscv

Disassembly of section .text:
main:
       0:	17 05 00 00 	auipc	a0, 0
       4:	93 05 05 00 	mv	a1, a0
		0000000000000004:  R_RISCV_PCREL_LO12_I	timeout_handler+4

timeout_handler:
       8:	67 80 00 00 	ret

That R_RISCV_PCREL_LO12_I seems stray to me (and indeed GNU ld fails to link after this happens, a quick look at the code suggests it can't find the pcrel_hi relocation but I'm no expert about it).

Apparently the .p2align directive has an impact and removing it (and introducing nops accordingly) seems to hide the issue.

Looking at the difference between the two cases, the problematic one has two extra fragments one MCAlignmentFragment followed by a MCDataFragment. I'm not sure what causes the second relocation be left there.

I will investigate further, but perhaps you have a rough idea of what might be going on here?

Thanks!