Thread (41 messages) 41 messages, 7 authors, 2022-07-08

Re: [RFC PATCH v3 11/12] powerpc: Remove unreachable() from WARN_ON() (gcc issue ?)

From: Christophe Leroy <hidden>
Date: 2022-07-01 11:40:45
Also in: linux-arm-kernel, lkml

Hi Segher, your help might be welcome,

Le 01/07/2022 à 08:56, Sathvika Vasireddy a écrit :
Hi Chen,

Thanks for pitching in and providing your inputs :-)

On 01/07/22 07:43, Chen Zhongjin wrote:
quoted
Hi everyone,

Hope I'm not too late for this discussion.

I'm not familiar with ppc so it spent me some time to reproduce this. 
But at last I didn't make it.

What I did:

1 checkout to tip/objtool/core

2 apply this patch

3 recover the unreachable() after WARN_ENTRY(..

4 compile on defconfig/allyesconfig

If anyone can point out which file will generate this problem it will 
be perfect.
To reproduce this problem, you may want to apply this patch series on 
top of objtool/core branch of the tip tree, and compile on 
ppc64le_defconfig.

There are a couple of C files that are generating these warnings. One 
such file is: arch/powerpc/kvm/../../../virt/kvm/kvm_main.o which gives
*arch/powerpc/kvm/../../../virt/kvm/kvm_main.o: warning: objtool: 
kvm_mmu_notifier_release+0x6c: unannotated intra-function call* warning.

With unreachable() in __WARN_FLAGS(), we get unannotated intra-function 
call warnings, but without unreachable() like in patch 11/12 or with 
just the builtin variant of unreachable (__builtin_unreachable()) 
instead of unreachable(), we do not get those warnings.

I made a simple test aside our issue to see if I get something similar 
to ARM. I don't if it is the same at the end, but it looks odd anyway:

int test(int x)
{
	switch(x) {
	case 0 : return 3;
	case 1 : return 5;
	default : unreachable();
	}
}

0000000000001c80 <test>:
     1c80:       a6 02 08 7c     mflr    r0
     1c84:       01 00 00 48     bl      1c84 <test+0x4>
                         1c84: R_PPC64_REL24     _mcount
     1c88:       00 00 23 2c     cmpdi   r3,0
     1c8c:       14 00 82 41     beq     1ca0 <test+0x20>
     1c90:       01 00 03 2c     cmpwi   r3,1
     1c94:       05 00 60 38     li      r3,5
     1c98:       20 00 82 4d     beqlr
     1c9c:       00 00 42 60     ori     r2,r2,0
     1ca0:       03 00 60 38     li      r3,3
     1ca4:       20 00 80 4e     blr

So it looks like it takes no real benefit from the unreachable marking.

Now with __builtin_unreachable() instead of unreachable() :

int test(int x)
{
	switch(x) {
	case 0 : return 3;
	case 1 : return 5;
	default : __builtin_unreachable();
	}
}

0000000000001c80 <test>:
     1c80:       a6 02 08 7c     mflr    r0
     1c84:       01 00 00 48     bl      1c84 <test+0x4>
                         1c84: R_PPC64_REL24     _mcount
     1c88:       00 00 63 20     subfic  r3,r3,0
     1c8c:       10 19 63 7c     subfe   r3,r3,r3
     1c90:       bc 07 63 54     rlwinm  r3,r3,0,30,30
     1c94:       03 00 63 38     addi    r3,r3,3
     1c98:       20 00 80 4e     blr


Here the generated code takes advantage of the unreachable marking and 
results in a branchless code.


Christophe
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help