[PATCH] fix zero builds for "unknown" architectures

Coleen Phillimore coleen.phillimore at oracle.com
Thu Sep 10 03:54:44 UTC 2015



On 9/9/15 11:44 PM, David Holmes wrote:
> Hi Coleen,
>
> This looks right to me now. For the record though there should be a 
> RFR email with the bug id etc.

Thanks David.   For the record, I'll send an RFR.  The bug was created 
after Matthias sent the patch.

Coleen

>
> Thanks,
> David
>
> On 10/09/2015 1:12 PM, Coleen Phillimore wrote:
>>
>> David pointed out to me offline that the order of ARM conditionals are
>> incorrect, because AARCH64 is a special ARM so I've reversed them:
>>
>> +#if defined(AMD64)
>> +  strncpy(cpuinfo, "x86_64", length);
>> +#elif defined(IA32)
>> +  strncpy(cpuinfo, "x86_32", length);
>> +#elif defined(IA64)
>> +  strncpy(cpuinfo, "IA64", length);
>> +#elif defined(SPARC)
>> +  strncpy(cpuinfo, "sparcv9", length);
>> +#elif defined(AARCH64)
>> +  strncpy(cpuinfo, "AArch64", length);
>> +#elif defined(ARM)
>> +  strncpy(cpuinfo, "ARM", length);
>> +#elif defined(PPC)
>> +  strncpy(cpuinfo, "PPC64", length);
>> +#elif defined(ZERO_LIBARCH)
>> +  strncpy(cpuinfo, ZERO_LIBARCH, length);
>> +#else
>> +  strncpy(cpuinfo, "unknown", length);
>> +#endif
>>
>> Does this look right?
>>
>> Thanks,
>> Coleen
>>
>> On 9/9/15 9:42 PM, Coleen Phillimore wrote:
>>>
>>>
>>> On 9/9/15 7:59 PM, David Holmes wrote:
>>>> On 10/09/2015 2:56 AM, Coleen Phillimore wrote:
>>>>>
>>>>> I think I like patch #1 better.   I will sponsor it and make sure it
>>>>> works on my machine.   Thank you for fixing this on the other
>>>>> platforms.
>>>>
>>>> I think there should be a way to detect an unknown arch via the build
>>>> mechanics. I would think an unknown arch would also impact includes
>>>> of <os>_<arch> headers ... unless the "arch" is Zero in that case? In
>>>> which case maybe we should be checking for Zero here?
>>>>
>>>> Also need to check the ARM versus ARM32 - as with PPC/PPC32/PPC64 I
>>>> think plain ARM means 32-bit or 64-bit, but Aarch64 covers 64-bit.
>>>
>>> I'm not sure what this comment means.   The code does check ARM, but
>>> it doesn't have specific PPC checks for 32 and 64 bit.  I think the
>>> PPC people verified this change works for them.
>>>
>>> +#if defined(AMD64)
>>> +  strncpy(cpuinfo, "x86_64", length);
>>> +#elif defined(IA32)
>>> +  strncpy(cpuinfo, "x86_32", length);
>>> +#elif defined(IA64)
>>> +  strncpy(cpuinfo, "IA64", length);
>>> +#elif defined(SPARC)
>>> +  strncpy(cpuinfo, "sparcv9", length);
>>> +#elif defined(ARM)
>>> +  strncpy(cpuinfo, "ARM", length);
>>> +#elif defined(AARCH64)
>>> +  strncpy(cpuinfo, "AArch64", length);
>>> +#elif defined(PPC)
>>> +  strncpy(cpuinfo, "PPC64", length);
>>> +#elif defined(ZERO_LIBARCH)
>>> +  strncpy(cpuinfo, ZERO_LIBARCH, length);
>>> +#else
>>> +  strncpy(cpuinfo, "unknown", length);
>>> +#endif
>>>
>>> And your other mail suggests a default case, which both of the parts
>>> of Matthias's patch has.
>>>
>>> I was in the process of checking this in under the small change only
>>> needs one reviewer rule.
>>>
>>> Coleen
>>>>
>>>> David
>>>>
>>>>> Coleen
>>>>>
>>>>> On 9/9/15 12:19 PM, Matthias Klose wrote:
>>>>>> seen with jdk9 / tag jdk9-b80. zero builds which don't match one of
>>>>>> the hotspot
>>>>>> architectures fail to build in src/os/linux/vm/os_linux.cpp, because
>>>>>> there is no
>>>>>> default / or else clause:
>>>>>>
>>>>>> const char* search_string = IA32_ONLY("model name") 
>>>>>> AMD64_ONLY("model
>>>>>> name")
>>>>>>                              IA64_ONLY("") SPARC_ONLY("cpu")
>>>>>>                              ARM32_ONLY("Processor")
>>>>>> PPC_ONLY("Processor")
>>>>>> AARCH64_ONLY("Processor");
>>>>>>
>>>>>> and:
>>>>>>
>>>>>>    strncpy(cpuinfo, IA32_ONLY("x86_32") AMD64_ONLY("x86_32")
>>>>>>                     IA64_ONLY("IA64") SPARC_ONLY("sparcv9")
>>>>>>                     ARM32_ONLY("ARM") PPC_ONLY("PPC64")
>>>>>> AARCH64_ONLY("AArch64"),
>>>>>> length);
>>>>>>
>>>>>> attached are two alternate patches how to fix this, either by not
>>>>>> using the
>>>>>> *_ONLY macros, or by defining an UNKOWN_ARCH_ONLY macro.
>>>>>>
>>>>>> Two other issues:
>>>>>>   - The zero builds only define ARM, not ARM32, so the clause
>>>>>>     should be ARM_ONLY (or at least an ARM_ONLY added).
>>>>>>   - The cpuinfo string seems to be wrong for AMD64_ONLY.
>>>>>>
>>>>>> Verified that zero builds without errors with on of these patches.
>>>>>>
>>>>>> Attaching both patches.
>>>>>>
>>>>>>    Matthias
>>>>>>
>>>>>
>>>
>>



More information about the hotspot-dev mailing list