Re: [kvm-unit-tests PATCH v1 1/6] s390x: initial infrastructure

David Hildenbrand
2017-05-16T08:22:01Z

On 16.05.2017 09:58, Christian Borntraeger wrote:
> On 05/16/2017 09:54 AM, David Hildenbrand wrote:
>> On 15.05.2017 11:19, Christian Borntraeger wrote:
>>> Nice to see somebody doing this for s390x.
>>>
>>>
>>> On 05/12/2017 12:58 PM, David Hildenbrand wrote:
>>> [...]
>>>
>>>> diff --git a/s390x/Makefile b/s390x/Makefile
>>>> new file mode 100644
>>>> index 0000000..f9468bb
>>>> --- /dev/null
>>>> +++ b/s390x/Makefile
>>>> @@ -0,0 +1,32 @@
>>>> +all: test_cases
>>>> +
>>>> +test_cases: $(tests)
>>>> +
>>>> +CFLAGS += -std=gnu99
>>>> +CFLAGS += -ffreestanding
>>>> +CFLAGS += -Wextra
>>>> +CFLAGS += -I lib
>>>> +CFLAGS += -O2
>>>> +CFLAGS += -msoft-float
>>>
>>>
>>> If you enable the AFP register control bit in your startup code, you can probably
>>> avoid the soft-float thing.
>>>
>>
>> Looks like we can drop that completely.
>>
>> a) no floating point operations are used either way.
>>
>> b) adding a very basic division results in
>>
>> static void test_fp(void)
>> {
>> double a, b;
>>
>> b = 6.5;
>> a = 2.0;
>>
>> b = b / a;
>>
>> report("test fp", b == 3.25);
>> }
>>
>> (compiled with -O0)
>>
>> s390x/selftest.o: In function `test_fp':
>> /home/dhildenb/git/kvm-unit-tests/s390x/selftest.c:21: undefined
>> reference to `__divdf3'
>> /home/dhildenb/git/kvm-unit-tests/s390x/selftest.c:23: undefined
>> reference to `__eqdf2'
>>
>> So we would need a soft float lib (as we're using -nostdlib)
>>
>> c) dropping -msoft-float
>>
>> A DDB (DIVIDE long BFP) is generated. That seems to work under TCG
>> without any exception. I assume the check for AFP-register control is
>> missing. I assume, that test would not pass under KVM.
>>
>>
>> For now I will simply drop -msoft-float.
>
> This will allow gcc to spill/fill into floating point registers, which are
> not all available without afp-register control. It will probably work because
> the gr->fpr move will not be used for -march=z900, but I suggest to not rely
> on that.
>

Good point, so enabling AFP sounds right. And that should be valid on
any z/Architecture system (base feature set).

Thanks!

--

Thanks,

David