diff mbox series

[v4,12/12] tests/meson: Only build softfloat objects if TCG is selected

Message ID 20210415163304.4120052-13-philmd@redhat.com
State New
Headers show
Series qtests: Check accelerator available at runtime via QMP 'query-accels' | expand

Commit Message

Philippe Mathieu-Daudé April 15, 2021, 4:33 p.m. UTC
From: Philippe Mathieu-Daudé <f4bug@amsat.org>

The previous attempt (commit f77147cd4de) doesn't work as
expected, as we still have CONFIG_TCG=1 when using:

  configure --disable-system --disable-user

Now than we have removed the use of CONFIG_TCG from target-dependent
files in tests/qtest/, we can remove the unconditional definition of
CONFIG_TCG in config_host.

This avoid to build a bunch of unrequired objects when building with
--disable-tcg (in particular the softfloat tests):

Before:

  $ make
  [1/812] Generating trace-qom.h with a custom command
  ...

After:

  $ make
  [1/349] Generating trace-qom.h with a custom command
  ...

A difference of 463 objects...

Reported-by: Claudio Fontana <cfontana@suse.de>
Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
v3: Include Paolo's feedback:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg793872.html
therefore o not include Alex's R-b tag.

Cc: Richard Henderson <richard.henderson@linaro.org>
Cc: Alex Bennée <alex.bennee@linaro.org>
Cc: Emilio G. Cota <cota@braap.org>
---
 meson.build | 1 -
 1 file changed, 1 deletion(-)

Comments

Philippe Mathieu-Daudé April 28, 2021, 4:38 p.m. UTC | #1
Alex, Richard, do you mind reviewing this one please?

On 4/15/21 6:33 PM, Philippe Mathieu-Daudé wrote:
> From: Philippe Mathieu-Daudé <f4bug@amsat.org>
> 
> The previous attempt (commit f77147cd4de) doesn't work as
> expected, as we still have CONFIG_TCG=1 when using:
> 
>   configure --disable-system --disable-user
> 
> Now than we have removed the use of CONFIG_TCG from target-dependent
> files in tests/qtest/, we can remove the unconditional definition of
> CONFIG_TCG in config_host.
> 
> This avoid to build a bunch of unrequired objects when building with
> --disable-tcg (in particular the softfloat tests):
> 
> Before:
> 
>   $ make
>   [1/812] Generating trace-qom.h with a custom command
>   ...
> 
> After:
> 
>   $ make
>   [1/349] Generating trace-qom.h with a custom command
>   ...
> 
> A difference of 463 objects...
> 
> Reported-by: Claudio Fontana <cfontana@suse.de>
> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> v3: Include Paolo's feedback:
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg793872.html
> therefore o not include Alex's R-b tag.
> 
> Cc: Richard Henderson <richard.henderson@linaro.org>
> Cc: Alex Bennée <alex.bennee@linaro.org>
> Cc: Emilio G. Cota <cota@braap.org>
> ---
>  meson.build | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/meson.build b/meson.build
> index c6f4b0cf5e8..623cbe50685 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -262,7 +262,6 @@
>                          language: ['c', 'cpp', 'objc'])
>  
>    accelerators += 'CONFIG_TCG'
> -  config_host += { 'CONFIG_TCG': 'y' }
>  endif
>  
>  if 'CONFIG_KVM' not in accelerators and get_option('kvm').enabled()
>
Alex Bennée April 28, 2021, 5:06 p.m. UTC | #2
Philippe Mathieu-Daudé <philmd@redhat.com> writes:

> Alex, Richard, do you mind reviewing this one please?

Isn't it already merged (with my r-b tag no less ;-)

  f77147cd4de8c726f89b2702f7a9d0c9711d8875
  Author:     Philippe Mathieu-Daudé <philmd@redhat.com>
  AuthorDate: Fri Jan 22 21:44:31 2021 +0100
  Commit:     Paolo Bonzini <pbonzini@redhat.com>
  CommitDate: Mon Feb 8 14:43:55 2021 +0100

>
> On 4/15/21 6:33 PM, Philippe Mathieu-Daudé wrote:
>> From: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> 
>> The previous attempt (commit f77147cd4de) doesn't work as
>> expected, as we still have CONFIG_TCG=1 when using:
>> 
>>   configure --disable-system --disable-user
>> 
>> Now than we have removed the use of CONFIG_TCG from target-dependent
>> files in tests/qtest/, we can remove the unconditional definition of
>> CONFIG_TCG in config_host.
>> 
>> This avoid to build a bunch of unrequired objects when building with
>> --disable-tcg (in particular the softfloat tests):
>> 
>> Before:
>> 
>>   $ make
>>   [1/812] Generating trace-qom.h with a custom command
>>   ...
>> 
>> After:
>> 
>>   $ make
>>   [1/349] Generating trace-qom.h with a custom command
>>   ...
>> 
>> A difference of 463 objects...
>> 
>> Reported-by: Claudio Fontana <cfontana@suse.de>
>> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>> v3: Include Paolo's feedback:
>> https://www.mail-archive.com/qemu-devel@nongnu.org/msg793872.html
>> therefore o not include Alex's R-b tag.
>> 
>> Cc: Richard Henderson <richard.henderson@linaro.org>
>> Cc: Alex Bennée <alex.bennee@linaro.org>
>> Cc: Emilio G. Cota <cota@braap.org>
>> ---
>>  meson.build | 1 -
>>  1 file changed, 1 deletion(-)
>> 
>> diff --git a/meson.build b/meson.build
>> index c6f4b0cf5e8..623cbe50685 100644
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -262,7 +262,6 @@
>>                          language: ['c', 'cpp', 'objc'])
>>  
>>    accelerators += 'CONFIG_TCG'
>> -  config_host += { 'CONFIG_TCG': 'y' }
>>  endif
>>  
>>  if 'CONFIG_KVM' not in accelerators and get_option('kvm').enabled()
>>
Philippe Mathieu-Daudé April 28, 2021, 5:32 p.m. UTC | #3
On 4/28/21 7:06 PM, Alex Bennée wrote:
> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
> 
>> Alex, Richard, do you mind reviewing this one please?
> 
> Isn't it already merged (with my r-b tag no less ;-)
> 
>   f77147cd4de8c726f89b2702f7a9d0c9711d8875

See ...

>   Author:     Philippe Mathieu-Daudé <philmd@redhat.com>
>   AuthorDate: Fri Jan 22 21:44:31 2021 +0100
>   Commit:     Paolo Bonzini <pbonzini@redhat.com>
>   CommitDate: Mon Feb 8 14:43:55 2021 +0100
> 
>>
>> On 4/15/21 6:33 PM, Philippe Mathieu-Daudé wrote:
>>> From: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>>
>>> The previous attempt (commit f77147cd4de) doesn't work as

... ^ this comment :(

>>> expected, as we still have CONFIG_TCG=1 when using:
>>>
>>>   configure --disable-system --disable-user
>>>
>>> Now than we have removed the use of CONFIG_TCG from target-dependent
>>> files in tests/qtest/, we can remove the unconditional definition of
>>> CONFIG_TCG in config_host.
>>>
>>> This avoid to build a bunch of unrequired objects when building with
>>> --disable-tcg (in particular the softfloat tests):
>>>
>>> Before:
>>>
>>>   $ make
>>>   [1/812] Generating trace-qom.h with a custom command
>>>   ...
>>>
>>> After:
>>>
>>>   $ make
>>>   [1/349] Generating trace-qom.h with a custom command
>>>   ...
>>>
>>> A difference of 463 objects...
>>>
>>> Reported-by: Claudio Fontana <cfontana@suse.de>
>>> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>> ---
>>> v3: Include Paolo's feedback:
>>> https://www.mail-archive.com/qemu-devel@nongnu.org/msg793872.html
>>> therefore o not include Alex's R-b tag.
>>>
>>> Cc: Richard Henderson <richard.henderson@linaro.org>
>>> Cc: Alex Bennée <alex.bennee@linaro.org>
>>> Cc: Emilio G. Cota <cota@braap.org>
>>> ---
>>>  meson.build | 1 -
>>>  1 file changed, 1 deletion(-)
>>>
>>> diff --git a/meson.build b/meson.build
>>> index c6f4b0cf5e8..623cbe50685 100644
>>> --- a/meson.build
>>> +++ b/meson.build
>>> @@ -262,7 +262,6 @@
>>>                          language: ['c', 'cpp', 'objc'])
>>>  
>>>    accelerators += 'CONFIG_TCG'
>>> -  config_host += { 'CONFIG_TCG': 'y' }
>>>  endif
>>>  
>>>  if 'CONFIG_KVM' not in accelerators and get_option('kvm').enabled()
>>>
> 
>
Alex Bennée April 28, 2021, 7:04 p.m. UTC | #4
Philippe Mathieu-Daudé <philmd@redhat.com> writes:

> On 4/28/21 7:06 PM, Alex Bennée wrote:
>> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
>> 
>>> Alex, Richard, do you mind reviewing this one please?
>> 
>> Isn't it already merged (with my r-b tag no less ;-)
>> 
>>   f77147cd4de8c726f89b2702f7a9d0c9711d8875
>
> See ...
>
>>   Author:     Philippe Mathieu-Daudé <philmd@redhat.com>
>>   AuthorDate: Fri Jan 22 21:44:31 2021 +0100
>>   Commit:     Paolo Bonzini <pbonzini@redhat.com>
>>   CommitDate: Mon Feb 8 14:43:55 2021 +0100
>> 
>>>
>>> On 4/15/21 6:33 PM, Philippe Mathieu-Daudé wrote:
>>>> From: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>>>
>>>> The previous attempt (commit f77147cd4de) doesn't work as
>
> ... ^ this comment :(

Ahh - my tooling was confused having searched by the subject title ;-)

>
>>>> expected, as we still have CONFIG_TCG=1 when using:
>>>>
>>>>   configure --disable-system --disable-user
>>>>
>>>> Now than we have removed the use of CONFIG_TCG from target-dependent
>>>> files in tests/qtest/, we can remove the unconditional definition of
>>>> CONFIG_TCG in config_host.
>>>>
>>>> This avoid to build a bunch of unrequired objects when building with
>>>> --disable-tcg (in particular the softfloat tests):
>>>>
>>>> Before:
>>>>
>>>>   $ make
>>>>   [1/812] Generating trace-qom.h with a custom command
>>>>   ...
>>>>
>>>> After:
>>>>
>>>>   $ make
>>>>   [1/349] Generating trace-qom.h with a custom command
>>>>   ...
>>>>
>>>> A difference of 463 objects...
>>>>
>>>> Reported-by: Claudio Fontana <cfontana@suse.de>
>>>> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
>>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>>> ---
>>>> v3: Include Paolo's feedback:
>>>> https://www.mail-archive.com/qemu-devel@nongnu.org/msg793872.html
>>>> therefore o not include Alex's R-b tag.
>>>>
>>>> Cc: Richard Henderson <richard.henderson@linaro.org>
>>>> Cc: Alex Bennée <alex.bennee@linaro.org>
>>>> Cc: Emilio G. Cota <cota@braap.org>
>>>> ---
>>>>  meson.build | 1 -
>>>>  1 file changed, 1 deletion(-)
>>>>
>>>> diff --git a/meson.build b/meson.build
>>>> index c6f4b0cf5e8..623cbe50685 100644
>>>> --- a/meson.build
>>>> +++ b/meson.build
>>>> @@ -262,7 +262,6 @@
>>>>                          language: ['c', 'cpp', 'objc'])
>>>>  
>>>>    accelerators += 'CONFIG_TCG'
>>>> -  config_host += { 'CONFIG_TCG': 'y' }
>>>>  endif
>>>>  
>>>>  if 'CONFIG_KVM' not in accelerators and get_option('kvm').enabled()
>>>>
>> 
>>
Philippe Mathieu-Daudé April 28, 2021, 7:12 p.m. UTC | #5
On 4/28/21 9:04 PM, Alex Bennée wrote:
> 
> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
> 
>> On 4/28/21 7:06 PM, Alex Bennée wrote:
>>> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
>>>
>>>> Alex, Richard, do you mind reviewing this one please?
>>>
>>> Isn't it already merged (with my r-b tag no less ;-)
>>>
>>>   f77147cd4de8c726f89b2702f7a9d0c9711d8875
>>
>> See ...
>>
>>>   Author:     Philippe Mathieu-Daudé <philmd@redhat.com>
>>>   AuthorDate: Fri Jan 22 21:44:31 2021 +0100
>>>   Commit:     Paolo Bonzini <pbonzini@redhat.com>
>>>   CommitDate: Mon Feb 8 14:43:55 2021 +0100
>>>
>>>>
>>>> On 4/15/21 6:33 PM, Philippe Mathieu-Daudé wrote:
>>>>> From: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>>>>
>>>>> The previous attempt (commit f77147cd4de) doesn't work as
>>
>> ... ^ this comment :(
> 
> Ahh - my tooling was confused having searched by the subject title ;-)

Oh I see, I'll rename as:
"tests/meson: Only build softfloat objects if TCG is selected (again)".
Philippe Mathieu-Daudé April 29, 2021, 12:57 p.m. UTC | #6
On 4/28/21 9:12 PM, Philippe Mathieu-Daudé wrote:
> On 4/28/21 9:04 PM, Alex Bennée wrote:
>>
>> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
>>
>>> On 4/28/21 7:06 PM, Alex Bennée wrote:
>>>> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
>>>>
>>>>> Alex, Richard, do you mind reviewing this one please?
>>>>
>>>> Isn't it already merged (with my r-b tag no less ;-)
>>>>
>>>>   f77147cd4de8c726f89b2702f7a9d0c9711d8875
>>>
>>> See ...
>>>
>>>>   Author:     Philippe Mathieu-Daudé <philmd@redhat.com>
>>>>   AuthorDate: Fri Jan 22 21:44:31 2021 +0100
>>>>   Commit:     Paolo Bonzini <pbonzini@redhat.com>
>>>>   CommitDate: Mon Feb 8 14:43:55 2021 +0100
>>>>
>>>>>
>>>>> On 4/15/21 6:33 PM, Philippe Mathieu-Daudé wrote:
>>>>>> From: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>>>>>
>>>>>> The previous attempt (commit f77147cd4de) doesn't work as
>>>
>>> ... ^ this comment :(
>>
>> Ahh - my tooling was confused having searched by the subject title ;-)
> 
> Oh I see, I'll rename as:
> "tests/meson: Only build softfloat objects if TCG is selected (again)".

BTW you indeed reviewed/tested v1 of this patch:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg793900.html
but after addressing Paolo's comments I preferred to remove your tags.
Alex Bennée April 30, 2021, 4:37 p.m. UTC | #7
Philippe Mathieu-Daudé <philmd@redhat.com> writes:

> From: Philippe Mathieu-Daudé <f4bug@amsat.org>
>
> The previous attempt (commit f77147cd4de) doesn't work as
> expected, as we still have CONFIG_TCG=1 when using:
>
>   configure --disable-system --disable-user
>
> Now than we have removed the use of CONFIG_TCG from target-dependent
> files in tests/qtest/, we can remove the unconditional definition of
> CONFIG_TCG in config_host.
>
> This avoid to build a bunch of unrequired objects when building with
> --disable-tcg (in particular the softfloat tests):

Hmm something else gets broken because I bisected a check-tcg failure to
this commit:

  timeout --foreground 15  /home/alex.bennee/lsrc/qemu.git/builds/bisect/qemu-system-aarch64 -monitor none -display none -chardev file,path=memory-record.out,id=output -icount shift=5,rr=record,rrfile=record.bin  -M virt -cpu max -display none -semihosting-config enable=on,target=native,chardev=output -kernel memory
  qemu-system-aarch64: Please enable icount to use record/replay
  /home/alex.bennee/lsrc/qemu.git/tests/tcg/aarch64/Makefile.softmmu-target:49: recipe for target 'run-memory-record' failed
  make[2]: *** [run-memory-record] Error 1
  make[2]: Leaving directory '/home/alex.bennee/lsrc/qemu.git/builds/bisect/tests/tcg/aarch64-softmmu'
  /home/alex.bennee/lsrc/qemu.git/tests/tcg/Makefile.qemu:102: recipe for target 'run-guest-tests' failed
  make[1]: *** [run-guest-tests] Error 2
  make[1]: Leaving directory '/home/alex.bennee/lsrc/qemu.git/builds/bisect'
  /home/alex.bennee/lsrc/qemu.git/tests/Makefile.include:63: recipe for target 'run-tcg-tests-aarch64-softmmu' failed
  make: *** [run-tcg-tests-aarch64-softmmu] Error 2

So it looks like it breaks icount somehow.

>
> Before:
>
>   $ make
>   [1/812] Generating trace-qom.h with a custom command
>   ...
>
> After:
>
>   $ make
>   [1/349] Generating trace-qom.h with a custom command
>   ...
>
> A difference of 463 objects...
>
> Reported-by: Claudio Fontana <cfontana@suse.de>
> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> v3: Include Paolo's feedback:
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg793872.html
> therefore o not include Alex's R-b tag.
>
> Cc: Richard Henderson <richard.henderson@linaro.org>
> Cc: Alex Bennée <alex.bennee@linaro.org>
> Cc: Emilio G. Cota <cota@braap.org>
> ---
>  meson.build | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/meson.build b/meson.build
> index c6f4b0cf5e8..623cbe50685 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -262,7 +262,6 @@
>                          language: ['c', 'cpp', 'objc'])
>  
>    accelerators += 'CONFIG_TCG'
> -  config_host += { 'CONFIG_TCG': 'y' }
>  endif
>  
>  if 'CONFIG_KVM' not in accelerators and get_option('kvm').enabled()
diff mbox series

Patch

diff --git a/meson.build b/meson.build
index c6f4b0cf5e8..623cbe50685 100644
--- a/meson.build
+++ b/meson.build
@@ -262,7 +262,6 @@ 
                         language: ['c', 'cpp', 'objc'])
 
   accelerators += 'CONFIG_TCG'
-  config_host += { 'CONFIG_TCG': 'y' }
 endif
 
 if 'CONFIG_KVM' not in accelerators and get_option('kvm').enabled()