[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v5 00/10] tcg mips64 and mips r6 improvements
From: |
Richard Henderson |
Subject: |
Re: [Qemu-devel] [PATCH v5 00/10] tcg mips64 and mips r6 improvements |
Date: |
Mon, 5 Dec 2016 07:55:04 -0800 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 |
On 12/05/2016 01:41 AM, Jin Guojie wrote:
> ------------------ Original ------------------
> From: "Richard Henderson";<address@hidden>;
> Send time: Thursday, Dec 1, 2016 11:52 PM
> To: "Jin Guojie"<address@hidden>; "qemu-devel"<address@hidden>;
> Cc: "Aurelien Jarno"<address@hidden>; "James Hogan"<address@hidden>;
> Subject: Re: [PATCH v5 00/10] tcg mips64 and mips r6 improvements
>
> Thanks a lot for Richard's careful review.
> I have some different opinions for discussion:
>
>> @@ -1233,7 +1235,8 @@ static void tcg_out_tlb_load(TCGContext *s, TCGReg
>> base,
>> - mask = (target_ulong)TARGET_PAGE_MASK | ((1 << a_bits) - 1);
>> + mask = TARGET_PAGE_MASK | ((1 << a_bits) - 1);
>> You need the target_ulong cast here for mips64.
>> See patch ebb90a005da67147245cd38fb04a965a87a961b7.
>
> Since mask is defined as a C type "target_ulong" at the beginning of the
> function,
> I guess an implicit type cast should be already done by the compiler.
> So your change is functionally the same with v5 patch.
> To test my idea, I wrote a small case and compiled it on an x86_64 host:
>
> main()
> {
> int a_bits = 2;
> int page_mask = 0xfffff000; /* X86 4KB page*/
> unsigned int mask = page_mask | ((1 << a_bits) - 1);
> unsigned long m = mask;
> printf("mask=%lx\n", m);
> }
>
> $ gcc a.c
> $ file a.out
> a.out: Mach-O 64-bit executable x86_64
> $ ./a.out
> mask=fffff003
You misunderstand. For a 64-bit guest, the result we're looking for is
0xfffffffffffff003.
(1) the type of a_bits is unsigned int, not int,
(2) which means the expression "page_mask | ((1 << a_bits) - 1)"
becomes unsigned int,
(3) which means that the assignment to mask gets truncated.
> The output is exactly an unsigned 32-bit quantity.
> I didn't see a wrong result generated.
> Could you teach me where is my mistake?
For a 64-bit guest we need a 64-bit quantity. For a 32-bit guest... we need to
match the load that we issue from cmp_off. If use use LWU, then we need an
unsigned 32-bit quantity; if we use LW, then we need a signed 32-bit quantity.
r~
- [Qemu-devel] [PATCH v5 01/10] tcg-mips: Move bswap code to a subroutine, (continued)
- [Qemu-devel] [PATCH v5 01/10] tcg-mips: Move bswap code to a subroutine, Jin Guojie, 2016/12/01
- [Qemu-devel] [PATCH v5 05/10] tcg-mips: Adjust move functions for mips64, Jin Guojie, 2016/12/01
- [Qemu-devel] [PATCH v5 06/10] tcg-mips: Adjust load/store functions for mips64, Jin Guojie, 2016/12/01
- [Qemu-devel] [PATCH v5 07/10] tcg-mips: Adjust prologue for mips64, Jin Guojie, 2016/12/01
- [Qemu-devel] [PATCH v5 03/10] tcg-mips: Support 64-bit opcodes, Jin Guojie, 2016/12/01
- [Qemu-devel] [PATCH v5 08/10] tcg-mips: Add tcg unwind info, Jin Guojie, 2016/12/01
- [Qemu-devel] [PATCH v5 09/10] tcg-mips: Adjust calling conventions for mips64, Jin Guojie, 2016/12/01
- [Qemu-devel] [PATCH v5 10/10] tcg-mips: Adjust qemu_ld/st for mips64, Jin Guojie, 2016/12/01
- Re: [Qemu-devel] [PATCH v5 00/10] tcg mips64 and mips r6 improvements, Richard Henderson, 2016/12/01
- Re: [Qemu-devel] [PATCH v5 00/10] tcg mips64 and mips r6 improvements, James Hogan, 2016/12/02
- Re: [Qemu-devel] [PATCH v5 00/10] tcg mips64 and mips r6 improvements, Aurelien Jarno, 2016/12/02
- [Qemu-devel] [PATCH v5 00/10] tcg mips64 and mips r6 improvements, YunQiang Su, 2016/12/02