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

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

[Uros Bizjak]

Hello!

After recent discussions, I would like to propose a transition to
-fomit-frame-pointer for x86_32.

The transition should be smooth as much as possible, should have
option to revert to old behaviour and still providing path for the
improvement. And we have learned something from cld issues, too
(cough, cough...).

I support the idea to change x86_32 defaults w.r.t. frame pointer (and
unwind tables) to the same defaults as x86_64 has.

The patch should also introduce --enable-frame-pointer configure
option (off by default) that would revert back to old x86_32
behaviour. So, if there are codes that depend on FP, their users (or
distributions) should either (re-)configure the compiler with
--enable-frame-pointer or they should use older compiler - 4.5.x will
still be supported for many years. OTOH, it looks that users don't
care that much whether backtraces on x86_64 are totally accurate, so
IMO the sky won't fall down if x86_32 misses some backtraces in the
same way. And as I have learned from the discussion, the problem is
fixable with some effort on the user's side, thus fixing both targets
in one shot.

Of course, this change and the option to revert to the previous
behaviour should be announced and documented in GCC release notes for
4.6.0.

IMO, we have to bite the bullet from time to time in order to improve
the generated code. We should not claim that gcc is
"no-code-left-behind compiler" - from my experience, introducing new
compiler always means that some parts of the code have to be fixed (as
in case of the change to -fno-strict-aliasing).

Uros.

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

[Michael Witten]

On Sun, Aug 8, 2010 at 09:56, Uros Bizjak <ubizjak@gmail.com> wrote:
>
> IMO, we have to bite the bullet from time to time in order to improve
> the generated code.

What's your performance function?

I like backtraces.

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

[Uros Bizjak]

On Sun, Aug 8, 2010 at 5:00 PM, Michael Witten <mfwitten@gmail.com> wrote:
> On Sun, Aug 8, 2010 at 09:56, Uros Bizjak <ubizjak@gmail.com> wrote:
>>
>> IMO, we have to bite the bullet from time to time in order to improve
>> the generated code.
>
> What's your performance function?

See measurements at [1].

>
> I like backtraces.
>

So, you will be able to use --enable-frame-pointer configure option.

[1] http://gcc.gnu.org/ml/gcc-patches/2010-07/msg02034.html

Uros.

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

[Bernd Schmidt]

On 08/08/2010 05:08 PM, Uros Bizjak wrote:
> So, you will be able to use --enable-frame-pointer configure option.

Or, presumably the -fno-omit-frame-pointer command line option?


Bernd

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

[Andi Kleen]

> > I like backtraces.
> >
> 
> So, you will be able to use --enable-frame-pointer configure option.
> 
> [1] http://gcc.gnu.org/ml/gcc-patches/2010-07/msg02034.html

... or use backtrace()

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

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

[Michael Witten]

On Sun, Aug 8, 2010 at 10:08, Uros Bizjak <ubizjak@gmail.com> wrote:
> On Sun, Aug 8, 2010 at 5:00 PM, Michael Witten <mfwitten@gmail.com> wrote:
>> On Sun, Aug 8, 2010 at 09:56, Uros Bizjak <ubizjak@gmail.com> wrote:
>>>
>>> IMO, we have to bite the bullet from time to time in order to improve
>>> the generated code.
>>
>> What's your performance function?
>
> See measurements at [1].

The point is that *my* performance function may be a measure of the
availability of decent backtraces.

> So, you will be able to use --enable-frame-pointer configure option.

Most of the computing world would probably benefit far more from
usable backtraces than from increases in efficiency; those individuals
who fret about efficiency are likely better suited to fiddling with
non-default flags.

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

[Michael Witten]

On Sun, Aug 8, 2010 at 10:30, Andi Kleen <andi@firstfloor.org> wrote:
>> > I like backtraces.
>> >
>>
>> So, you will be able to use --enable-frame-pointer configure option.
>>
>> [1] http://gcc.gnu.org/ml/gcc-patches/2010-07/msg02034.html
>
> ... or use backtrace()

What kind of black magic would you expect backtrace() to use?

From the `info backtrace' text:

  Note that certain compiler optimizations
  may interfere with obtaining a valid
  backtrace.  Function inlining causes
  the inlined function to not have a stack
  frame; tail call optimization replaces
  one stack frame with another; frame pointer
  elimination will stop `backtrace' from
  interpreting the stack contents correctly.

Specifically, note the last sentence:

  frame pointer elimination will stop `backtrace'
  from interpreting the stack contents correctly.

Sincerely,
Michael Witten

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

[Andi Kleen]

On Sun, Aug 08, 2010 at 12:00:47PM -0500, Michael Witten wrote:
> On Sun, Aug 8, 2010 at 10:30, Andi Kleen <andi@firstfloor.org> wrote:
> >> > I like backtraces.
> >> >
> >>
> >> So, you will be able to use --enable-frame-pointer configure option.
> >>
> >> [1] http://gcc.gnu.org/ml/gcc-patches/2010-07/msg02034.html
> >
> > ... or use backtrace()
> 
> What kind of black magic would you expect backtrace() to use?

dwarf2 unwind tables.

-Andi

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

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

[H.J. Lu]

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

On Sun, Aug 8, 2010 at 7:56 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
> Hello!
>
> After recent discussions, I would like to propose a transition to
> -fomit-frame-pointer for x86_32.
>
> The transition should be smooth as much as possible, should have
> option to revert to old behaviour and still providing path for the
> improvement. And we have learned something from cld issues, too
> (cough, cough...).
>
> I support the idea to change x86_32 defaults w.r.t. frame pointer (and
> unwind tables) to the same defaults as x86_64 has.
>
> The patch should also introduce --enable-frame-pointer configure
> option (off by default) that would revert back to old x86_32
> behaviour. So, if there are codes that depend on FP, their users (or
> distributions) should either (re-)configure the compiler with
> --enable-frame-pointer or they should use older compiler - 4.5.x will
> still be supported for many years. OTOH, it looks that users don't
> care that much whether backtraces on x86_64 are totally accurate, so
> IMO the sky won't fall down if x86_32 misses some backtraces in the
> same way. And as I have learned from the discussion, the problem is
> fixable with some effort on the user's side, thus fixing both targets
> in one shot.
>
> Of course, this change and the option to revert to the previous
> behaviour should be announced and documented in GCC release notes for
> 4.6.0.
>
> IMO, we have to bite the bullet from time to time in order to improve
> the generated code. We should not claim that gcc is
> "no-code-left-behind compiler" - from my experience, introducing new
> compiler always means that some parts of the code have to be fixed (as
> in case of the change to -fno-strict-aliasing).
>
> Uros.
>

I tested this patch on Linux/ia32 and Linux/x86-64. There are no regressions.

I don't have good wording for document:

--
For 32-bit x86 targets, it is not enabled at @option{-Os} by default.
This option also can be disabled by default on 32-bit x86 targets by
configuring GCC with the @option{--enable-frame-pointer} configure
option.
--

isn't very accurate.  Any suggestions?

Thanks.


-- 
H.J.
---
2010-08-09  H.J. Lu  <hongjiu.lu@intel.com>

	* config.gcc: Handle --enable-frame-pointer.

	* configure.ac: Add --enable-frame-pointer.
	* configure: Regenerated.

	* config/i386/i386.c (override_options): If not optimize for
	size, use -fomit-frame-pointer and -fasynchronous-unwind-tables
	by default for 32-bit code unless configured with
	--enable-frame-pointer.

[-- Attachment #2: gcc-i386-fp-5.patch --]
[-- Type: text/plain, Size: 7063 bytes --]

2010-08-09  H.J. Lu  <hongjiu.lu@intel.com>

	* config.gcc: Handle --enable-frame-pointer.

	* configure.ac: Add --enable-frame-pointer.
	* configure: Regenerated.

	* config/i386/i386.c (override_options): If not optimize for
	size, use -fomit-frame-pointer and -fasynchronous-unwind-tables
	by default for 32-bit code unless configured with
	--enable-frame-pointer.

diff --git a/gcc/config.gcc b/gcc/config.gcc
index 9170fc8..62dd9f6 100644
--- a/gcc/config.gcc
+++ b/gcc/config.gcc
@@ -406,6 +406,9 @@ i[34567]86-*-*)
 	if test "x$enable_cld" = xyes; then
 		tm_defines="${tm_defines} USE_IX86_CLD=1"
 	fi
+	if test "x$enable_frame_pointer" = xyes; then
+		tm_defines="${tm_defines} USE_IX86_FRAME_POINTER=1"
+	fi
 	tm_file="vxworks-dummy.h ${tm_file}"
 	;;
 x86_64-*-*)
@@ -413,6 +416,9 @@ x86_64-*-*)
 	if test "x$enable_cld" = xyes; then
 		tm_defines="${tm_defines} USE_IX86_CLD=1"
 	fi
+	if test "x$enable_frame_pointer" = xyes; then
+		tm_defines="${tm_defines} USE_IX86_FRAME_POINTER=1"
+	fi
 	tm_file="vxworks-dummy.h ${tm_file}"
 	;;
 esac
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 1877730..c0b657b 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -2979,32 +2979,6 @@ override_options (bool main_args_p)
   if (TARGET_MACHO && TARGET_64BIT)
     flag_pic = 2;
 
-  /* 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)
-    {
-      if (flag_zee == 2)
-        flag_zee = 1;
-      /* 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_zee == 2)
-        flag_zee = 0;
-      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;
-    }
-
   /* Need to check -mtune=generic first.  */
   if (ix86_tune_string)
     {
@@ -3292,6 +3266,49 @@ override_options (bool main_args_p)
   for (i = 0; i < X86_TUNE_LAST; ++i)
     ix86_tune_features[i] = !!(initial_ix86_tune_features[i] & ix86_tune_mask);
 
+  if ((x86_accumulate_outgoing_args & ix86_tune_mask)
+      && !(target_flags_explicit & MASK_ACCUMULATE_OUTGOING_ARGS)
+      && !optimize_size)
+    target_flags |= MASK_ACCUMULATE_OUTGOING_ARGS;
+
+  /* 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)
+    {
+      if (flag_zee == 2)
+        flag_zee = 1;
+      /* 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_zee == 2)
+        flag_zee = 0;
+      /* If not optimize for size, use -fomit-frame-pointer and
+	 -fasynchronous-unwind-tables by default for 32-bit code
+	 unless configured with --enable-frame-pointer.  */
+#ifdef USE_IX86_FRAME_POINTER
+      if (flag_omit_frame_pointer == 2)
+	flag_omit_frame_pointer = 0;
+      if (flag_asynchronous_unwind_tables == 2)
+	flag_asynchronous_unwind_tables = 0;
+#else
+      if (flag_omit_frame_pointer == 2)
+	flag_omit_frame_pointer
+	  = !!(target_flags & MASK_ACCUMULATE_OUTGOING_ARGS);
+      if (flag_asynchronous_unwind_tables == 2)
+	flag_asynchronous_unwind_tables
+	  = !!(target_flags & MASK_ACCUMULATE_OUTGOING_ARGS);
+#endif
+      if (flag_pcc_struct_return == 2)
+	flag_pcc_struct_return = DEFAULT_PCC_STRUCT_RETURN;
+    }
+
   if (optimize_size)
     ix86_cost = &ix86_size_cost;
   else
@@ -3574,11 +3591,6 @@ override_options (bool main_args_p)
 	       prefix, suffix, sw);
     }
 
-  if ((x86_accumulate_outgoing_args & ix86_tune_mask)
-      && !(target_flags_explicit & MASK_ACCUMULATE_OUTGOING_ARGS)
-      && !optimize_size)
-    target_flags |= MASK_ACCUMULATE_OUTGOING_ARGS;
-
   /* ??? Unwind info is not correct around the CFG unless either a frame
      pointer is present or M_A_O_A is set.  Fixing this requires rewriting
      unwind info generation to be aware of the CFG and propagating states
diff --git a/gcc/configure b/gcc/configure
index aa61cd6..ad548fc 100755
--- a/gcc/configure
+++ b/gcc/configure
@@ -898,6 +898,7 @@ with_system_libunwind
 enable_secureplt
 enable_leading_mingw64_underscores
 enable_cld
+enable_frame_pointer
 enable_win32_registry
 enable_static
 with_pic
@@ -1597,6 +1598,7 @@ Optional Features:
   --enable-leading-mingw64-underscores
                           Enable leading underscores on 64 bit mingw targets
   --enable-cld            enable -mcld by default for 32bit x86
+  --enable-frame-pointer  enable -fno-omit-frame-pointer by default for 32bit x86
   --disable-win32-registry
                           disable lookup of installation paths in the
                           Registry on Windows hosts
@@ -10708,6 +10710,24 @@ else
 fi
 
 
+# Check whether --enable-frame-pointer was given.
+if test "${enable_frame_pointer+set}" = set; then :
+  enableval=$enable_frame_pointer;
+else
+
+case $target_os in
+linux*)
+  # Enable -fomit-frame-pointer by default for Linux.
+  enable_frame_pointer=no
+  ;;
+*)
+  enable_frame_pointer=yes
+  ;;
+esac
+
+fi
+
+
 # Windows32 Registry support for specifying GCC installation paths.
 # Check whether --enable-win32-registry was given.
 if test "${enable_win32_registry+set}" = set; then :
@@ -17109,7 +17129,7 @@ else
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 17112 "configure"
+#line 17132 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H
@@ -17215,7 +17235,7 @@ else
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 17218 "configure"
+#line 17238 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H
diff --git a/gcc/configure.ac b/gcc/configure.ac
index 24d38aa..dd0b198 100644
--- a/gcc/configure.ac
+++ b/gcc/configure.ac
@@ -1580,6 +1580,20 @@ AC_ARG_ENABLE(cld,
 [  --enable-cld            enable -mcld by default for 32bit x86], [],
 [enable_cld=no])
 
+AC_ARG_ENABLE(frame-pointer,
+[  --enable-frame-pointer  enable -fno-omit-frame-pointer by default for 32bit x86], [],
+[
+case $target_os in
+linux*)
+  # Enable -fomit-frame-pointer by default for Linux.
+  enable_frame_pointer=no
+  ;;
+*)
+  enable_frame_pointer=yes
+  ;;
+esac
+])
+
 # Windows32 Registry support for specifying GCC installation paths.
 AC_ARG_ENABLE(win32-registry,
 [  --disable-win32-registry