[Qemu-devel] [PATCH 1/2] tcg-arm: Implement tcg_register_jit

[Qemu-devel] [PATCH 1/2] tcg-arm: Implement tcg_register_jit

[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);
 }
 
+/* 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));
+}
-- 
1.8.1.4

[Qemu-devel] [PATCH 2/2] tcg: Fix high_pc fields in .debug_info

[Richard Henderson]

I don't think the debugger actually looks at this for anything,
using the correct .debug_frame contents, but might as well get
it all correct.

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 tcg/tcg.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tcg/tcg.c b/tcg/tcg.c
index 1d8099c..8ea43b3 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -2659,9 +2659,9 @@ static void tcg_register_jit_int(void *buf_ptr, size_t buf_size,
     img->sym[1].st_size = buf_size;
 
     img->di.cu_low_pc = buf;
-    img->di.cu_high_pc = buf_size;
+    img->di.cu_high_pc = buf + buf_size;
     img->di.fn_low_pc = buf;
-    img->di.fn_high_pc = buf_size;
+    img->di.fn_high_pc = buf + buf_size;
 
 #ifdef DEBUG_JIT
     /* Enable this block to be able to debug the ELF image file creation.
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH 1/2] tcg-arm: Implement tcg_register_jit

[Richard Henderson]

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));
> +}
> 

Re: [Qemu-devel] [PATCH 1/2] tcg-arm: Implement tcg_register_jit

[li guang]

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));
> +}

Re: [Qemu-devel] [PATCH 1/2] tcg-arm: Implement tcg_register_jit

[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?

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~

Re: [Qemu-devel] [PATCH 1/2] tcg-arm: Implement tcg_register_jit

[Andreas Färber]

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~

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH 1/2] tcg-arm: Implement tcg_register_jit

[Richard Henderson]

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~

Re: [Qemu-devel] [PATCH 1/2] tcg-arm: Implement tcg_register_jit

[Peter Maydell]

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

Re: [Qemu-devel] [PATCH 1/2] tcg-arm: Implement tcg_register_jit

[Richard Henderson]

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~

Re: [Qemu-devel] [PATCH 1/2] tcg-arm: Implement tcg_register_jit

[li guang]

在 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~

Re: [Qemu-devel] [PATCH 1/2] tcg-arm: Implement tcg_register_jit

[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.


r~

Re: [Qemu-devel] [PATCH 1/2] tcg-arm: Implement tcg_register_jit

[li guang]

在 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!