This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] i386: Replace frame pointer with stack pointer in debug insns
- From: Uros Bizjak <ubizjak at gmail dot com>
- To: "H.J. Lu" <hjl dot tools at gmail dot com>
- Cc: "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>
- Date: Sun, 13 Aug 2017 20:12:52 +0200
- Subject: Re: [PATCH] i386: Replace frame pointer with stack pointer in debug insns
- Authentication-results: sourceware.org; auth=none
- References: <20170812013228.GA3607@gmail.com> <CAFULd4bdKLz57AB9=F2KtPVpev1SZhDwREh7aeBtqnqJfcj+4w@mail.gmail.com> <CAMe9rOrzTCY+3DfeO2jcJN+MLyu2JiB0BEUFLsdKvm57wecwgA@mail.gmail.com>
On Sun, Aug 13, 2017 at 7:35 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Sun, Aug 13, 2017 at 9:15 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
>> On Sat, Aug 12, 2017 at 3:32 AM, H.J. Lu <hjl.tools@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.