RFC: Turn on -fomit-frame-pointer and -fasynchronous-unwind-tables for Linux/i386

RFC: Turn on -fomit-frame-pointer and -fasynchronous-unwind-tables for Linux/i386

[H.J. Lu]

[-- Attachment #1: Type: text/plain, Size: 6344 bytes --]

Hi,

This patch turns on -fomit-frame-pointer and -fasynchronous-unwind-tables
for Linux/i386.  Tested on Linux/ia32 and Linux/x86-64.  I am also
enclosing a spreadsheet of comparison of shared library segment sizes
in gcc.  Segment size differences range from -1% to 30%. The smaller
the DSO size is, the bigger its size increases. Overall, the size
difference is close to 0%. This comparison may not be typical since
C++ and Java libraries have .eh_frame sections anyway.  Any comments?

Thanks.



H.J.
---
gcc/

2010-07-14  H.J. Lu  <hongjiu.lu@intel.com>

	* config/i386/i386.c (override_options): Default
	flag_omit_frame_pointer and flag_asynchronous_unwind_tables
	to TARGET_SUBTARGET32_OMIT_FRAME_POINTER_DEFAULT and
	TARGET_SUBTARGET32_ASYNCHRONOUS_UNWIND_TABLES_DEFAULT.

	* config/i386/i386.h (TARGET_SUBTARGET32_OMIT_FRAME_POINTER_DEFAULT):
	New.
	(TARGET_SUBTARGET32_ASYNCHRONOUS_UNWIND_TABLES_DEFAULT): Likewise.
	* config/i386/linux.h (TARGET_SUBTARGET32_OMIT_FRAME_POINTER_DEFAULT):
	Likewise.
	(TARGET_SUBTARGET32_ASYNCHRONOUS_UNWIND_TABLES_DEFAULT): Likewise.
	* config/i386/linux64.h (TARGET_SUBTARGET32_OMIT_FRAME_POINTER_DEFAULT):
	Likewise.
	(TARGET_SUBTARGET32_ASYNCHRONOUS_UNWIND_TABLES_DEFAULT): Likewise.

gcc/testsuite/

2010-07-15  H.J. Lu  <hongjiu.lu@intel.com>

	* gcc.target/i386/frame-pointer-1.c: New.
	* gcc.target/i386/frame-pointer-2.c: Likewise.
	* gcc.target/i386/frame-pointer-3.c: Likewise.
	* gcc.target/i386/frame-pointer-4.c: Likewise.

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 4fd2aab..fe30bfd 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -2985,10 +2985,23 @@ override_options (bool main_args_p)
     {
       if (flag_zee == 2)
         flag_zee = 0;
+      /* Unwind info is not correct around the CFG unless either a
+	 frame pointer is present or -maccumulate-outgoing-args is
+	 set. When both -fasynchronous-unwind-tables and
+	 -fomit-frame-pointer are turned on by default, turn off
+	 both if -mno-accumulate-outgoing-args is used.  */
       if (flag_omit_frame_pointer == 2)
-	flag_omit_frame_pointer = 0;
+	flag_omit_frame_pointer
+	  = (TARGET_SUBTARGET32_OMIT_FRAME_POINTER_DEFAULT
+	     && (!TARGET_SUBTARGET32_ASYNCHRONOUS_UNWIND_TABLES_DEFAULT
+		 || !(target_flags_explicit
+		      & MASK_ACCUMULATE_OUTGOING_ARGS)));
       if (flag_asynchronous_unwind_tables == 2)
-	flag_asynchronous_unwind_tables = 0;
+	flag_asynchronous_unwind_tables
+	  = (TARGET_SUBTARGET32_ASYNCHRONOUS_UNWIND_TABLES_DEFAULT
+	     && (!TARGET_SUBTARGET32_OMIT_FRAME_POINTER_DEFAULT
+		 || !(target_flags_explicit
+		      & MASK_ACCUMULATE_OUTGOING_ARGS)));
       if (flag_pcc_struct_return == 2)
 	flag_pcc_struct_return = DEFAULT_PCC_STRUCT_RETURN;
     }
diff --git a/gcc/config/i386/i386.h b/gcc/config/i386/i386.h
index eb3eb9f..c0ae95f 100644
--- a/gcc/config/i386/i386.h
+++ b/gcc/config/i386/i386.h
@@ -467,6 +467,8 @@ extern tree x86_mfence;
 /* Extra bits to force on w/ 32-bit mode.  */
 #define TARGET_SUBTARGET32_DEFAULT 0
 #define TARGET_SUBTARGET32_ISA_DEFAULT 0
+#define TARGET_SUBTARGET32_OMIT_FRAME_POINTER_DEFAULT 0
+#define TARGET_SUBTARGET32_ASYNCHRONOUS_UNWIND_TABLES_DEFAULT 0
 
 /* Extra bits to force on w/ 64-bit mode.  */
 #define TARGET_SUBTARGET64_DEFAULT 0
diff --git a/gcc/config/i386/linux.h b/gcc/config/i386/linux.h
index 81dfd1e..61d53b5 100644
--- a/gcc/config/i386/linux.h
+++ b/gcc/config/i386/linux.h
@@ -219,3 +219,10 @@ along with GCC; see the file COPYING3.  If not see
 /* i386 glibc provides __stack_chk_guard in %gs:0x14.  */
 #define TARGET_THREAD_SSP_OFFSET	0x14
 #endif
+
+/* Turn on -fomit-frame-pointer and -fasynchronous-unwind-tables by
+   default.  */
+#undef TARGET_SUBTARGET32_OMIT_FRAME_POINTER_DEFAULT
+#define TARGET_SUBTARGET32_OMIT_FRAME_POINTER_DEFAULT 1
+#undef TARGET_SUBTARGET32_ASYNCHRONOUS_UNWIND_TABLES_DEFAULT
+#define TARGET_SUBTARGET32_ASYNCHRONOUS_UNWIND_TABLES_DEFAULT 1
diff --git a/gcc/config/i386/linux64.h b/gcc/config/i386/linux64.h
index 33b4dc9..5a02205 100644
--- a/gcc/config/i386/linux64.h
+++ b/gcc/config/i386/linux64.h
@@ -123,3 +123,10 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
    x86_64 glibc provides it in %fs:0x28.  */
 #define TARGET_THREAD_SSP_OFFSET	(TARGET_64BIT ? 0x28 : 0x14)
 #endif
+
+/* Turn on -fomit-frame-pointer and -fasynchronous-unwind-tables by
+   default.  */
+#undef TARGET_SUBTARGET32_OMIT_FRAME_POINTER_DEFAULT
+#define TARGET_SUBTARGET32_OMIT_FRAME_POINTER_DEFAULT 1
+#undef TARGET_SUBTARGET32_ASYNCHRONOUS_UNWIND_TABLES_DEFAULT
+#define TARGET_SUBTARGET32_ASYNCHRONOUS_UNWIND_TABLES_DEFAULT 1
diff --git a/gcc/testsuite/gcc.target/i386/frame-pointer-1.c b/gcc/testsuite/gcc.target/i386/frame-pointer-1.c
new file mode 100644
index 0000000..2495d09
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/frame-pointer-1.c
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -mtune=i686" } */
+/* { dg-require-effective-target ilp32 } */
+
+void bar (int);
+
+void
+foo (void)
+{
+  bar (1);
+}
diff --git a/gcc/testsuite/gcc.target/i386/frame-pointer-2.c b/gcc/testsuite/gcc.target/i386/frame-pointer-2.c
new file mode 100644
index 0000000..1478aba
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/frame-pointer-2.c
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -mtune=i686 -mno-accumulate-outgoing-args" } */
+/* { dg-require-effective-target ilp32 } */
+
+void bar (int);
+
+void
+foo (void)
+{
+  bar (1);
+}
diff --git a/gcc/testsuite/gcc.target/i386/frame-pointer-3.c b/gcc/testsuite/gcc.target/i386/frame-pointer-3.c
new file mode 100644
index 0000000..2ff965f
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/frame-pointer-3.c
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -mtune=i686 -mpush-args" } */
+/* { dg-require-effective-target ilp32 } */
+
+void bar (int);
+
+void
+foo (void)
+{
+  bar (1);
+}
diff --git a/gcc/testsuite/gcc.target/i386/frame-pointer-4.c b/gcc/testsuite/gcc.target/i386/frame-pointer-4.c
new file mode 100644
index 0000000..2b1bf52
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/frame-pointer-4.c
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -mtune=i686 -mpush-args -mno-accumulate-outgoing-args" } */
+/* { dg-require-effective-target ilp32 } */
+
+void bar (int);
+
+void
+foo (void)
+{
+  bar (1);
+}

[-- Attachment #2: gcc frame pointer.xlsx --]
[-- Type: application/vnd.openxmlformats-officedocument.spreadsheetml.sheet, Size: 12803 bytes --]

Re: RFC: Turn on -fomit-frame-pointer and -fasynchronous-unwind-tables for Linux/i386

[Chris Lattner]

On Jul 15, 2010, at 10:42 AM, H.J. Lu wrote:

> Hi,
> 
> This patch turns on -fomit-frame-pointer and -fasynchronous-unwind-tables
> for Linux/i386.  Tested on Linux/ia32 and Linux/x86-64.  I am also
> enclosing a spreadsheet of comparison of shared library segment sizes
> in gcc.  Segment size differences range from -1% to 30%. The smaller
> the DSO size is, the bigger its size increases. Overall, the size
> difference is close to 0%. This comparison may not be typical since
> C++ and Java libraries have .eh_frame sections anyway.  Any comments?

Does glibc backtrace() look at eh_frame on i386?  I suspect that most unwinders don't.  This is a borderline ABI change.

-Chris

Re: RFC: Turn on -fomit-frame-pointer and -fasynchronous-unwind-tables for Linux/i386

[H.J. Lu]

On Thu, Jul 15, 2010 at 11:08 AM, Chris Lattner <clattner@apple.com> wrote:
>
> On Jul 15, 2010, at 10:42 AM, H.J. Lu wrote:
>
>> Hi,
>>
>> This patch turns on -fomit-frame-pointer and -fasynchronous-unwind-tables
>> for Linux/i386.  Tested on Linux/ia32 and Linux/x86-64.  I am also
>> enclosing a spreadsheet of comparison of shared library segment sizes
>> in gcc.  Segment size differences range from -1% to 30%. The smaller
>> the DSO size is, the bigger its size increases. Overall, the size
>> difference is close to 0%. This comparison may not be typical since
>> C++ and Java libraries have .eh_frame sections anyway.  Any comments?
>
> Does glibc backtrace() look at eh_frame on i386?  I suspect that most unwinders don't.  This is a borderline ABI change.
>

It does:

http://sourceware.org/git/?p=glibc.git;a=blob_plain;f=sysdeps/i386/backtrace.c;hb=HEAD


-- 
H.J.

Re: RFC: Turn on -fomit-frame-pointer and -fasynchronous-unwind-tables for Linux/i386

[H.J. Lu]

[-- Attachment #1: Type: text/plain, Size: 1173 bytes --]

On Thu, Jul 15, 2010 at 11:16 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Thu, Jul 15, 2010 at 11:08 AM, Chris Lattner <clattner@apple.com> wrote:
>>
>> On Jul 15, 2010, at 10:42 AM, H.J. Lu wrote:
>>
>>> Hi,
>>>
>>> This patch turns on -fomit-frame-pointer and -fasynchronous-unwind-tables
>>> for Linux/i386.  Tested on Linux/ia32 and Linux/x86-64.  I am also
>>> enclosing a spreadsheet of comparison of shared library segment sizes
>>> in gcc.  Segment size differences range from -1% to 30%. The smaller
>>> the DSO size is, the bigger its size increases. Overall, the size
>>> difference is close to 0%. This comparison may not be typical since
>>> C++ and Java libraries have .eh_frame sections anyway.  Any comments?
>>
>> Does glibc backtrace() look at eh_frame on i386?  I suspect that most unwinders don't.  This is a borderline ABI change.
>>
>
> It does:
>
> http://sourceware.org/git/?p=glibc.git;a=blob_plain;f=sysdeps/i386/backtrace.c;hb=HEAD
>

I updated the spread sheet with glibc 12.0 DSO sizes
I think -fomit-frame-pointer and -fasynchronous-unwind-tables
will increase size of DSO written in C by about 10%.


-- 
H.J.

[-- Attachment #2: gcc frame pointer.xlsx --]
[-- Type: application/vnd.openxmlformats-officedocument.spreadsheetml.sheet, Size: 34149 bytes --]

Re: RFC: Turn on -fomit-frame-pointer and -fasynchronous-unwind-tables for Linux/i386

[Roland McGrath]

> I updated the spread sheet with glibc 12.0 DSO sizes
> I think -fomit-frame-pointer and -fasynchronous-unwind-tables
> will increase size of DSO written in C by about 10%.

Are you talking about code size or text size?  Of course
-fasynchronous-unwind-tables increases the size of .eh_frame,
but that is not usually a performance issue.  The actual code
size increase is what affects hot path cache load and so forth.


Thanks,
Roland

Re: RFC: Turn on -fomit-frame-pointer and -fasynchronous-unwind-tables for Linux/i386

[H.J. Lu]

On Thu, Jul 15, 2010 at 12:49 PM, Roland McGrath <roland@redhat.com> wrote:
>> I updated the spread sheet with glibc 12.0 DSO sizes
>> I think -fomit-frame-pointer and -fasynchronous-unwind-tables
>> will increase size of DSO written in C by about 10%.
>
> Are you talking about code size or text size?  Of course
> -fasynchronous-unwind-tables increases the size of .eh_frame,
> but that is not usually a performance issue.  The actual code
> size increase is what affects hot path cache load and so forth.

-fomit-frame-pointer increases libc.so code size by 1%. We
have to save/restore EBP when using it GPR. But for some reason,
gcc will choose EBP even when ECX/EDX are available:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44958


-- 
H.J.

Re: RFC: Turn on -fomit-frame-pointer and -fasynchronous-unwind-tables for Linux/i386

[Chris Lattner]

On Jul 15, 2010, at 1:46 PM, H.J. Lu wrote:

> On Thu, Jul 15, 2010 at 12:49 PM, Roland McGrath <roland@redhat.com> wrote:
>>> I updated the spread sheet with glibc 12.0 DSO sizes
>>> I think -fomit-frame-pointer and -fasynchronous-unwind-tables
>>> will increase size of DSO written in C by about 10%.
>> 
>> Are you talking about code size or text size?  Of course
>> -fasynchronous-unwind-tables increases the size of .eh_frame,
>> but that is not usually a performance issue.  The actual code
>> size increase is what affects hot path cache load and so forth.
> 
> -fomit-frame-pointer increases libc.so code size by 1%. We
> have to save/restore EBP when using it GPR. But for some reason,
> gcc will choose EBP even when ECX/EDX are available:
> 
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44958

Another issue is that X86 encoding of 42(%esp) takes more space than 42(%ebp).  The former requires a SIB byte, the later doesn't.

-Chris

Re: RFC: Turn on -fomit-frame-pointer and -fasynchronous-unwind-tables for Linux/i386

[Andi Kleen]

"H.J. Lu" <hjl.tools@gmail.com> writes:

> On Thu, Jul 15, 2010 at 12:49 PM, Roland McGrath <roland@redhat.com> wrote:
>>> I updated the spread sheet with glibc 12.0 DSO sizes
>>> I think -fomit-frame-pointer and -fasynchronous-unwind-tables
>>> will increase size of DSO written in C by about 10%.
>>
>> Are you talking about code size or text size?  Of course
>> -fasynchronous-unwind-tables increases the size of .eh_frame,
>> but that is not usually a performance issue.  The actual code
>> size increase is what affects hot path cache load and so forth.
>
> -fomit-frame-pointer increases libc.so code size by 1%. We
> have to save/restore EBP when using it GPR. But for some reason,
> gcc will choose EBP even when ECX/EDX are available:
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44958

Ideal would be if -Os could automatically switch on -fframe-pointer
when the function has many local variables on the stack
(reference through %rbp is one byte smaller than through another GPR)
and otherwise keep it off.

I believe MSVC does that and it's a useful optimization for x86.

But it's presumably hard to do in gcc because FP or no FP
gets decided fairly early. Still perhaps it could be done
with a heuristic?

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only.

Re: RFC: Turn on -fomit-frame-pointer and -fasynchronous-unwind-tables for Linux/i386

[Dave Korn]

On 16/07/2010 09:13, Andi Kleen wrote:

> (reference through %rbp is one byte smaller than through another GPR)

> I believe MSVC does that and it's a useful optimization for x86.

  I can't tell if you meant x86_64 or %ebp but those two statements don't
quite go together as-is! :)

    cheers,
      DaveK

Re: RFC: Turn on -fomit-frame-pointer and -fasynchronous-unwind-tables for Linux/i386

[Andi Kleen]

On Fri, Jul 16, 2010 at 05:21:16PM +0100, Dave Korn wrote:
> On 16/07/2010 09:13, Andi Kleen wrote:
> 
> > (reference through %rbp is one byte smaller than through another GPR)
> 
> > I believe MSVC does that and it's a useful optimization for x86.
> 
>   I can't tell if you meant x86_64 or %ebp but those two statements don't
> quite go together as-is! :)

The optimization is valid for 32bit and 64bit.

When you have a lot of variable references onto the stack it's smaller
to generate a frame pointer.

However on 64bit since there are more registers this should generally happen somewhat 
less often, but there is always code that exceeds even the 16 registers on 64bit
too.

The problem in gcc would be really to figure this out in advance.

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only.

Re: RFC: Turn on -fomit-frame-pointer and -fasynchronous-unwind-tables for Linux/i386

[Dave Korn]

On 16/07/2010 20:24, Andi Kleen wrote:
> On Fri, Jul 16, 2010 at 05:21:16PM +0100, Dave Korn wrote:
>> On 16/07/2010 09:13, Andi Kleen wrote:
>>
>>> (reference through %rbp is one byte smaller than through another GPR)
>>> I believe MSVC does that and it's a useful optimization for x86.
>>   I can't tell if you meant x86_64 or %ebp but those two statements don't
>> quite go together as-is! :)
> 
> The optimization is valid for 32bit and 64bit.

  There's no such thing as %rbp in 32-bit x86 is all I meant.  Does the "one
byte smaller" also apply to %ebp indirection on x86?

    cheers,
      DaveK

Re: RFC: Turn on -fomit-frame-pointer and -fasynchronous-unwind-tables for Linux/i386

[Andi Kleen]

On Sat, Jul 17, 2010 at 08:59:24PM +0100, Dave Korn wrote:
> On 16/07/2010 20:24, Andi Kleen wrote:
> > On Fri, Jul 16, 2010 at 05:21:16PM +0100, Dave Korn wrote:
> >> On 16/07/2010 09:13, Andi Kleen wrote:
> >>
> >>> (reference through %rbp is one byte smaller than through another GPR)
> >>> I believe MSVC does that and it's a useful optimization for x86.
> >>   I can't tell if you meant x86_64 or %ebp but those two statements don't
> >> quite go together as-is! :)
> > 
> > The optimization is valid for 32bit and 64bit.
> 
>   There's no such thing as %rbp in 32-bit x86 is all I meant.  Does the "one
> byte smaller" also apply to %ebp indirection on x86?

Yes. The two encodings are exactly the same.

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only.