* [Qemu-devel] [PATCH] alpha: Fix build error for linux-user
@ 2016-06-16 18:56 Pranith Kumar
2016-06-16 19:07 ` Richard Henderson
0 siblings, 1 reply; 9+ messages in thread
From: Pranith Kumar @ 2016-06-16 18:56 UTC (permalink / raw)
To: Richard Henderson, open list:All patches CC here
Using gcc 6.1 for alpha-linux-user target we see the following build
error:
/mnt/devops/code/qemu/target-alpha/translate.c: In function ‘in_superpage’:
/mnt/devops/code/qemu/target-alpha/translate.c:454:52: error: self-comparison always evaluates to true [-Werror=tautological-compare]
&& addr >> TARGET_VIRT_ADDR_SPACE_BITS == addr >> 63);
Fix it by replacing (addr >> 63) by '1' which is what it evaluates to
since addr is negative.
Signed-off-by: Pranith Kumar <bobby.prani@gmail.com>
---
target-alpha/translate.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target-alpha/translate.c b/target-alpha/translate.c
index f9b2426..31da6ea 100644
--- a/target-alpha/translate.c
+++ b/target-alpha/translate.c
@@ -451,7 +451,7 @@ static bool in_superpage(DisasContext *ctx, int64_t addr)
return ((ctx->tb->flags & TB_FLAGS_USER_MODE) == 0
&& addr < 0
&& ((addr >> 41) & 3) == 2
- && addr >> TARGET_VIRT_ADDR_SPACE_BITS == addr >> 63);
+ && addr >> TARGET_VIRT_ADDR_SPACE_BITS == 1);
}
static bool use_goto_tb(DisasContext *ctx, uint64_t dest)
--
2.9.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [Qemu-devel] [PATCH] alpha: Fix build error for linux-user
2016-06-16 18:56 [Qemu-devel] [PATCH] alpha: Fix build error for linux-user Pranith Kumar
@ 2016-06-16 19:07 ` Richard Henderson
2016-06-16 19:15 ` Pranith Kumar
2016-06-17 18:04 ` Paolo Bonzini
0 siblings, 2 replies; 9+ messages in thread
From: Richard Henderson @ 2016-06-16 19:07 UTC (permalink / raw)
To: Pranith Kumar, open list:All patches CC here
On 06/16/2016 11:56 AM, Pranith Kumar wrote:
> Using gcc 6.1 for alpha-linux-user target we see the following build
> error:
>
> /mnt/devops/code/qemu/target-alpha/translate.c: In function ‘in_superpage’:
> /mnt/devops/code/qemu/target-alpha/translate.c:454:52: error: self-comparison always evaluates to true [-Werror=tautological-compare]
> && addr >> TARGET_VIRT_ADDR_SPACE_BITS == addr >> 63);
>
> Fix it by replacing (addr >> 63) by '1' which is what it evaluates to
> since addr is negative.
>
> Signed-off-by: Pranith Kumar <bobby.prani@gmail.com>
> ---
> target-alpha/translate.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target-alpha/translate.c b/target-alpha/translate.c
> index f9b2426..31da6ea 100644
> --- a/target-alpha/translate.c
> +++ b/target-alpha/translate.c
> @@ -451,7 +451,7 @@ static bool in_superpage(DisasContext *ctx, int64_t addr)
> return ((ctx->tb->flags & TB_FLAGS_USER_MODE) == 0
> && addr < 0
> && ((addr >> 41) & 3) == 2
> - && addr >> TARGET_VIRT_ADDR_SPACE_BITS == addr >> 63);
> + && addr >> TARGET_VIRT_ADDR_SPACE_BITS == 1);
> }
This fix is incorrect.
(1) addr is not always negative.
(2) in_superpage is only relevant for alpha-softmmu, where
TARGET_VIRT_ADDR_SPACE_BITS is not 63.
r~
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [Qemu-devel] [PATCH] alpha: Fix build error for linux-user
2016-06-16 19:07 ` Richard Henderson
@ 2016-06-16 19:15 ` Pranith Kumar
2016-06-17 0:43 ` Laurent Vivier
2016-06-17 18:04 ` Paolo Bonzini
1 sibling, 1 reply; 9+ messages in thread
From: Pranith Kumar @ 2016-06-16 19:15 UTC (permalink / raw)
To: Richard Henderson; +Cc: open list:All patches CC here
On Thu, Jun 16, 2016 at 3:07 PM, Richard Henderson <rth@twiddle.net> wrote:
> On 06/16/2016 11:56 AM, Pranith Kumar wrote:
>> Using gcc 6.1 for alpha-linux-user target we see the following build
>> error:
>>
>> /mnt/devops/code/qemu/target-alpha/translate.c: In function ‘in_superpage’:
>> /mnt/devops/code/qemu/target-alpha/translate.c:454:52: error: self-comparison always evaluates to true [-Werror=tautological-compare]
>> && addr >> TARGET_VIRT_ADDR_SPACE_BITS == addr >> 63);
>>
>> Fix it by replacing (addr >> 63) by '1' which is what it evaluates to
>> since addr is negative.
>>
>> Signed-off-by: Pranith Kumar <bobby.prani@gmail.com>
>> ---
>> target-alpha/translate.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/target-alpha/translate.c b/target-alpha/translate.c
>> index f9b2426..31da6ea 100644
>> --- a/target-alpha/translate.c
>> +++ b/target-alpha/translate.c
>> @@ -451,7 +451,7 @@ static bool in_superpage(DisasContext *ctx, int64_t addr)
>> return ((ctx->tb->flags & TB_FLAGS_USER_MODE) == 0
>> && addr < 0
>> && ((addr >> 41) & 3) == 2
>> - && addr >> TARGET_VIRT_ADDR_SPACE_BITS == addr >> 63);
>> + && addr >> TARGET_VIRT_ADDR_SPACE_BITS == 1);
>> }
>
> This fix is incorrect.
>
> (1) addr is not always negative.
> (2) in_superpage is only relevant for alpha-softmmu, where
> TARGET_VIRT_ADDR_SPACE_BITS is not 63.
If you see line 2 of the condition you check for (addr < 0). Only if
this evaluates to true do you check the right shift value of addr.
Reg. (2), I think that is what gcc is erroring out for.
TARGET_VIRT_ADDR_SPACE_BITS is 63 for linux-user and we are comparing
(addr >> 63) with itself. GCC rightly says that it is a tautological
compare and errors out. May be we can have a better work around.
Thanks!
--
Pranith
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH] alpha: Fix build error for linux-user
2016-06-16 19:15 ` Pranith Kumar
@ 2016-06-17 0:43 ` Laurent Vivier
2016-06-17 17:20 ` Pranith Kumar
0 siblings, 1 reply; 9+ messages in thread
From: Laurent Vivier @ 2016-06-17 0:43 UTC (permalink / raw)
To: Pranith Kumar, Richard Henderson; +Cc: open list:All patches CC here
Le 16/06/2016 à 21:15, Pranith Kumar a écrit :
> On Thu, Jun 16, 2016 at 3:07 PM, Richard Henderson <rth@twiddle.net> wrote:
>> On 06/16/2016 11:56 AM, Pranith Kumar wrote:
>>> Using gcc 6.1 for alpha-linux-user target we see the following build
>>> error:
>>>
>>> /mnt/devops/code/qemu/target-alpha/translate.c: In function ‘in_superpage’:
>>> /mnt/devops/code/qemu/target-alpha/translate.c:454:52: error: self-comparison always evaluates to true [-Werror=tautological-compare]
>>> && addr >> TARGET_VIRT_ADDR_SPACE_BITS == addr >> 63);
>>>
>>> Fix it by replacing (addr >> 63) by '1' which is what it evaluates to
>>> since addr is negative.
>>>
>>> Signed-off-by: Pranith Kumar <bobby.prani@gmail.com>
>>> ---
>>> target-alpha/translate.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/target-alpha/translate.c b/target-alpha/translate.c
>>> index f9b2426..31da6ea 100644
>>> --- a/target-alpha/translate.c
>>> +++ b/target-alpha/translate.c
>>> @@ -451,7 +451,7 @@ static bool in_superpage(DisasContext *ctx, int64_t addr)
>>> return ((ctx->tb->flags & TB_FLAGS_USER_MODE) == 0
>>> && addr < 0
>>> && ((addr >> 41) & 3) == 2
>>> - && addr >> TARGET_VIRT_ADDR_SPACE_BITS == addr >> 63);
>>> + && addr >> TARGET_VIRT_ADDR_SPACE_BITS == 1);
>>> }
>>
>> This fix is incorrect.
>>
>> (1) addr is not always negative.
>> (2) in_superpage is only relevant for alpha-softmmu, where
>> TARGET_VIRT_ADDR_SPACE_BITS is not 63.
>
> If you see line 2 of the condition you check for (addr < 0). Only if
> this evaluates to true do you check the right shift value of addr.
>
> Reg. (2), I think that is what gcc is erroring out for.
> TARGET_VIRT_ADDR_SPACE_BITS is 63 for linux-user and we are comparing
> (addr >> 63) with itself. GCC rightly says that it is a tautological
> compare and errors out. May be we can have a better work around.
Perhaps something like:
#ifndef CONFIG_USER_ONLY
static bool in_superpage(DisasContext *ctx, int64_t addr)
{
...
}
#endif
...
#ifdef CONFIG_USER_ONLY
pc_mask = ~TARGET_PAGE_MASK;
#else
if (in_superpage(&ctx, pc_start)) {
pc_mask = (1ULL << 41) - 1;
} else {
pc_mask = ~TARGET_PAGE_MASK;
}
#endif
Laurent
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [Qemu-devel] [PATCH] alpha: Fix build error for linux-user
2016-06-17 0:43 ` Laurent Vivier
@ 2016-06-17 17:20 ` Pranith Kumar
0 siblings, 0 replies; 9+ messages in thread
From: Pranith Kumar @ 2016-06-17 17:20 UTC (permalink / raw)
To: Laurent Vivier; +Cc: Richard Henderson, open list:All patches CC here
On Thu, Jun 16, 2016 at 8:43 PM, Laurent Vivier <laurent@vivier.eu> wrote:
>
>
> Le 16/06/2016 à 21:15, Pranith Kumar a écrit :
>> On Thu, Jun 16, 2016 at 3:07 PM, Richard Henderson <rth@twiddle.net> wrote:
>>> On 06/16/2016 11:56 AM, Pranith Kumar wrote:
>>>> Using gcc 6.1 for alpha-linux-user target we see the following build
>>>> error:
>>>>
>>>> /mnt/devops/code/qemu/target-alpha/translate.c: In function ‘in_superpage’:
>>>> /mnt/devops/code/qemu/target-alpha/translate.c:454:52: error: self-comparison always evaluates to true [-Werror=tautological-compare]
>>>> && addr >> TARGET_VIRT_ADDR_SPACE_BITS == addr >> 63);
>>>>
>>>> Fix it by replacing (addr >> 63) by '1' which is what it evaluates to
>>>> since addr is negative.
>>>>
>>>> Signed-off-by: Pranith Kumar <bobby.prani@gmail.com>
>>>> ---
>>>> target-alpha/translate.c | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/target-alpha/translate.c b/target-alpha/translate.c
>>>> index f9b2426..31da6ea 100644
>>>> --- a/target-alpha/translate.c
>>>> +++ b/target-alpha/translate.c
>>>> @@ -451,7 +451,7 @@ static bool in_superpage(DisasContext *ctx, int64_t addr)
>>>> return ((ctx->tb->flags & TB_FLAGS_USER_MODE) == 0
>>>> && addr < 0
>>>> && ((addr >> 41) & 3) == 2
>>>> - && addr >> TARGET_VIRT_ADDR_SPACE_BITS == addr >> 63);
>>>> + && addr >> TARGET_VIRT_ADDR_SPACE_BITS == 1);
>>>> }
>>>
>>> This fix is incorrect.
>>>
>>> (1) addr is not always negative.
>>> (2) in_superpage is only relevant for alpha-softmmu, where
>>> TARGET_VIRT_ADDR_SPACE_BITS is not 63.
>>
>> If you see line 2 of the condition you check for (addr < 0). Only if
>> this evaluates to true do you check the right shift value of addr.
>>
>> Reg. (2), I think that is what gcc is erroring out for.
>> TARGET_VIRT_ADDR_SPACE_BITS is 63 for linux-user and we are comparing
>> (addr >> 63) with itself. GCC rightly says that it is a tautological
>> compare and errors out. May be we can have a better work around.
>
> Perhaps something like:
>
> #ifndef CONFIG_USER_ONLY
> static bool in_superpage(DisasContext *ctx, int64_t addr)
> {
> ...
> }
> #endif
> ...
>
> #ifdef CONFIG_USER_ONLY
> pc_mask = ~TARGET_PAGE_MASK;
> #else
> if (in_superpage(&ctx, pc_start)) {
> pc_mask = (1ULL << 41) - 1;
> } else {
> pc_mask = ~TARGET_PAGE_MASK;
> }
> #endif
>
OK, I will use this.
Thanks!
--
Pranith
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH] alpha: Fix build error for linux-user
2016-06-16 19:07 ` Richard Henderson
2016-06-16 19:15 ` Pranith Kumar
@ 2016-06-17 18:04 ` Paolo Bonzini
2016-06-17 18:07 ` Pranith Kumar
1 sibling, 1 reply; 9+ messages in thread
From: Paolo Bonzini @ 2016-06-17 18:04 UTC (permalink / raw)
To: Richard Henderson, Pranith Kumar, qemu-devel
On 16/06/2016 21:07, Richard Henderson wrote:
>> && ((addr >> 41) & 3) == 2
>> - && addr >> TARGET_VIRT_ADDR_SPACE_BITS == addr >> 63);
>> + && addr >> TARGET_VIRT_ADDR_SPACE_BITS == 1);
What you want here is
+ addr >> TARGET_VIRT_ADDR_SPACE_BITS == -1
since that's what addr >> 63 is. With this change the patch should be fine.
Alternatively, combining this condition with addr < 0 should be
(uint64_t)addr >= (-1ull << TARGET_VIRT_ADDR_SPACE_BITS)
I think (I might have an off by one here, sorry). If you go this way,
it's of course nicer to change the argument type to uint64_t.
Paolo
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH] alpha: Fix build error for linux-user
2016-06-17 18:04 ` Paolo Bonzini
@ 2016-06-17 18:07 ` Pranith Kumar
2016-06-17 18:09 ` Richard Henderson
0 siblings, 1 reply; 9+ messages in thread
From: Pranith Kumar @ 2016-06-17 18:07 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Richard Henderson, qemu-devel
On Fri, Jun 17, 2016 at 2:04 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
>
> On 16/06/2016 21:07, Richard Henderson wrote:
>>> && ((addr >> 41) & 3) == 2
>>> - && addr >> TARGET_VIRT_ADDR_SPACE_BITS == addr >> 63);
>>> + && addr >> TARGET_VIRT_ADDR_SPACE_BITS == 1);
>
> What you want here is
>
> + addr >> TARGET_VIRT_ADDR_SPACE_BITS == -1
>
> since that's what addr >> 63 is. With this change the patch should be fine.
Isn't (addr >> 63) supposed to be 1? How can it be -1?
--
Pranith
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH] alpha: Fix build error for linux-user
2016-06-17 18:07 ` Pranith Kumar
@ 2016-06-17 18:09 ` Richard Henderson
2016-06-17 18:12 ` Pranith Kumar
0 siblings, 1 reply; 9+ messages in thread
From: Richard Henderson @ 2016-06-17 18:09 UTC (permalink / raw)
To: Pranith Kumar, Paolo Bonzini; +Cc: qemu-devel
On 06/17/2016 11:07 AM, Pranith Kumar wrote:
> On Fri, Jun 17, 2016 at 2:04 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>
>>
>> On 16/06/2016 21:07, Richard Henderson wrote:
>>>> && ((addr >> 41) & 3) == 2
>>>> - && addr >> TARGET_VIRT_ADDR_SPACE_BITS == addr >> 63);
>>>> + && addr >> TARGET_VIRT_ADDR_SPACE_BITS == 1);
>>
>> What you want here is
>>
>> + addr >> TARGET_VIRT_ADDR_SPACE_BITS == -1
>>
>> since that's what addr >> 63 is. With this change the patch should be fine.
>
> Isn't (addr >> 63) supposed to be 1? How can it be -1?
Signed right shift.
I'll prepare a patch for all of this.
r~
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH] alpha: Fix build error for linux-user
2016-06-17 18:09 ` Richard Henderson
@ 2016-06-17 18:12 ` Pranith Kumar
0 siblings, 0 replies; 9+ messages in thread
From: Pranith Kumar @ 2016-06-17 18:12 UTC (permalink / raw)
To: Richard Henderson; +Cc: Paolo Bonzini, qemu-devel
On Fri, Jun 17, 2016 at 2:09 PM, Richard Henderson <rth@twiddle.net> wrote:
> On 06/17/2016 11:07 AM, Pranith Kumar wrote:
>> On Fri, Jun 17, 2016 at 2:04 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>>
>>>
>>> On 16/06/2016 21:07, Richard Henderson wrote:
>>>>> && ((addr >> 41) & 3) == 2
>>>>> - && addr >> TARGET_VIRT_ADDR_SPACE_BITS == addr >> 63);
>>>>> + && addr >> TARGET_VIRT_ADDR_SPACE_BITS == 1);
>>>
>>> What you want here is
>>>
>>> + addr >> TARGET_VIRT_ADDR_SPACE_BITS == -1
>>>
>>> since that's what addr >> 63 is. With this change the patch should be fine.
>>
>> Isn't (addr >> 63) supposed to be 1? How can it be -1?
>
> Signed right shift.
Ah, got it. :)
>
> I'll prepare a patch for all of this.
Thanks!
--
Pranith
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2016-06-17 18:12 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-06-16 18:56 [Qemu-devel] [PATCH] alpha: Fix build error for linux-user Pranith Kumar
2016-06-16 19:07 ` Richard Henderson
2016-06-16 19:15 ` Pranith Kumar
2016-06-17 0:43 ` Laurent Vivier
2016-06-17 17:20 ` Pranith Kumar
2016-06-17 18:04 ` Paolo Bonzini
2016-06-17 18:07 ` Pranith Kumar
2016-06-17 18:09 ` Richard Henderson
2016-06-17 18:12 ` Pranith Kumar
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).