User account creation filtered due to spam.

Bug 47466 - c++ __builtin_expect() regression
Summary: c++ __builtin_expect() regression
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: middle-end (show other bugs)
Version: 4.5.2
: P3 normal
Target Milestone: 5.0
Assignee: Not yet assigned to anyone
URL:
Keywords: missed-optimization
Depends on:
Blocks:
 
Reported: 2011-01-26 01:24 UTC by Guillaume Morin
Modified: 2016-01-06 02:29 UTC (History)
0 users

See Also:
Host:
Target:
Build:
Known to work: 5.0
Known to fail: 4.8.4
Last reconfirmed: 2011-12-15 00:00:00


Attachments
test case (248 bytes, application/octet-stream)
2011-01-26 01:24 UTC, Guillaume Morin
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Guillaume Morin 2011-01-26 01:24:34 UTC
Created attachment 23127 [details]
test case

This is a followup from http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42233

While the example given there now works, a slightly different example leads to the same problem: the "unlikely" path is favored by the compiler (the likely case will branch).

$ cat foo.cpp
struct EnumStruct {
    enum Enum { ONE, TWO, THREE };
    char e;
    EnumStruct(Enum _e) : e(_e) {}
    operator Enum() const { return (Enum)e; }
};

extern void unlikelyCall();

#define unlikely(x) __builtin_expect(!!(x), 0)

int test_expect(const EnumStruct& s)  {
    if (unlikely(s != EnumStruct::ONE && s != EnumStruct::TWO)) {
        unlikelyCall();
        return 1;
    }
    return 0;
}
int test_expect2(const EnumStruct& s)  {
    if (unlikely(s != EnumStruct::ONE) &&
        unlikely(s != EnumStruct::TWO)) {
        unlikelyCall();
        return 1;
    }
    return 0;
}
$ g++45 -O3 -S -o foo.S foo.cpp
$ cat foo.S | c++filt 
	.file	"foo.cpp"
	.text
	.p2align 4,,15
.globl test_expect(EnumStruct const&)
	.type	test_expect(EnumStruct const&), @function
test_expect(EnumStruct const&):
.LFB4:
	.cfi_startproc
	subq	$8, %rsp
	.cfi_def_cfa_offset 16
	movsbl	(%rdi), %edx
	xorl	%eax, %eax
	testl	%edx, %edx
	je	.L2
	cmpl	$1, %edx
	je	.L2
	call	unlikelyCall()
	movl	$1, %eax
.L2:
	addq	$8, %rsp
	.cfi_def_cfa_offset 8
	ret
	.cfi_endproc
.LFE4:
	.size	test_expect(EnumStruct const&), .-test_expect(EnumStruct const&)
	.p2align 4,,15
.globl test_expect2(EnumStruct const&)
	.type	test_expect2(EnumStruct const&), @function
test_expect2(EnumStruct const&):
.LFB5:
	.cfi_startproc
	subq	$8, %rsp
	.cfi_def_cfa_offset 16
	movsbl	(%rdi), %edx
	testl	%edx, %edx
	jne	.L10
	xorl	%eax, %eax
.L7:
	addq	$8, %rsp
	.cfi_remember_state
	.cfi_def_cfa_offset 8
	ret
.L10:
	.cfi_restore_state
	xorl	%eax, %eax
	subl	$1, %edx
	je	.L7
	call	unlikelyCall()
	movl	$1, %eax
	jmp	.L7
	.cfi_endproc
.LFE5:
	.size	test_expect2(EnumStruct const&), .-test_expect2(EnumStruct const&)
	.ident	"GCC: (GNU) .5.2 20100819 (prerelease)"


Same symptom as bug 42233.  The unlikely path is favored and breaking the "unlilkely(foo && bar)" into "unlikely(foo) && unlikely(bar)" works around the problem.

I verified that this example leads to the likely path being favored with gcc 4.1.2 from CentOS, 4.2.1 from FreeBSD.  gcc 4.3 seems to have the same problem
Comment 1 Guillaume Morin 2011-01-26 18:26:34 UTC
A recent 4.6 snapshot shows the same issue

	.file	"foo.cpp"
	.text
	.p2align 4,,15
	.globl	test_expect(EnumStruct const&)
	.type	test_expect(EnumStruct const&), @function
test_expect(EnumStruct const&):
.LFB4:
	.cfi_startproc
	subq	$8, %rsp
	.cfi_def_cfa_offset 16
	movsbl	(%rdi), %edx
	xorl	%eax, %eax
	testl	%edx, %edx
	je	.L2
	cmpl	$1, %edx
	je	.L2
	call	unlikelyCall()
	movl	$1, %eax
.L2:
	addq	$8, %rsp
	.cfi_def_cfa_offset 8
	ret
	.cfi_endproc
.LFE4:
	.size	test_expect(EnumStruct const&), .-test_expect(EnumStruct const&)
	.p2align 4,,15
	.globl	test_expect2(EnumStruct const&)
	.type	test_expect2(EnumStruct const&), @function
test_expect2(EnumStruct const&):
.LFB5:
	.cfi_startproc
	subq	$8, %rsp
	.cfi_def_cfa_offset 16
	movsbl	(%rdi), %edx
	xorl	%eax, %eax
	testl	%edx, %edx
	jne	.L10
.L7:
	addq	$8, %rsp
	.cfi_remember_state
	.cfi_def_cfa_offset 8
	ret
.L10:
	.cfi_restore_state
	subl	$1, %edx
	je	.L7
	.p2align 4,,5
	call	unlikelyCall()
	movl	$1, %eax
	jmp	.L7
	.cfi_endproc
.LFE5:
	.size	test_expect2(EnumStruct const&), .-test_expect2(EnumStruct const&)
	.ident	"GCC: (Debian 20110116-1) 4.6.0 20110116 (experimental) [trunk revision 168860]"
	.section	.note.GNU-stack,"",@progbits
Comment 2 Andrew Pinski 2011-12-15 22:29:48 UTC
Confirmed, the C testcase works though.
Comment 3 Andrew Pinski 2016-01-06 02:29:26 UTC
This is fixed now.  I don't know what fixed it.