Re: [PATCH] WARN_ONCE(): use bool for boolean flag

[Cesar Eduardo Barros]

Cesar Eduardo Barros escreveu:
> Daniel Walker escreveu:
> > On Sun, 2009-09-27 at 14:24 -0300, Cesar Eduardo Barros wrote:
> >
> > > In fact, I was expecting no change at all, since gcc should be able 
> > > to see it is being treated as a boolean (perhaps I am trusting gcc 
> > > too much). And to make matters even more confusing, my own test 
> > > changing all __ret_warn_once to bool and dropping the !! caused an 
> > > _increase_ of 598 bytes (x86-64 defconfig).
> > >
> > >     text    data     bss     dec     hex filename
> > > 8100553 1207148  991988 10299689         9d2929 vmlinux.warnret.before
> > > 8101119 1207180  991988 10300287         9d2b7f vmlinux.warnret.after
> > >
> > > (And yes, data increased again.)
> >
> > If this was just your regular base line config , then that is odd .. I
> > also would think worse case would be no size reduction .. I did my
> > compile test on x86-32 btw..
>
> I will try looking at the first function which shows a difference in 
> size (which appears to be handle_irq) and see what I can find.

I just took a quick look, and it does seem to be bad code generation 
(the gcc on this machine is a bit old). The question is, is the gain in 
less buggy gcc versions enough to offset the loss in older and buggier 
gcc versions?

The function in question (stack_overflow_check() in 
arch/x86/kernel/irq_64.c) has a somewhat complex expression in the call 
to WARN_ON, which gcc seems to be pessimizing in this case (it is 
storing the boolean in a register just to test it again).

I will send the patch I am using in the next email.

gcc (Ubuntu 4.3.2-1ubuntu12) 4.3.2

--- /dev/fd/63	2009-09-27 14:59:26.124947107 -0300
+++ /dev/fd/62	2009-09-27 14:59:26.144947152 -0300
@@ -246,14 +246,14 @@
 	pushq	%rbp
 #APP
 # 14 
"/scratch/build/cesarb/linux/linux-2.6/arch/x86/include/asm/current.h" 1
-	movq %gs:per_cpu__current_task,%rcx
+	movq %gs:per_cpu__current_task,%rax
 # 0 "" 2
 #NO_APP
 	movq	%rsp, %rbp
 	pushq	%rbx
 	movl	%edi, %ebx
 	subq	$8, %rsp
-	movq	8(%rcx), %r8
+	movq	8(%rax), %r8
 	movq	152(%rsi), %rdx
 	cmpq	%r8, %rdx
 	jb	.L24
@@ -262,28 +262,40 @@
 	ja	.L24
 	leaq	400(%r8), %rax
 	cmpq	%rax, %rdx
-	jae	.L24
+	setb	%al
+	movzbl	%al, %eax
+	jmp	.L25
+.L24:
+	xorl	%eax, %eax
+.L25:
+	testl	%eax, %eax
+	je	.L26
 	cmpb	$0, __warned.21424(%rip)
-	jne	.L24
+	jne	.L26
 	movq	%rdx, %r9
-	addq	$1112, %rcx
-	movq	$.LC3, %rdx
 	movl	$47, %esi
+	movq	$.LC3, %rdx
+#APP
+# 14 
"/scratch/build/cesarb/linux/linux-2.6/arch/x86/include/asm/current.h" 1
+	movq %gs:per_cpu__current_task,%rcx
+# 0 "" 2
+#NO_APP
 	movq	$.LC0, %rdi
+	addq	$1112, %rcx
 	xorl	%eax, %eax
 	call	warn_slowpath_fmt
 	movb	$1, __warned.21424(%rip)
-.L24:
+.L26:
 	movl	%ebx, %edi
 	call	irq_to_desc
 	xorl	%edx, %edx
 	testq	%rax, %rax
-	je	.L26
+	je	.L28
 	movq	%rax, %rsi
 	movl	%ebx, %edi
 	call	*24(%rax)
 	movb	$1, %dl
-.L26:
+.L28:
 	movb	%dl, %al
 	popq	%rdx
 	popq	%rbx


--
Cesar Eduardo Barros
cesarb@xxxxxxxxxx
cesar.barros@xxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/