This is an archive of the discontinued LLVM Phabricator instance.

[LoongArch] Allow %pc_lo12 relocs in JIRL's immediate operand position
ClosedPublic

Authored by xen0n on Jan 21 2023, 2:22 AM.

Details

Summary

Currently, gcc-13 will generate such assembly when -mcmodel=medium,
which is ostensibly a dirty hack to allow bigger offsets for extern
function calls without having to add more reloc types. This is not the
best way to accomplish the original goal, but such usages will appear
soon and we have to support it anyway.

Example:

c
extern int foo(int);

int bar(int x) {
    return foo(x + 123);
}

will produce the following (simplified) assembly when compiled with
-O2 -mcmodel=medium:

    .globl  bar
    .type   bar, @function
bar:
    .cfi_startproc
    addi.w  $r4,$r4,123
    pcalau12i   $r12,%pc_hi20(foo)
    jirl    $r0,$r12,%pc_lo12(foo)
    .cfi_endproc

Diff Detail

Event Timeline

xen0n created this revision.Jan 21 2023, 2:22 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 21 2023, 2:22 AM
Herald added a subscriber: hiraditya. · View Herald Transcript
xen0n requested review of this revision.Jan 21 2023, 2:22 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 21 2023, 2:22 AM
xry111 accepted this revision.Jan 21 2023, 3:36 AM

LGTM though I really dislike depending on some undocumented behavior of PCALA_LO12.

Happe new Lunar year!

This revision is now accepted and ready to land.Jan 21 2023, 3:36 AM
SixWeining accepted this revision.Jan 21 2023, 3:47 AM

LGTM. Happy Chinese new year!

wangleiat accepted this revision.Jan 21 2023, 4:42 AM

LGTM.
春节快乐。:)

xen0n added a comment.Jan 21 2023, 5:09 AM

I only realized it's the Chinese New Year's Eve after finishing this and seeing your comments. /facepalm

Thanks and Happy New Year!

By the way, using PCALA_LO12 like this means lld will need to peek at the instruction and see if it's a jirl to handle the PCALA_LO12 relocation properly. I guess we'll need to investigate this issue a little. I hope it won't cause too much trouble.

xen0n added a comment.Jan 21 2023, 5:18 AM

LGTM though I really dislike depending on some undocumented behavior of PCALA_LO12.

I dislike it too (a longer rant can be seen in the LLD patch btw), but it's already too late to change: gcc-13 is already at Stage 4 and glibc really had to be changed to allow linking Firefox/Chromium. It's a pity that your PR was ignored, but we might be able to pull off the transition eventually after 1 or 2 years, given the current trend of new world adoption and mainstream distros' porting progress. Not all is lost ;-)

Happe new Lunar year!

Happy new year! 春节快乐!

xen0n added a comment.Jan 21 2023, 5:40 AM

By the way, using PCALA_LO12 like this means lld will need to peek at the instruction and see if it's a jirl to handle the PCALA_LO12 relocation properly. I guess we'll need to investigate this issue a little. I hope it won't cause too much trouble.

I've checked and there is one (and only one) similar case: X86 R_386_GOT32 and R_386_GOT32X need to read the input section too. At least there is precedent so we're technically not alone... and we're only a PR away from having this kludge documented.

MaskRay accepted this revision.Jan 21 2023, 5:43 AM