1. 14 Aug, 2021 1 commit
    • Christophe Leroy's avatar
      powerpc/bug: Remove specific powerpc BUG_ON() and WARN_ON() on PPC32 · db87a719
      Christophe Leroy authored
      powerpc BUG_ON() and WARN_ON() are based on using twnei instruction.
      
      For catching simple conditions like a variable having value 0, this
      is efficient because it does the test and the trap at the same time.
      But most conditions used with BUG_ON or WARN_ON are more complex and
      forces GCC to format the condition into a 0 or 1 value in a register.
      This will usually require 2 to 3 instructions.
      
      The most efficient solution would be to use __builtin_trap() because
      GCC is able to optimise the use of the different trap instructions
      based on the requested condition, but this is complex if not
      impossible for the following reasons:
      - __builtin_trap() is a non-recoverable instruction, so it can't be
      used for WARN_ON
      - Knowing which line of code generated the trap would require the
      analysis of DWARF information. This is not a feature we have today.
      
      As mentioned in commit 8d4fbcfb ("Fix WARN_ON() on bitfield ops")
      the way WARN_ON() is implemented is suboptimal. That commit also
      mentions an issue with 'long long' condition. It fixed it for
      WARN_ON() but the same problem still exists today with BUG_ON() on
      PPC32. It will be fixed by using the generic implementation.
      
      By using the generic implementation, gcc will naturally generate a
      branch to the unconditional trap generated by BUG().
      
      As modern powerpc implement zero-cycle branch,
      that's even more efficient.
      
      And for the functions using WARN_ON() and its return, the test
      on return from WARN_ON() is now also used for the WARN_ON() itself.
      
      On PPC64 we don't want it because we want to be able to use CFAR
      register to track how we entered the code that trapped. The CFAR
      register would be clobbered by the branch.
      
      A simple test function:
      
      	unsigned long test9w(unsigned long a, unsigned long b)
      	{
      		if (WARN_ON(!b))
      			return 0;
      		return a / b;
      	}
      
      Before the patch:
      
      	0000046c <test9w>:
      	 46c:	7c 89 00 34 	cntlzw  r9,r4
      	 470:	55 29 d9 7e 	rlwinm  r9,r9,27,5,31
      	 474:	0f 09 00 00 	twnei   r9,0
      	 478:	2c 04 00 00 	cmpwi   r4,0
      	 47c:	41 82 00 0c 	beq     488 <test9w+0x1c>
      	 480:	7c 63 23 96 	divwu   r3,r3,r4
      	 484:	4e 80 00 20 	blr
      
      	 488:	38 60 00 00 	li      r3,0
      	 48c:	4e 80 00 20 	blr
      
      After the patch:
      
      	00000468 <test9w>:
      	 468:	2c 04 00 00 	cmpwi   r4,0
      	 46c:	41 82 00 0c 	beq     478 <test9w+0x10>
      	 470:	7c 63 23 96 	divwu   r3,r3,r4
      	 474:	4e 80 00 20 	blr
      
      	 478:	0f e0 00 00 	twui    r0,0
      	 47c:	38 60 00 00 	li      r3,0
      	 480:	4e 80 00 20 	blr
      
      So we see before the patch we need 3 instructions on the likely path
      to handle the WARN_ON(). With the patch the trap goes on the unlikely
      path.
      
      See below the difference at the entry of system_call_exception where
      we have several BUG_ON(), allthough less impressing.
      
      With the patch:
      
      	00000000 <system_call_exception>:
      	   0:	81 6a 00 84 	lwz     r11,132(r10)
      	   4:	90 6a 00 88 	stw     r3,136(r10)
      	   8:	71 60 00 02 	andi.   r0,r11,2
      	   c:	41 82 00 70 	beq     7c <system_call_exception+0x7c>
      	  10:	71 60 40 00 	andi.   r0,r11,16384
      	  14:	41 82 00 6c 	beq     80 <system_call_exception+0x80>
      	  18:	71 6b 80 00 	andi.   r11,r11,32768
      	  1c:	41 82 00 68 	beq     84 <system_call_exception+0x84>
      	  20:	94 21 ff e0 	stwu    r1,-32(r1)
      	  24:	93 e1 00 1c 	stw     r31,28(r1)
      	  28:	7d 8c 42 e6 	mftb    r12
      	...
      	  7c:	0f e0 00 00 	twui    r0,0
      	  80:	0f e0 00 00 	twui    r0,0
      	  84:	0f e0 00 00 	twui    r0,0
      
      Without the patch:
      
      	00000000 <system_call_exception>:
      	   0:	94 21 ff e0 	stwu    r1,-32(r1)
      	   4:	93 e1 00 1c 	stw     r31,28(r1)
      	   8:	90 6a 00 88 	stw     r3,136(r10)
      	   c:	81 6a 00 84 	lwz     r11,132(r10)
      	  10:	69 60 00 02 	xori    r0,r11,2
      	  14:	54 00 ff fe 	rlwinm  r0,r0,31,31,31
      	  18:	0f 00 00 00 	twnei   r0,0
      	  1c:	69 60 40 00 	xori    r0,r11,16384
      	  20:	54 00 97 fe 	rlwinm  r0,r0,18,31,31
      	  24:	0f 00 00 00 	twnei   r0,0
      	  28:	69 6b 80 00 	xori    r11,r11,32768
      	  2c:	55 6b 8f fe 	rlwinm  r11,r11,17,31,31
      	  30:	0f 0b 00 00 	twnei   r11,0
      	  34:	7d 8c 42 e6 	mftb    r12
      Signed-off-by: default avatarChristophe Leroy <christophe.leroy@csgroup.eu>
      Signed-off-by: default avatarMichael Ellerman <mpe@ellerman.id.au>
      Link: https://lore.kernel.org/r/b286e07fb771a664b631cd07a40b09c06f26e64b.1618331881.git.christophe.leroy@csgroup.eu
      db87a719
  2. 13 Aug, 2021 11 commits
  3. 10 Aug, 2021 28 commits