On 20/07/16 14:03, Jiong Wang wrote:
> Those stack adjustment sequences inside aarch64_expand_prologue/epilogue
> are doing exactly what's aarch64_add_constant offered, except they also
> need to be aware of dwarf generation.
>
> This patch teach existed aarch64_add_constant about dwarf generation and
> currently SP register is supported. Whenever SP is updated, there
> should be CFA update, we then mark these instructions as frame related,
> and if the update is too complex for gcc to guess the adjustment, we
> attach explicit annotation.
>
> Both dwarf frame info size and pro/epilogue scheduling are improved after
> this patch as aarch64_add_constant has better utilization of scratch
> register.
>
> OK for trunk?
>
> gcc/
> 2016-07-20 Jiong Wang <jiong.w...@arm.com>
>
> * config/aarch64/aarch64.c (aarch64_add_constant): Mark
> instruction as frame related when it is. Generate CFA
> annotation when it's necessary.
> (aarch64_expand_prologue): Use aarch64_add_constant.
> (aarch64_expand_epilogue): Likewise.
>
Are you sure using aarch64_add_constant is unconditionally safe? Stack
adjustments need to be done very carefully to ensure that we never
transiently deallocate part of the stack.
R.
>
> build-const-3.patch
>
>
> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index
> 41844a101247c939ecb31f8a8c17cf79759255aa..b38f3f1e8f85a5f3191d0c96080327dac7b2eaed
> 100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -1874,6 +1874,8 @@ aarch64_add_constant (machine_mode mode, int regnum,
> int scratchreg,
> {
> HOST_WIDE_INT mdelta = abs_hwi (delta);
> rtx this_rtx = gen_rtx_REG (mode, regnum);
> + bool frame_related_p = (regnum == SP_REGNUM);
> + rtx_insn *insn;
>
> /* Do nothing if mdelta is zero. */
> if (!mdelta)
> @@ -1882,7 +1884,8 @@ aarch64_add_constant (machine_mode mode, int regnum,
> int scratchreg,
> /* We only need single instruction if the offset fit into add/sub. */
> if (aarch64_uimm12_shift (mdelta))
> {
> - emit_insn (gen_add2_insn (this_rtx, GEN_INT (delta)));
> + insn = emit_insn (gen_add2_insn (this_rtx, GEN_INT (delta)));
> + RTX_FRAME_RELATED_P (insn) = frame_related_p;
> return;
> }
>
> @@ -1895,15 +1898,23 @@ aarch64_add_constant (machine_mode mode, int regnum,
> int scratchreg,
> HOST_WIDE_INT low_off = mdelta & 0xfff;
>
> low_off = delta < 0 ? -low_off : low_off;
> - emit_insn (gen_add2_insn (this_rtx, GEN_INT (low_off)));
> - emit_insn (gen_add2_insn (this_rtx, GEN_INT (delta - low_off)));
> + insn = emit_insn (gen_add2_insn (this_rtx, GEN_INT (low_off)));
> + RTX_FRAME_RELATED_P (insn) = frame_related_p;
> + insn = emit_insn (gen_add2_insn (this_rtx, GEN_INT (delta - low_off)));
> + RTX_FRAME_RELATED_P (insn) = frame_related_p;
> return;
> }
>
> /* Otherwise use generic function to handle all other situations. */
> rtx scratch_rtx = gen_rtx_REG (mode, scratchreg);
> aarch64_internal_mov_immediate (scratch_rtx, GEN_INT (delta), true, mode);
> - emit_insn (gen_add2_insn (this_rtx, scratch_rtx));
> + insn = emit_insn (gen_add2_insn (this_rtx, scratch_rtx));
> + if (frame_related_p)
> + {
> + RTX_FRAME_RELATED_P (insn) = frame_related_p;
> + rtx adj = plus_constant (mode, this_rtx, delta);
> + add_reg_note (insn , REG_CFA_ADJUST_CFA, gen_rtx_SET (this_rtx, adj));
> + }
> }
>
> static bool
> @@ -3038,36 +3049,7 @@ aarch64_expand_prologue (void)
> frame_size -= (offset + crtl->outgoing_args_size);
> fp_offset = 0;
>
> - if (frame_size >= 0x1000000)
> - {
> - rtx op0 = gen_rtx_REG (Pmode, IP0_REGNUM);
> - emit_move_insn (op0, GEN_INT (-frame_size));
> - insn = emit_insn (gen_add2_insn (stack_pointer_rtx, op0));
> -
> - add_reg_note (insn, REG_CFA_ADJUST_CFA,
> - gen_rtx_SET (stack_pointer_rtx,
> - plus_constant (Pmode, stack_pointer_rtx,
> - -frame_size)));
> - RTX_FRAME_RELATED_P (insn) = 1;
> - }
> - else if (frame_size > 0)
> - {
> - int hi_ofs = frame_size & 0xfff000;
> - int lo_ofs = frame_size & 0x000fff;
> -
> - if (hi_ofs)
> - {
> - insn = emit_insn (gen_add2_insn
> - (stack_pointer_rtx, GEN_INT (-hi_ofs)));
> - RTX_FRAME_RELATED_P (insn) = 1;
> - }
> - if (lo_ofs)
> - {
> - insn = emit_insn (gen_add2_insn
> - (stack_pointer_rtx, GEN_INT (-lo_ofs)));
> - RTX_FRAME_RELATED_P (insn) = 1;
> - }
> - }
> + aarch64_add_constant (Pmode, SP_REGNUM, IP0_REGNUM, -frame_size);
> }
> else
> frame_size = -1;
> @@ -3287,31 +3269,7 @@ aarch64_expand_epilogue (bool for_sibcall)
> if (need_barrier_p)
> emit_insn (gen_stack_tie (stack_pointer_rtx, stack_pointer_rtx));
>
> - if (frame_size >= 0x1000000)
> - {
> - rtx op0 = gen_rtx_REG (Pmode, IP0_REGNUM);
> - emit_move_insn (op0, GEN_INT (frame_size));
> - insn = emit_insn (gen_add2_insn (stack_pointer_rtx, op0));
> - }
> - else
> - {
> - int hi_ofs = frame_size & 0xfff000;
> - int lo_ofs = frame_size & 0x000fff;
> -
> - if (hi_ofs && lo_ofs)
> - {
> - insn = emit_insn (gen_add2_insn
> - (stack_pointer_rtx, GEN_INT (hi_ofs)));
> - RTX_FRAME_RELATED_P (insn) = 1;
> - frame_size = lo_ofs;
> - }
> - insn = emit_insn (gen_add2_insn
> - (stack_pointer_rtx, GEN_INT (frame_size)));
> - }
> -
> - /* Reset the CFA to be SP + 0. */
> - add_reg_note (insn, REG_CFA_DEF_CFA, stack_pointer_rtx);
> - RTX_FRAME_RELATED_P (insn) = 1;
> + aarch64_add_constant (Pmode, SP_REGNUM, IP0_REGNUM, frame_size);
> }
>
> /* Stack adjustment for exception handler. */
>
