On 17/01/17 15:11, Jiong Wang wrote:
>
>
> On 17/01/17 13:57, Richard Earnshaw (lists) wrote:
>> On 16/01/17 14:29, Jiong Wang wrote:
>>>
>>>> I can see the reason for doing this is if you want to seperate the
>>>> interpretion
>>>> of GCC CFA reg-note and the final DWARF CFA operation. My
>>>> understanding is all
>>>> reg notes defined in gcc/reg-note.def should have general meaning,
>>>> even the
>>>> CFA_WINDOW_SAVE. For those which are architecture specific we might
>>>> need a
>>>> mechanism to define them in backend only.
>>>> For general reg-notes in gcc/reg-note.def, they are not always have
>>>> the
>>>> corresponding standard DWARF CFA operation, for example
>>>> CFA_WINDOW_SAVE,
>>>> therefore if we want to achieve what you described, I think we also
>>>> need to
>>>> define a new target hook which maps a GCC CFA reg-note into
>>>> architecture DWARF
>>>> CFA operation.
>>>>
>>>> Regards,
>>>> Jiong
>>>>
>>>>
>>> Here is the patch.
>>>
>> Hmm, I really wasn't expecting any more than something like the
>> following in dwarf2cfi.c:
>>
>> @@ -2098,7 +2098,9 @@ dwarf2out_frame_debug (rtx_insn *insn)
>> handled_one = true;
>> break;
>>
>> + case REG_CFA_TOGGLE_RA_MANGLE:
>> case REG_CFA_WINDOW_SAVE:
>> + /* We overload both of these operations onto the same DWARF
>> opcode. */
>> dwarf2out_frame_debug_cfa_window_save ();
>> handled_one = true;
>> break;
>>
>> This keeps the two reg notes separate within the compiler, but emits the
>> same dwarf operation during final output. This avoids the need for new
>> hooks or anything more complicated.
>
> This was my initial thoughts and the patch would be very small as you've
> demonstrated. I later moved to this complexer patch as I am thinking it's
> better to completely treat notes in reg-notes.def as having generic
> meaning and
> maps them to standard DWARF CFA if there is, otherwise maps them to target
> private DWARF CFA through this new hook. This give other targets a
> chance to
> map, for example REG_CFA_TOGGLE_RA_MANGLE, to their architecture DWARF
> number.
>
> The introduction of new hook looks be very low risk in this stage, the only
> painful thing is the header file needs to be reorganized as we need to
> use some
> DWARF type and reg-note type in targhooks.c.
>
> Anyway, if the new hook patch is too heavy, I have attached the the
> simplified
> version which simply defines the new REG_CFA_TOGGLE_RA_MANGLE and maps
> to same
> code of REG_CFA_WINDOW_SAVE.
>
>
Yes, this is much more like what I had in mind.
OK.
R.
> gcc/
>
> 2017-01-17 Jiong Wang <jiong.w...@arm.com>
>
> * reg-notes.def (CFA_TOGGLE_RA_MANGLE): New reg-note.
> * combine-stack-adj.c (no_unhandled_cfa): Handle
> REG_CFA_TOGGLE_RA_MANGLE.
> * dwarf2cfi.c
> (dwarf2out_frame_debug): Handle REG_CFA_TOGGLE_RA_MANGLE.
> * config/aarch64/aarch64.c (aarch64_expand_prologue): Generates
> DWARF
> info for return address signing.
> (aarch64_expand_epilogue): Likewise.
>
>
> k.patch
>
>
> diff --git a/gcc/combine-stack-adj.c b/gcc/combine-stack-adj.c
> index
> 20cd59ad08329e9f4f834bfc01d6f9ccc4485283..9ec14a3e44363f35f6419c38233ce5eebddd3458
> 100644
> --- a/gcc/combine-stack-adj.c
> +++ b/gcc/combine-stack-adj.c
> @@ -208,6 +208,7 @@ no_unhandled_cfa (rtx_insn *insn)
> case REG_CFA_SET_VDRAP:
> case REG_CFA_WINDOW_SAVE:
> case REG_CFA_FLUSH_QUEUE:
> + case REG_CFA_TOGGLE_RA_MANGLE:
> return false;
> }
>
> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index
> 3bcad76b68b6ea7c9d75d150d79c45fb74d6bf0d..6451b08191cf1a44aed502930da8603111f6e8ca
> 100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -3553,7 +3553,11 @@ aarch64_expand_prologue (void)
>
> /* Sign return address for functions. */
> if (aarch64_return_address_signing_enabled ())
> - emit_insn (gen_pacisp ());
> + {
> + insn = emit_insn (gen_pacisp ());
> + add_reg_note (insn, REG_CFA_TOGGLE_RA_MANGLE, const0_rtx);
> + RTX_FRAME_RELATED_P (insn) = 1;
> + }
>
> if (flag_stack_usage_info)
> current_function_static_stack_size = frame_size;
> @@ -3707,7 +3711,11 @@ aarch64_expand_epilogue (bool for_sibcall)
> */
> if (aarch64_return_address_signing_enabled ()
> && (for_sibcall || !TARGET_ARMV8_3 || crtl->calls_eh_return))
> - emit_insn (gen_autisp ());
> + {
> + insn = emit_insn (gen_autisp ());
> + add_reg_note (insn, REG_CFA_TOGGLE_RA_MANGLE, const0_rtx);
> + RTX_FRAME_RELATED_P (insn) = 1;
> + }
>
> /* Stack adjustment for exception handler. */
> if (crtl->calls_eh_return)
> diff --git a/gcc/dwarf2cfi.c b/gcc/dwarf2cfi.c
> index
> 2748e2fa48e4794181496b26df9b51b7e51e7b84..2a527c9fecab091dccb417492e5dbb2ade244be2
> 100644
> --- a/gcc/dwarf2cfi.c
> +++ b/gcc/dwarf2cfi.c
> @@ -2098,7 +2098,9 @@ dwarf2out_frame_debug (rtx_insn *insn)
> handled_one = true;
> break;
>
> + case REG_CFA_TOGGLE_RA_MANGLE:
> case REG_CFA_WINDOW_SAVE:
> + /* We overload both of these operations onto the same DWARF opcode. */
> dwarf2out_frame_debug_cfa_window_save ();
> handled_one = true;
> break;
> diff --git a/gcc/reg-notes.def b/gcc/reg-notes.def
> index
> ead4a9f58e8621288ee765e029c673640fdf38f4..175da119b6a534b04bd154f2c69dd087afd474ea
> 100644
> --- a/gcc/reg-notes.def
> +++ b/gcc/reg-notes.def
> @@ -177,6 +177,11 @@ REG_NOTE (CFA_WINDOW_SAVE)
> the rest of the compiler as a CALL_INSN. */
> REG_NOTE (CFA_FLUSH_QUEUE)
>
> +/* Attached to insns that are RTX_FRAME_RELATED_P, toggling the mangling
> status
> + of return address. Currently it's only used by AArch64. The argument is
> + ignored. */
> +REG_NOTE (CFA_TOGGLE_RA_MANGLE)
> +
> /* Indicates what exception region an INSN belongs in. This is used
> to indicate what region to which a call may throw. REGION 0
> indicates that a call cannot throw at all. REGION -1 indicates
>
