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: "H.J. Lu" <hjl dot tools at gmail dot com>
- To: Uros Bizjak <ubizjak 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 10:35:57 -0700
- 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>
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?
>>
>> 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. */
>> + 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.