On Sun, Aug 13, 2017 at 7:35 PM, H.J. Lu <hjl.to...@gmail.com> wrote:
> On Sun, Aug 13, 2017 at 9:15 AM, Uros Bizjak <ubiz...@gmail.com> wrote:
>> On Sat, Aug 12, 2017 at 3:32 AM, H.J. Lu <hjl.to...@gmail.com> wrote:
>>> When we eliminate frame pointer, we should also replace frame pointer
>>> with stack pointer - UNITS_PER_WORD in debug insns.  This patch fixed:
>>>
>>> FAIL: gcc.dg/guality/pr58791-5.c   -Os  line pr58791-5.c:20 b1 == 9
>>> FAIL: gcc.dg/guality/pr58791-5.c   -Os  line pr58791-5.c:20 b2 == 73
>>> FAIL: gcc.dg/guality/pr58791-5.c   -Os  line pr58791-5.c:20 b3 == 585
>>> FAIL: gcc.dg/guality/pr58791-5.c   -Os  line pr58791-5.c:20 b4 == 4681
>>> FAIL: gcc.dg/guality/pr59776.c   -Os  line pr59776.c:17 s1.f == 5.0
>>> FAIL: gcc.dg/guality/pr59776.c   -Os  line pr59776.c:17 s1.g == 6.0
>>> FAIL: gcc.dg/guality/pr59776.c   -Os  line pr59776.c:17 s2.g == 6.0
>>> FAIL: gcc.dg/guality/pr59776.c   -Os  line pr59776.c:20 s1.f == 5.0
>>> FAIL: gcc.dg/guality/pr59776.c   -Os  line pr59776.c:20 s1.g == 6.0
>>> FAIL: gcc.dg/guality/pr59776.c   -Os  line pr59776.c:20 s2.f == 5.0
>>> FAIL: gcc.dg/guality/pr59776.c   -Os  line pr59776.c:20 s2.g == 6.0
>>>
>>> on Linux/i386.
>>>
>>> Tested on i686 and x86-64.  OK for trunk?

OK wit ha small comment adjustment below.

Thanks,
Uros.

>>> H.J.
>>> ---
>>>
>>>         PR target/81820
>>>         * config/i386/i386.c (ix86_finalize_stack_frame_flags): Replace
>>>         frame pointer with stack pointer - UNITS_PER_WORD in debug insns.
>>> ---
>>>  gcc/config/i386/i386.c | 36 ++++++++++++++++++++++++++++++++++++
>>>  1 file changed, 36 insertions(+)
>>>
>>> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
>>> index b04321a8d40..0094f2c4441 100644
>>> --- a/gcc/config/i386/i386.c
>>> +++ b/gcc/config/i386/i386.c
>>> @@ -14281,6 +14281,42 @@ ix86_finalize_stack_frame_flags (void)
>>>        df_scan_blocks ();
>>>        df_compute_regs_ever_live (true);
>>>        df_analyze ();
>>> +
>>> +      if (flag_var_tracking)
>>> +       {
>>> +         /* Since frame pointer is no longer needed, replace it with
>>> +            stack pointer - UNITS_PER_WORD in debug insns.  */

... Since frame pointer is no longer available, ...

>>> +         df_ref ref, next;
>>> +         for (ref = DF_REG_USE_CHAIN (HARD_FRAME_POINTER_REGNUM);
>>> +              ref; ref = next)
>>> +           {
>>> +             rtx_insn *insn = DF_REF_INSN (ref);
>>> +             /* Make sure the next ref is for a different instruction,
>>> +                so that we're not affected by the rescan.  */
>>> +             next = DF_REF_NEXT_REG (ref);
>>> +             while (next && DF_REF_INSN (next) == insn)
>>> +               next = DF_REF_NEXT_REG (next);
>>
>> Can you please explain why the above loop is needed?
>>
>
> After df_insn_rescan below is called, DF_REF_NEXT_REG (ref)
> may point to the current debug instruction we just updated.
> gcc.dg/guality/pr58791-5.c is such an example.  If I take out the loop:
>
> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
> index 0094f2c4441..cef3a6e4816 100644
> --- a/gcc/config/i386/i386.c
> +++ b/gcc/config/i386/i386.c
> @@ -14294,8 +14294,6 @@ ix86_finalize_stack_frame_flags (void)
>          /* Make sure the next ref is for a different instruction,
>        so that we're not affected by the rescan.  */
>          next = DF_REF_NEXT_REG (ref);
> -        while (next && DF_REF_INSN (next) == insn)
> -     next = DF_REF_NEXT_REG (next);
>
>          if (DEBUG_INSN_P (insn))
>       {
>
> I got:
>
> [hjl@gnu-tools-1 gcc]$ ./xgcc -B./ -m32 -Os -g
> /export/gnu/import/git/sources/gcc/gcc/testsuite/gcc.dg/guality/pr58791-5.c
> during RTL pass: pro_and_epilogue
> /export/gnu/import/git/sources/gcc/gcc/testsuite/gcc.dg/guality/pr58791-5.c:
> In function ‘foo’:
> /export/gnu/import/git/sources/gcc/gcc/testsuite/gcc.dg/guality/pr58791-5.c:22:1:
> internal compiler error: Segmentation fault
>  }
>  ^
> 0xf0cae1 crash_signal
> /export/gnu/import/git/sources/gcc/gcc/toplev.c:341
> 0x1329df6 ix86_finalize_stack_frame_flags
> /export/gnu/import/git/sources/gcc/gcc/config/i386/i386.c:14293
> 0x132a5be ix86_expand_prologue()
> /export/gnu/import/git/sources/gcc/gcc/config/i386/i386.c:14440
> 0x165edaf gen_prologue()
> /export/gnu/import/git/sources/gcc/gcc/config/i386/i386.md:12464
> 0x130c25d target_gen_prologue
> /export/gnu/import/git/sources/gcc/gcc/config/i386/i386.md:18659
> 0xb30b5e make_prologue_seq
> /export/gnu/import/git/sources/gcc/gcc/function.c:5835
> 0xb30d3e thread_prologue_and_epilogue_insns()
> /export/gnu/import/git/sources/gcc/gcc/function.c:5952
> 0xb31d46 rest_of_handle_thread_prologue_and_epilogue
> /export/gnu/import/git/sources/gcc/gcc/function.c:6443
> 0xb31dc8 execute
> /export/gnu/import/git/sources/gcc/gcc/function.c:6485
> Please submit a full bug report,
> with preprocessed source if appropriate.
> Please include the complete backtrace with any bug report.
> See <https://gcc.gnu.org/bugs/> for instructions.
> [hjl@gnu-tools-1 gcc]$
>
>
>> Thanks,
>> Uros.
>>
>>> +             if (DEBUG_INSN_P (insn))
>>> +               {
>>> +                 bool changed = false;
>>> +                 for (; ref != next; ref = DF_REF_NEXT_REG (ref))
>>> +                   {
>>> +                     rtx *loc = DF_REF_LOC (ref);
>>> +                     if (*loc == hard_frame_pointer_rtx)
>>> +                       {
>>> +                         *loc = plus_constant (Pmode,
>>> +                                               stack_pointer_rtx,
>>> +                                               -UNITS_PER_WORD);
>>> +                         changed = true;
>>> +                       }
>>> +                   }
>>> +                 if (changed)
>>> +                   df_insn_rescan (insn);
>>> +               }
>>> +           }
>>> +       }
>>> +
>>>        recompute_frame_layout_p = true;
>>>      }
>>>
>>> --
>>> 2.13.4
>>>
>
>
>
> --
> H.J.

Reply via email to