Message ID | 3849486f-84ee-31ad-e01c-93dcdb8ca1fa@foss.arm.com |
---|---|
State | Superseded |
Headers | show |
On 21/10/16 09:30, Jiong Wang wrote: > Currently, GCC only support DW_CFA_expression in dwarf module, this patch > extend the support to DW_CFA_val_expression which share the same code > mostly the same code with DW_CFA_expression. > > Meanwhile the existed dwarf expression parser only allows expressions > which can be represented using GCC RTL. If one operation doesn't have > a correspondent GCC RTL operator, then there is no way to attach that > information in reg-note. > > This patch extends the current dwarf expression support to unlimited > number of operations by using PARALLEL, and unlimited type of operations > by using UNSPEC. > > All DW_OP_* of the expression are grouped together inside the PARALLEL, > and those operations which don't have RTL mapping are wrapped by > UNSPEC. The parsing algorithm is simply something like: > > foreach elem inside PARALLEL > if (UNSPEC) > { > dw_op_code = INTVAL (XVECEXP (elem, 0, 0)); > oprnd1 = INTVAL (XVECEXP (elem, 0, 1)); > oprnd2 = INTVAL (XVECEXP (elem, 0, 2)); > } > else > call standard RTL parser. > > Any comments on the approach? Ping ~ > > Thanks. > > gcc/ > 2016-10-20 Jiong Wang <jiong.wang@arm.com> > > * reg-notes.def (CFA_VAL_EXPRESSION): New entry. > * dwarf2cfi.c (dwarf2out_frame_debug_cfa_val_expression): New > function. > (dwarf2out_frame_debug): Support REG_CFA_VAL_EXPRESSION. > (output_cfa_loc): Support DW_CFA_val_expression. > (output_cfa_loc_raw): Likewise. > (output_cfi): Likewise. > (output_cfi_directive): Likewise. > * dwarf2out.c (dw_cfi_oprnd1_desc): Support > DW_CFA_val_expression. > (dw_cfi_oprnd2_desc): Likewise. > (mem_loc_descriptor): Recognize new pattern generated for value > expression. >
On 10/21/2016 04:30 AM, Jiong Wang wrote: > All DW_OP_* of the expression are grouped together inside the PARALLEL, > and those operations which don't have RTL mapping are wrapped by > UNSPEC. The parsing algorithm is simply something like: > > foreach elem inside PARALLEL > if (UNSPEC) > { > dw_op_code = INTVAL (XVECEXP (elem, 0, 0)); > oprnd1 = INTVAL (XVECEXP (elem, 0, 1)); > oprnd2 = INTVAL (XVECEXP (elem, 0, 2)); > } > else > call standard RTL parser. > > Any comments on the approach? If you're going to use UNSPEC, why not put the DWARF operator in the second operand? Jason
On 31/10/16 19:50, Jason Merrill wrote: > On 10/21/2016 04:30 AM, Jiong Wang wrote: >> All DW_OP_* of the expression are grouped together inside the PARALLEL, >> and those operations which don't have RTL mapping are wrapped by >> UNSPEC. The parsing algorithm is simply something like: >> >> foreach elem inside PARALLEL >> if (UNSPEC) >> { >> dw_op_code = INTVAL (XVECEXP (elem, 0, 0)); >> oprnd1 = INTVAL (XVECEXP (elem, 0, 1)); >> oprnd2 = INTVAL (XVECEXP (elem, 0, 2)); >> } >> else >> call standard RTL parser. >> >> Any comments on the approach? > > If you're going to use UNSPEC, why not put the DWARF operator in the > second operand? Hi Jason, Thanks for the review, but I still don't understand your meaning. Do you mean I should simply put the DWARF operator at XVECEXP (UNSPEC_RTX, 0, 2) instead of at XVECEXP (UNSPEC_RTX, 0, 0), and the new parsing algorithm will be the following ? foreach elem inside PARALLEL if (UNSPEC) { oprnd1 = INTVAL (XVECEXP (elem, 0, 0)); oprnd2 = INTVAL (XVECEXP (elem, 0, 1)); dw_op_code = INTVAL (XVECEXP (elem, 0, 2)); } else call standard RTL parser. I actually don't see the benefit of this change, could you please give more comments on this? For this patch, suppose the unwinding rule for register A is poping two values from dwarf evalutaion stack, do some complex processing based on the two values, then push back the result on to stack. We can generate the dwarf value expression description like: (set (reg A) (parallel [(reg A) (reg B) (UNSPEC [(const_int DW_OP_XXX) (const0_rtx) (const0_rtx)] UNSPEC_NUM) then readelf dump will be something like === DW_CFA_val_expression: A (DW_OP_bregB: 0; DW_OP_bregC: 0; DW_OP_XXX) We can't do such description based on current GCC dwarf code, right?
On Tue, Nov 1, 2016 at 11:12 AM, Jiong Wang <jiong.wang@foss.arm.com> wrote: > On 31/10/16 19:50, Jason Merrill wrote: >> >> On 10/21/2016 04:30 AM, Jiong Wang wrote: >>> >>> All DW_OP_* of the expression are grouped together inside the PARALLEL, >>> and those operations which don't have RTL mapping are wrapped by >>> UNSPEC. The parsing algorithm is simply something like: >>> >>> foreach elem inside PARALLEL >>> if (UNSPEC) >>> { >>> dw_op_code = INTVAL (XVECEXP (elem, 0, 0)); >>> oprnd1 = INTVAL (XVECEXP (elem, 0, 1)); >>> oprnd2 = INTVAL (XVECEXP (elem, 0, 2)); >>> } >>> else >>> call standard RTL parser. >>> >>> Any comments on the approach? >> >> >> If you're going to use UNSPEC, why not put the DWARF operator in the >> second operand? > > Thanks for the review, but I still don't understand your meaning. > > Do you mean I should simply put the DWARF operator at XVECEXP (UNSPEC_RTX, > 0, 2) instead of at XVECEXP (UNSPEC_RTX, 0, 0) No, at XINT (UNSPEC_RTX, 1). The documentation of UNSPEC says, /* A machine-specific operation. 1st operand is a vector of operands being used by the operation so that any needed reloads can be done. 2nd operand is a unique value saying which of a number of machine-specific operations is to be performed. Jason
On 01/11/16 15:24, Jason Merrill wrote: > On Tue, Nov 1, 2016 at 11:12 AM, Jiong Wang <jiong.wang@foss.arm.com> wrote: >> On 31/10/16 19:50, Jason Merrill wrote: >>> On 10/21/2016 04:30 AM, Jiong Wang wrote: >>>> All DW_OP_* of the expression are grouped together inside the PARALLEL, >>>> and those operations which don't have RTL mapping are wrapped by >>>> UNSPEC. The parsing algorithm is simply something like: >>>> >>>> foreach elem inside PARALLEL >>>> if (UNSPEC) >>>> { >>>> dw_op_code = INTVAL (XVECEXP (elem, 0, 0)); >>>> oprnd1 = INTVAL (XVECEXP (elem, 0, 1)); >>>> oprnd2 = INTVAL (XVECEXP (elem, 0, 2)); >>>> } >>>> else >>>> call standard RTL parser. >>>> >>>> Any comments on the approach? >>> >>> If you're going to use UNSPEC, why not put the DWARF operator in the >>> second operand? >> Thanks for the review, but I still don't understand your meaning. >> >> Do you mean I should simply put the DWARF operator at XVECEXP (UNSPEC_RTX, >> 0, 2) instead of at XVECEXP (UNSPEC_RTX, 0, 0) > No, at XINT (UNSPEC_RTX, 1). The documentation of UNSPEC says, > > /* A machine-specific operation. > 1st operand is a vector of operands being used by the operation so > that > any needed reloads can be done. > 2nd operand is a unique value saying which of a number of > machine-specific > operations is to be performed. Aha, understood now, thanks for the clarification. You mean we simply reuse the UNSPEC number field, so the RTX will be (UNSPEC [((reg) (reg)] DW_OP_XXX) Yeah, I do have tried to do that, but later give up, one reason I remember is suppose we want to push two value on the stack, the second value is an address, which we want a follow up DW_OP_deref to operate on that. then the value expression will be (set (reg A) (parallel [(reg A) (UNSPEC [DW_OP_deref, const0_rtx, const0_rtx] UNSPEC_PRIVATE_DW); (UNSPEC [DW_OP_XXX (const0_rtx) (const0_rtx)] UNSPEC_PRIVATE_DW)) And there might be some other expressions we need some complex RAW encoding, so it seems to me if we want to offer user the most general way to do this, then it's better to encode the DWARF operation inside UNSPEC as reuse the UNSPEC number then you need to make sure there is no overlap with other backend UNSPEC enumeration number. Does this explanation make sense to you? Thanks.
On Tue, Nov 1, 2016 at 11:59 AM, Jiong Wang <jiong.wang@foss.arm.com> wrote: > On 01/11/16 15:24, Jason Merrill wrote: >> On Tue, Nov 1, 2016 at 11:12 AM, Jiong Wang <jiong.wang@foss.arm.com> wrote: >>> On 31/10/16 19:50, Jason Merrill wrote: >>>> On 10/21/2016 04:30 AM, Jiong Wang wrote: >>>>> >>>>> All DW_OP_* of the expression are grouped together inside the PARALLEL, >>>>> and those operations which don't have RTL mapping are wrapped by >>>>> UNSPEC. The parsing algorithm is simply something like: >>>>> >>>>> foreach elem inside PARALLEL >>>>> if (UNSPEC) >>>>> { >>>>> dw_op_code = INTVAL (XVECEXP (elem, 0, 0)); >>>>> oprnd1 = INTVAL (XVECEXP (elem, 0, 1)); >>>>> oprnd2 = INTVAL (XVECEXP (elem, 0, 2)); >>>>> } >>>>> else >>>>> call standard RTL parser. >>>>> >>>>> Any comments on the approach? >>>> >>>> If you're going to use UNSPEC, why not put the DWARF operator in the >>>> second operand? >>> >>> Thanks for the review, but I still don't understand your meaning. >>> >>> Do you mean I should simply put the DWARF operator at XVECEXP >>> (UNSPEC_RTX, 0, 2) instead of at XVECEXP (UNSPEC_RTX, 0, 0) >> >> No, at XINT (UNSPEC_RTX, 1). The documentation of UNSPEC says, >> >> /* A machine-specific operation. >> 1st operand is a vector of operands being used by the operation so >> that any needed reloads can be done. >> 2nd operand is a unique value saying which of a number of >> machine-specific operations is to be performed. > > Aha, understood now, thanks for the clarification. > > You mean we simply reuse the UNSPEC number field, so the RTX will be > > (UNSPEC > [((reg) (reg)] > DW_OP_XXX) > > Yeah, I do have tried to do that, but later give up, one reason I remember is suppose we > want to push two value on the stack, the second value is an address, which we want a > follow up DW_OP_deref to operate on that. then the value expression will be > > (set (reg A) > (parallel > [(reg A) > > (UNSPEC > [DW_OP_deref, const0_rtx, const0_rtx] > UNSPEC_PRIVATE_DW); > > (UNSPEC > [DW_OP_XXX (const0_rtx) (const0_rtx)] > UNSPEC_PRIVATE_DW)) > > And there might be some other expressions we need some complex RAW encoding, Why can't you do this putting the OP in the number field of both UNSPECs? > so it seems to me if we want to offer user the most general way to do this, then it's > better to encode the DWARF operation inside UNSPEC as reuse the UNSPEC number then you need to make > sure there is no overlap with other backend UNSPEC enumeration number. It seems to me that a CFA_*expression note would never use target UNSPEC codes, and a DWARF UNSPEC would never appear outside of such a note, so we don't need to worry about conflicts. Jason
On 01/11/16 16:48, Jason Merrill wrote: > On Tue, Nov 1, 2016 at 11:59 AM, Jiong Wang <jiong.wang@foss.arm.com> wrote: >> On 01/11/16 15:24, Jason Merrill wrote: >>> On Tue, Nov 1, 2016 at 11:12 AM, Jiong Wang <jiong.wang@foss.arm.com> wrote: >>>> On 31/10/16 19:50, Jason Merrill wrote: >>>>> On 10/21/2016 04:30 AM, Jiong Wang wrote: >>>>>> All DW_OP_* of the expression are grouped together inside the PARALLEL, >>>>>> and those operations which don't have RTL mapping are wrapped by >>>>>> UNSPEC. The parsing algorithm is simply something like: >>>>>> >>>>>> foreach elem inside PARALLEL >>>>>> if (UNSPEC) >>>>>> { >>>>>> dw_op_code = INTVAL (XVECEXP (elem, 0, 0)); >>>>>> oprnd1 = INTVAL (XVECEXP (elem, 0, 1)); >>>>>> oprnd2 = INTVAL (XVECEXP (elem, 0, 2)); >>>>>> } >>>>>> else >>>>>> call standard RTL parser. >>>>>> >>>>>> Any comments on the approach? >>>>> If you're going to use UNSPEC, why not put the DWARF operator in the >>>>> second operand? >>>> Thanks for the review, but I still don't understand your meaning. >>>> >>>> Do you mean I should simply put the DWARF operator at XVECEXP >>>> (UNSPEC_RTX, 0, 2) instead of at XVECEXP (UNSPEC_RTX, 0, 0) >>> No, at XINT (UNSPEC_RTX, 1). The documentation of UNSPEC says, >>> >>> /* A machine-specific operation. >>> 1st operand is a vector of operands being used by the operation so >>> that any needed reloads can be done. >>> 2nd operand is a unique value saying which of a number of >>> machine-specific operations is to be performed. >> Aha, understood now, thanks for the clarification. >> >> You mean we simply reuse the UNSPEC number field, so the RTX will be >> >> (UNSPEC >> [((reg) (reg)] >> DW_OP_XXX) >> >> Yeah, I do have tried to do that, but later give up, one reason I remember is suppose we >> want to push two value on the stack, the second value is an address, which we want a >> follow up DW_OP_deref to operate on that. then the value expression will be >> >> (set (reg A) >> (parallel >> [(reg A) >> >> (UNSPEC >> [DW_OP_deref, const0_rtx, const0_rtx] >> UNSPEC_PRIVATE_DW); >> >> (UNSPEC >> [DW_OP_XXX (const0_rtx) (const0_rtx)] >> UNSPEC_PRIVATE_DW)) >> >> And there might be some other expressions we need some complex RAW encoding, > Why can't you do this putting the OP in the number field of both UNSPECs? I was demoing the RTX based on my current approach, and simplfy want to say we only need to define one unspec number (UNSPEC_PRIVATE_DW), while if we putting the OP in the number field of both UNSPECs, we need two unspec number, and we might need more for other similar expressions. If we don't need to worry about the conflicts, then your suggestion is definitely better. I will do more tests on this. Besides this issue, do you think the PARALLEL + UNSPEC based approach to represent DWARF RAW expression is acceptable? Thanks. Regards, Jiong > >> so it seems to me if we want to offer user the most general way to do this, then it's >> better to encode the DWARF operation inside UNSPEC as reuse the UNSPEC number then you need to make >> sure there is no overlap with other backend UNSPEC enumeration number. > It seems to me that a CFA_*expression note would never use target > UNSPEC codes, and a DWARF UNSPEC would never appear outside of such a > note, so we don't need to worry about conflicts. > > Jason
On Tue, Nov 1, 2016 at 1:02 PM, Jiong Wang <jiong.wang@foss.arm.com> wrote: > Besides this issue, do you think the PARALLEL + UNSPEC based approach to > represent DWARF RAW expression is acceptable? Yes. Jason
diff --git a/gcc/dwarf2cfi.c b/gcc/dwarf2cfi.c index 6491d5aaf4c4a21241cc718bfff1016f6d149951..b8c88fbae1df80a2664a414d8ae016a5343bf435 100644 --- a/gcc/dwarf2cfi.c +++ b/gcc/dwarf2cfi.c @@ -1235,7 +1235,7 @@ dwarf2out_frame_debug_cfa_register (rtx set) reg_save (sregno, dregno, 0); } -/* A subroutine of dwarf2out_frame_debug, process a REG_CFA_EXPRESSION note. */ +/* A subroutine of dwarf2out_frame_debug, process a REG_CFA_EXPRESSION note. */ static void dwarf2out_frame_debug_cfa_expression (rtx set) @@ -1267,6 +1267,29 @@ dwarf2out_frame_debug_cfa_expression (rtx set) update_row_reg_save (cur_row, regno, cfi); } +/* A subroutine of dwarf2out_frame_debug, process a REG_CFA_VAL_EXPRESSION + note. */ + +static void +dwarf2out_frame_debug_cfa_val_expression (rtx set) +{ + rtx dest = SET_DEST (set); + gcc_assert (REG_P (dest)); + + rtx span = targetm.dwarf_register_span (dest); + gcc_assert (!span); + + rtx src = SET_SRC (set); + dw_cfi_ref cfi = new_cfi (); + cfi->dw_cfi_opc = DW_CFA_val_expression; + cfi->dw_cfi_oprnd1.dw_cfi_reg_num = dwf_regno (dest); + cfi->dw_cfi_oprnd2.dw_cfi_loc + = mem_loc_descriptor (src, GET_MODE (src), + GET_MODE (dest), VAR_INIT_STATUS_INITIALIZED); + add_cfi (cfi); + update_row_reg_save (cur_row, dwf_regno (dest), cfi); +} + /* A subroutine of dwarf2out_frame_debug, process a REG_CFA_RESTORE note. */ static void @@ -2033,10 +2056,16 @@ dwarf2out_frame_debug (rtx_insn *insn) break; case REG_CFA_EXPRESSION: + case REG_CFA_VAL_EXPRESSION: n = XEXP (note, 0); if (n == NULL) n = single_set (insn); - dwarf2out_frame_debug_cfa_expression (n); + + if (REG_NOTE_KIND (note) == REG_CFA_EXPRESSION) + dwarf2out_frame_debug_cfa_expression (n); + else + dwarf2out_frame_debug_cfa_val_expression (n); + handled_one = true; break; @@ -3015,7 +3044,8 @@ output_cfa_loc (dw_cfi_ref cfi, int for_eh) dw_loc_descr_ref loc; unsigned long size; - if (cfi->dw_cfi_opc == DW_CFA_expression) + if (cfi->dw_cfi_opc == DW_CFA_expression + || cfi->dw_cfi_opc == DW_CFA_val_expression) { unsigned r = DWARF2_FRAME_REG_OUT (cfi->dw_cfi_oprnd1.dw_cfi_reg_num, for_eh); @@ -3041,7 +3071,8 @@ output_cfa_loc_raw (dw_cfi_ref cfi) dw_loc_descr_ref loc; unsigned long size; - if (cfi->dw_cfi_opc == DW_CFA_expression) + if (cfi->dw_cfi_opc == DW_CFA_expression + || cfi->dw_cfi_opc == DW_CFA_val_expression) { unsigned r = DWARF2_FRAME_REG_OUT (cfi->dw_cfi_oprnd1.dw_cfi_reg_num, 1); @@ -3188,6 +3219,7 @@ output_cfi (dw_cfi_ref cfi, dw_fde_ref fde, int for_eh) case DW_CFA_def_cfa_expression: case DW_CFA_expression: + case DW_CFA_val_expression: output_cfa_loc (cfi, for_eh); break; @@ -3302,16 +3334,13 @@ output_cfi_directive (FILE *f, dw_cfi_ref cfi) break; case DW_CFA_def_cfa_expression: - if (f != asm_out_file) - { - fprintf (f, "\t.cfi_def_cfa_expression ...\n"); - break; - } - /* FALLTHRU */ case DW_CFA_expression: + case DW_CFA_val_expression: if (f != asm_out_file) { - fprintf (f, "\t.cfi_cfa_expression ...\n"); + fprintf (f, "\t.cfi_%scfa_%sexpression ...\n", + cfi->dw_cfi_opc == DW_CFA_def_cfa_expression ? "def_" : "", + cfi->dw_cfi_opc == DW_CFA_val_expression ? "val_" : ""); break; } fprintf (f, "\t.cfi_escape %#x,", cfi->dw_cfi_opc); diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c index 4a3df339df2c6a6816ac8b8dbdb2466a7492c592..525abccd64d88ec0f3ba4ed59508879748106625 100644 --- a/gcc/dwarf2out.c +++ b/gcc/dwarf2out.c @@ -518,6 +518,7 @@ dw_cfi_oprnd1_desc (enum dwarf_call_frame_info cfi) case DW_CFA_def_cfa_register: case DW_CFA_register: case DW_CFA_expression: + case DW_CFA_val_expression: return dw_cfi_oprnd_reg_num; case DW_CFA_def_cfa_offset: @@ -551,6 +552,7 @@ dw_cfi_oprnd2_desc (enum dwarf_call_frame_info cfi) return dw_cfi_oprnd_reg_num; case DW_CFA_expression: + case DW_CFA_val_expression: return dw_cfi_oprnd_loc; default: @@ -14236,6 +14238,59 @@ mem_loc_descriptor (rtx rtl, machine_mode mode, resolve_one_addr (&rtl); goto symref; + /* RTL sequences inside PARALLEL are raw expression representation. + + mem_loc_descriptor can be used to build generic DWARF expressions for + DW_CFA_expression ad DW_CFA_val_expression where the expression may can + not be represented using normal RTL sequences. In this case, group all + expression operations (DW_OP_*) inside a PARALLEL. For those DW_OP which + doesn't have RTL mapping, wrap it using UNSPEC. The logic for parsing + PARALLEL sequences is: + + foreach elem inside PARALLEL + if (UNSPEC) + dw_op = UNSPEC elem 0 (the DWARF operation code) + oprnd1 = UNSPEC elem 1 + oprnd2 = UNSPEC elem 2 + else + call standard RTL parser (recursive call of mem_loc_descriptor) */ + case PARALLEL: + { + int index = 0; + dw_loc_descr_ref exp_result = NULL; + + for (; index < XVECLEN (rtl, 0); index++) + { + rtx elem = XVECEXP (rtl, 0, index); + if (GET_CODE (elem) == UNSPEC) + { + /* Each DWARF operation should always contain two operands, for + those operands not used, const0_rtx is passed. Plus the + operation code itself, there will be three elements for each + DWARF operation description UNSPEC. */ + gcc_assert (XVECLEN (elem, 0) == 3); + + HOST_WIDE_INT dw_op = INTVAL (XVECEXP (elem, 0, 0)); + HOST_WIDE_INT oprnd1 = INTVAL (XVECEXP (elem, 0, 1)); + HOST_WIDE_INT oprnd2 = INTVAL (XVECEXP (elem, 0, 2)); + exp_result = + new_loc_descr ((enum dwarf_location_atom) dw_op, oprnd1, + oprnd2); + } + else + exp_result = + mem_loc_descriptor (elem, mode, mem_mode, + VAR_INIT_STATUS_INITIALIZED); + + if (!mem_loc_result) + mem_loc_result = exp_result; + else + add_loc_descr (&mem_loc_result, exp_result); + } + + break; + } + default: if (flag_checking) { diff --git a/gcc/reg-notes.def b/gcc/reg-notes.def index 5374169b9a417d3695b685bfce2b44ff5d8c89e1..962dbb8007e5150d733344dd5a75495d9adf6a79 100644 --- a/gcc/reg-notes.def +++ b/gcc/reg-notes.def @@ -149,6 +149,11 @@ REG_NOTE (CFA_REGISTER) store of a register to an arbitrary (non-validated) memory address. */ REG_NOTE (CFA_EXPRESSION) +/* Attached to insns that are RTX_FRAME_RELATED_P, but are too complex + for FRAME_RELATED_EXPR intuition. The DWARF expression computes the value of + the given register. */ +REG_NOTE (CFA_VAL_EXPRESSION) + /* Attached to insns that are RTX_FRAME_RELATED_P, with the information that this is a restore operation, i.e. will result in DW_CFA_restore or the like. Either the attached rtx, or the destination of the insn's