Turn on -fomit-frame-pointer by default for 32bit x86?

Turn on -fomit-frame-pointer by default for 32bit x86?

[H.J. Lu]

Hi,

I checked the default code for 32bit. We currently generate

[hjl@gnu-6 tmp]$ cat x.c
void
foo ()
{
}
[hjl@gnu-6 tmp]$ /usr/gcc-4.5/bin/gcc -m32 -S x.c -O3
[hjl@gnu-6 tmp]$ cat x.s
	.file	"x.c"
	.text
.globl foo
	.type	foo, @function
foo:
	pushl	%ebp
	movl	%esp, %ebp
	popl	%ebp
	ret

Can we turn on -fomit-frame-pointer by default for 32bit?


-- 
H.J.

Re: Turn on -fomit-frame-pointer by default for 32bit x86?

[Richard Guenther]

On Tue, Jul 13, 2010 at 8:15 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> Hi,
>
> I checked the default code for 32bit. We currently generate
>
> [hjl@gnu-6 tmp]$ cat x.c
> void
> foo ()
> {
> }
> [hjl@gnu-6 tmp]$ /usr/gcc-4.5/bin/gcc -m32 -S x.c -O3
> [hjl@gnu-6 tmp]$ cat x.s
>        .file   "x.c"
>        .text
> .globl foo
>        .type   foo, @function
> foo:
>        pushl   %ebp
>        movl    %esp, %ebp
>        popl    %ebp
>        ret
>
> Can we turn on -fomit-frame-pointer by default for 32bit?

No, we don't have unwind information by default for 32bit.

Richard.

>
> --
> H.J.
>

Re: Turn on -fomit-frame-pointer by default for 32bit x86?

[H.J. Lu]

On Tue, Jul 13, 2010 at 1:40 PM, Richard Guenther
<richard.guenther@gmail.com> wrote:
> On Tue, Jul 13, 2010 at 8:15 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> Hi,
>>
>> I checked the default code for 32bit. We currently generate
>>
>> [hjl@gnu-6 tmp]$ cat x.c
>> void
>> foo ()
>> {
>> }
>> [hjl@gnu-6 tmp]$ /usr/gcc-4.5/bin/gcc -m32 -S x.c -O3
>> [hjl@gnu-6 tmp]$ cat x.s
>>        .file   "x.c"
>>        .text
>> .globl foo
>>        .type   foo, @function
>> foo:
>>        pushl   %ebp
>>        movl    %esp, %ebp
>>        popl    %ebp
>>        ret
>>
>> Can we turn on -fomit-frame-pointer by default for 32bit?
>
> No, we don't have unwind information by default for 32bit.
>

What will stop working when -fomit-frame-pointer is on?

-- 
H.J.

Re: Turn on -fomit-frame-pointer by default for 32bit x86?

[Andrew Pinski]

On Tue, Jul 13, 2010 at 1:59 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> What will stop working when -fomit-frame-pointer is on?

backtraces when debugging information is not turned on.

-- Pinski

Re: Turn on -fomit-frame-pointer by default for 32bit x86?

[Andrew Pinski]

On Tue, Jul 13, 2010 at 2:01 PM, Andrew Pinski <pinskia@gmail.com> wrote:
> On Tue, Jul 13, 2010 at 1:59 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> What will stop working when -fomit-frame-pointer is on?
>
> backtraces when debugging information is not turned on.

See http://gcc.gnu.org/ml/gcc-patches/2004-08/msg01033.html and many
more.  This is not the first time this has been discussed.

Re: Turn on -fomit-frame-pointer by default for 32bit x86?

[H.J. Lu]

On Tue, Jul 13, 2010 at 2:02 PM, Andrew Pinski <pinskia@gmail.com> wrote:
> On Tue, Jul 13, 2010 at 2:01 PM, Andrew Pinski <pinskia@gmail.com> wrote:
>> On Tue, Jul 13, 2010 at 1:59 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>> What will stop working when -fomit-frame-pointer is on?
>>
>> backtraces when debugging information is not turned on.
>
> See http://gcc.gnu.org/ml/gcc-patches/2004-08/msg01033.html and many
> more.  This is not the first time this has been discussed.

Most of 32bit x86 assembly codes in glibc have .eh_frame section.
If backtrace is absolutely needed, they can add -fasynchronous-unwind-tables
or -fno-omit-frame-pointer.


-- 
H.J.

Re: Turn on -fomit-frame-pointer by default for 32bit x86?

[Richard Guenther]

On Tue, Jul 13, 2010 at 11:44 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Tue, Jul 13, 2010 at 2:02 PM, Andrew Pinski <pinskia@gmail.com> wrote:
>> On Tue, Jul 13, 2010 at 2:01 PM, Andrew Pinski <pinskia@gmail.com> wrote:
>>> On Tue, Jul 13, 2010 at 1:59 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>> What will stop working when -fomit-frame-pointer is on?
>>>
>>> backtraces when debugging information is not turned on.
>>
>> See http://gcc.gnu.org/ml/gcc-patches/2004-08/msg01033.html and many
>> more.  This is not the first time this has been discussed.
>
> Most of 32bit x86 assembly codes in glibc have .eh_frame section.
> If backtrace is absolutely needed, they can add -fasynchronous-unwind-tables
> or -fno-omit-frame-pointer.

We build opensuse with -fomit-frame-pointer -fasynchronous-unwind-tables.
If you want to make -fomit-frame-pointer the default then you should enable
unwind tables by default.

Richard.

>
> --
> H.J.
>

Re: Turn on -fomit-frame-pointer by default for 32bit x86?

[Dave Korn]

On 13/07/2010 22:44, H.J. Lu wrote:
> On Tue, Jul 13, 2010 at 2:02 PM, Andrew Pinski <pinskia@gmail.com> wrote:
>> On Tue, Jul 13, 2010 at 2:01 PM, Andrew Pinski <pinskia@gmail.com> wrote:
>>> On Tue, Jul 13, 2010 at 1:59 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>> What will stop working when -fomit-frame-pointer is on?
>>> backtraces when debugging information is not turned on.
>> See http://gcc.gnu.org/ml/gcc-patches/2004-08/msg01033.html and many
>> more.  This is not the first time this has been discussed.
> 
> Most of 32bit x86 assembly codes in glibc have .eh_frame section.
> If backtrace is absolutely needed, they can add -fasynchronous-unwind-tables
> or -fno-omit-frame-pointer.
> 

  Alas not all the x86 world is a glibc.  So if you want to do this it has to
be target-dependent I think.

    cheers,
      DaveK

Re: Turn on -fomit-frame-pointer by default for 32bit x86?

[H.J. Lu]

On Wed, Jul 14, 2010 at 1:09 AM, Richard Guenther
<richard.guenther@gmail.com> wrote:
> On Tue, Jul 13, 2010 at 11:44 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Tue, Jul 13, 2010 at 2:02 PM, Andrew Pinski <pinskia@gmail.com> wrote:
>>> On Tue, Jul 13, 2010 at 2:01 PM, Andrew Pinski <pinskia@gmail.com> wrote:
>>>> On Tue, Jul 13, 2010 at 1:59 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>>> What will stop working when -fomit-frame-pointer is on?
>>>>
>>>> backtraces when debugging information is not turned on.
>>>
>>> See http://gcc.gnu.org/ml/gcc-patches/2004-08/msg01033.html and many
>>> more.  This is not the first time this has been discussed.
>>
>> Most of 32bit x86 assembly codes in glibc have .eh_frame section.
>> If backtrace is absolutely needed, they can add -fasynchronous-unwind-tables
>> or -fno-omit-frame-pointer.
>
> We build opensuse with -fomit-frame-pointer -fasynchronous-unwind-tables.
> If you want to make -fomit-frame-pointer the default then you should enable
> unwind tables by default.
>

I will try and make it target/OS dependent.

-- 
H.J.

Re: Turn on -fomit-frame-pointer by default for 32bit x86?

[H.J. Lu]

On Wed, Jul 14, 2010 at 6:46 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Wed, Jul 14, 2010 at 1:09 AM, Richard Guenther
> <richard.guenther@gmail.com> wrote:
>> On Tue, Jul 13, 2010 at 11:44 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>> On Tue, Jul 13, 2010 at 2:02 PM, Andrew Pinski <pinskia@gmail.com> wrote:
>>>> On Tue, Jul 13, 2010 at 2:01 PM, Andrew Pinski <pinskia@gmail.com> wrote:
>>>>> On Tue, Jul 13, 2010 at 1:59 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>>>> What will stop working when -fomit-frame-pointer is on?
>>>>>
>>>>> backtraces when debugging information is not turned on.
>>>>
>>>> See http://gcc.gnu.org/ml/gcc-patches/2004-08/msg01033.html and many
>>>> more.  This is not the first time this has been discussed.
>>>
>>> Most of 32bit x86 assembly codes in glibc have .eh_frame section.
>>> If backtrace is absolutely needed, they can add -fasynchronous-unwind-tables
>>> or -fno-omit-frame-pointer.
>>
>> We build opensuse with -fomit-frame-pointer -fasynchronous-unwind-tables.
>> If you want to make -fomit-frame-pointer the default then you should enable
>> unwind tables by default.
>>
>
> I will try and make it target/OS dependent.
>

Here is a patch. How does it look?


-- 
H.J.
--
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

Re: Turn on -fomit-frame-pointer by default for 32bit x86?

[Andi Kleen]

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

> [hjl@gnu-6 tmp]$ /usr/gcc-4.5/bin/gcc -m32 -S x.c -O3
> [hjl@gnu-6 tmp]$ cat x.s
> 	.file	"x.c"
> 	.text
> .globl foo
> 	.type	foo, @function
> foo:
> 	pushl	%ebp
> 	movl	%esp, %ebp
> 	popl	%ebp
> 	ret
>
> Can we turn on -fomit-frame-pointer by default for 32bit?

I think it's a good idea, but in general one side effect one should
be aware of is that it breaks many profilers with calltrace per sample
support. Those tend to not be able to read unwind tables.


-Andi

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

Re: Turn on -fomit-frame-pointer by default for 32bit x86?

[Jeff Law]

On 07/16/10 13:35, Andi Kleen wrote:
> "H.J. Lu"<hjl.tools@gmail.com>  writes:
>
>    
>> [hjl@gnu-6 tmp]$ /usr/gcc-4.5/bin/gcc -m32 -S x.c -O3
>> [hjl@gnu-6 tmp]$ cat x.s
>> 	.file	"x.c"
>> 	.text
>> .globl foo
>> 	.type	foo, @function
>> foo:
>> 	pushl	%ebp
>> 	movl	%esp, %ebp
>> 	popl	%ebp
>> 	ret
>>
>> Can we turn on -fomit-frame-pointer by default for 32bit?
>>      
> I think it's a good idea, but in general one side effect one should
> be aware of is that it breaks many profilers with calltrace per sample
> support. Those tend to not be able to read unwind tables.
>    
Speaking of, I sent you a message on fentry about a month ago, but 
didn't get a reply:

I was ready to commit, except that we get a few new testsuite failures 
which I think touch on the issue of implicit setting of 
-fomit-frame-pointer issue.

  See:
/* Set the default values for switches whose default depends on 
TARGET_64BIT
      in case they weren't overwritten by command line options.  */
   if (TARGET_64BIT)
     {
       /* Mach-O doesn't support omitting the frame pointer for now.  */
       if (flag_omit_frame_pointer == 2)
         flag_omit_frame_pointer = (TARGET_MACHO ? 0 : 1);
       if (flag_asynchronous_unwind_tables == 2)
         flag_asynchronous_unwind_tables = 1;
       if (flag_pcc_struct_return == 2)
         flag_pcc_struct_return = 0;
     }
   else
     {
       if (flag_omit_frame_pointer == 2)
         flag_omit_frame_pointer = 0;
       if (flag_asynchronous_unwind_tables == 2)
         flag_asynchronous_unwind_tables = 0;
       if (flag_pcc_struct_return == 2)
         flag_pcc_struct_return = DEFAULT_PCC_STRUCT_RETURN;
     }



For x86_64, -fomit-frame-pointer is implicitly set; therefore with your 
patch someone compiling with -O2 -pg will get a diagnostic informing 
them those options are incompatible except with -mfentry is added.  I 
don't think you intended to get a diagnostic in this case.  It's 
certainly a behaviour change.

Jeff