From patchwork Tue Nov 6 17:56:42 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [AArch64] fix missing Dwarf call frame information in the epilogue Date: Tue, 06 Nov 2012 07:56:42 -0000 From: Yufeng Zhang X-Patchwork-Id: 197528 Message-Id: <50994F5A.4080104@arm.com> To: Richard Henderson Cc: "gcc-patches@gcc.gnu.org" Hi, Many thanks for reviewing. Please find the updated patch. The explicit calls to gen_rtx_PLUS and GEN_INT have been replaced by plus_constant, and the call to aarch64_set_frame_expr has been replaced with add_reg_note (REG_CFA_ADJUST_CFA). I'll clean up other cases in aarch64.c in a separate patch. OK to commit? Thanks, Yufeng gcc/ChangeLog 2012-11-06 Yufeng Zhang * config/aarch64/aarch64.c (aarch64_expand_prologue): For the load-pair with writeback instruction, replace aarch64_set_frame_expr with add_reg_note (REG_CFA_ADJUST_CFA); add new local variable 'cfa_reg' and use it. gcc/testsuite/ChangeLog 2012-11-06 Yufeng Zhang * gcc.target/aarch64/dwarf-cfa-reg.c: New file. On 09/12/12 19:37, Richard Henderson wrote: > On 09/12/2012 09:10 AM, Yufeng Zhang wrote: >> aarch64_set_frame_expr (gen_rtx_SET >> (Pmode, >> stack_pointer_rtx, >> - gen_rtx_PLUS (Pmode, stack_pointer_rtx, >> + gen_rtx_PLUS (Pmode, cfa_reg, >> GEN_INT (offset)))); > > We'd prefer to use > > plus_constant (Pmode, cfa_reg, offset) > > instead of the explicit call to gen_rtx_PLUS and GEN_INT. > It would appear that the entire aarch64.c file ought to > be audited for that. > > Also, use of the REG_CFA_* notes is strongly encouraged over > use of REG_FRAME_RELATED_EXPR. > > There's all sorts of work involved in turning R_F_R_E into > R_CFA_* notes, depending on a rather large state machine. > This state machine was developed when only prologues were > annotated for unwinding, and therefore one cannot expect it > to work reliably for epilogues. > > A long-term goal is to convert all targets to use R_CFA_* > exclusively, as that preserves much more information present > in the structure of the code of the prologue generator. It > means less work within the compiler, and eventually being able > to remove a rather large hunk of state-machine code. > > > r~ > diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c index b36be90..8a2d7ba 100644 --- a/gcc/config/aarch64/aarch64.c +++ b/gcc/config/aarch64/aarch64.c @@ -1777,7 +1777,7 @@ aarch64_expand_prologue (void) - original_frame_size - cfun->machine->frame.saved_regs_size); - /* Store pairs and load pairs have a range only of +/- 512. */ + /* Store pairs and load pairs have a range only -512 to 504. */ if (offset >= 512) { /* When the frame has a large size, an initial decrease is done on @@ -1923,6 +1923,7 @@ aarch64_expand_epilogue (bool for_sibcall) HOST_WIDE_INT original_frame_size, frame_size, offset; HOST_WIDE_INT fp_offset; rtx insn; + rtx cfa_reg; aarch64_layout_frame (); original_frame_size = get_frame_size () + cfun->machine->saved_varargs_size; @@ -1935,7 +1936,9 @@ aarch64_expand_epilogue (bool for_sibcall) - original_frame_size - cfun->machine->frame.saved_regs_size); - /* Store pairs and load pairs have a range only of +/- 512. */ + cfa_reg = frame_pointer_needed ? hard_frame_pointer_rtx : stack_pointer_rtx; + + /* Store pairs and load pairs have a range only -512 to 504. */ if (offset >= 512) { offset = original_frame_size + cfun->machine->frame.saved_regs_size; @@ -1966,6 +1969,10 @@ aarch64_expand_epilogue (bool for_sibcall) hard_frame_pointer_rtx, GEN_INT (- fp_offset))); RTX_FRAME_RELATED_P (insn) = 1; + /* As SP is set to (FP - fp_offset), according to the rules in + dwarf2cfi.c:dwarf2out_frame_debug_expr, CFA should be calculated + from the value of SP from now on. */ + cfa_reg = stack_pointer_rtx; } aarch64_save_or_restore_callee_save_registers @@ -2003,11 +2010,9 @@ aarch64_expand_epilogue (bool for_sibcall) GEN_INT (offset), GEN_INT (GET_MODE_SIZE (DImode) + offset))); RTX_FRAME_RELATED_P (XVECEXP (PATTERN (insn), 0, 2)) = 1; - aarch64_set_frame_expr (gen_rtx_SET - (Pmode, - stack_pointer_rtx, - gen_rtx_PLUS (Pmode, stack_pointer_rtx, - GEN_INT (offset)))); + add_reg_note (insn, REG_CFA_ADJUST_CFA, + (gen_rtx_SET (Pmode, stack_pointer_rtx, + plus_constant (cfa_reg, offset)))); } /* The first part of a frame-related parallel insn @@ -2027,7 +2032,6 @@ aarch64_expand_epilogue (bool for_sibcall) RTX_FRAME_RELATED_P (insn) = 1; } } - else { insn = emit_insn (gen_add2_insn (stack_pointer_rtx, diff --git a/gcc/testsuite/gcc.target/aarch64/dwarf-cfa-reg.c b/gcc/testsuite/gcc.target/aarch64/dwarf-cfa-reg.c new file mode 100644 index 0000000..cce8815 --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/dwarf-cfa-reg.c @@ -0,0 +1,14 @@ +/* Verify that CFA register is restored to SP after FP is restored. */ +/* { dg-do compile } */ +/* { dg-options "-O0 -gdwarf-2" } */ +/* { dg-final { scan-assembler ".cfi_restore 30" } } */ +/* { dg-final { scan-assembler ".cfi_restore 29" } } */ +/* { dg-final { scan-assembler ".cfi_def_cfa 31, 0" } } */ +/* { dg-final { scan-assembler "ret" } } */ + +int bar (unsigned int); + +int foo (void) +{ + return bar (0xcafe); +}