Patchwork xtensa, hw: remove unnecessary include of dyngen-exec.h

login
register
mail settings
Submitter Peter Portante
Date April 25, 2012, 6:47 p.m.
Message ID <1335379678-27524-1-git-send-email-peter.portante@redhat.com>
Download mbox | patch
Permalink /patch/155059/
State New
Headers show

Comments

Peter Portante - April 25, 2012, 6:47 p.m.
Signed-off-by: Peter Portante <peter.portante@redhat.com>
---
 hw/spapr_hcall.c |    1 -
 xtensa-semi.c    |    1 -
 2 files changed, 0 insertions(+), 2 deletions(-)
Max Filippov - April 25, 2012, 8:07 p.m.
On 04/25/2012 10:47 PM, Peter Portante wrote:
> Signed-off-by: Peter Portante<peter.portante@redhat.com>
> ---
>   hw/spapr_hcall.c |    1 -
>   xtensa-semi.c    |    1 -
>   2 files changed, 0 insertions(+), 2 deletions(-)
>
> diff --git a/hw/spapr_hcall.c b/hw/spapr_hcall.c
> index 94bb504..88c1fab 100644
> --- a/hw/spapr_hcall.c
> +++ b/hw/spapr_hcall.c
> @@ -1,6 +1,5 @@
>   #include "sysemu.h"
>   #include "cpu.h"
> -#include "dyngen-exec.h"
>   #include "qemu-char.h"
>   #include "sysemu.h"
>   #include "qemu-char.h"
> diff --git a/xtensa-semi.c b/xtensa-semi.c
> index 5754b77..8c97a02 100644
> --- a/xtensa-semi.c
> +++ b/xtensa-semi.c
> @@ -30,7 +30,6 @@
>   #include<string.h>
>   #include<stddef.h>
>   #include "cpu.h"
> -#include "dyngen-exec.h"
>   #include "helpers.h"
>   #include "qemu-log.h"
>

Regarding xtensa part: it does not apply now that helpers.h is renamed to helper.h
Otherwise Acked-by: Max Filippov <jcmvbkbc@gmail.com>
Andreas Färber - April 25, 2012, 8:30 p.m.
Am 25.04.2012 20:47, schrieb Peter Portante:
> Signed-off-by: Peter Portante <peter.portante@redhat.com>
> ---
>  hw/spapr_hcall.c |    1 -
>  xtensa-semi.c    |    1 -
>  2 files changed, 0 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/spapr_hcall.c b/hw/spapr_hcall.c
> index 94bb504..88c1fab 100644
> --- a/hw/spapr_hcall.c
> +++ b/hw/spapr_hcall.c
> @@ -1,6 +1,5 @@
>  #include "sysemu.h"
>  #include "cpu.h"
> -#include "dyngen-exec.h"
>  #include "qemu-char.h"
>  #include "sysemu.h"
>  #include "qemu-char.h"

Careful! This part depends on http://patchwork.ozlabs.org/patch/154522/
getting applied first - I was surprised that you didn't send it as a
two-patch series alongside the "emv" v2. (cc'ing qemu-ppc)

> diff --git a/xtensa-semi.c b/xtensa-semi.c
> index 5754b77..8c97a02 100644
> --- a/xtensa-semi.c
> +++ b/xtensa-semi.c
> @@ -30,7 +30,6 @@
>  #include <string.h>
>  #include <stddef.h>
>  #include "cpu.h"
> -#include "dyngen-exec.h"
>  #include "helpers.h"
>  #include "qemu-log.h"
>  

This part is independent of the above. If you rebase it and split it off
into its own patch (e.g., git checkout -p) then Max can apply it to his
xtensa queue already.

Andreas
Peter Portante - April 25, 2012, 8:31 p.m.
On 04/25/2012 04:07 PM, Max Filippov wrote:
> On 04/25/2012 10:47 PM, Peter Portante wrote:
>> Signed-off-by: Peter Portante<peter.portante@redhat.com>
>> ---
>>   hw/spapr_hcall.c |    1 -
>>   xtensa-semi.c    |    1 -
>>   2 files changed, 0 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/spapr_hcall.c b/hw/spapr_hcall.c
>> index 94bb504..88c1fab 100644
>> --- a/hw/spapr_hcall.c
>> +++ b/hw/spapr_hcall.c
>> @@ -1,6 +1,5 @@
>>   #include "sysemu.h"
>>   #include "cpu.h"
>> -#include "dyngen-exec.h"
>>   #include "qemu-char.h"
>>   #include "sysemu.h"
>>   #include "qemu-char.h"
>> diff --git a/xtensa-semi.c b/xtensa-semi.c
>> index 5754b77..8c97a02 100644
>> --- a/xtensa-semi.c
>> +++ b/xtensa-semi.c
>> @@ -30,7 +30,6 @@
>>   #include<string.h>
>>   #include<stddef.h>
>>   #include "cpu.h"
>> -#include "dyngen-exec.h"
>>   #include "helpers.h"
>>   #include "qemu-log.h"
>>
>
> Regarding xtensa part: it does not apply now that helpers.h is renamed 
> to helper.h
Hi Max,

I am not entirely sure I understand what you wrote above. I am probably 
missing a subtlety above, so pardon me ahead of time.

For the file xtensa-semi.c, the references it makes to "env" are 
satisfied by the argument declaration. Looking through helper.h and 
def-helper.h, for completeness, I don't see any references to "env" 
either. So it appears that dyngen-exec.h does not need to be included 
(and in fact, it is born out by a successful compile).

Do I understand things correctly?

Thanks, -peter

> Otherwise Acked-by: Max Filippov <jcmvbkbc@gmail.com>
>
Peter Portante - April 25, 2012, 8:39 p.m.
On 04/25/2012 04:30 PM, Andreas Färber wrote:
> Am 25.04.2012 20:47, schrieb Peter Portante:
>> Signed-off-by: Peter Portante<peter.portante@redhat.com>
>> ---
>>   hw/spapr_hcall.c |    1 -
>>   xtensa-semi.c    |    1 -
>>   2 files changed, 0 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/spapr_hcall.c b/hw/spapr_hcall.c
>> index 94bb504..88c1fab 100644
>> --- a/hw/spapr_hcall.c
>> +++ b/hw/spapr_hcall.c
>> @@ -1,6 +1,5 @@
>>   #include "sysemu.h"
>>   #include "cpu.h"
>> -#include "dyngen-exec.h"
>>   #include "qemu-char.h"
>>   #include "sysemu.h"
>>   #include "qemu-char.h"
> Careful! This part depends on http://patchwork.ozlabs.org/patch/154522/
> getting applied first - I was surprised that you didn't send it as a
> two-patch series alongside the "emv" v2. (cc'ing qemu-ppc)
Ugh, thanks for pointing that out. I should have done that. I was too 
focused on the bug first.

I'll re-submit this first change once the above patch is in the tree.
>> diff --git a/xtensa-semi.c b/xtensa-semi.c
>> index 5754b77..8c97a02 100644
>> --- a/xtensa-semi.c
>> +++ b/xtensa-semi.c
>> @@ -30,7 +30,6 @@
>>   #include<string.h>
>>   #include<stddef.h>
>>   #include "cpu.h"
>> -#include "dyngen-exec.h"
>>   #include "helpers.h"
>>   #include "qemu-log.h"
>>
> This part is independent of the above. If you rebase it and split it off
> into its own patch (e.g., git checkout -p) then Max can apply it to his
> xtensa queue already.
Thanks, I'll do that.

-peter

>
> Andreas
>
Max Filippov - April 25, 2012, 8:56 p.m.
On 04/26/2012 12:31 AM, Peter Portante wrote:
> On 04/25/2012 04:07 PM, Max Filippov wrote:
>> On 04/25/2012 10:47 PM, Peter Portante wrote:
>>> Signed-off-by: Peter Portante<peter.portante@redhat.com>
>>> ---
>>> hw/spapr_hcall.c | 1 -
>>> xtensa-semi.c | 1 -
>>> 2 files changed, 0 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/hw/spapr_hcall.c b/hw/spapr_hcall.c
>>> index 94bb504..88c1fab 100644
>>> --- a/hw/spapr_hcall.c
>>> +++ b/hw/spapr_hcall.c
>>> @@ -1,6 +1,5 @@
>>> #include "sysemu.h"
>>> #include "cpu.h"
>>> -#include "dyngen-exec.h"
>>> #include "qemu-char.h"
>>> #include "sysemu.h"
>>> #include "qemu-char.h"
>>> diff --git a/xtensa-semi.c b/xtensa-semi.c
>>> index 5754b77..8c97a02 100644
>>> --- a/xtensa-semi.c
>>> +++ b/xtensa-semi.c
>>> @@ -30,7 +30,6 @@
>>> #include<string.h>
>>> #include<stddef.h>
>>> #include "cpu.h"
>>> -#include "dyngen-exec.h"
>>> #include "helpers.h"
>>> #include "qemu-log.h"
>>>
>>
>> Regarding xtensa part: it does not apply now that helpers.h is renamed to helper.h
> Hi Max,
>
> I am not entirely sure I understand what you wrote above. I am probably missing a subtlety above, so pardon me ahead of
> time.
>
> For the file xtensa-semi.c, the references it makes to "env" are satisfied by the argument declaration. Looking through
> helper.h and def-helper.h, for completeness, I don't see any references to "env" either. So it appears that
> dyngen-exec.h does not need to be included (and in fact, it is born out by a successful compile).
>
> Do I understand things correctly?

Yes, you're right and the change itself is fine, but the patch is based on the old version of xtensa-semi.c,
that had #include "helpers.h". It was renamed to "helper.h" in the commit 16c1deae215d4aac5b9b4fc090844b92852a0c5b.
Peter Portante - April 25, 2012, 9:16 p.m.
On 04/25/2012 04:56 PM, Max Filippov wrote:
> On 04/26/2012 12:31 AM, Peter Portante wrote:
>> On 04/25/2012 04:07 PM, Max Filippov wrote:
>>> On 04/25/2012 10:47 PM, Peter Portante wrote:
>>>> Signed-off-by: Peter Portante<peter.portante@redhat.com>
>>>> ---
>>>> hw/spapr_hcall.c | 1 -
>>>> xtensa-semi.c | 1 -
>>>> 2 files changed, 0 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/hw/spapr_hcall.c b/hw/spapr_hcall.c
>>>> index 94bb504..88c1fab 100644
>>>> --- a/hw/spapr_hcall.c
>>>> +++ b/hw/spapr_hcall.c
>>>> @@ -1,6 +1,5 @@
>>>> #include "sysemu.h"
>>>> #include "cpu.h"
>>>> -#include "dyngen-exec.h"
>>>> #include "qemu-char.h"
>>>> #include "sysemu.h"
>>>> #include "qemu-char.h"
>>>> diff --git a/xtensa-semi.c b/xtensa-semi.c
>>>> index 5754b77..8c97a02 100644
>>>> --- a/xtensa-semi.c
>>>> +++ b/xtensa-semi.c
>>>> @@ -30,7 +30,6 @@
>>>> #include<string.h>
>>>> #include<stddef.h>
>>>> #include "cpu.h"
>>>> -#include "dyngen-exec.h"
>>>> #include "helpers.h"
>>>> #include "qemu-log.h"
>>>>
>>>
>>> Regarding xtensa part: it does not apply now that helpers.h is 
>>> renamed to helper.h
>> Hi Max,
>>
>> I am not entirely sure I understand what you wrote above. I am 
>> probably missing a subtlety above, so pardon me ahead of
>> time.
>>
>> For the file xtensa-semi.c, the references it makes to "env" are 
>> satisfied by the argument declaration. Looking through
>> helper.h and def-helper.h, for completeness, I don't see any 
>> references to "env" either. So it appears that
>> dyngen-exec.h does not need to be included (and in fact, it is born 
>> out by a successful compile).
>>
>> Do I understand things correctly?
>
> Yes, you're right and the change itself is fine, but the patch is 
> based on the old version of xtensa-semi.c,
> that had #include "helpers.h". It was renamed to "helper.h" in the 
> commit 16c1deae215d4aac5b9b4fc090844b92852a0c5b.

I see that now. You meant that the patch does not apply without 
conflicts now, not that it no longer applies in the abstract.

I am going to resubmit the patch against your changes shortly, without 
the changes to hw/spapr_hcall.c.

Thanks, -peter

Patch

diff --git a/hw/spapr_hcall.c b/hw/spapr_hcall.c
index 94bb504..88c1fab 100644
--- a/hw/spapr_hcall.c
+++ b/hw/spapr_hcall.c
@@ -1,6 +1,5 @@ 
 #include "sysemu.h"
 #include "cpu.h"
-#include "dyngen-exec.h"
 #include "qemu-char.h"
 #include "sysemu.h"
 #include "qemu-char.h"
diff --git a/xtensa-semi.c b/xtensa-semi.c
index 5754b77..8c97a02 100644
--- a/xtensa-semi.c
+++ b/xtensa-semi.c
@@ -30,7 +30,6 @@ 
 #include <string.h>
 #include <stddef.h>
 #include "cpu.h"
-#include "dyngen-exec.h"
 #include "helpers.h"
 #include "qemu-log.h"