Re: [Qemu-devel] [PATCH] target/m68k: Change fpu_rom from const static array to switch

[Kamil Rytarowski <n54@gmx.com>]

[-- Attachment #1: Type: text/plain, Size: 2645 bytes --]

On 04.09.2017 16:41, Philippe Mathieu-Daudé wrote:
> On 09/04/2017 11:09 AM, Laurent Vivier wrote:
>> Le 04/09/2017 à 15:54, Kamil Rytarowski a écrit :
>>> On 04.09.2017 02:27, Philippe Mathieu-Daudé wrote:
>>>> On 09/03/2017 02:05 PM, Laurent Vivier wrote:
>>>>> Le 03/09/2017 à 18:31, Kamil Rytarowski a écrit :
>>>>>> GCC 4.7.2 on SunOS reports that the values assigned to array members
>>>>>> are not
>>>>>> real constants:
>>>>>>
>>>>>> target/m68k/fpu_helper.c:32:5: error: initializer element is not
>>>>>> constant
>>>>>> target/m68k/fpu_helper.c:32:5: error: (near initialization for
>>>>>> 'fpu_rom[0]')
>>>>>> rules.mak:66: recipe for target 'target/m68k/fpu_helper.o' failed
>>>>>>
>>>>>> Convert the array to switch() to workaround the issue.
>>>>>
>>>>> I don't like the idea. It's really an array and should be managed
>>>>> as an
>>>>> array.
>>>>
>>>> I agree with Laurent.
>>>>
>>>>>
>>>>> Could you try to use make_floatx80_init() instead of make_floatx80() ?
>>>>
>>>> I guess the problem comes from the macro which cast as not const:
>>>>
>>>> #define make_floatx80(exp, mant) ((floatx80) { mant, exp })
>>>>
>>>
>>> Adding const to make_floatx80 - ((const floatx80) { mant, exp }) - does
>>> not fix the problem.
>>>
>>>> make_floatx80_init() doesn't cast so it might work,
>>>> else we could add a macro such const_floatx80():
>>>>
>>>> #define const_floatx80(exp, mant) ((const floatx80) { mant, exp })
>>>
>>> Switching make_floatx80() to make_floatx80_init() in works.
>>>
>>> How about floatx80_zero, floatx80_one etc? We cannot translate them to
>>> make_floatx80_init() as this breaks in other parts of the code in
>>> functions like:
>>>
>>> void helper_fldpi_ST0(CPUX86State *env)
>>> {
>>>      ST0 = floatx80_pi;
>>> }
>>>
>>> -- target/i386/fpu_helper.c
>>>
>>
>> You can:
>>
>> either replace the "#define floatx80_pi make_floatx80(...)" by a "const
>> floatx80 floatx80_pi = make_floatx80_init(...)"
> 
> this won't work with:
> 
> const floatx80 2pi = floatx80_mul(floatx80_pi, floatx80_pi, &s);
> 
> however this should work fine:
> 
> #define floatx80_pi make_floatx80_init(0x4000, 0xc90fdaa22168c235LL)
> 

This breaks as mentioned above, in target/i386/fpu_helper.c.

>>
>> or replace all the macros in the m68k/fpu_helper.c array by
>> make_floatx80_init(...)
>>

I've copied the float_* numbers to target/m68k/fpu_helper.c and it
builds on SmartOS machine with GCC 4.7.2 (from Joyent shell) and on my
local NetBSD workstation with Clang 5.0.0

>> Thanks,
>> Laurent
>>

I'm going to submit the new patch soon.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]