[Qemu-devel] [PATCH RFC 1/2] tests/tcg: Allow targets to set the optimization level

David Hildenbrand posted 2 patches 6 years, 8 months ago
Maintainers: David Hildenbrand <david@redhat.com>, Cornelia Huck <cohuck@redhat.com>, "Alex Bennée" <alex.bennee@linaro.org>, "Philippe Mathieu-Daudé" <f4bug@amsat.org>, Richard Henderson <rth@twiddle.net>
[Qemu-devel] [PATCH RFC 1/2] tests/tcg: Allow targets to set the optimization level
Posted by David Hildenbrand 6 years, 8 months ago
We sometimes want basic optimizations, e.g. for constant propagation
into inlined functions.

Especially for inline asm with immediates;

static inline void some_instr(uint8_t imm)
{
    asm volatile("...", :: "i" (imm));
}

static void test()
{
    some_instr(1);
}

Which is impossible with -O0. So make -O0 the default but let
targets override it.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 tests/tcg/Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/tcg/Makefile b/tests/tcg/Makefile
index bf06415390..bd0bace0d5 100644
--- a/tests/tcg/Makefile
+++ b/tests/tcg/Makefile
@@ -47,7 +47,7 @@ skip-test = @printf "  SKIPPED %s on $(TARGET_NAME) because %s\n" $1 $2
 TESTS=
 
 # Start with a blank slate, the build targets get to add stuff first
-CFLAGS=
+CFLAGS=-O0
 QEMU_CFLAGS=
 LDFLAGS=
 
@@ -70,7 +70,7 @@ ifneq ($(TARGET_BASE_ARCH),$(TARGET_NAME))
 endif
 
 # Add the common build options
-CFLAGS+=-Wall -O0 -g -fno-strict-aliasing
+CFLAGS+=-Wall -g -fno-strict-aliasing
 ifeq ($(BUILD_STATIC),y)
 LDFLAGS+=-static
 endif
-- 
2.17.2


Re: [Qemu-devel] [PATCH RFC 1/2] tests/tcg: Allow targets to set the optimization level
Posted by Richard Henderson 6 years, 8 months ago
On 2/27/19 3:14 AM, David Hildenbrand wrote:
> Especially for inline asm with immediates;
> 
> static inline void some_instr(uint8_t imm)
> {
>     asm volatile("...", :: "i" (imm));
> }
> 
> static void test()
> {
>     some_instr(1);
> }

FWIW, __attribute__((always_inline)) should work even with -O0.


r~

Re: [Qemu-devel] [PATCH RFC 1/2] tests/tcg: Allow targets to set the optimization level
Posted by Alex Bennée 6 years, 8 months ago
David Hildenbrand <david@redhat.com> writes:

> We sometimes want basic optimizations, e.g. for constant propagation
> into inlined functions.
>
> Especially for inline asm with immediates;

I agree you need this, but...

<snip>
>  # Start with a blank slate, the build targets get to add stuff first
> -CFLAGS=
> +CFLAGS=-O0
>  QEMU_CFLAGS=
>  LDFLAGS=
>
> @@ -70,7 +70,7 @@ ifneq ($(TARGET_BASE_ARCH),$(TARGET_NAME))
>  endif
>
>  # Add the common build options
> -CFLAGS+=-Wall -O0 -g -fno-strict-aliasing
> +CFLAGS+=-Wall -g -fno-strict-aliasing


You don't need to do this - we already have EXTRA_CFLAGS which will
always be at the end of the compile invocation:

  # enable vectors and optimisation to vectorise for this test
  vglv: CFLAGS+=-march=z13 -m64
  vglv: EXTRA_CFLAGS+=-O2


>  ifeq ($(BUILD_STATIC),y)
>  LDFLAGS+=-static
>  endif


--
Alex Bennée

Re: [Qemu-devel] [PATCH RFC 1/2] tests/tcg: Allow targets to set the optimization level
Posted by David Hildenbrand 6 years, 8 months ago
On 27.02.19 12:46, Alex Bennée wrote:
> 
> David Hildenbrand <david@redhat.com> writes:
> 
>> We sometimes want basic optimizations, e.g. for constant propagation
>> into inlined functions.
>>
>> Especially for inline asm with immediates;
> 
> I agree you need this, but...
> 
> <snip>
>>  # Start with a blank slate, the build targets get to add stuff first
>> -CFLAGS=
>> +CFLAGS=-O0
>>  QEMU_CFLAGS=
>>  LDFLAGS=
>>
>> @@ -70,7 +70,7 @@ ifneq ($(TARGET_BASE_ARCH),$(TARGET_NAME))
>>  endif
>>
>>  # Add the common build options
>> -CFLAGS+=-Wall -O0 -g -fno-strict-aliasing
>> +CFLAGS+=-Wall -g -fno-strict-aliasing
> 
> 
> You don't need to do this - we already have EXTRA_CFLAGS which will
> always be at the end of the compile invocation:
> 
>   # enable vectors and optimisation to vectorise for this test
>   vglv: CFLAGS+=-march=z13 -m64
>   vglv: EXTRA_CFLAGS+=-O2
> 

Cool, missed that - thanks!


-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH RFC 1/2] tests/tcg: Allow targets to set the optimization level
Posted by David Hildenbrand 6 years, 8 months ago
On 27.02.19 12:46, Alex Bennée wrote:
> 
> David Hildenbrand <david@redhat.com> writes:
> 
>> We sometimes want basic optimizations, e.g. for constant propagation
>> into inlined functions.
>>
>> Especially for inline asm with immediates;
> 
> I agree you need this, but...
> 
> <snip>
>>  # Start with a blank slate, the build targets get to add stuff first
>> -CFLAGS=
>> +CFLAGS=-O0
>>  QEMU_CFLAGS=
>>  LDFLAGS=
>>
>> @@ -70,7 +70,7 @@ ifneq ($(TARGET_BASE_ARCH),$(TARGET_NAME))
>>  endif
>>
>>  # Add the common build options
>> -CFLAGS+=-Wall -O0 -g -fno-strict-aliasing
>> +CFLAGS+=-Wall -g -fno-strict-aliasing
> 
> 
> You don't need to do this - we already have EXTRA_CFLAGS which will
> always be at the end of the compile invocation:
> 
>   # enable vectors and optimisation to vectorise for this test
>   vglv: CFLAGS+=-march=z13 -m64
>   vglv: EXTRA_CFLAGS+=-O2

For whatever reason I can't get this to work. EXTRA_CFLAGS+=-O2 is
always silently dropped:

make SHELL='sh -x' run-tcg-tests-s390x-linux-user

+ /home/dhildenb/git/qemu/tests/docker/docker.py cc --user 100813 --cc
s390x-linux-gnu-gcc -i qemu:debian-s390x-cross -s
/home/dhildenb/git/qemu -- -march=zEC12 -m64 -Wall -O0 -g
-fno-strict-aliasing -march=z13
/home/dhildenb/git/qemu/tests/tcg/s390x/vlgv.c -o vlgv -static
/home/dhildenb/git/qemu/tests/tcg/s390x/vlgv.c: In function 'vlgv':
/home/dhildenb/git/qemu/tests/tcg/s390x/vlgv.c:8:5: warning: asm operand
3 probably doesn't match constraints
     asm volatile("vlgv %[r1], %[v3], 0(%[a2]), %[m4]\n"
     ^~~
/home/dhildenb/git/qemu/tests/tcg/s390x/vlgv.c:8:5: error: impossible
constraint in 'asm'



-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH RFC 1/2] tests/tcg: Allow targets to set the optimization level
Posted by David Hildenbrand 6 years, 8 months ago
On 27.02.19 13:42, David Hildenbrand wrote:
> On 27.02.19 12:46, Alex Bennée wrote:
>>
>> David Hildenbrand <david@redhat.com> writes:
>>
>>> We sometimes want basic optimizations, e.g. for constant propagation
>>> into inlined functions.
>>>
>>> Especially for inline asm with immediates;
>>
>> I agree you need this, but...
>>
>> <snip>
>>>  # Start with a blank slate, the build targets get to add stuff first
>>> -CFLAGS=
>>> +CFLAGS=-O0
>>>  QEMU_CFLAGS=
>>>  LDFLAGS=
>>>
>>> @@ -70,7 +70,7 @@ ifneq ($(TARGET_BASE_ARCH),$(TARGET_NAME))
>>>  endif
>>>
>>>  # Add the common build options
>>> -CFLAGS+=-Wall -O0 -g -fno-strict-aliasing
>>> +CFLAGS+=-Wall -g -fno-strict-aliasing
>>
>>
>> You don't need to do this - we already have EXTRA_CFLAGS which will
>> always be at the end of the compile invocation:
>>
>>   # enable vectors and optimisation to vectorise for this test
>>   vglv: CFLAGS+=-march=z13 -m64
>>   vglv: EXTRA_CFLAGS+=-O2
> 
> For whatever reason I can't get this to work. EXTRA_CFLAGS+=-O2 is
> always silently dropped:
> 
> make SHELL='sh -x' run-tcg-tests-s390x-linux-user
> 
> + /home/dhildenb/git/qemu/tests/docker/docker.py cc --user 100813 --cc
> s390x-linux-gnu-gcc -i qemu:debian-s390x-cross -s
> /home/dhildenb/git/qemu -- -march=zEC12 -m64 -Wall -O0 -g
> -fno-strict-aliasing -march=z13
> /home/dhildenb/git/qemu/tests/tcg/s390x/vlgv.c -o vlgv -static
> /home/dhildenb/git/qemu/tests/tcg/s390x/vlgv.c: In function 'vlgv':
> /home/dhildenb/git/qemu/tests/tcg/s390x/vlgv.c:8:5: warning: asm operand
> 3 probably doesn't match constraints
>      asm volatile("vlgv %[r1], %[v3], 0(%[a2]), %[m4]\n"
>      ^~~
> /home/dhildenb/git/qemu/tests/tcg/s390x/vlgv.c:8:5: error: impossible
> constraint in 'asm'
> 

vlgv: CFLAGS+=-march=z13 -O2

However works. I am very confused now. I guess this is about evaluation
order or something like that.

-- 

Thanks,

David / dhildenb