Do not set -fomit-frame-pointer if TARGET_OMIT_LEAF_FRAME_POINTER_P.

Hi.

I'm not fully sure about the change, but -momit-leaf-frame-pointer
probably should not globally omit frame pointers?

Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

Ready to be installed?
Thanks,
Martin

gcc/ChangeLog:

2020-01-03  Martin Liska  <mliska@suse.cz>

	PR tree-optimization/92860
	* config/i386/i386-options.c (ix86_option_override_internal):
	Do not enable -fomit-frame-pointer with -momit-leaf-frame-pointer
	as it will globally omit pointers (like in ira.c).
---
  gcc/config/i386/i386-options.c | 2 --
  1 file changed, 2 deletions(-)

PING^1

On 1/3/20 12:23 PM, Martin Liška wrote:
> Hi.
> 
> I'm not fully sure about the change, but -momit-leaf-frame-pointer
> probably should not globally omit frame pointers?
> 
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
> 
> Ready to be installed?
> Thanks,
> Martin
> 
> gcc/ChangeLog:
> 
> 2020-01-03  Martin Liska  <mliska@suse.cz>
> 
>      PR tree-optimization/92860
>      * config/i386/i386-options.c (ix86_option_override_internal):
>      Do not enable -fomit-frame-pointer with -momit-leaf-frame-pointer
>      as it will globally omit pointers (like in ira.c).
> ---
>   gcc/config/i386/i386-options.c | 2 --
>   1 file changed, 2 deletions(-)
> 
>

> PING^1
> 
> On 1/3/20 12:23 PM, Martin Liška wrote:
> > Hi.
> > 
> > I'm not fully sure about the change, but -momit-leaf-frame-pointer
> > probably should not globally omit frame pointers?
> > 
> > Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
> > 
> > Ready to be installed?
> > Thanks,
> > Martin
> > 
> > gcc/ChangeLog:
> > 
> > 2020-01-03  Martin Liska  <mliska@suse.cz>
> > 
> >      PR tree-optimization/92860
> >      * config/i386/i386-options.c (ix86_option_override_internal):
> >      Do not enable -fomit-frame-pointer with -momit-leaf-frame-pointer
> >      as it will globally omit pointers (like in ira.c).

Does gcc omit frame pointer of leaf functions after your change?
My recollection how this code works is that flag_omit_frame_pointer is
needed for regalloc to consider omitting at first plce and then it
queries frame_pointer_required hook wihch if only
-momit-leaf-frame-pointer is used will return true for all non-leaf
functions so effectivly only leaf ones are omitted.

If you stop setting the flag I think IRA will not consider omitting at
all and whole flag will become noop.

Honza

On 1/14/20 12:18 PM, Jan Hubicka wrote:
>> PING^1
>>
>> On 1/3/20 12:23 PM, Martin Liška wrote:
>>> Hi.
>>>
>>> I'm not fully sure about the change, but -momit-leaf-frame-pointer
>>> probably should not globally omit frame pointers?
>>>
>>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>>>
>>> Ready to be installed?
>>> Thanks,
>>> Martin
>>>
>>> gcc/ChangeLog:
>>>
>>> 2020-01-03  Martin Liska  <mliska@suse.cz>
>>>
>>>       PR tree-optimization/92860
>>>       * config/i386/i386-options.c (ix86_option_override_internal):
>>>       Do not enable -fomit-frame-pointer with -momit-leaf-frame-pointer
>>>       as it will globally omit pointers (like in ira.c).
> 
> Does gcc omit frame pointer of leaf functions after your change?
> My recollection how this code works is that flag_omit_frame_pointer is
> needed for regalloc to consider omitting at first plce and then it
> queries frame_pointer_required hook wihch if only
> -momit-leaf-frame-pointer is used will return true for all non-leaf
> functions so effectivly only leaf ones are omitted.

I've got it.

> 
> If you stop setting the flag I think IRA will not consider omitting at
> all and whole flag will become noop.

Yes, then patch is then incorrect. We would needs something like:

diff --git a/gcc/config/i386/i386-options.c b/gcc/config/i386/i386-options.c
index 2acc9fb0cfe..ec538724581 100644
--- a/gcc/config/i386/i386-options.c
+++ b/gcc/config/i386/i386-options.c
@@ -1671,6 +1671,9 @@ ix86_recompute_optlev_based_flags (struct gcc_options *opts,
             opts->x_flag_pcc_struct_return = DEFAULT_PCC_STRUCT_RETURN;
         }
      }
+
+  if (TARGET_OMIT_LEAF_FRAME_POINTER_P (opts->x_target_flags))
+    opts->x_flag_omit_frame_pointer = 1;
  }
  
  /* Implement TARGET_OVERRIDE_OPTIONS_AFTER_CHANGE hook.  */
@@ -2437,8 +2440,6 @@ ix86_option_override_internal (bool main_args_p,
    /* Keep nonleaf frame pointers.  */
    if (opts->x_flag_omit_frame_pointer)
      opts->x_target_flags &= ~MASK_OMIT_LEAF_FRAME_POINTER;
-  else if (TARGET_OMIT_LEAF_FRAME_POINTER_P (opts->x_target_flags))
-    opts->x_flag_omit_frame_pointer = 1;
  
    /* If we're doing fast math, we don't care about comparison order
       wrt NaNs.  This lets us use a shorter comparison sequence.  */

But it does not work as opts->x_target_flags is changed for some reason
for a per-function opts.

Martin

> 
> Honza
>

On Fri, 2020-01-03 at 12:23 +0100, Martin Liška wrote:
> Hi.
> 
> I'm not fully sure about the change, but -momit-leaf-frame-pointer
> probably should not globally omit frame pointers?
> 
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
> 
> Ready to be installed?
> Thanks,
> Martin
> 
> gcc/ChangeLog:
> 
> 2020-01-03  Martin Liska  <mliska@suse.cz>
> 
> 	PR tree-optimization/92860
> 	* config/i386/i386-options.c (ix86_option_override_internal):
> 	Do not enable -fomit-frame-pointer with -momit-leaf-frame-pointer
> 	as it will globally omit pointers (like in ira.c).
But when TARGET_OMIT_LEAF_FRAME_POINTER is on, then for a non-leaf
function ix86_frame_pointer_required returns true. 

So AFAICT we don't globally omit frame-pointers when this option is
turned on.

jeff