| Message ID | 1369430452-27598-1-git-send-email-rth@twiddle.net |
|---|---|
| State | New |
| Headers | show |
Ping. r~ On 05/24/2013 02:20 PM, Richard Henderson wrote: > Allows unwinding past the code_gen_buffer. > > Signed-off-by: Richard Henderson <rth@twiddle.net> > --- > tcg/arm/tcg-target.c | 91 ++++++++++++++++++++++++++++++++++++++++++++++------ > 1 file changed, 82 insertions(+), 9 deletions(-) > > diff --git a/tcg/arm/tcg-target.c b/tcg/arm/tcg-target.c > index 3d43412..4a691b1 100644 > --- a/tcg/arm/tcg-target.c > +++ b/tcg/arm/tcg-target.c > @@ -2100,23 +2100,31 @@ static inline void tcg_out_movi(TCGContext *s, TCGType type, > tcg_out_movi32(s, COND_AL, ret, arg); > } > > +/* Compute frame size via macros, to share between tcg_target_qemu_prologue > + and tcg_register_jit. */ > + > +#define PUSH_SIZE ((11 - 4 + 1 + 1) * sizeof(tcg_target_long)) > + > +#define FRAME_SIZE \ > + ((PUSH_SIZE \ > + + TCG_STATIC_CALL_ARGS_SIZE \ > + + CPU_TEMP_BUF_NLONGS * sizeof(long) \ > + + TCG_TARGET_STACK_ALIGN - 1) \ > + & -TCG_TARGET_STACK_ALIGN) > + > static void tcg_target_qemu_prologue(TCGContext *s) > { > - int frame_size; > + int stack_addend; > > /* Calling convention requires us to save r4-r11 and lr. */ > /* stmdb sp!, { r4 - r11, lr } */ > tcg_out32(s, (COND_AL << 28) | 0x092d4ff0); > > - /* Allocate the local stack frame. */ > - frame_size = TCG_STATIC_CALL_ARGS_SIZE; > - frame_size += CPU_TEMP_BUF_NLONGS * sizeof(long); > - /* We saved an odd number of registers above; keep an 8 aligned stack. */ > - frame_size = ((frame_size + TCG_TARGET_STACK_ALIGN - 1) > - & -TCG_TARGET_STACK_ALIGN) + 4; > + /* Reserve callee argument and tcg temp space. */ > + stack_addend = FRAME_SIZE - PUSH_SIZE; > > tcg_out_dat_rI(s, COND_AL, ARITH_SUB, TCG_REG_CALL_STACK, > - TCG_REG_CALL_STACK, frame_size, 1); > + TCG_REG_CALL_STACK, stack_addend, 1); > tcg_set_frame(s, TCG_REG_CALL_STACK, TCG_STATIC_CALL_ARGS_SIZE, > CPU_TEMP_BUF_NLONGS * sizeof(long)); > > @@ -2127,8 +2135,73 @@ static void tcg_target_qemu_prologue(TCGContext *s) > > /* Epilogue. We branch here via tb_ret_addr. */ > tcg_out_dat_rI(s, COND_AL, ARITH_ADD, TCG_REG_CALL_STACK, > - TCG_REG_CALL_STACK, frame_size, 1); > + TCG_REG_CALL_STACK, stack_addend, 1); > > /* ldmia sp!, { r4 - r11, pc } */ > tcg_out32(s, (COND_AL << 28) | 0x08bd8ff0); > } > + > +typedef struct { > + uint32_t len __attribute__((aligned((sizeof(void *))))); > + uint32_t id; > + uint8_t version; > + char augmentation[1]; > + uint8_t code_align; > + uint8_t data_align; > + uint8_t return_column; > +} DebugFrameCIE; > + > +typedef struct { > + uint32_t len __attribute__((aligned((sizeof(void *))))); > + uint32_t cie_offset; > + tcg_target_long func_start __attribute__((packed)); > + tcg_target_long func_len __attribute__((packed)); > + uint8_t def_cfa[4]; > + uint8_t reg_ofs[18]; > +} DebugFrameFDE; > + > +typedef struct { > + DebugFrameCIE cie; > + DebugFrameFDE fde; > +} DebugFrame; > + > +#define ELF_HOST_MACHINE EM_ARM > + > +static DebugFrame debug_frame = { > + .cie.len = sizeof(DebugFrameCIE)-4, /* length after .len member */ > + .cie.id = -1, > + .cie.version = 1, > + .cie.code_align = 1, > + .cie.data_align = 0x7c, /* sleb128 -4 */ > + .cie.return_column = 14, > + > + .fde.len = sizeof(DebugFrameFDE)-4, /* length after .len member */ > + .fde.def_cfa = { > + 12, 13, /* DW_CFA_def_cfa sp, ... */ > + (FRAME_SIZE & 0x7f) | 0x80, /* ... uleb128 FRAME_SIZE */ > + (FRAME_SIZE >> 7) > + }, > + .fde.reg_ofs = { > + /* The following must match the stmdb in the prologue. */ > + 0x8e, 1, /* DW_CFA_offset, lr, -4 */ > + 0x8b, 2, /* DW_CFA_offset, r11, -8 */ > + 0x8a, 3, /* DW_CFA_offset, r10, -12 */ > + 0x89, 4, /* DW_CFA_offset, r9, -16 */ > + 0x88, 5, /* DW_CFA_offset, r8, -20 */ > + 0x87, 6, /* DW_CFA_offset, r7, -24 */ > + 0x86, 7, /* DW_CFA_offset, r6, -28 */ > + 0x85, 8, /* DW_CFA_offset, r5, -32 */ > + 0x84, 9, /* DW_CFA_offset, r4, -36 */ > + } > +}; > + > +void tcg_register_jit(void *buf, size_t buf_size) > +{ > + /* We're expecting a 2 byte uleb128 encoded value. */ > + assert(FRAME_SIZE >> 14 == 0); > + > + debug_frame.fde.func_start = (tcg_target_long) buf; > + debug_frame.fde.func_len = buf_size; > + > + tcg_register_jit_int(buf, buf_size, &debug_frame, sizeof(debug_frame)); > +} >
Hi, Richard, 在 2013-05-24五的 14:20 -0700，Richard Henderson写道： > Allows unwinding past the code_gen_buffer. > > Signed-off-by: Richard Henderson <rth@twiddle.net> > --- > tcg/arm/tcg-target.c | 91 ++++++++++++++++++++++++++++++++++++++++++++++------ > 1 file changed, 82 insertions(+), 9 deletions(-) > > diff --git a/tcg/arm/tcg-target.c b/tcg/arm/tcg-target.c > index 3d43412..4a691b1 100644 > --- a/tcg/arm/tcg-target.c > +++ b/tcg/arm/tcg-target.c > @@ -2100,23 +2100,31 @@ static inline void tcg_out_movi(TCGContext *s, TCGType type, > tcg_out_movi32(s, COND_AL, ret, arg); > } > > + > +typedef struct { > + uint32_t len __attribute__((aligned((sizeof(void *))))); > + uint32_t cie_offset; > + tcg_target_long func_start __attribute__((packed)); > + tcg_target_long func_len __attribute__((packed)); suspicious usage of packed attribute here, since tcg_targe_long is either 32 or 64 bits, not a struct or union. Thanks! > + uint8_t def_cfa[4]; > + uint8_t reg_ofs[18]; > +} DebugFrameFDE; > + > +typedef struct { > + DebugFrameCIE cie; > + DebugFrameFDE fde; > +} DebugFrame; > + > +#define ELF_HOST_MACHINE EM_ARM > + > +static DebugFrame debug_frame = { > + .cie.len = sizeof(DebugFrameCIE)-4, /* length after .len member */ > + .cie.id = -1, > + .cie.version = 1, > + .cie.code_align = 1, > + .cie.data_align = 0x7c, /* sleb128 -4 */ > + .cie.return_column = 14, > + > + .fde.len = sizeof(DebugFrameFDE)-4, /* length after .len member */ > + .fde.def_cfa = { > + 12, 13, /* DW_CFA_def_cfa sp, ... */ > + (FRAME_SIZE & 0x7f) | 0x80, /* ... uleb128 FRAME_SIZE */ > + (FRAME_SIZE >> 7) > + }, > + .fde.reg_ofs = { > + /* The following must match the stmdb in the prologue. */ > + 0x8e, 1, /* DW_CFA_offset, lr, -4 */ > + 0x8b, 2, /* DW_CFA_offset, r11, -8 */ > + 0x8a, 3, /* DW_CFA_offset, r10, -12 */ > + 0x89, 4, /* DW_CFA_offset, r9, -16 */ > + 0x88, 5, /* DW_CFA_offset, r8, -20 */ > + 0x87, 6, /* DW_CFA_offset, r7, -24 */ > + 0x86, 7, /* DW_CFA_offset, r6, -28 */ > + 0x85, 8, /* DW_CFA_offset, r5, -32 */ > + 0x84, 9, /* DW_CFA_offset, r4, -36 */ > + } > +}; > + > +void tcg_register_jit(void *buf, size_t buf_size) > +{ > + /* We're expecting a 2 byte uleb128 encoded value. */ > + assert(FRAME_SIZE >> 14 == 0); > + > + debug_frame.fde.func_start = (tcg_target_long) buf; > + debug_frame.fde.func_len = buf_size; > + > + tcg_register_jit_int(buf, buf_size, &debug_frame, sizeof(debug_frame)); > +}
On 06/04/2013 06:56 PM, li guang wrote: >> > +typedef struct { >> > + uint32_t len __attribute__((aligned((sizeof(void *))))); >> > + uint32_t cie_offset; >> > + tcg_target_long func_start __attribute__((packed)); >> > + tcg_target_long func_len __attribute__((packed)); > suspicious usage of packed attribute here, > since tcg_targe_long is either 32 or 64 bits, > not a struct or union. > > Thanks! > Your question is worded poorly -- what has struct/union got to do with it? One can adjust the alignment of any type. Perhaps you don't know what it is that __attribute__((packed)) actually does? While it's true that for ARM all four of these data members are 32-bit, and thus none of the attributes are required, it's not actually wrong. Given that this sort of boiler-plate tends to get copied from target to target, and since the attributes *are* required for 64-bit hosts, I prefer to keep all such structures defined similarly. r~
Am 05.06.2013 14:49, schrieb Richard Henderson: > On 06/04/2013 06:56 PM, li guang wrote: >>>> +typedef struct { >>>> + uint32_t len __attribute__((aligned((sizeof(void *))))); >>>> + uint32_t cie_offset; >>>> + tcg_target_long func_start __attribute__((packed)); >>>> + tcg_target_long func_len __attribute__((packed)); >> suspicious usage of packed attribute here, >> since tcg_targe_long is either 32 or 64 bits, >> not a struct or union. >> >> Thanks! >> > > Your question is worded poorly -- what has struct/union got to do with it? One > can adjust the alignment of any type. Perhaps you don't know what it is that > __attribute__((packed)) actually does? To me the English word "packed" refers to a struct containing no alignment padding, i.e. sizeof(the struct) = sum(sizeof(each field)). The use of __attribute__((packed)) on an individual field while quite possibly valid is unusual and I believe we have a QEMU_PACKED macro. So why can't you apply QEMU_PACKED to the whole struct? Because of the contradicting void* alignment attribute of the first field? Cheers, Andreas > While it's true that for ARM all four of these data members are 32-bit, and > thus none of the attributes are required, it's not actually wrong. Given that > this sort of boiler-plate tends to get copied from target to target, and since > the attributes *are* required for 64-bit hosts, I prefer to keep all such > structures defined similarly. > > > r~
On 06/05/2013 06:02 AM, Andreas Färber wrote: > So why can't you apply QEMU_PACKED to the whole struct? Because of the > contradicting void* alignment attribute of the first field? Actually, that might work. I'll give it a shot on x86_64 and change all of the uses if it does work. r~
On 5 June 2013 14:04, Richard Henderson <rth@twiddle.net> wrote: > On 06/05/2013 06:02 AM, Andreas Färber wrote: >> So why can't you apply QEMU_PACKED to the whole struct? Because of the >> contradicting void* alignment attribute of the first field? > > Actually, that might work. I'll give it a shot on x86_64 and change all > of the uses if it does work. Bear in mind that making the entire structure 'packed' means gcc treats it as being potentially completely unaligned (ie attribute 'packed' means 'packed and not at all aligned', not just 'packed'). This isn't a big deal except for structs where we care about atomicity, though, which I don't think is the case here. -- PMM
On 06/05/2013 06:10 AM, Peter Maydell wrote: > This isn't a big deal except for structs > where we care about atomicity, though, which I don't think > is the case here. Indeed not. This is just more dwarf debug info for gdb's use. r~
在 2013-06-05三的 05:49 -0700，Richard Henderson写道： > On 06/04/2013 06:56 PM, li guang wrote: > >> > +typedef struct { > >> > + uint32_t len __attribute__((aligned((sizeof(void *))))); > >> > + uint32_t cie_offset; > >> > + tcg_target_long func_start __attribute__((packed)); > >> > + tcg_target_long func_len __attribute__((packed)); > > suspicious usage of packed attribute here, > > since tcg_targe_long is either 32 or 64 bits, > > not a struct or union. > > > > Thanks! > > > > Your question is worded poorly -- what has struct/union got to do with it? One > can adjust the alignment of any type. Perhaps you don't know what it is that > __attribute__((packed)) actually does? referred from gcc manual: "This attribute, attached to struct or union type definition, specifies that each member (other than zero-width bitfields) of the structure or union is placed to minimize the memory required." so, what I mean is as the manual said, this attribute mostly applies to struct or union. > > While it's true that for ARM all four of these data members are 32-bit, and > thus none of the attributes are required, it's not actually wrong. Given that > this sort of boiler-plate tends to get copied from target to target, and since > the attributes *are* required for 64-bit hosts, forgive me if the question is fool, packed means seize minimal memory size, right? and any chance tcg_target_long smaller than 64 bits in 64-bit case? > I prefer to keep all such > structures defined similarly. > > > r~
On 06/05/2013 05:28 PM, li guang wrote: > 在 2013-06-05三的 05:49 -0700，Richard Henderson写道： >> On 06/04/2013 06:56 PM, li guang wrote: >>>>> +typedef struct { >>>>> + uint32_t len __attribute__((aligned((sizeof(void *))))); >>>>> + uint32_t cie_offset; >>>>> + tcg_target_long func_start __attribute__((packed)); >>>>> + tcg_target_long func_len __attribute__((packed)); >>> suspicious usage of packed attribute here, >>> since tcg_targe_long is either 32 or 64 bits, >>> not a struct or union. >>> >>> Thanks! >>> >> >> Your question is worded poorly -- what has struct/union got to do with it? One >> can adjust the alignment of any type. Perhaps you don't know what it is that >> __attribute__((packed)) actually does? > > referred from gcc manual: > "This attribute, attached to struct or union type definition, specifies > that each member (other than zero-width bitfields) of the structure or > union is placed to minimize the memory required." > > so, what I mean is as the manual said, this attribute mostly applies to > struct or union. >From the gcc manual: @cindex @code{packed} attribute The @code{packed} attribute specifies that a variable or structure field should have the smallest possible alignment---one byte for a variable, and one bit for a field, unless you specify a larger value with the @code{aligned} attribute. Notice "or structure field", which is exactly what I have above. > forgive me if the question is fool, > packed means seize minimal memory size, right? It really means minimal alignment. Which means that no padding will be added to ensure alignment. Which can lead to a reduction in memory size, but that's not the major point. r~
在 2013-06-06四的 05:48 -0700，Richard Henderson写道： > On 06/05/2013 05:28 PM, li guang wrote: > > 在 2013-06-05三的 05:49 -0700，Richard Henderson写道： > >> On 06/04/2013 06:56 PM, li guang wrote: > >>>>> +typedef struct { > >>>>> + uint32_t len __attribute__((aligned((sizeof(void *))))); > >>>>> + uint32_t cie_offset; > >>>>> + tcg_target_long func_start __attribute__((packed)); > >>>>> + tcg_target_long func_len __attribute__((packed)); > >>> suspicious usage of packed attribute here, > >>> since tcg_targe_long is either 32 or 64 bits, > >>> not a struct or union. > >>> > >>> Thanks! > >>> > >> > >> Your question is worded poorly -- what has struct/union got to do with it? One > >> can adjust the alignment of any type. Perhaps you don't know what it is that > >> __attribute__((packed)) actually does? > > > > referred from gcc manual: > > "This attribute, attached to struct or union type definition, specifies > > that each member (other than zero-width bitfields) of the structure or > > union is placed to minimize the memory required." > > > > so, what I mean is as the manual said, this attribute mostly applies to > > struct or union. > > >From the gcc manual: > > @cindex @code{packed} attribute > The @code{packed} attribute specifies that a variable or structure field > should have the smallest possible alignment---one byte for a variable, > and one bit for a field, unless you specify a larger value with the > @code{aligned} attribute. > > Notice "or structure field", which is exactly what I have above. > > > forgive me if the question is fool, > > packed means seize minimal memory size, right? > > It really means minimal alignment. Which means that no padding will be added > to ensure alignment. Which can lead to a reduction in memory size, but that's > not the major point. > > right, Thanks!
diff --git a/tcg/arm/tcg-target.c b/tcg/arm/tcg-target.c index 3d43412..4a691b1 100644 --- a/tcg/arm/tcg-target.c +++ b/tcg/arm/tcg-target.c @@ -2100,23 +2100,31 @@ static inline void tcg_out_movi(TCGContext *s, TCGType type, tcg_out_movi32(s, COND_AL, ret, arg); } +/* Compute frame size via macros, to share between tcg_target_qemu_prologue + and tcg_register_jit. */ + +#define PUSH_SIZE ((11 - 4 + 1 + 1) * sizeof(tcg_target_long)) + +#define FRAME_SIZE \ + ((PUSH_SIZE \ + + TCG_STATIC_CALL_ARGS_SIZE \ + + CPU_TEMP_BUF_NLONGS * sizeof(long) \ + + TCG_TARGET_STACK_ALIGN - 1) \ + & -TCG_TARGET_STACK_ALIGN) + static void tcg_target_qemu_prologue(TCGContext *s) { - int frame_size; + int stack_addend; /* Calling convention requires us to save r4-r11 and lr. */ /* stmdb sp!, { r4 - r11, lr } */ tcg_out32(s, (COND_AL << 28) | 0x092d4ff0); - /* Allocate the local stack frame. */ - frame_size = TCG_STATIC_CALL_ARGS_SIZE; - frame_size += CPU_TEMP_BUF_NLONGS * sizeof(long); - /* We saved an odd number of registers above; keep an 8 aligned stack. */ - frame_size = ((frame_size + TCG_TARGET_STACK_ALIGN - 1) - & -TCG_TARGET_STACK_ALIGN) + 4; + /* Reserve callee argument and tcg temp space. */ + stack_addend = FRAME_SIZE - PUSH_SIZE; tcg_out_dat_rI(s, COND_AL, ARITH_SUB, TCG_REG_CALL_STACK, - TCG_REG_CALL_STACK, frame_size, 1); + TCG_REG_CALL_STACK, stack_addend, 1); tcg_set_frame(s, TCG_REG_CALL_STACK, TCG_STATIC_CALL_ARGS_SIZE, CPU_TEMP_BUF_NLONGS * sizeof(long)); @@ -2127,8 +2135,73 @@ static void tcg_target_qemu_prologue(TCGContext *s) /* Epilogue. We branch here via tb_ret_addr. */ tcg_out_dat_rI(s, COND_AL, ARITH_ADD, TCG_REG_CALL_STACK, - TCG_REG_CALL_STACK, frame_size, 1); + TCG_REG_CALL_STACK, stack_addend, 1); /* ldmia sp!, { r4 - r11, pc } */ tcg_out32(s, (COND_AL << 28) | 0x08bd8ff0); } + +typedef struct { + uint32_t len __attribute__((aligned((sizeof(void *))))); + uint32_t id; + uint8_t version; + char augmentation[1]; + uint8_t code_align; + uint8_t data_align; + uint8_t return_column; +} DebugFrameCIE; + +typedef struct { + uint32_t len __attribute__((aligned((sizeof(void *))))); + uint32_t cie_offset; + tcg_target_long func_start __attribute__((packed)); + tcg_target_long func_len __attribute__((packed)); + uint8_t def_cfa[4]; + uint8_t reg_ofs[18]; +} DebugFrameFDE; + +typedef struct { + DebugFrameCIE cie; + DebugFrameFDE fde; +} DebugFrame; + +#define ELF_HOST_MACHINE EM_ARM + +static DebugFrame debug_frame = { + .cie.len = sizeof(DebugFrameCIE)-4, /* length after .len member */ + .cie.id = -1, + .cie.version = 1, + .cie.code_align = 1, + .cie.data_align = 0x7c, /* sleb128 -4 */ + .cie.return_column = 14, + + .fde.len = sizeof(DebugFrameFDE)-4, /* length after .len member */ + .fde.def_cfa = { + 12, 13, /* DW_CFA_def_cfa sp, ... */ + (FRAME_SIZE & 0x7f) | 0x80, /* ... uleb128 FRAME_SIZE */ + (FRAME_SIZE >> 7) + }, + .fde.reg_ofs = { + /* The following must match the stmdb in the prologue. */ + 0x8e, 1, /* DW_CFA_offset, lr, -4 */ + 0x8b, 2, /* DW_CFA_offset, r11, -8 */ + 0x8a, 3, /* DW_CFA_offset, r10, -12 */ + 0x89, 4, /* DW_CFA_offset, r9, -16 */ + 0x88, 5, /* DW_CFA_offset, r8, -20 */ + 0x87, 6, /* DW_CFA_offset, r7, -24 */ + 0x86, 7, /* DW_CFA_offset, r6, -28 */ + 0x85, 8, /* DW_CFA_offset, r5, -32 */ + 0x84, 9, /* DW_CFA_offset, r4, -36 */ + } +}; + +void tcg_register_jit(void *buf, size_t buf_size) +{ + /* We're expecting a 2 byte uleb128 encoded value. */ + assert(FRAME_SIZE >> 14 == 0); + + debug_frame.fde.func_start = (tcg_target_long) buf; + debug_frame.fde.func_len = buf_size; + + tcg_register_jit_int(buf, buf_size, &debug_frame, sizeof(debug_frame)); +}
Allows unwinding past the code_gen_buffer. Signed-off-by: Richard Henderson <rth@twiddle.net> --- tcg/arm/tcg-target.c | 91 ++++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 82 insertions(+), 9 deletions(-)