[PowerPC] On PPC64LE, clang -fpatchable-function-entry=1 records function beginning instead of first NOP emitted

[hbathini]

On PPC64 little-endian Linux -fpatchable-function-entry support is added with PR#151569. __patchable_function_entries records the current function beginning when there are no prefix NOPs. But on ppc64le, this ends up pointing at Global Entry Point instead of the first NOP emitted.

$ cat test.c
attribute((noinline)) void foo(void) {}

int main() {
foo();
return 0;
}
$
$
$
$ clang -g -O0 -fpatchable-function-entry=1 -c test.c -o test.o
$
$
$
$ objdump -d test.o

test.o: file format elf64-powerpcle

Disassembly of section .text:

0000000000000000 :
0: 00 00 00 60 nop
4: 20 00 80 4e blr
...
14: 00 00 00 60 nop
18: 00 00 00 60 nop
1c: 00 00 00 60 nop

0000000000000020

:
20: 00 00 4c 3c addis r2,r12,0
24: 00 00 42 38 addi r2,r2,0
28: 00 00 00 60 nop
2c: a6 02 08 7c mflr r0
30: f8 ff e1 fb std r31,-8(r1)
34: 91 ff 21 f8 stdu r1,-112(r1)
38: 80 00 01 f8 std r0,128(r1)
3c: 78 0b 3f 7c mr r31,r1
40: 00 00 60 38 li r3,0
44: 64 00 7f 90 stw r3,100(r31)
48: 01 00 00 48 bl 48 <main+0x28>
4c: 00 00 00 60 nop
50: 00 00 60 38 li r3,0
54: 70 00 21 38 addi r1,r1,112
58: 10 00 01 e8 ld r0,16(r1)
5c: f8 ff e1 eb ld r31,-8(r1)
60: a6 03 08 7c mtlr r0
64: 20 00 80 4e blr
...
$
$
$
$ objdump -r -j __patchable_function_entries test.o

test.o: file format elf64-powerpcle

RELOCATION RECORDS FOR [__patchable_function_entries]:
OFFSET TYPE VALUE
0000000000000000 R_PPC64_ADDR64 .text

RELOCATION RECORDS FOR [__patchable_function_entries]:
OFFSET TYPE VALUE
0000000000000000 R_PPC64_ADDR64 .text+0x0000000000000020

$

Expected the second record to point at .text+0x28 instead of .text+0x20

[llvmbot]

@llvm/issue-subscribers-backend-powerpc

Author: Hari Bathini (hbathini)

On PPC64 little-endian Linux -fpatchable-function-entry support is added with PR#151569. __patchable_function_entries records the current function beginning when there are no prefix NOPs. But on ppc64le, this ends up pointing at Global Entry Point instead of the first NOP emitted.

> $ cat test.c
> attribute((noinline)) void foo(void) {}
>
> int main() {
> foo();
> return 0;
> }
> $
> $
> $
> $ clang -g -O0 -fpatchable-function-entry=1 -c test.c -o test.o
> $
> $
> $
> $ objdump -d test.o
>
> test.o: file format elf64-powerpcle
>
>
> Disassembly of section .text:
>
> 0000000000000000 <foo>:
> 0: 00 00 00 60 nop
> 4: 20 00 80 4e blr
> ...
> 14: 00 00 00 60 nop
> 18: 00 00 00 60 nop
> 1c: 00 00 00 60 nop
>
> 0000000000000020 <main>:
> 20: 00 00 4c 3c addis r2,r12,0
> 24: 00 00 42 38 addi r2,r2,0
> 28: 00 00 00 60 nop
> 2c: a6 02 08 7c mflr r0
> 30: f8 ff e1 fb std r31,-8(r1)
> 34: 91 ff 21 f8 stdu r1,-112(r1)
> 38: 80 00 01 f8 std r0,128(r1)
> 3c: 78 0b 3f 7c mr r31,r1
> 40: 00 00 60 38 li r3,0
> 44: 64 00 7f 90 stw r3,100(r31)
> 48: 01 00 00 48 bl 48 <main+0x28>
> 4c: 00 00 00 60 nop
> 50: 00 00 60 38 li r3,0
> 54: 70 00 21 38 addi r1,r1,112
> 58: 10 00 01 e8 ld r0,16(r1)
> 5c: f8 ff e1 eb ld r31,-8(r1)
> 60: a6 03 08 7c mtlr r0
> 64: 20 00 80 4e blr
> ...
> $
> $
> $
> $ objdump -r -j __patchable_function_entries test.o
>
> test.o: file format elf64-powerpcle
>
> RELOCATION RECORDS FOR [__patchable_function_entries]:
> OFFSET TYPE VALUE
> 0000000000000000 R_PPC64_ADDR64 .text
>
>
> RELOCATION RECORDS FOR [__patchable_function_entries]:
> OFFSET TYPE VALUE
> 0000000000000000 R_PPC64_ADDR64 .text+0x0000000000000020
>
>
> $

Expected the second record to point at .text+0x28 instead of .text+0x20

[nemanjai]

@chenzheng1030 I'm not sure if you still contribute to PPC, but it seems that you implemented this (at least for PPC32 and BE PPC64). Do you think this has something to do with the fact that we don't handle this in PPCInstrInfo::getInstSizeInBytes()?

[nemanjai]

@lei137 It appears that you don't get notified for PPC bugs that get opened. It would probably be more meaningful for you to get notified than me at this point :)

[chenzheng1030]

@chenzheng1030 I'm not sure if you still contribute to PPC, but it seems that you implemented this (at least for PPC32 and BE PPC64). Do you think this has something to do with the fact that we don't handle this in PPCInstrInfo::getInstSizeInBytes()?

Hi Nemanjai, great to work with you again:) I am currently not working on the upstream LLVM and also not on the PPC targets. I saw @maryammo recently added -fpatchable-function-entry support for PPC64LE in #151569. Could you please help to have a look at this bug? @maryammo Thanks.

[maryammo]

@chenzheng1030 I'm not sure if you still contribute to PPC, but it seems that you implemented this (at least for PPC32 and BE PPC64). Do you think this has something to do with the fact that we don't handle this in PPCInstrInfo::getInstSizeInBytes()?

Hi Nemanjai, great to work with you again:) I am currently not working on the upstream LLVM and also not on the PPC targets. I saw @maryammo recently added -fpatchable-function-entry support for PPC64LE in #151569. Could you please help to have a look at this bug? @maryammo Thanks.

Hello @chenzheng1030 and @nemanjai, Clang currently records GEP for ppc64le while GCC records LEP for ppc64le, so this is to investigate and match Clang behavior with GCC as discussed with Hari. We will look into this later. Thanks.